From 07697131ab367709f50045fd755714ddca6a9fe7 Mon Sep 17 00:00:00 2001 From: Fred L Sharp Date: Tue, 5 Oct 2021 12:51:14 -0500 Subject: [PATCH 1/4] First pass k8s support for static env var config --- servo/connectors/kubernetes.py | 14 +++++++++++++- servo/connectors/opsani_dev.py | 2 ++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/servo/connectors/kubernetes.py b/servo/connectors/kubernetes.py index e39445545..23b277e4b 100644 --- a/servo/connectors/kubernetes.py +++ b/servo/connectors/kubernetes.py @@ -3348,6 +3348,17 @@ async def _configure_tuning_pod_template_spec(self) -> None: container = Container(container_obj, None) servo.logger.debug(f"Initialized new tuning container from Pod spec template: {container.name}") + if self.container_config.static_environment_variables: + if container.obj.env is None: + container.obj.env = [] + + # TODO: test and/or add support for overriding existing values + env_list = [ + kubernetes_asyncio.client.V1EnvVar(name=k, value=v) + for k, v in self.container_config.static_environment_variables.items() + ] + container.obj.env.extend(env_list) + if self.tuning_container: servo.logger.debug(f"Copying resource requirements from existing tuning pod container '{self.tuning_pod.name}/{self.tuning_container.name}'") resource_requirements = self.tuning_container.resources @@ -3995,7 +4006,8 @@ class ContainerConfiguration(servo.BaseConfiguration): command: Optional[str] # TODO: create model... cpu: CPU memory: Memory - env: Optional[List[str]] # TODO: create model... + env: Optional[List[str]] # (adjustable environment variables) TODO: create model... + static_environment_variables: Optional[Dict[str, str]] diff --git a/servo/connectors/opsani_dev.py b/servo/connectors/opsani_dev.py index 565f18cec..fc7d2423b 100644 --- a/servo/connectors/opsani_dev.py +++ b/servo/connectors/opsani_dev.py @@ -56,6 +56,7 @@ class OpsaniDevConfiguration(servo.BaseConfiguration): port: Optional[Union[pydantic.StrictInt, str]] = None cpu: CPU memory: Memory + static_environment_variables: Optional[Dict[str, str]] prometheus_base_url: str = PROMETHEUS_SIDECAR_BASE_URL timeout: servo.Duration = "5m" settlement: Optional[servo.Duration] = pydantic.Field( @@ -107,6 +108,7 @@ def generate_kubernetes_config( alias="main", cpu=self.cpu, memory=self.memory, + static_environment_variables=self.static_environment_variables, ) ], ) From ae6c8042754702f8592550cf6823e8b45bfc6223 Mon Sep 17 00:00:00 2001 From: Fred L Sharp Date: Tue, 5 Oct 2021 14:54:49 -0500 Subject: [PATCH 2/4] Add tests, error if env vars used in saturation mode - Check env var configuration during DeploymentOptimization.create and raise NotImplementedError if present - Expand test coverage for tuning pod creation - Add static_environment_variables to tuning pod creation tests and validations - Update opsani_dev_test TestConfig.test_generate with static_environment_variables config key - Add opsani_dev_test TestConfig.test_genrate_kubernetes_config including static_environment_variables input and validation --- servo/connectors/kubernetes.py | 3 ++ tests/connectors/kubernetes_test.py | 77 +++++++++++++++++++++++++---- tests/connectors/opsani_dev_test.py | 25 +++++++++- 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/servo/connectors/kubernetes.py b/servo/connectors/kubernetes.py index 23b277e4b..a83c98b1e 100644 --- a/servo/connectors/kubernetes.py +++ b/servo/connectors/kubernetes.py @@ -2925,6 +2925,9 @@ async def create( f'no container named "{container_config.name}" exists in the Pod (found {names})' ) + if container_config.static_environment_variables: + raise NotImplementedError("Configurable environment variables are not currently supported under Deployment optimization (saturation mode)") + name = container_config.alias or ( f"{deployment.name}/{container.name}" if container else deployment.name ) diff --git a/tests/connectors/kubernetes_test.py b/tests/connectors/kubernetes_test.py index 1f195cc47..9d81a4b4f 100644 --- a/tests/connectors/kubernetes_test.py +++ b/tests/connectors/kubernetes_test.py @@ -45,7 +45,7 @@ ) from servo.errors import AdjustmentFailedError, AdjustmentRejectedError import servo.runner -from servo.types import Adjustment +from servo.types import Adjustment, Component, Description, Replicas from tests.helpers import * @@ -1164,12 +1164,41 @@ async def test_read_pod(self, config, kube) -> None: ## # Canary Tests - # async def test_create_canary(self, tuning_config, namespace: str) -> None: - # connector = KubernetesConnector(config=tuning_config) - # dep = await Deployment.read("fiber-http", namespace) - # debug(dep) - # description = await connector.startup() - # debug(description) + async def test_create_tuning( + self, + tuning_config: KubernetesConfiguration, + kube: kubetest.client.TestClient + ) -> None: + tuning_config.deployments[0].containers[0].static_environment_variables = { "FOO": "BAR" } + connector = KubernetesConnector(config=tuning_config) + description = await connector.describe() + + assert description == Description(components=[ + Component( + name='fiber-http/fiber-http', + settings=[ + CPU(name='cpu', type='range', pinned=True, value="125m", min="125m", max="875m", step="125m", request="125m", limit="125m", get=['request', 'limit'], set=['request', 'limit']), + Memory(name='mem', type='range', pinned=True, value=134217728, min=134217728, max=805306368, step=33554432, request=134217728, limit=134217728, get=['request', 'limit'], set=['request', 'limit']), + Replicas(name='replicas', type='range', pinned=True, value=1, min=0, max=99999, step=1) + ] + ), + Component( + name='fiber-http/fiber-http-tuning', + settings=[ + CPU(name='cpu', type='range', pinned=False, value="125m", min="125m", max="875m", step="125m", request="125m", limit="125m", get=['request', 'limit'], set=['request', 'limit']), + Memory(name='mem', type='range', pinned=False, value=134217728, min=134217728, max=805306368, step=33554432, request=134217728, limit=134217728, get=['request', 'limit'], set=['request', 'limit']), + Replicas(name='replicas', type='range', pinned=True, value=1, min=0, max=1, step=1) + ] + ) + ]) + + tuning_pod = kube.get_pods()["fiber-http-tuning"] + assert tuning_pod.obj.metadata.annotations["opsani.com/opsani_tuning_for"] == "fiber-http/fiber-http-tuning" + assert tuning_pod.obj.metadata.labels["opsani_role"] == "tuning" + target_container = next(filter(lambda c: c.name == "fiber-http" , tuning_pod.obj.spec.containers)) + assert target_container.resources.requests == {'cpu': '125m', 'memory': '128Mi'} + assert target_container.resources.limits == {'cpu': '125m', 'memory': '128Mi'} + assert target_container.env == [kubernetes.client.models.V1EnvVar(name="FOO", value="BAR")] async def test_adjust_tuning_insufficient_mem( self, @@ -2381,10 +2410,40 @@ def _rollout_tuning_config(self, tuning_config: KubernetesConfiguration) -> Kube ## # Canary Tests - async def test_create_rollout_tuning(self, _rollout_tuning_config: KubernetesConfiguration, namespace: str) -> None: + async def test_create_rollout_tuning( + self, _rollout_tuning_config: KubernetesConfiguration, kube: kubetest.client.TestClient, namespace: str + ) -> None: + _rollout_tuning_config.rollouts[0].containers[0].static_environment_variables = { "FOO": "BAR" } connector = KubernetesConnector(config=_rollout_tuning_config) rol = await Rollout.read("fiber-http", namespace) - await connector.describe() + description = await connector.describe() + + assert description == Description(components=[ + Component( + name='fiber-http/fiber-http', + settings=[ + CPU(name='cpu', type='range', pinned=True, value="125m", min="125m", max="875m", step="125m", request="125m", limit="125m", get=['request', 'limit'], set=['request', 'limit']), + Memory(name='mem', type='range', pinned=True, value=134217728, min=134217728, max=805306368, step=33554432, request=134217728, limit=134217728, get=['request', 'limit'], set=['request', 'limit']), + Replicas(name='replicas', type='range', pinned=True, value=1, min=0, max=99999, step=1) + ] + ), + Component( + name='fiber-http/fiber-http-tuning', + settings=[ + CPU(name='cpu', type='range', pinned=False, value="125m", min="125m", max="875m", step="125m", request="125m", limit="125m", get=['request', 'limit'], set=['request', 'limit']), + Memory(name='mem', type='range', pinned=False, value=134217728, min=134217728, max=805306368, step=33554432, request=134217728, limit=134217728, get=['request', 'limit'], set=['request', 'limit']), + Replicas(name='replicas', type='range', pinned=True, value=1, min=0, max=1, step=1) + ] + ) + ]) + + tuning_pod = kube.get_pods()["fiber-http-tuning"] + assert tuning_pod.obj.metadata.annotations["opsani.com/opsani_tuning_for"] == "fiber-http/fiber-http-tuning" + assert tuning_pod.obj.metadata.labels["opsani_role"] == "tuning" + target_container = next(filter(lambda c: c.name == "fiber-http" , tuning_pod.obj.spec.containers)) + assert target_container.resources.requests == {'cpu': '125m', 'memory': '128Mi'} + assert target_container.resources.limits == {'cpu': '125m', 'memory': '128Mi'} + assert target_container.env == [kubernetes.client.models.V1EnvVar(name="FOO", value="BAR")] # verify tuning pod is registered as service endpoint service = await servo.connectors.kubernetes.Service.read("fiber-http", namespace) diff --git a/tests/connectors/opsani_dev_test.py b/tests/connectors/opsani_dev_test.py index 69c5c40e4..4bef9384b 100644 --- a/tests/connectors/opsani_dev_test.py +++ b/tests/connectors/opsani_dev_test.py @@ -70,7 +70,10 @@ def rollout_checks(rollout_config: servo.connectors.opsani_dev.OpsaniDevConfigur class TestConfig: def test_generate(self) -> None: config = servo.connectors.opsani_dev.OpsaniDevConfiguration.generate() - assert list(config.dict().keys()) == ['description', 'namespace', 'deployment', 'rollout', 'container', 'service', 'port', 'cpu', 'memory', 'prometheus_base_url', 'timeout', 'settlement'] + assert list(config.dict().keys()) == [ + 'description', 'namespace', 'deployment', 'rollout', 'container', 'service','port', 'cpu', 'memory', + 'static_environment_variables', 'prometheus_base_url', 'timeout', 'settlement' + ] def test_generate_yaml(self) -> None: config = servo.connectors.opsani_dev.OpsaniDevConfiguration.generate() @@ -91,6 +94,26 @@ def test_assign_optimizer(self) -> None: config = servo.connectors.opsani_dev.OpsaniDevConfiguration.generate() config.__optimizer__ = None + def test_generate_kubernetes_config(self) -> None: + opsani_dev_config = servo.connectors.opsani_dev.OpsaniDevConfiguration( + namespace="test", + deployment="fiber-http", + container="fiber-http", + service="fiber-http", + cpu=servo.connectors.kubernetes.CPU(min="125m", max="4000m", step="125m"), + memory=servo.connectors.kubernetes.Memory(min="128 MiB", max="4.0 GiB", step="128 MiB"), + static_environment_variables={"FOO": "BAR", "BAZ": 1}, + __optimizer__=servo.configuration.Optimizer(id="test.com/foo", token="12345") + ) + kubernetes_config = opsani_dev_config.generate_kubernetes_config() + assert kubernetes_config.namespace == "test" + assert kubernetes_config.deployments[0].namespace == "test" + assert kubernetes_config.deployments[0].name == "fiber-http" + assert kubernetes_config.deployments[0].containers[0].name == "fiber-http" + assert kubernetes_config.deployments[0].containers[0].cpu == servo.connectors.kubernetes.CPU(min="125m", max="4000m", step="125m") + assert kubernetes_config.deployments[0].containers[0].memory == servo.connectors.kubernetes.Memory(min="128 MiB", max="4.0 GiB", step="128 MiB") + assert kubernetes_config.deployments[0].containers[0].static_environment_variables == {"FOO": "BAR", "BAZ": "1"} + def test_generate_rollout_config(self) -> None: rollout_config = servo.connectors.opsani_dev.OpsaniDevConfiguration( namespace="test", From 2238833ab07fc9f1dfddcaeca90502aa3035df9b Mon Sep 17 00:00:00 2001 From: Fred L Sharp Date: Fri, 8 Oct 2021 16:30:04 -0500 Subject: [PATCH 3/4] Remove duplicate env vars from tuning pod template - Update static_environment_variables logic to filter out existing environment variables with same name(s) as configured variables - Expand integration test coverage to verify existing environment variables are overriden by static_environment_variables --- servo/connectors/kubernetes.py | 7 ++++++- tests/connectors/kubernetes_test.py | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/servo/connectors/kubernetes.py b/servo/connectors/kubernetes.py index a83c98b1e..455abb778 100644 --- a/servo/connectors/kubernetes.py +++ b/servo/connectors/kubernetes.py @@ -3355,7 +3355,12 @@ async def _configure_tuning_pod_template_spec(self) -> None: if container.obj.env is None: container.obj.env = [] - # TODO: test and/or add support for overriding existing values + # Filter out vars with the same name as the ones we are setting + container.obj.env = list(filter( + lambda e: e.name not in self.container_config.static_environment_variables, + container.obj.env + )) + env_list = [ kubernetes_asyncio.client.V1EnvVar(name=k, value=v) for k, v in self.container_config.static_environment_variables.items() diff --git a/tests/connectors/kubernetes_test.py b/tests/connectors/kubernetes_test.py index 9d81a4b4f..71ff2c364 100644 --- a/tests/connectors/kubernetes_test.py +++ b/tests/connectors/kubernetes_test.py @@ -1169,7 +1169,12 @@ async def test_create_tuning( tuning_config: KubernetesConfiguration, kube: kubetest.client.TestClient ) -> None: + # verify existing env vars are overriden by config var with same name + main_dep = kube.get_deployments()["fiber-http"] + main_dep.obj.spec.template.spec.containers[0].env = [kubernetes.client.models.V1EnvVar(name="FOO", value="BAZ")] + main_dep.api_client.patch_namespaced_deployment(main_dep.name, main_dep.namespace, main_dep.obj) tuning_config.deployments[0].containers[0].static_environment_variables = { "FOO": "BAR" } + connector = KubernetesConnector(config=tuning_config) description = await connector.describe() From b297bae53e73465a10c144254deb26d8037a28d3 Mon Sep 17 00:00:00 2001 From: Fred L Sharp Date: Fri, 8 Oct 2021 16:40:17 -0500 Subject: [PATCH 4/4] whitespace nit --- servo/connectors/kubernetes.py | 4 ++-- tests/connectors/kubernetes_test.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/servo/connectors/kubernetes.py b/servo/connectors/kubernetes.py index 455abb778..b9754be40 100644 --- a/servo/connectors/kubernetes.py +++ b/servo/connectors/kubernetes.py @@ -3357,8 +3357,8 @@ async def _configure_tuning_pod_template_spec(self) -> None: # Filter out vars with the same name as the ones we are setting container.obj.env = list(filter( - lambda e: e.name not in self.container_config.static_environment_variables, - container.obj.env + lambda e: e.name not in self.container_config.static_environment_variables, + container.obj.env )) env_list = [ diff --git a/tests/connectors/kubernetes_test.py b/tests/connectors/kubernetes_test.py index 71ff2c364..0a08b7a51 100644 --- a/tests/connectors/kubernetes_test.py +++ b/tests/connectors/kubernetes_test.py @@ -1174,7 +1174,7 @@ async def test_create_tuning( main_dep.obj.spec.template.spec.containers[0].env = [kubernetes.client.models.V1EnvVar(name="FOO", value="BAZ")] main_dep.api_client.patch_namespaced_deployment(main_dep.name, main_dep.namespace, main_dep.obj) tuning_config.deployments[0].containers[0].static_environment_variables = { "FOO": "BAR" } - + connector = KubernetesConnector(config=tuning_config) description = await connector.describe()