From aa80afdec2c83e2a67221902e7245cdffc3a6cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20P=C3=A1ez?= Date: Wed, 20 Dec 2023 14:00:31 +0100 Subject: [PATCH] Make pod_name length equal to HOST_NAME_MAX --- .../cncf/kubernetes/kubernetes_helper_functions.py | 10 ++++------ airflow/providers/cncf/kubernetes/operators/pod.py | 7 ++++--- airflow/providers/cncf/kubernetes/pod_generator.py | 12 ++++++++---- .../providers/cncf/kubernetes/operators/test_pod.py | 2 +- .../kubernetes/test_kubernetes_helper_functions.py | 8 ++++---- .../providers/cncf/kubernetes/test_pod_generator.py | 2 +- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/airflow/providers/cncf/kubernetes/kubernetes_helper_functions.py b/airflow/providers/cncf/kubernetes/kubernetes_helper_functions.py index 0a0bb9b582622..c9d94a2a9a5cf 100644 --- a/airflow/providers/cncf/kubernetes/kubernetes_helper_functions.py +++ b/airflow/providers/cncf/kubernetes/kubernetes_helper_functions.py @@ -34,6 +34,8 @@ alphanum_lower = string.ascii_lowercase + string.digits +POD_NAME_MAX_LENGTH = 63 # Matches Linux kernel's HOST_NAME_MAX default value minus 1. + def rand_str(num): """Generate random lowercase alphanumeric string of length num. @@ -43,7 +45,7 @@ def rand_str(num): return "".join(secrets.choice(alphanum_lower) for _ in range(num)) -def add_pod_suffix(*, pod_name: str, rand_len: int = 8, max_len: int = 80) -> str: +def add_pod_suffix(*, pod_name: str, rand_len: int = 8, max_len: int = POD_NAME_MAX_LENGTH) -> str: """Add random string to pod name while staying under max length. :param pod_name: name of the pod @@ -59,16 +61,12 @@ def create_pod_id( dag_id: str | None = None, task_id: str | None = None, *, - max_length: int = 80, + max_length: int = POD_NAME_MAX_LENGTH, unique: bool = True, ) -> str: """ Generate unique pod ID given a dag_id and / or task_id. - The default of 80 for max length is somewhat arbitrary, mainly a balance between - content and not overwhelming terminal windows of reasonable width. The true - upper limit is 253, and this is enforced in construct_pod. - :param dag_id: DAG ID :param task_id: Task ID :param max_length: max number of characters diff --git a/airflow/providers/cncf/kubernetes/operators/pod.py b/airflow/providers/cncf/kubernetes/operators/pod.py index 5153b85965be4..6e84e131ce7ee 100644 --- a/airflow/providers/cncf/kubernetes/operators/pod.py +++ b/airflow/providers/cncf/kubernetes/operators/pod.py @@ -51,6 +51,7 @@ convert_volume_mount, ) from airflow.providers.cncf.kubernetes.hooks.kubernetes import KubernetesHook +from airflow.providers.cncf.kubernetes.kubernetes_helper_functions import POD_NAME_MAX_LENGTH from airflow.providers.cncf.kubernetes.pod_generator import PodGenerator from airflow.providers.cncf.kubernetes.triggers.pod import KubernetesPodTrigger from airflow.providers.cncf.kubernetes.utils import xcom_sidecar # type: ignore[attr-defined] @@ -92,7 +93,7 @@ def _rand_str(num): return "".join(secrets.choice(alphanum_lower) for _ in range(num)) -def _add_pod_suffix(*, pod_name, rand_len=8, max_len=253): +def _add_pod_suffix(*, pod_name, rand_len=8, max_len=POD_NAME_MAX_LENGTH): """Add random string to pod name while staying under max len. TODO: when min airflow version >= 2.5, delete this function and import from kubernetes_helper_functions. @@ -107,7 +108,7 @@ def _create_pod_id( dag_id: str | None = None, task_id: str | None = None, *, - max_length: int = 80, + max_length: int = POD_NAME_MAX_LENGTH, unique: bool = True, ) -> str: """ @@ -962,7 +963,7 @@ def build_pod_request_obj(self, context: Context | None = None) -> k8s.V1Pod: if not pod.metadata.name: pod.metadata.name = _create_pod_id( - task_id=self.task_id, unique=self.random_name_suffix, max_length=80 + task_id=self.task_id, unique=self.random_name_suffix, max_length=POD_NAME_MAX_LENGTH ) elif self.random_name_suffix: # user has supplied pod name, we're just adding suffix diff --git a/airflow/providers/cncf/kubernetes/pod_generator.py b/airflow/providers/cncf/kubernetes/pod_generator.py index da4c27a831b2e..1e991cb6d959a 100644 --- a/airflow/providers/cncf/kubernetes/pod_generator.py +++ b/airflow/providers/cncf/kubernetes/pod_generator.py @@ -41,7 +41,11 @@ AirflowException, RemovedInAirflow3Warning, ) -from airflow.providers.cncf.kubernetes.kubernetes_helper_functions import add_pod_suffix, rand_str +from airflow.providers.cncf.kubernetes.kubernetes_helper_functions import ( + POD_NAME_MAX_LENGTH, + add_pod_suffix, + rand_str, +) from airflow.providers.cncf.kubernetes.pod_generator_deprecated import ( PodDefaults, PodGenerator as PodGeneratorDeprecated, @@ -380,11 +384,11 @@ def construct_pod( - executor_config - dynamic arguments """ - if len(pod_id) > 253: + if len(pod_id) > POD_NAME_MAX_LENGTH: warnings.warn( - "pod_id supplied is longer than 253 characters; truncating and adding unique suffix." + f"pod_id supplied is longer than {POD_NAME_MAX_LENGTH} characters; truncating and adding unique suffix." ) - pod_id = add_pod_suffix(pod_name=pod_id, max_len=253) + pod_id = add_pod_suffix(pod_name=pod_id, max_len=POD_NAME_MAX_LENGTH) try: image = pod_override_object.spec.containers[0].image # type: ignore if not image: diff --git a/tests/providers/cncf/kubernetes/operators/test_pod.py b/tests/providers/cncf/kubernetes/operators/test_pod.py index 8402f0f6b2399..853512a20dd53 100644 --- a/tests/providers/cncf/kubernetes/operators/test_pod.py +++ b/tests/providers/cncf/kubernetes/operators/test_pod.py @@ -1347,7 +1347,7 @@ def test_task_id_as_name_with_suffix_very_long(self): pod = k.build_pod_request_obj({}) assert ( re.match( - r"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-[a-z0-9]{8}", + r"a{54}-[a-z0-9]{8}", pod.metadata.name, ) is not None diff --git a/tests/providers/cncf/kubernetes/test_kubernetes_helper_functions.py b/tests/providers/cncf/kubernetes/test_kubernetes_helper_functions.py index 4f97f0a1da6db..9601e5c28e8ce 100644 --- a/tests/providers/cncf/kubernetes/test_kubernetes_helper_functions.py +++ b/tests/providers/cncf/kubernetes/test_kubernetes_helper_functions.py @@ -87,14 +87,14 @@ def test_create_pod_id_dag_and_task(self, dag_id, task_id, expected, create_pod_ def test_create_pod_id_dag_too_long_with_suffix(self, create_pod_id): actual = create_pod_id("0" * 254) - assert len(actual) == 80 - assert re.match(r"0{71}-[a-z0-9]{8}", actual) + assert len(actual) == 63 + assert re.match(r"0{54}-[a-z0-9]{8}", actual) assert re.match(pod_name_regex, actual) def test_create_pod_id_dag_too_long_non_unique(self, create_pod_id): actual = create_pod_id("0" * 254, unique=False) - assert len(actual) == 80 - assert re.match(r"0{80}", actual) + assert len(actual) == 63 + assert re.match(r"0{63}", actual) assert re.match(pod_name_regex, actual) @pytest.mark.parametrize("unique", [True, False]) diff --git a/tests/providers/cncf/kubernetes/test_pod_generator.py b/tests/providers/cncf/kubernetes/test_pod_generator.py index 969bf26bc5e5a..682af0378085c 100644 --- a/tests/providers/cncf/kubernetes/test_pod_generator.py +++ b/tests/providers/cncf/kubernetes/test_pod_generator.py @@ -572,7 +572,7 @@ def test_ensure_max_identifier_length(self, mock_rand_str): base_worker_pod=worker_config, ) - assert result.metadata.name == "a" * 244 + "-" + self.rand_str + assert result.metadata.name == "a" * 54 + "-" + self.rand_str for v in result.metadata.labels.values(): assert len(v) <= 63