From 2b00ef9d5d27af58e2fd35a37170af9df347d928 Mon Sep 17 00:00:00 2001 From: Pelle Johansson Date: Fri, 20 Dec 2024 12:31:39 +0100 Subject: [PATCH 1/2] Add a dns-search-domains config option. This sets the podSpec.dnsConfig.searches. The main use-case I have in mind is to search services in different namespaces, e.g. if you have a hierarchical namespace setup it can be used to search the parent namespace automatically. Internal ticket: CDX-24. --- docs/operator_guide.md | 5 ++- fiaas_deploy_daemon/config.py | 24 ++++++++---- .../kubernetes/deployment/deployer.py | 14 ++++++- pyproject.toml | 2 +- .../kubernetes/test_deployment_deploy.py | 38 ++++++++++++++++--- tests/fiaas_deploy_daemon/test_config.py | 1 + 6 files changed, 68 insertions(+), 16 deletions(-) diff --git a/docs/operator_guide.md b/docs/operator_guide.md index 32d2da24..558c2c62 100644 --- a/docs/operator_guide.md +++ b/docs/operator_guide.md @@ -91,7 +91,7 @@ Used when collecting metrics to DataDog. See [Metrics](#metrics). ### datadog-activate-sleep -Flag to set a Lifecycle in the datadog container with a preStop that will execute a sleep of the pre-stop-delay of the main container plus 5 seconds. +Flag to set a Lifecycle in the datadog container with a preStop that will execute a sleep of the pre-stop-delay of the main container plus 5 seconds. ### pre-stop-delay @@ -222,6 +222,9 @@ This option Controls when unhealthy running pods can be evicted. The default 'If By default, [enableServiceLinks](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#podspec-v1-core) is set to true, which means that environment variables are automatically created for each service in the same namespace. This parameter allows you to disable this feature by setting disable-service-links to true. This can be useful in scenarios where you have a large number of services and you want to reduce the number of environment variables that are automatically created. For new installations, it's recommended to disable this feature to streamline the environment setup. However, for existing installations, proceed with caution when disabling this feature. This is due to potential compatibility issues, as some services might rely on these automatically created environment variables for communication. +### dns-search-domains + +Additional search domains to put in the pod spec `dnsConfig.searches`. Empty by default. Deploying an application ------------------------ diff --git a/fiaas_deploy_daemon/config.py b/fiaas_deploy_daemon/config.py index 23663c0e..e7e3523e 100644 --- a/fiaas_deploy_daemon/config.py +++ b/fiaas_deploy_daemon/config.py @@ -256,7 +256,7 @@ def _parse_args(self, args): parser.add_argument( "--deployment-max-surge", help="maximum number of extra pods that can be scheduled above the desired " - "number of pods during an update", + "number of pods during an update", default="25%", type=_int_or_unicode, ) @@ -271,7 +271,7 @@ def _parse_args(self, args): "--ready-check-timeout-multiplier", type=int, help="Multiply default ready check timeout (replicas * initialDelaySeconds) with this " - + "number of seconds (default: %(default)s)", + + "number of seconds (default: %(default)s)", default=10, ) parser.add_argument( @@ -303,8 +303,10 @@ def _parse_args(self, args): default=False, ) parser.add_argument( - "--include-status-in-app", help="Include status subresource in application CRD", default=False, - action="store_true" + "--include-status-in-app", + help="Include status subresource in application CRD", + default=False, + action="store_true", ) parser.add_argument( "--list-of-roles", @@ -326,7 +328,7 @@ def _parse_args(self, args): "--pdb-max-unavailable", help="The maximum number of pods that can be unavailable after an eviction", default="1", - type=_int_or_unicode + type=_int_or_unicode, ) parser.add_argument( "--pdb-unhealthy-pod-eviction-policy", @@ -345,8 +347,16 @@ def _parse_args(self, args): help=( "Disable service links in the podspec. Recommended " " for new installations. Existing installations may need to skip this." - ), - action="store_false" + ), + action="store_false", + ) + parser.add_argument( + "--dns-search-domains", + help="list of extra dns search domains", + default=[], + action="append", + type=str, + dest="search_domains", ) usage_reporting_parser = parser.add_argument_group("Usage Reporting", USAGE_REPORTING_LONG_HELP) usage_reporting_parser.add_argument( diff --git a/fiaas_deploy_daemon/deployer/kubernetes/deployment/deployer.py b/fiaas_deploy_daemon/deployer/kubernetes/deployment/deployer.py index da62ba16..4391fd89 100644 --- a/fiaas_deploy_daemon/deployer/kubernetes/deployment/deployer.py +++ b/fiaas_deploy_daemon/deployer/kubernetes/deployment/deployer.py @@ -17,6 +17,7 @@ import logging import shlex +from fiaas_deploy_daemon.config import Configuration from k8s.client import NotFound from k8s.models.common import ObjectMeta from k8s.models.deployment import ( @@ -42,6 +43,7 @@ HTTPHeader, Lifecycle, ObjectFieldSelector, + PodDNSConfig, PodSpec, Probe, ResourceFieldSelector, @@ -67,7 +69,9 @@ class DeploymentDeployer(object): MINIMUM_GRACE_PERIOD = 30 DATADOG_PRE_STOP_DELAY = 5 - def __init__(self, config, datadog, prometheus, deployment_secrets, owner_references, extension_hook): + def __init__( + self, config: Configuration, datadog, prometheus, deployment_secrets, owner_references, extension_hook + ): self._datadog: DataDog = datadog self._prometheus: Prometheus = prometheus self._secrets: Secrets = deployment_secrets @@ -88,6 +92,7 @@ def __init__(self, config, datadog, prometheus, deployment_secrets, owner_refere self._enable_service_account_per_app = config.enable_service_account_per_app # We set enable_service_links to None, when not explicitly disabled, because we want to use the default value in kubernetes. self._enable_service_links = False if config.enable_service_links is False else None + self._search_domains = config.search_domains @retry_on_upsert_conflict(max_value_seconds=5, max_tries=5) def deploy(self, app_spec, selector, labels, besteffort_qos_is_required): @@ -122,6 +127,12 @@ def deploy(self, app_spec, selector, labels, besteffort_qos_is_required): ) ] + dns_config = None + if self._search_domains: + dns_config = PodDNSConfig( + searches=self._search_domains, + ) + automount_service_account_token = app_spec.admin_access init_containers = [] service_account_name = app_spec.name if self._enable_service_account_per_app else "default" @@ -134,6 +145,7 @@ def deploy(self, app_spec, selector, labels, besteffort_qos_is_required): automountServiceAccountToken=automount_service_account_token, terminationGracePeriodSeconds=self._grace_period, enableServiceLinks=self._enable_service_links, + dnsConfig=dns_config, ) pod_labels = merge_dicts(app_spec.labels.pod, _add_status_label(labels)) diff --git a/pyproject.toml b/pyproject.toml index a7b032c9..63f815d2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,7 @@ dependencies = [ "pinject == 0.14.1", "decorator < 5.0.0", # 5.0.0 and later drops py2 support (transitive dep from pinject) "six >= 1.12.0", - "k8s == 0.27.3", + "k8s == 0.28.0", "appdirs == 1.4.3", "requests-toolbelt == 0.10.1", "backoff == 1.8.0", diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py index 55c3f47f..ac22e0ae 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py @@ -330,13 +330,20 @@ def test_deploy_new_deployment( owner_references.apply.assert_called_with(TypeMatcher(Deployment), app_spec) extension_hook.apply.assert_called_once_with(TypeMatcher(Deployment), app_spec) - @pytest.mark.parametrize("enable_service_links, expected_result", [ - (True, None), - (False, False), - (None, None) # If enable_service_links is None, it should default to None - ]) + @pytest.mark.parametrize( + "enable_service_links, expected_result", + [(True, None), (False, False), (None, None)], # If enable_service_links is None, it should default to None + ) def test_enable_service_links( - self, enable_service_links, expected_result, config, datadog, prometheus, secrets, owner_references, extension_hook + self, + enable_service_links, + expected_result, + config, + datadog, + prometheus, + secrets, + owner_references, + extension_hook, ): config.enable_service_links = enable_service_links deployer = DeploymentDeployer(config, datadog, prometheus, secrets, owner_references, extension_hook) @@ -500,6 +507,25 @@ def test_replicas_when_autoscaler_enabled( secrets.apply.assert_called_once_with(DeploymentMatcher(), app_spec) extension_hook.apply.assert_called_once_with(TypeMatcher(Deployment), app_spec) + @pytest.mark.usefixtures("get") + def test_search_domains( + self, post, config, app_spec, datadog, prometheus, secrets, owner_references, extension_hook + ): + config.search_domains = ["example.com", "example.org"] + expected_deployment = create_expected_deployment(config, app_spec) + expected_deployment["spec"]["template"]["spec"]["dnsConfig"] = { + "searches": ["example.com", "example.org"], + } + mock_response = create_autospec(Response) + mock_response.json.return_value = expected_deployment + post.side_effect = None + post.return_value = mock_response + + deployer = DeploymentDeployer(config, datadog, prometheus, secrets, owner_references, extension_hook) + deployer.deploy(app_spec, SELECTOR, LABELS, False) + + pytest.helpers.assert_any_call(post, DEPLOYMENTS_URI, expected_deployment) + def create_expected_deployment( config, diff --git a/tests/fiaas_deploy_daemon/test_config.py b/tests/fiaas_deploy_daemon/test_config.py index 63723f23..404baec3 100644 --- a/tests/fiaas_deploy_daemon/test_config.py +++ b/tests/fiaas_deploy_daemon/test_config.py @@ -58,6 +58,7 @@ def test_default_parameter_values(self): assert config.disable_deprecated_managed_env_vars is False assert config.tls_certificate_ready is False assert config.enable_service_links is True + assert config.search_domains == [] @pytest.mark.parametrize( "arg,key", From cd1306032c29796b19be4bd13850fa8f484d364b Mon Sep 17 00:00:00 2001 From: Pelle Johansson Date: Thu, 2 Jan 2025 10:55:28 +0100 Subject: [PATCH 2/2] Rename search_domains to dns_search_domains everywhere. --- fiaas_deploy_daemon/config.py | 2 +- .../deployer/kubernetes/deployment/deployer.py | 6 +++--- .../deployer/kubernetes/test_deployment_deploy.py | 2 +- tests/fiaas_deploy_daemon/test_config.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fiaas_deploy_daemon/config.py b/fiaas_deploy_daemon/config.py index e7e3523e..0d4ed968 100644 --- a/fiaas_deploy_daemon/config.py +++ b/fiaas_deploy_daemon/config.py @@ -356,7 +356,7 @@ def _parse_args(self, args): default=[], action="append", type=str, - dest="search_domains", + dest="dns_search_domains", ) usage_reporting_parser = parser.add_argument_group("Usage Reporting", USAGE_REPORTING_LONG_HELP) usage_reporting_parser.add_argument( diff --git a/fiaas_deploy_daemon/deployer/kubernetes/deployment/deployer.py b/fiaas_deploy_daemon/deployer/kubernetes/deployment/deployer.py index 4391fd89..a4551ae3 100644 --- a/fiaas_deploy_daemon/deployer/kubernetes/deployment/deployer.py +++ b/fiaas_deploy_daemon/deployer/kubernetes/deployment/deployer.py @@ -92,7 +92,7 @@ def __init__( self._enable_service_account_per_app = config.enable_service_account_per_app # We set enable_service_links to None, when not explicitly disabled, because we want to use the default value in kubernetes. self._enable_service_links = False if config.enable_service_links is False else None - self._search_domains = config.search_domains + self._dns_search_domains = config.dns_search_domains @retry_on_upsert_conflict(max_value_seconds=5, max_tries=5) def deploy(self, app_spec, selector, labels, besteffort_qos_is_required): @@ -128,9 +128,9 @@ def deploy(self, app_spec, selector, labels, besteffort_qos_is_required): ] dns_config = None - if self._search_domains: + if self._dns_search_domains: dns_config = PodDNSConfig( - searches=self._search_domains, + searches=self._dns_search_domains, ) automount_service_account_token = app_spec.admin_access diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py index ac22e0ae..2ca73b93 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py @@ -511,7 +511,7 @@ def test_replicas_when_autoscaler_enabled( def test_search_domains( self, post, config, app_spec, datadog, prometheus, secrets, owner_references, extension_hook ): - config.search_domains = ["example.com", "example.org"] + config.dns_search_domains = ["example.com", "example.org"] expected_deployment = create_expected_deployment(config, app_spec) expected_deployment["spec"]["template"]["spec"]["dnsConfig"] = { "searches": ["example.com", "example.org"], diff --git a/tests/fiaas_deploy_daemon/test_config.py b/tests/fiaas_deploy_daemon/test_config.py index 404baec3..963b53fa 100644 --- a/tests/fiaas_deploy_daemon/test_config.py +++ b/tests/fiaas_deploy_daemon/test_config.py @@ -58,7 +58,7 @@ def test_default_parameter_values(self): assert config.disable_deprecated_managed_env_vars is False assert config.tls_certificate_ready is False assert config.enable_service_links is True - assert config.search_domains == [] + assert config.dns_search_domains == [] @pytest.mark.parametrize( "arg,key",