Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add extra_constraints, which doesn't override default constraints. #236

Merged
merged 2 commits into from
Feb 5, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions docs/source/yelpsoa_configs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,24 @@ instance MAY have:
drain_method. Valid parameters are any of the kwargs defined for the
specified bounce_method in `drain_lib <drain_lib.html>`_.

* ``constraints``: Specifies placement constraints for services. Should be
defined as an array within an array (E.g ``[["habitat", "GROUP_BY"]]``).
Defaults to ``[["<discover_location_type>, "GROUP_BY"]]`` where
``<discover_location_type>`` is defined by the ``discover`` attribute in
``smartstack.yaml``. For more details and other constraint types, see the
* ``constraints``: Overrides the default placement constraints for services.
Should be defined as an array of arrays (E.g ``[["habitat", "GROUP_BY"]]``
or ``[["habitat", "GROUP_BY"], ["hostname", "UNIQUE"]]``). Defaults to
``[["<discover_location_type>, "GROUP_BY"], ["pool", "LIKE", <pool>],
[<deploy_blacklist_type>, "UNLIKE", <deploy_blacklist_value>], ...]``
where ``<discover_location_type>`` is defined by the ``discover`` attribute
in ``smartstack.yaml``, ``<pool>`` is defined by the ``pool`` attribute in
``marathon.yaml``, and ``deploy_blacklist_type`` and
``deploy_blacklist_value`` are defined in the ``deploy_blacklist`` attribute
in marathon.yaml. For more details and other constraint types, see the
official `Marathon constraint documentation
<https://mesosphere.github.io/marathon/docs/constraints.html>`_.

* ``extra_constraints``: Adds to the default placement constraints for
services. This acts the same as ``constraints``, but adds to the default
constraints instead of replacing them. See ``constraints`` for details on
format and the default constraints.

* ``cmd``: The command that is executed. Can be used as an alternative to
args for containers without an `entrypoint
<https://docs.docker.com/reference/builder/#entrypoint>`_. This value is
Expand Down
29 changes: 20 additions & 9 deletions paasta_tools/marathon_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,26 @@ def get_constraints(self, service_namespace_config):
if 'constraints' in self.config_dict:
return self.config_dict.get('constraints')
else:
discover_level = service_namespace_config.get_discover()
locations = get_mesos_slaves_grouped_by_attribute(
attribute=discover_level, blacklist=self.get_deploy_blacklist())
pool = self.get_pool()

deploy_constraints = deploy_blacklist_to_constraints(self.get_deploy_blacklist())
routing_constraints = [[discover_level, "GROUP_BY", str(len(locations))]]
pool_constraints = [["pool", "LIKE", pool]]
return routing_constraints + deploy_constraints + pool_constraints
constraints = self.config_dict.get('extra_constraints', [])
constraints.extend(self.get_routing_constraints(service_namespace_config))
constraints.extend(self.get_deploy_constraints())
constraints.extend(self.get_pool_constraints())
return constraints

def get_routing_constraints(self, service_namespace_config):
discover_level = service_namespace_config.get_discover()
locations = get_mesos_slaves_grouped_by_attribute(
attribute=discover_level, blacklist=self.get_deploy_blacklist())

routing_constraints = [[discover_level, "GROUP_BY", str(len(locations))]]
return routing_constraints

def get_deploy_constraints(self):
return deploy_blacklist_to_constraints(self.get_deploy_blacklist())

def get_pool_constraints(self):
pool = self.get_pool()
return [["pool", "LIKE", pool]]

def format_marathon_app_dict(self, app_id, docker_url, docker_volumes, service_namespace_config):
"""Create the configuration that will be passed to the Marathon REST API.
Expand Down
23 changes: 22 additions & 1 deletion tests/test_marathon_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1030,7 +1030,7 @@ def test_get_constraints_in_config(self):
service='fake_name',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rename line 1027 to "test_get_constraints_in_config_overrides_other_constraints" or something like that?

cluster='fake_cluster',
instance='fake_instance',
config_dict={'constraints': 'so_many_walls'},
config_dict={'constraints': 'so_many_walls', 'extra_constraints': [['ignore', 'this']]},
branch_dict={},
)
with mock.patch(
Expand Down Expand Up @@ -1060,6 +1060,27 @@ def test_get_constraints_default(self):
]
assert fake_conf.get_constraints(fake_service_namespace_config) == expected_constraints

def test_get_constraints_extra_constraints(self):
fake_service_namespace_config = marathon_tools.ServiceNamespaceConfig()
fake_conf = marathon_tools.MarathonServiceConfig(
service='fake_name',
cluster='fake_cluster',
instance='fake_instance',
config_dict={'extra_constraints': [['extra', 'constraint']]},
branch_dict={},
)
with mock.patch(
'paasta_tools.marathon_tools.get_mesos_slaves_grouped_by_attribute',
autospec=True,
) as get_slaves_patch:
get_slaves_patch.return_value = {'fake_region': {}}
expected_constraints = [
['extra', 'constraint'],
["region", "GROUP_BY", "1"],
["pool", "LIKE", "default"],
]
assert fake_conf.get_constraints(fake_service_namespace_config) == expected_constraints

def test_get_constraints_from_discover(self):
fake_service_namespace_config = marathon_tools.ServiceNamespaceConfig({
'mode': 'http',
Expand Down