Have setup_kubernetes_job prioritize services whose git_shas are changing, so that real bounces take priority over big bounces.

Author: EvanKrall

paasta_tools/setup_kubernetes_job.py

@@ -23,6 +23,7 @@
 import argparse
 import logging
 import sys
+from typing import Dict
 from typing import List
 from typing import Optional
 from typing import Sequence
@@ -213,6 +214,12 @@ def setup_kube_deployments(
             for deployment in existing_kube_deployments
         }
 
+        existing_git_shas: Dict[Tuple[str, str, str], List[str]] = {}
+        for deployment in existing_kube_deployments:
+            existing_git_shas.setdefault(
+                (deployment.service, deployment.instance, deployment.namespace), []
+            ).append(deployment.git_sha)
+
     applications = [
         create_application_object(
             cluster=cluster,
@@ -223,6 +230,31 @@ 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 git_sha matches an existing git_sha, and 0 otherwise. This will cause applications that need a new git_sha
+        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.
+        """
+        ok, app = ok_app
+        if ok:
+            if app.kube_deployment.git_sha in existing_git_shas.get(
+                (
+                    app.kube_deployment.service,
+                    app.kube_deployment.instance,
+                    app.kube_deployment.namespace,
+                ),
+                [],
+            ):
+                return 1
+            else:
+                return 0
+        else:
+            return 2
+
+    applications.sort(key=sort_key)
+
     api_updates = 0
     for _, app in applications:
         if app:

tests/test_setup_kubernetes_job.py

@@ -654,3 +654,100 @@ 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",
+                namespace="paasta",
+            ),
+        )
+        fm_app = mock.Mock(
+            kube_deployment=mock.Mock(
+                git_sha="2",
+                service="kurupt",
+                instance="fm",
+                namespace="paasta",
+            ),
+        )
+
+        def create_application_object(
+            cluster: str,
+            soa_dir: str,
+            service_instance_config: KubernetesDeploymentConfig,
+        ):
+            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 = 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)"
+        )