diff --git a/.gitignore b/.gitignore index cb0388c093..660fe9e3e0 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,5 @@ unique-run # Coverage artifacts .coverage + +unique-run diff --git a/paasta_tools/kubernetes_tools.py b/paasta_tools/kubernetes_tools.py index 694025f1ac..d852f77c97 100644 --- a/paasta_tools/kubernetes_tools.py +++ b/paasta_tools/kubernetes_tools.py @@ -148,6 +148,7 @@ from paasta_tools.utils import DockerVolume from paasta_tools.utils import get_config_hash from paasta_tools.utils import get_git_sha_from_dockerurl +from paasta_tools.utils import KubeContainerResourceRequest from paasta_tools.utils import load_service_instance_config from paasta_tools.utils import load_system_paasta_config from paasta_tools.utils import load_v2_deployments_json @@ -201,6 +202,11 @@ DEFAULT_USE_PROMETHEUS_CPU = False DEFAULT_USE_PROMETHEUS_UWSGI = True DEFAULT_USE_RESOURCE_METRICS_CPU = True +DEFAULT_SIDECAR_REQUEST: KubeContainerResourceRequest = { + "cpu": 0.1, + "memory": "1024Mi", + "ephemeral-storage": "256Mi", +} # conditions is None when creating a new HPA, but the client raises an error in that case. @@ -290,17 +296,6 @@ def _set_disrupted_pods(self: Any, disrupted_pods: Mapping[str, datetime]) -> No self._disrupted_pods = disrupted_pods -KubeContainerResourceRequest = TypedDict( - "KubeContainerResourceRequest", - { - "cpu": float, - "memory": str, - "ephemeral-storage": str, - }, - total=False, -) - - SidecarResourceRequirements = TypedDict( "SidecarResourceRequirements", { @@ -1055,7 +1050,10 @@ def get_hacheck_sidecar_container( ) ) ), - resources=self.get_sidecar_resource_requirements("hacheck"), + resources=self.get_sidecar_resource_requirements( + "hacheck", + system_paasta_config, + ), name=HACHECK_POD_NAME, env=self.get_kubernetes_environment() + [hacheck_registrations_env], ports=[V1ContainerPort(container_port=6666)], @@ -1082,7 +1080,10 @@ def get_uwsgi_exporter_sidecar_container( return V1Container( image=system_paasta_config.get_uwsgi_exporter_sidecar_image_url(), - resources=self.get_sidecar_resource_requirements("uwsgi_exporter"), + resources=self.get_sidecar_resource_requirements( + "uwsgi_exporter", + system_paasta_config, + ), name=UWSGI_EXPORTER_POD_NAME, env=self.get_kubernetes_environment() + [stats_port_env], ports=[V1ContainerPort(container_port=9117)], @@ -1126,7 +1127,9 @@ def get_gunicorn_exporter_sidecar_container( if self.should_run_gunicorn_exporter_sidecar(): return V1Container( image=system_paasta_config.get_gunicorn_exporter_sidecar_image_url(), - resources=self.get_sidecar_resource_requirements("gunicorn_exporter"), + resources=self.get_sidecar_resource_requirements( + "gunicorn_exporter", system_paasta_config + ), name=GUNICORN_EXPORTER_POD_NAME, env=self.get_kubernetes_environment(), ports=[V1ContainerPort(container_port=9117)], @@ -1306,15 +1309,36 @@ def get_resource_requirements(self) -> V1ResourceRequirements: return V1ResourceRequirements(limits=limits, requests=requests) def get_sidecar_resource_requirements( - self, sidecar_name: str + self, + sidecar_name: str, + system_paasta_config: SystemPaastaConfig, ) -> V1ResourceRequirements: + """ + Sidecar request/limits are set with varying levels of priority, with + elements further down the list taking precedence: + * hard-coded paasta default + * SystemPaastaConfig + * per-service soaconfig overrides + + Additionally, for the time being we do not expose a way to set + limits separately from requests - these values will always mirror + each other + + NOTE: changing any of these will cause a bounce of all services that + run the sidecars affected by the resource change + """ config = self.config_dict.get("sidecar_resource_requirements", {}).get( sidecar_name, {} ) + sidecar_requirements_config = ( + system_paasta_config.get_sidecar_requirements_config().get( + sidecar_name, DEFAULT_SIDECAR_REQUEST + ) + ) requests: KubeContainerResourceRequest = { - "cpu": 0.1, - "memory": "1024Mi", - "ephemeral-storage": "256Mi", + "cpu": sidecar_requirements_config.get("cpu"), + "memory": sidecar_requirements_config.get("memory"), + "ephemeral-storage": sidecar_requirements_config.get("ephemeral-storage"), } requests.update(config.get("requests", {})) diff --git a/paasta_tools/utils.py b/paasta_tools/utils.py index 29cc57e59b..d67d1808d9 100644 --- a/paasta_tools/utils.py +++ b/paasta_tools/utils.py @@ -349,6 +349,17 @@ class DockerParameter(TypedDict): value: str +KubeContainerResourceRequest = TypedDict( + "KubeContainerResourceRequest", + { + "cpu": float, + "memory": str, + "ephemeral-storage": str, + }, + total=False, +) + + def safe_deploy_blacklist(input: UnsafeDeployBlacklist) -> DeployBlacklist: return [(t, l) for t, l in input] @@ -2017,6 +2028,7 @@ class SystemPaastaConfigDict(TypedDict, total=False): spark_kubeconfig: str kube_clusters: Dict spark_use_eks_default: bool + sidecar_requirements_config: Dict[str, KubeContainerResourceRequest] def load_system_paasta_config( @@ -2096,6 +2108,11 @@ def __repr__(self) -> str: def get_spark_use_eks_default(self) -> bool: return self.config_dict.get("spark_use_eks_default", False) + def get_sidecar_requirements_config( + self, + ) -> Dict[str, KubeContainerResourceRequest]: + return self.config_dict.get("sidecar_requirements_config", {}) + def get_tron_default_pool_override(self) -> str: """Get the default pool override variable defined in this host's cluster config file. diff --git a/tests/test_kubernetes_tools.py b/tests/test_kubernetes_tools.py index 52a3a405ac..cf49372f85 100644 --- a/tests/test_kubernetes_tools.py +++ b/tests/test_kubernetes_tools.py @@ -516,6 +516,21 @@ def test_get_sidecar_containers(self): "paasta_tools.kubernetes_tools.KubernetesDeploymentConfig.get_readiness_check_script", autospec=True, return_value=["/nail/blah.sh"], + ), mock.patch( + "paasta_tools.kubernetes_tools.KubernetesDeploymentConfig.get_sidecar_resource_requirements", + autospec=True, + return_value={ + "limits": { + "cpu": 0.1, + "memory": "1024Mi", + "ephemeral-storage": "256Mi", + }, + "requests": { + "cpu": 0.1, + "memory": "1024Mi", + "ephemeral-storage": "256Mi", + }, + }, ): mock_system_config = mock.Mock( get_hacheck_sidecar_image_url=mock.Mock( @@ -656,9 +671,10 @@ def test_get_sidecar_resource_requirements(self): }, } } + system_paasta_config = mock.Mock() assert self.deployment.get_sidecar_resource_requirements( - "hacheck" + "hacheck", system_paasta_config ) == V1ResourceRequirements( limits={"cpu": 0.3, "memory": "1025Mi", "ephemeral-storage": "257Mi"}, requests={"cpu": 0.2, "memory": "1024Mi", "ephemeral-storage": "256Mi"}, @@ -675,9 +691,9 @@ def test_get_sidecar_resource_requirements_default_limits(self): }, } } - + system_paasta_config = mock.Mock() assert self.deployment.get_sidecar_resource_requirements( - "hacheck" + "hacheck", system_paasta_config ) == V1ResourceRequirements( limits={"cpu": 0.2, "memory": "1025Mi", "ephemeral-storage": "257Mi"}, requests={"cpu": 0.2, "memory": "1025Mi", "ephemeral-storage": "257Mi"}, @@ -690,11 +706,27 @@ def test_get_sidecar_resource_requirements_default_requirements(self): except KeyError: pass + system_paasta_config = mock.Mock( + get_sidecar_requirements_config=mock.Mock( + return_value={ + "hacheck": { + "cpu": 0.1, + "memory": "512Mi", + "ephemeral-storage": "256Mi", + }, + "uwsgi_exporter": { + "cpu": 0.1, + "memory": "512Mi", + "ephemeral-storage": "256Mi", + }, + } + ) + ) assert self.deployment.get_sidecar_resource_requirements( - "hacheck" + "hacheck", system_paasta_config ) == V1ResourceRequirements( - limits={"cpu": 0.1, "memory": "1024Mi", "ephemeral-storage": "256Mi"}, - requests={"cpu": 0.1, "memory": "1024Mi", "ephemeral-storage": "256Mi"}, + limits={"cpu": 0.1, "memory": "512Mi", "ephemeral-storage": "256Mi"}, + requests={"cpu": 0.1, "memory": "512Mi", "ephemeral-storage": "256Mi"}, ) def test_get_sidecar_resource_requirements_limits_override_default_requirements( @@ -706,9 +738,24 @@ def test_get_sidecar_resource_requirements_limits_override_default_requirements( "limits": {"cpu": 1.0}, } } - + system_paasta_config = mock.Mock( + get_sidecar_requirements_config=mock.Mock( + return_value={ + "hacheck": { + "cpu": 0.1, + "memory": "1024Mi", + "ephemeral-storage": "256Mi", + }, + "uwsgi_exporter": { + "cpu": 0.1, + "memory": "1024Mi", + "ephemeral-storage": "256Mi", + }, + } + ) + ) assert self.deployment.get_sidecar_resource_requirements( - "hacheck" + "hacheck", system_paasta_config ) == V1ResourceRequirements( limits={"cpu": 1.0, "memory": "1024Mi", "ephemeral-storage": "256Mi"}, requests={"cpu": 0.1, "memory": "1024Mi", "ephemeral-storage": "256Mi"},