Skip to content

Commit

Permalink
Merge pull request #3729 from Yelp/u/cuza/making-skj-and-ckj-aware-of…
Browse files Browse the repository at this point in the history 
…-downthenup-bounces-across-namespaces

making skj and ckj aware of downthenup bounces across namespaces
  • Loading branch information
@cuza
cuza authored Oct 25, 2023
2 parents 406bb5b + ad79077 commit dc7ad32
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 38 deletions.
14 changes: 10 additions & 4 deletions paasta_tools/cleanup_kubernetes_jobs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,16 @@ def instance_is_not_bouncing(
if isinstance(application, DeploymentWrapper):
existing_app = application.item
if (
existing_app.metadata.namespace == instance_config.get_namespace()
and (
instance_config.get_instances()
<= (existing_app.status.ready_replicas or 0)
(
existing_app.metadata.namespace != instance_config.get_namespace()
and (instance_config.get_bounce_method() == "downthenup")
)
or (
existing_app.metadata.namespace == instance_config.get_namespace()
and (
instance_config.get_instances()
<= (existing_app.status.ready_replicas or 0)
)
)
) or instance_config.get_desired_state() == "stop":
return True
Expand Down
27 changes: 21 additions & 6 deletions paasta_tools/setup_kubernetes_job.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,14 @@ def setup_kube_deployments(
eks: bool = False,
) -> bool:

if service_instance_configs_list:
existing_kube_deployments = set(list_all_paasta_deployments(kube_client))
existing_apps = {
(deployment.service, deployment.instance, deployment.namespace)
for deployment in existing_kube_deployments
}
if not service_instance_configs_list:
return True

existing_kube_deployments = set(list_all_paasta_deployments(kube_client))
existing_apps = {
(deployment.service, deployment.instance, deployment.namespace)
for deployment in existing_kube_deployments
}

applications = [
create_application_object(
Expand Down Expand Up @@ -266,6 +268,19 @@ def setup_kube_deployments(
app.kube_deployment.instance,
app.kube_deployment.namespace,
) not in existing_apps:
if app.soa_config.get_bounce_method() == "downthenup":
if any(
(
existing_app[:2]
== (
app.kube_deployment.service,
app.kube_deployment.instance,
)
)
for existing_app in existing_apps
):
# For downthenup, we don't want to create until cleanup_kubernetes_job has cleaned up the instance in the other namespace.
continue
log.info(f"Creating {app} because it does not exist yet.")
app.create(kube_client)
app_dimensions["deploy_event"] = "create"
Expand Down
114 changes: 86 additions & 28 deletions tests/test_setup_kubernetes_job.py
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
# from typing import Sequence
from typing import List
from typing import Tuple
from typing import Union

import mock
import pytest
from kubernetes.client import V1Deployment
Expand All @@ -10,6 +13,7 @@
from paasta_tools.kubernetes_tools import InvalidKubernetesConfig
from paasta_tools.kubernetes_tools import KubeDeployment
from paasta_tools.kubernetes_tools import KubernetesDeploymentConfig
from paasta_tools.kubernetes_tools import KubernetesDeploymentConfigDict
from paasta_tools.setup_kubernetes_job import create_application_object
from paasta_tools.setup_kubernetes_job import get_kubernetes_deployment_config
from paasta_tools.setup_kubernetes_job import get_service_instances_with_valid_names
Expand Down Expand Up @@ -51,7 +55,7 @@ def test_main_logging():
service="my-service",
instance="my-instance",
cluster="cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
)
mock_service_instance_configs_list.return_value = [
Expand Down Expand Up @@ -80,7 +84,7 @@ def test_main_logging():
service="my-service",
instance="my-instance",
cluster="cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
False,
Expand All @@ -90,7 +94,7 @@ def test_main_logging():
service="my-service",
instance="my-instance",
cluster="cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
True,
Expand Down Expand Up @@ -213,7 +217,7 @@ def test_get_kubernetes_deployment_config():
instance="instance",
cluster="fake_cluster",
soa_dir="nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
)
mock_load_kubernetes_service_config_no_cache.side_effect = None
Expand All @@ -232,7 +236,7 @@ def test_get_kubernetes_deployment_config():
instance="instance",
cluster="fake_cluster",
soa_dir="nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
)
Expand Down Expand Up @@ -278,7 +282,7 @@ def test_get_eks_deployment_config():
instance="instance",
cluster="fake_cluster",
soa_dir="nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
)
mock_load_eks_service_config_no_cache.side_effect = None
Expand All @@ -298,7 +302,7 @@ def test_get_eks_deployment_config():
instance="instance",
cluster="fake_cluster",
soa_dir="nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
)
Expand All @@ -315,7 +319,7 @@ def test_get_eks_deployment_config():
instance="instance",
cluster="fake_cluster",
soa_dir="nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
),
Expand All @@ -326,7 +330,7 @@ def test_get_eks_deployment_config():
instance="instance",
cluster="fake_cluster",
soa_dir="nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
),
Expand Down Expand Up @@ -408,7 +412,7 @@ def test_create_application_object(eks_flag, mock_service_config):
instance="fm",
cluster="fake_cluster",
soa_dir="/nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
False,
Expand All @@ -419,7 +423,7 @@ def test_create_application_object(eks_flag, mock_service_config):
instance="fm",
cluster="fake_cluster",
soa_dir="/nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
True,
Expand Down Expand Up @@ -448,7 +452,14 @@ def simple_create_application_object(
fake_app.update = fake_update
fake_app.update_related_api_objects = fake_update_related_api_objects
fake_app.item = None
fake_app.soa_config = None
fake_app.soa_config = KubernetesDeploymentConfig(
service=service_instance_config.service,
cluster=cluster,
instance=service_instance_config.instance,
config_dict=service_instance_config.config_dict,
branch_dict=None,
soa_dir=soa_dir,
)
fake_app.__str__ = lambda app: "fake_app"
return True, fake_app

Expand All @@ -465,10 +476,12 @@ def simple_create_application_object(
) as mock_no_metrics, mock.patch(
"paasta_tools.setup_kubernetes_job.get_kubernetes_deployment_config",
autospec=True,
) as mock_service_instance_configs_list:
):
mock_client = mock.Mock()
# No instances created
mock_service_instance_configs_list = []
mock_service_instance_configs_list: List[
Tuple[bool, Union[KubernetesDeploymentConfig, EksDeploymentConfig]]
] = []
setup_kube_deployments(
kube_client=mock_client,
service_instance_configs_list=mock_service_instance_configs_list,
Expand Down Expand Up @@ -499,6 +512,47 @@ def simple_create_application_object(
mock_log_obj.info.reset_mock()
mock_no_metrics.reset_mock()

# Skipping downthenup instance cuz of existing_apps
fake_create.reset_mock()
fake_update.reset_mock()
fake_update_related_api_objects.reset_mock()
mock_list_all_paasta_deployments.return_value = [
KubeDeployment(
service="kurupt",
instance="fm",
git_sha="2",
namespace="paastasvc-kurupt",
image_version="extrastuff-1",
config_sha="1",
replicas=1,
)
]
mock_downthenup_kube_deploy_config = KubernetesDeploymentConfig(
service="kurupt",
instance="fm",
cluster="fake_cluster",
soa_dir="/nail/blah",
config_dict=KubernetesDeploymentConfigDict(bounce_method="downthenup"),
branch_dict=None,
)
mock_service_instance_configs_list = [
(True, mock_downthenup_kube_deploy_config)
]
setup_kube_deployments(
kube_client=mock_client,
service_instance_configs_list=mock_service_instance_configs_list,
cluster="fake_cluster",
soa_dir="/nail/blah",
metrics_interface=mock_no_metrics,
eks=eks_flag,
)
assert fake_create.call_count == 0
assert fake_update.call_count == 0
assert fake_update_related_api_objects.call_count == 0
assert mock_no_metrics.emit_event.call_count == 0
mock_log_obj.info.reset_mock()
mock_no_metrics.reset_mock()

# Update when gitsha changed
fake_create.reset_mock()
fake_update.reset_mock()
Expand Down Expand Up @@ -629,7 +683,7 @@ def simple_create_application_object(
instance="garage",
cluster="fake_cluster",
soa_dir="/nail/blah",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
)
mock_service_instance_configs_list = [
Expand Down Expand Up @@ -698,21 +752,21 @@ def simple_create_application_object(
service="kurupt",
instance="fm",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
KubernetesDeploymentConfig(
service="kurupt",
instance="garage",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
KubernetesDeploymentConfig(
service="kurupt",
instance="radio",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
False,
Expand All @@ -722,21 +776,21 @@ def simple_create_application_object(
service="kurupt",
instance="fm",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
EksDeploymentConfig(
service="kurupt",
instance="garage",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
EksDeploymentConfig(
service="kurupt",
instance="radio",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
True,
Expand All @@ -758,7 +812,9 @@ def test_setup_kube_deployments_rate_limit(
"paasta_tools.setup_kubernetes_job.log", autospec=True
) as mock_log_obj:
mock_client = mock.Mock()
mock_service_instance_configs_list = [
mock_service_instance_configs_list: List[
Tuple[bool, Union[KubernetesDeploymentConfig, EksDeploymentConfig]]
] = [
(True, mock_kube_deploy_config_fm),
(True, mock_kube_deploy_config_garage),
(True, mock_kube_deploy_config_radio),
Expand Down Expand Up @@ -801,14 +857,14 @@ def test_setup_kube_deployments_rate_limit(
service="fake",
instance="instance",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
KubernetesDeploymentConfig(
service="mock",
instance="instance",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
False,
Expand All @@ -818,14 +874,14 @@ def test_setup_kube_deployments_rate_limit(
service="fake",
instance="instance",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
EksDeploymentConfig(
service="mock",
instance="instance",
cluster="fake_cluster",
config_dict={},
config_dict=KubernetesDeploymentConfigDict(),
branch_dict=None,
),
True,
Expand All @@ -844,7 +900,9 @@ def test_setup_kube_deployments_skip_malformed_apps(
"paasta_tools.setup_kubernetes_job.log", autospec=True
) as mock_log_obj:
mock_client = mock.Mock()
mock_service_instance_configs_list = [
mock_service_instance_configs_list: List[
Tuple[bool, Union[KubernetesDeploymentConfig, EksDeploymentConfig]]
] = [
(True, mock_kube_deploy_config_fake),
(True, mock_kube_deploy_config_mock),
]
Expand Down

0 comments on commit dc7ad32

Please sign in to comment.