Skip to content

Commit

Permalink
Make pod_name length equal to HOST_NAME_MAX (#36332)
Browse files Browse the repository at this point in the history
  • Loading branch information
csp33 committed Dec 21, 2023
1 parent 55f421b commit 381922f
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 19 deletions.
10 changes: 4 additions & 6 deletions airflow/providers/cncf/kubernetes/kubernetes_helper_functions.py
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions airflow/providers/cncf/kubernetes/operators/pod.py
Expand Up @@ -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]
Expand Down Expand Up @@ -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.
Expand All @@ -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:
"""
Expand Down Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions airflow/providers/cncf/kubernetes/pod_generator.py
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/providers/cncf/kubernetes/operators/test_pod.py
Expand Up @@ -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
Expand Down
Expand Up @@ -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])
Expand Down
2 changes: 1 addition & 1 deletion tests/providers/cncf/kubernetes/test_pod_generator.py
Expand Up @@ -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

Expand Down

0 comments on commit 381922f

Please sign in to comment.