Merge 5a72d0b into a6b815c

paasta_itests/steps/setup_marathon_job_steps.py

@@ -23,13 +23,15 @@
 from paasta_tools import marathon_tools
 
 fake_service_name = 'fake_complete_service'
+fake_cluster_name = 'fake_cluster'
 fake_instance_name = 'fake_instance'
 fake_appid = 'fake--complete--service.fake--instance.gitdeadbeef.configdeadbeef2'
 fake_service_marathon_config = marathon_tools.MarathonServiceConfig(
-    fake_service_name,
-    fake_instance_name,
-    {},
-    {'docker_image': 'test-image'},
+    service=fake_service_name,
+    cluster=fake_cluster_name,
+    instance=fake_instance_name,
+    config_dict={},
+    branch_dict={'docker_image': 'test-image'},
 )
 fake_service_config = {
     'id': fake_appid,

paasta_tools/chronos_tools.py

@@ -159,29 +159,37 @@ def load_chronos_job_config(service, instance, cluster, load_deployments=True, s
         deployments_json = load_deployments_json(service, soa_dir=soa_dir)
         branch = get_paasta_branch(cluster=cluster, instance=instance)
         branch_dict = deployments_json.get_branch_dict(service, branch)
-    return ChronosJobConfig(service, instance, service_chronos_jobs[instance], branch_dict)
+    return ChronosJobConfig(
+        service=service,
+        cluster=cluster,
+        instance=instance,
+        config_dict=service_chronos_jobs[instance],
+        branch_dict=branch_dict,
+    )
 
 
 class ChronosJobConfig(InstanceConfig):
 
-    def __init__(self, service, job_name, config_dict, branch_dict):
-        super(ChronosJobConfig, self).__init__(config_dict, branch_dict)
-        self.service = service
-        self.job_name = job_name
-        self.config_dict = config_dict
-        self.branch_dict = branch_dict
+    def __init__(self, service, instance, cluster, config_dict, branch_dict):
+        super(ChronosJobConfig, self).__init__(
+            cluster=cluster,
+            instance=instance,
+            service=service,
+            config_dict=config_dict,
+            branch_dict=branch_dict,
+        )
 
     def __eq__(self, other):
         return ((self.service == other.service)
-                and (self.job_name == other.job_name)
+                and (self.get_job_name() == other.get_job_name())
                 and (self.config_dict == other.config_dict)
                 and (self.branch_dict == other.branch_dict))
 
     def get_service(self):
         return self.service
 
     def get_job_name(self):
-        return self.job_name
+        return self.instance
 
     def get_cmd(self):
         original_cmd = super(ChronosJobConfig, self).get_cmd()

paasta_tools/generate_deployments_for_service.py

@@ -48,9 +48,11 @@
 
 from paasta_tools import remote_git
 from paasta_tools.utils import atomic_file_write
-from paasta_tools.utils import get_paasta_branch
 from paasta_tools.utils import get_git_url
-
+from paasta_tools.utils import list_clusters
+from paasta_tools.utils import get_service_instance_list
+from paasta_tools.marathon_tools import load_marathon_service_config
+from paasta_tools.chronos_tools import load_chronos_job_config
 
 log = logging.getLogger('__main__')
 logging.basicConfig()
@@ -70,35 +72,33 @@ def parse_args():
     return args
 
 
-def get_branches_from_config_file(file_dir, filename):
-    """Get all branches defined in a single service configuration file.
-    A branch is defined for an instance if it has a 'branch' key, or
-    the branch name is paasta-{cluster}.{instance},
-    where cluster is the cluster the marathon or chronos file is defined for
-    (i.e. marathon-hab.yaml is for hab), and instance is the
-    instance name.
-
-    :param file_dir: The directory that the filename argument is in
-    :param filename: The name of the service configuration file to read from
-    :returns: A set of branch names listed in the configuration file
-    """
-    valid_branches = set([])
-    config = service_configuration_lib.read_service_information(os.path.join(file_dir, filename))
-    for instance in config:
-        target_branch = None
-        if 'branch' in config[instance]:
-            target_branch = config[instance]['branch']
-        else:
-            try:
-                # cluster may contain dashes (and frequently does) so
-                # reassemble the cluster after stripping the chronos/marathon prefix
-                cluster = '-'.join(filename.split('-')[1:]).split('.')[0]
-                target_branch = get_paasta_branch(cluster, instance)
-            except IndexError:
-                pass
-        if target_branch:
-            valid_branches.add(target_branch)
-    return valid_branches
+def get_instance_config_for_service(soa_dir, service):
+    for cluster in list_clusters(
+        service=service,
+        soa_dir=soa_dir,
+    ):
+        for _, instance in get_service_instance_list(
+            service=service,
+            cluster=cluster,
+            instance_type='marathon',
+        ):
+            yield load_marathon_service_config(
+                service=service,
+                instance=instance,
+                cluster=cluster,
+                soa_dir=soa_dir,
+            )
+        for _, instance in get_service_instance_list(
+            service=service,
+            cluster=cluster,
+            instance_type='chronos',
+        ):
+            yield load_chronos_job_config(
+                service=service,
+                instance=instance,
+                cluster=cluster,
+                soa_dir=soa_dir,
+            )
 
 
 def get_branches_for_service(soa_dir, service):
@@ -108,11 +108,11 @@ def get_branches_for_service(soa_dir, service):
     :param service: The service name to get branches for
     :returns: A list of branches defined in instances for the service
     """
-    valid_branches = set([])
-    working_dir = os.path.join(soa_dir, service)
-    for fname in os.listdir(working_dir):
-        if fname.startswith('marathon-') or fname.startswith('chronos-'):
-            valid_branches = valid_branches.union(get_branches_from_config_file(working_dir, fname))
+    valid_branches = set([config.get_deploy_group() for config in get_instance_config_for_service(
+        soa_dir=soa_dir,
+        service=service,
+    )])
+
     return valid_branches
 
 

paasta_tools/marathon_tools.py

@@ -151,10 +151,11 @@ def load_marathon_service_config(service, instance, cluster, load_deployments=Tr
         branch_dict = deployments_json.get_branch_dict(service, branch)
 
     return MarathonServiceConfig(
-        service,
-        instance,
-        general_config,
-        branch_dict,
+        service=service,
+        cluster=cluster,
+        instance=instance,
+        config_dict=general_config,
+        branch_dict=branch_dict,
     )
 
 
@@ -164,16 +165,19 @@ class InvalidMarathonConfig(Exception):
 
 class MarathonServiceConfig(InstanceConfig):
 
-    def __init__(self, service, instance, config_dict, branch_dict):
-        super(MarathonServiceConfig, self).__init__(config_dict, branch_dict)
-        self.service = service
-        self.instance = instance
-        self.config_dict = config_dict
-        self.branch_dict = branch_dict
+    def __init__(self, service, cluster, instance, config_dict, branch_dict):
+        super(MarathonServiceConfig, self).__init__(
+            cluster=cluster,
+            instance=instance,
+            service=service,
+            config_dict=config_dict,
+            branch_dict=branch_dict,
+        )
 
     def __repr__(self):
-        return "MarathonServiceConfig(%r, %r, %r, %r)" % (
+        return "MarathonServiceConfig(%r, %r, %r, %r, %r)" % (
             self.service,
+            self.cluster,
             self.instance,
             self.config_dict,
             self.branch_dict

paasta_tools/utils.py

@@ -68,9 +68,24 @@ class InvalidInstanceConfig(Exception):
 
 class InstanceConfig(dict):
 
-    def __init__(self, config_dict, branch_dict):
+    def __init__(self, cluster, instance, service, config_dict, branch_dict):
         self.config_dict = config_dict
         self.branch_dict = branch_dict
+        self.cluster = cluster
+        self.instance = instance
+        self.service = service
+
+    def get_cluster(self):
+        return self.cluster
+
+    def get_instance(self):
+        return self.instance
+
+    def get_service(self):
+        return self.service
+
+    def get_deploy_group(self):
+        return self.config_dict.get('deploy_group', '.'.join((self.get_cluster(), self.get_instance())))
 
     def get_mem(self):
         """Gets the memory required from the service's configuration.

tests/cli/test_cmds_local_run.py

@@ -822,9 +822,15 @@ def test_simulate_healthcheck_on_service_disabled(mock_sleep):
 @mock.patch('paasta_tools.cli.cmds.local_run.run_healthcheck_on_container', autospec=True)
 def test_simulate_healthcheck_on_service_enabled_success(mock_run_healthcheck_on_container, mock_sleep):
     mock_docker_client = mock.MagicMock(spec_set=docker.Client)
-    mock_service_manifest = MarathonServiceConfig('fake_name', 'fake_instance', {
-        'healthcheck_grace_period_seconds': 0,
-    }, {})
+    mock_service_manifest = MarathonServiceConfig(
+        service='fake_name',
+        cluster='fake_cluster',
+        instance='fake_instance',
+        config_dict={
+            'healthcheck_grace_period_seconds': 0,
+        },
+        branch_dict={},
+    )
     fake_container_id = 'fake_container_id'
     fake_mode = 'http'
     fake_url = 'http://fake_host/fake_status_path'
@@ -837,9 +843,15 @@ def test_simulate_healthcheck_on_service_enabled_success(mock_run_healthcheck_on
 @mock.patch('paasta_tools.cli.cmds.local_run.run_healthcheck_on_container', autospec=True)
 def test_simulate_healthcheck_on_service_enabled_failure(mock_run_healthcheck_on_container, mock_sleep):
     mock_docker_client = mock.MagicMock(spec_set=docker.Client)
-    mock_service_manifest = MarathonServiceConfig('fake_name', 'fake_instance', {
-        'healthcheck_grace_period_seconds': 0,
-    }, {})
+    mock_service_manifest = MarathonServiceConfig(
+        service='fake_name',
+        cluster='fake_cluster',
+        instance='fake_instance',
+        config_dict={
+            'healthcheck_grace_period_seconds': 0,
+        },
+        branch_dict={},
+    )
     mock_service_manifest
 
     fake_container_id = 'fake_container_id'
@@ -855,9 +867,15 @@ def test_simulate_healthcheck_on_service_enabled_failure(mock_run_healthcheck_on
             autospec=True, side_effect=[False, False, False, False, True])
 def test_simulate_healthcheck_on_service_enabled_partial_failure(mock_run_healthcheck_on_container, mock_sleep):
     mock_docker_client = mock.MagicMock(spec_set=docker.Client)
-    mock_service_manifest = MarathonServiceConfig('fake_name', 'fake_instance', {
-        'healthcheck_grace_period_seconds': 0,
-    }, {})
+    mock_service_manifest = MarathonServiceConfig(
+        service='fake_name',
+        cluster='fake_cluster',
+        instance='fake_instance',
+        config_dict={
+            'healthcheck_grace_period_seconds': 0,
+        },
+        branch_dict={},
+    )
 
     fake_container_id = 'fake_container_id'
     fake_mode = 'http'
@@ -881,9 +899,15 @@ def test_simulate_healthcheck_on_service_enabled_during_grace_period(
     # prevent grace period from ending
     mock_time.side_effect = [0, 0]
     mock_docker_client = mock.MagicMock(spec_set=docker.Client)
-    mock_service_manifest = MarathonServiceConfig('fake_name', 'fake_instance', {
-        'healthcheck_grace_period_seconds': 1,
-    }, {})
+    mock_service_manifest = MarathonServiceConfig(
+        service='fake_name',
+        cluster='fake_cluster',
+        instance='fake_instance',
+        config_dict={
+            'healthcheck_grace_period_seconds': 1,
+        },
+        branch_dict={},
+    )
 
     fake_container_id = 'fake_container_id'
     fake_mode = 'http'
@@ -907,10 +931,16 @@ def test_simulate_healthcheck_on_service_enabled_honors_grace_period(
     # change time to make sure we are sufficiently past grace period
     mock_time.side_effect = [0, 2]
     mock_docker_client = mock.MagicMock(spec_set=docker.Client)
-    mock_service_manifest = MarathonServiceConfig('fake_name', 'fake_instance', {
-        # only one healthcheck will be performed silently
-        'healthcheck_grace_period_seconds': 1,
-    }, {})
+    mock_service_manifest = MarathonServiceConfig(
+        service='fake_name',
+        cluster='fake_cluster',
+        instance='fake_instance',
+        config_dict={
+            # only one healthcheck will be performed silently
+            'healthcheck_grace_period_seconds': 1,
+        },
+        branch_dict={},
+    )
 
     fake_container_id = 'fake_container_id'
     fake_mode = 'http'