Have setup_kubernetes_job prioritize services whose git_shas are chan…

paasta_tools/kubernetes_tools.py

@@ -248,6 +248,10 @@ class KubeDeployment(NamedTuple):
     namespace: str
     replicas: Optional[int]
 
+    @property
+    def deployment_version(self) -> DeploymentVersion:
+        return DeploymentVersion(self.git_sha, self.image_version)
+
 
 class KubeCustomResource(NamedTuple):
     service: str

paasta_tools/setup_kubernetes_job.py

@@ -24,6 +24,7 @@
 import logging
 import sys
 import traceback
+from typing import Dict
 from typing import List
 from typing import Optional
 from typing import Sequence
@@ -45,6 +46,7 @@
 from paasta_tools.metrics import metrics_lib
 from paasta_tools.utils import decompose_job_id
 from paasta_tools.utils import DEFAULT_SOA_DIR
+from paasta_tools.utils import DeploymentVersion
 from paasta_tools.utils import InvalidJobNameError
 from paasta_tools.utils import load_system_paasta_config
 from paasta_tools.utils import NoConfigurationForServiceError
@@ -243,6 +245,14 @@ def setup_kube_deployments(
         for deployment in existing_kube_deployments
     }
 
+    existing_deployment_versions: Dict[
+        Tuple[str, str, str], List[DeploymentVersion]
+    ] = {}
+    for deployment in existing_kube_deployments:
+        existing_deployment_versions.setdefault(
+            (deployment.service, deployment.instance, deployment.namespace), []
+        ).append(deployment.deployment_version)
+
     applications = [
         create_application_object(
             cluster=cluster,
@@ -254,6 +264,39 @@ def setup_kube_deployments(
         else (_, None)
         for _, service_instance in service_instance_configs_list
     ]
+
+    def sort_key(ok_app: Tuple[bool, Optional[Application]]) -> int:
+        """This will return 1 if the desired deployment_version matches an existing deployment_version, and 0
+        otherwise. This will cause applications that need a new deployment_version to be handled first.
+        This prioritizes "real" bounces (developers pushing new service versions) over things that are likely to be big
+        bounces (config-only changes).
+        Most of the time, this won't matter much, as we should get through our backlog quickly, but when there is a
+        backlog, we want to avoid blocking developers.
+        """
+        _, app = ok_app
+        if app:
+            if (
+                app.kube_deployment.deployment_version
+                in existing_deployment_versions.get(
+                    (
+                        app.kube_deployment.service,
+                        app.kube_deployment.instance,
+                        app.kube_deployment.namespace,
+                    ),
+                    [],
+                )
+            ):
+                # Desired version exists, so this is just a configuration update; handle this second.
+                return 1
+            else:
+                # Desired version doesn't exist, handle this first.
+                return 0
+        else:
+            # Handle broken app last.
+            return 2
+
+    applications.sort(key=sort_key)
+
     api_updates = 0
     for _, app in applications:
         if app:

tests/test_setup_kubernetes_job.py

@@ -20,6 +20,7 @@
 from paasta_tools.setup_kubernetes_job import main
 from paasta_tools.setup_kubernetes_job import parse_args
 from paasta_tools.setup_kubernetes_job import setup_kube_deployments
+from paasta_tools.utils import DeploymentVersion
 from paasta_tools.utils import NoConfigurationForServiceError
 from paasta_tools.utils import NoDeploymentsAvailable
 
@@ -926,3 +927,105 @@ def test_setup_kube_deployments_skip_malformed_apps(
         assert mock_log_obj.exception.call_args_list[0] == mock.call(
             "Error while processing: fake_app"
         )
+
+
+def test_setup_kube_deployments_does_git_sha_changes_first():
+    with mock.patch(
+        "paasta_tools.setup_kubernetes_job.create_application_object",
+        autospec=True,
+    ) as mock_create_application_object, mock.patch(
+        "paasta_tools.setup_kubernetes_job.list_all_paasta_deployments", autospec=True
+    ) as mock_list_all_paasta_deployments, mock.patch(
+        "paasta_tools.setup_kubernetes_job.log", autospec=True
+    ) as mock_log_obj:
+        mock_client = mock.Mock()
+        mock_kube_deploy_config_fm = KubernetesDeploymentConfig(
+            service="kurupt",
+            instance="fm",
+            cluster="fake_cluster",
+            config_dict={},
+            branch_dict=None,
+        )
+        mock_kube_deploy_config_garage = KubernetesDeploymentConfig(
+            service="kurupt",
+            instance="garage",
+            cluster="fake_cluster",
+            config_dict={},
+            branch_dict=None,
+        )
+
+        mock_list_all_paasta_deployments.return_value = [
+            KubeDeployment(
+                service="kurupt",
+                instance="garage",
+                git_sha="1",
+                namespace="paasta",
+                image_version="extrastuff-1",
+                config_sha="1",
+                replicas=1,
+            ),
+            KubeDeployment(
+                service="kurupt",
+                instance="fm",
+                git_sha="1",
+                namespace="paasta",
+                image_version="extrastuff-1",
+                config_sha="1",
+                replicas=1,
+            ),
+        ]
+
+        mock_service_instance_configs_list = [
+            (True, mock_kube_deploy_config_garage),
+            (True, mock_kube_deploy_config_fm),
+        ]
+
+        garage_app = mock.Mock(
+            kube_deployment=mock.Mock(
+                git_sha="1",
+                service="kurupt",
+                instance="garage",
+                image_version="extrastuff-1",
+                namespace="paasta",
+                deployment_version=DeploymentVersion("1", "extrastuff-1"),
+            ),
+        )
+        fm_app = mock.Mock(
+            kube_deployment=mock.Mock(
+                git_sha="2",
+                service="kurupt",
+                instance="fm",
+                image_version="extrastuff-1",
+                namespace="paasta",
+                deployment_version=DeploymentVersion("2", "extrastuff-1"),
+            ),
+        )
+
+        def fake_create_application_object(
+            cluster: str,
+            soa_dir: str,
+            service_instance_config: KubernetesDeploymentConfig,
+            eks: bool = False,
+        ):
+            if service_instance_config.instance == "garage":
+                return (True, garage_app)
+            elif service_instance_config.instance == "fm":
+                return (True, fm_app)
+            raise ValueError("expecting instance of 'fm' or 'garage'")
+
+        mock_create_application_object.side_effect = fake_create_application_object
+
+        setup_kube_deployments(
+            kube_client=mock_client,
+            service_instance_configs_list=mock_service_instance_configs_list,
+            cluster="fake_cluster",
+            soa_dir="/nail/blah",
+            rate_limit=1,
+        )
+
+        # With rate_limit == 1, only the app with a different `git_sha` should be bounced.
+        assert garage_app.update.call_count == 0
+        assert fm_app.update.call_count == 1
+        mock_log_obj.info.assert_any_call(
+            "Not doing any further updates as we reached the limit (1)"
+        )