Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make sidecar resource limit configurable #3647

Merged
merged 2 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,5 @@ unique-run

# Coverage artifacts
.coverage

unique-run
60 changes: 42 additions & 18 deletions paasta_tools/kubernetes_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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",
{
Expand Down Expand Up @@ -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)],
Expand All @@ -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)],
Expand Down Expand Up @@ -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)],
Expand Down Expand Up @@ -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 = (
ajayOO8 marked this conversation as resolved.
Show resolved Hide resolved
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", {}))

Expand Down
17 changes: 17 additions & 0 deletions paasta_tools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down Expand Up @@ -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]
eks_cluster_aliases: Dict[str, str]


Expand Down Expand Up @@ -2097,6 +2109,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.

Expand Down
63 changes: 55 additions & 8 deletions tests/test_kubernetes_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"},
Expand All @@ -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"},
Expand All @@ -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(
Expand All @@ -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"},
Expand Down
Loading