diff --git a/cli/polyaxon/_containers/names.py b/cli/polyaxon/_containers/names.py index ac57b30b..a85f75d1 100644 --- a/cli/polyaxon/_containers/names.py +++ b/cli/polyaxon/_containers/names.py @@ -22,6 +22,20 @@ INIT_PREFIX = "init" SIDECAR_PREFIX = "sidecar" +# Container names are Kubernetes DNS labels, capped at 63 characters. +MAX_CONTAINER_NAME_LENGTH = 63 + + +def _trim(value: str, max_length: int) -> str: + """Cut a name component down to `max_length` characters. + + A DNS label must start and end with an alphanumeric character, so any + dashes exposed by the cut are stripped as well. + """ + if max_length <= 0: + return "" + return value[:max_length].strip("-") + def generate_container_name( prefix: str, suffix: Optional[str] = None, unique: bool = True @@ -33,12 +47,18 @@ def generate_container_name( if suffix: suffix = suffix.replace("_", "-") if unique: - suffix = "{}-{}".format(suffix, unique_value) + # Reserve room for the prefix, the two joining dashes and the + # unique value so that the generated name always stays within the + # Kubernetes limit, and trim the caller-supplied part only. + budget = MAX_CONTAINER_NAME_LENGTH - len(prefix) - len(unique_value) - 2 + suffix = "{}-{}".format(_trim(suffix, budget), unique_value) + else: + suffix = _trim(suffix, MAX_CONTAINER_NAME_LENGTH - len(prefix) - 1) else: suffix = unique_value - return "{}-{}".format(prefix, suffix) + return sanitize_container_name("{}-{}".format(prefix, suffix)) def sanitize_container_name(name: str) -> str: - name = name.replace("_", "-") - return name.lower() + name = name.replace("_", "-").lower() + return _trim(name, MAX_CONTAINER_NAME_LENGTH) diff --git a/cli/tests/test_containers/__init__.py b/cli/tests/test_containers/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/cli/tests/test_containers/test_names.py b/cli/tests/test_containers/test_names.py new file mode 100644 index 00000000..7c5f0a9e --- /dev/null +++ b/cli/tests/test_containers/test_names.py @@ -0,0 +1,67 @@ +import pytest + +from polyaxon._containers.names import ( + INIT_ARTIFACTS_CONTAINER_PREFIX, + INIT_GIT_CONTAINER_PREFIX, + MAX_CONTAINER_NAME_LENGTH, + generate_container_name, + sanitize_container_name, +) +from polyaxon._utils.test_utils import BaseTestCase + +LONG_CONNECTION_NAME = "very-long-artifacts-store-connection-name-for-the-team" + + +@pytest.mark.converter_mark +class TestContainerNames(BaseTestCase): + def test_generate_container_name_short_suffix_is_untouched(self): + name = generate_container_name( + INIT_ARTIFACTS_CONTAINER_PREFIX, "s3-store", False + ) + assert name == "polyaxon-init-artifacts-s3-store" + + def test_generate_container_name_replaces_underscores(self): + name = generate_container_name( + INIT_ARTIFACTS_CONTAINER_PREFIX, "my_store", False + ) + assert name == "polyaxon-init-artifacts-my-store" + + def test_generate_container_name_respects_k8s_limit(self): + for prefix in ( + INIT_ARTIFACTS_CONTAINER_PREFIX, + INIT_GIT_CONTAINER_PREFIX, + ): + for unique in (True, False): + name = generate_container_name(prefix, LONG_CONNECTION_NAME, unique) + assert len(name) <= MAX_CONTAINER_NAME_LENGTH + assert name.startswith(prefix) + + def test_generate_container_name_keeps_unique_value_when_truncating(self): + names = { + generate_container_name( + INIT_ARTIFACTS_CONTAINER_PREFIX, LONG_CONNECTION_NAME + ) + for _ in range(100) + } + # Truncation must not collapse distinct runs onto the same name. + assert len(names) == 100 + + def test_generate_container_name_is_a_valid_dns_label(self): + name = generate_container_name( + INIT_ARTIFACTS_CONTAINER_PREFIX, LONG_CONNECTION_NAME + ) + assert not name.startswith("-") + assert not name.endswith("-") + assert name == name.lower() + + def test_sanitize_container_name_truncates(self): + name = sanitize_container_name( + "polyaxon-init-artifacts-" + LONG_CONNECTION_NAME + ) + assert len(name) <= MAX_CONTAINER_NAME_LENGTH + assert not name.endswith("-") + + def test_sanitize_container_name_leaves_valid_names_alone(self): + assert sanitize_container_name("polyaxon_init_Artifacts") == ( + "polyaxon-init-artifacts" + )