Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use hash and trim long Helm release names instead of only trimming #390

Merged
merged 34 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
00db2d3
Use fullnameOverride insted of nameOverride for the deployments
raminqaf Nov 20, 2023
a2101ee
Fix Kafka Resetter fullnameOverride value
raminqaf Nov 20, 2023
5ea5b7a
Fix Kafka Resetter fullnameOverride value
raminqaf Nov 20, 2023
9fdbfed
Use sha-1 hash to shorten clean up job release names
raminqaf Nov 21, 2023
76e1d03
Update files
raminqaf Nov 21, 2023
a211004
Change helm release name to sha-1 hashed strings
raminqaf Nov 21, 2023
9db707e
Improve helm release name creation
raminqaf Nov 21, 2023
2ad0c05
Improve tests
raminqaf Nov 21, 2023
fcec5eb
Improve tests
raminqaf Nov 21, 2023
c178617
fix helm name
raminqaf Dec 6, 2023
27d7782
Merge branch 'main' into feature/use-fullname-override
raminqaf Dec 6, 2023
6a77faa
add reviews
raminqaf Dec 8, 2023
cd5f65d
Fix tests
raminqaf Dec 8, 2023
eca8256
Merge v3
raminqaf Dec 8, 2023
7e8bb11
Merge branch 'v3' of github.com:bakdata/kpops into feature/use-fullna…
raminqaf Dec 8, 2023
ebbd5b0
fix title
raminqaf Dec 8, 2023
c3f6afa
add clean suffix to helm clean release name
raminqaf Dec 11, 2023
33a5d54
add tests
raminqaf Dec 12, 2023
8992d75
Update files
raminqaf Dec 12, 2023
b1344e5
merge v3
raminqaf Dec 12, 2023
db6e874
Add migration guide
raminqaf Dec 12, 2023
678ca56
Merge branch 'v3' of github.com:bakdata/kpops into feature/use-fullna…
raminqaf Dec 12, 2023
edc540f
Update files
raminqaf Dec 12, 2023
29e5cb5
Update files
raminqaf Dec 12, 2023
5d98313
Merge branch 'v3' of github.com:bakdata/kpops into feature/use-fullna…
raminqaf Dec 21, 2023
873553b
merge v3
raminqaf Dec 21, 2023
539efab
Merge branch 'v3' into feature/use-fullname-override
raminqaf Dec 21, 2023
64f46d3
Merge branch 'v3' into feature/use-fullname-override
raminqaf Dec 21, 2023
5fa0844
Merge branch 'v3' into feature/use-fullname-override
raminqaf Dec 22, 2023
e1fe2ec
Fix typo
raminqaf Dec 22, 2023
13935eb
add nameOverride
raminqaf Jan 2, 2024
e2d6394
Update files
raminqaf Jan 2, 2024
659f53a
Update files
raminqaf Jan 2, 2024
cddd344
Update files
raminqaf Jan 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions docs/docs/schema/pipeline.json
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@
"additionalProperties": true,
"description": "Settings specific to producers.",
"properties": {
"nameOverride": {
"fullnameOverride": {
"anyOf": [
{
"type": "string"
Expand All @@ -588,8 +588,8 @@
}
],
"default": null,
"description": "Override name with this value",
"title": "Nameoverride"
"description": "Fully overrides the release and chart name",
"title": "Fullname override"
},
"streams": {
"allOf": [
Expand Down Expand Up @@ -863,7 +863,7 @@
"default": null,
"description": "Kubernetes Event-driven Autoscaling config"
},
"nameOverride": {
"fullnameOverride": {
"anyOf": [
{
"type": "string"
Expand All @@ -873,8 +873,8 @@
}
],
"default": null,
"description": "Override name with this value",
"title": "Nameoverride"
"description": "Fully overrides the release and chart name",
"title": "Fullname override"
},
"streams": {
"allOf": [
Expand Down
6 changes: 5 additions & 1 deletion docs/docs/user/migration-guide/v2-v3.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Migrate from V2 to V3

## [Use hash and trim long helm release names instead of only trimming](https://github.com/bakdata/kpops/pull/390)

KPOps handles long (more than 53 characters) Helm releases names differently. Helm will not find your (long) old release names anymore. Therefore, it is recommended that you should once destroy your pipeline with KPOps v2 to remove old Helm release names. After a clean destroy, re-deploy your pipeline with the KPOps v3.

## [Make Kafka REST Proxy & Kafka Connect hosts default and improve Schema Registry config](https://github.com/bakdata/kpops/pull/354)

The breaking changes target the `config.yaml` file:
Expand Down Expand Up @@ -48,7 +52,7 @@ The variable is now called `kafka_brokers`.
...
```

## [Move GitHub action to repsitory root](https://github.com/bakdata/kpops/pull/356)
## [Move GitHub action to repository root](https://github.com/bakdata/kpops/pull/356)

The location of the GitHub action has changed, and it's now available directly as `bakdata/kpops`.

Expand Down
24 changes: 16 additions & 8 deletions kpops/component_handlers/helm_wrapper/utils.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,30 @@
import hashlib
import logging

log = logging.getLogger("HelmUtils")


ENCODING = "utf-8"
RELEASE_NAME_MAX_LEN = 52


def trim_release_name(name: str, suffix: str = "") -> str:
"""Trim Helm release name while preserving suffix.
def create_helm_release_name(name: str, suffix: str = "") -> str:
"""Shortens the long Helm release name.

Creates a 52 character long release name if the name length exceeds the Helm release character length.
It first trims the string and fetches the first RELEASE_NAME_MAX_LEN - len(suffix) characters.
Then it replaces the last 6 characters with the SHA-1 encoded string (with "-") to avoid collision
and append the suffix if given.

:param name: The release name including optional suffix
:param name: The Helm release name to be shortened.
:param suffix: The release suffix to preserve
:return: Truncated release name.
:return: Trimmed + hashed version of the release name if it exceeds the Helm release character length otherwise the actual release name
"""
if len(name) > RELEASE_NAME_MAX_LEN:
new_name = name[: (RELEASE_NAME_MAX_LEN - len(suffix))] + suffix
exact_name = name[: RELEASE_NAME_MAX_LEN - len(suffix)]
hash_name = hashlib.sha1(name.encode(ENCODING)).hexdigest()
new_name = exact_name[:-6] + "-" + hash_name[:5] + suffix
log.critical(
f"Invalid Helm release name '{name}'. Truncating to {RELEASE_NAME_MAX_LEN} characters: \n {name} --> {new_name}"
f"Invalid Helm release name '{name}'. Truncating and hashing the release name: \n {name} --> {new_name}"
)
name = new_name
return new_name
return name
2 changes: 1 addition & 1 deletion kpops/component_handlers/kafka_connect/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class KafkaConnectResetterConfig(CamelCaseConfigModel):
class KafkaConnectResetterValues(CamelCaseConfigModel):
connector_type: Literal["source", "sink"]
config: KafkaConnectResetterConfig
name_override: str
fullname_override: str

# TODO(Ivan Yordanov): Replace with a function decorated with `@model_serializer`
# BEWARE! All default values are enforced, hard to replicate without
Expand Down
9 changes: 8 additions & 1 deletion kpops/components/base_components/helm_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
HelmTemplateFlags,
HelmUpgradeInstallFlags,
)
from kpops.component_handlers.helm_wrapper.utils import create_helm_release_name
from kpops.components.base_components.kubernetes_app import KubernetesApp
from kpops.utils.colorify import magentaify
from kpops.utils.docstring import describe_attr
Expand Down Expand Up @@ -67,7 +68,13 @@ def dry_run_handler(self) -> DryRunHandler:
@property
def helm_release_name(self) -> str:
"""The name for the Helm release. Can be overridden."""
return self.full_name
return create_helm_release_name(self.full_name)

@property
def clean_release_name(self) -> str:
"""The name for the Helm release for cleanup jobs. Can be overridden."""
suffix = "-clean"
return create_helm_release_name(self.helm_release_name, suffix)

@property
def helm_chart(self) -> str:
Expand Down
40 changes: 17 additions & 23 deletions kpops/components/base_components/kafka_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
HelmRepoConfig,
HelmUpgradeInstallFlags,
)
from kpops.component_handlers.helm_wrapper.utils import trim_release_name
from kpops.components.base_components.helm_app import HelmApp
from kpops.components.base_components.kubernetes_app import KubernetesAppConfig
from kpops.components.base_components.kubernetes_app import (
KubernetesAppConfig,
)
from kpops.utils.docstring import describe_attr
from kpops.utils.pydantic import CamelCaseConfigModel, DescConfigModel

Expand Down Expand Up @@ -40,14 +41,16 @@ class KafkaAppConfig(KubernetesAppConfig):
"""Settings specific to Kafka Apps.

:param streams: Kafka streams config
:param name_override: Override name with this value, defaults to None
:param fullname_override: Fully overrides the release and chart name, defaults to None
"""

streams: KafkaStreamsConfig = Field(
default=..., description=describe_attr("streams", __doc__)
)
name_override: str | None = Field(
default=None, description=describe_attr("name_override", __doc__)
fullname_override: str | None = Field(
default=None,
title="Fullname override",
description=describe_attr("fullname_override", __doc__),
)


Expand Down Expand Up @@ -108,28 +111,21 @@ def _run_clean_up_job(
:param values: The value YAML for the chart
:param dry_run: Dry run command
:param retain_clean_jobs: Whether to retain the cleanup job, defaults to False
:return:
"""
suffix = "-clean"
clean_up_release_name = trim_release_name(
self.helm_release_name + suffix, suffix
)
log.info(f"Uninstall old cleanup job for {clean_up_release_name}")
log.info(f"Uninstall old cleanup job for {self.clean_release_name}")

self.__uninstall_clean_up_job(clean_up_release_name, dry_run)
self.__uninstall_clean_up_job(self.clean_release_name, dry_run)

log.info(f"Init cleanup job for {clean_up_release_name}")
log.info(f"Init cleanup job for {self.clean_release_name}")

stdout = self.__install_clean_up_job(
clean_up_release_name, suffix, values, dry_run
)
stdout = self.__install_clean_up_job(self.clean_release_name, values, dry_run)

if dry_run:
self.dry_run_handler.print_helm_diff(stdout, clean_up_release_name, log)
self.dry_run_handler.print_helm_diff(stdout, self.clean_release_name, log)

if not retain_clean_jobs:
log.info(f"Uninstall cleanup job for {clean_up_release_name}")
self.__uninstall_clean_up_job(clean_up_release_name, dry_run)
log.info(f"Uninstall cleanup job for {self.clean_release_name}")
self.__uninstall_clean_up_job(self.clean_release_name, dry_run)

def __uninstall_clean_up_job(self, release_name: str, dry_run: bool) -> None:
"""Uninstall clean up job.
Expand All @@ -142,7 +138,6 @@ def __uninstall_clean_up_job(self, release_name: str, dry_run: bool) -> None:
def __install_clean_up_job(
self,
release_name: str,
suffix: str,
values: dict,
dry_run: bool,
) -> str:
Expand All @@ -152,11 +147,10 @@ def __install_clean_up_job(
:param suffix: Suffix to add to the release name, e.g. "-clean"
:param values: The Helm values for the chart
:param dry_run: Whether to do a dry run of the command
:return: Install clean up job with helm, return the output of the installation
:return: Return the output of the installation
"""
clean_up_release_name = trim_release_name(release_name, suffix)
return self.helm.upgrade_install(
clean_up_release_name,
release_name,
self.clean_up_helm_chart,
dry_run,
self.namespace,
Expand Down
7 changes: 3 additions & 4 deletions kpops/components/base_components/kafka_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
HelmTemplateFlags,
HelmUpgradeInstallFlags,
)
from kpops.component_handlers.helm_wrapper.utils import trim_release_name
from kpops.component_handlers.helm_wrapper.utils import create_helm_release_name
from kpops.component_handlers.kafka_connect.model import (
KafkaConnectorConfig,
KafkaConnectorType,
Expand Down Expand Up @@ -104,8 +104,7 @@ def helm(self) -> Helm:
@property
def _resetter_release_name(self) -> str:
suffix = "-clean"
clean_up_release_name = self.full_name + suffix
return trim_release_name(clean_up_release_name, suffix)
return create_helm_release_name(self.full_name + suffix, suffix)

@property
def _resetter_helm_chart(self) -> str:
Expand Down Expand Up @@ -244,7 +243,7 @@ def _get_kafka_connect_resetter_values(
**kwargs,
),
connector_type=self._connector_type.value,
name_override=self.full_name,
fullname_override=self.full_name + "-clean",
).model_dump(),
**self.resetter_values,
}
Expand Down
6 changes: 3 additions & 3 deletions kpops/pipeline_generator/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ def validate_unique_names(self) -> None:
@staticmethod
def _populate_component_name(component: PipelineComponent) -> None: # TODO: remove
with suppress(
AttributeError # Some components like Kafka Connect do not have a name_override attribute
AttributeError # Some components like Kafka Connect do not have a fullname_override attribute
):
if (app := getattr(component, "app")) and app.name_override is None:
app.name_override = component.full_name
if (app := getattr(component, "app")) and app.fullname_override is None:
app.fullname_override = component.full_name


def create_env_components_index(
Expand Down
48 changes: 26 additions & 22 deletions tests/component_handlers/helm_wrapper/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,33 @@
from kpops.component_handlers.helm_wrapper.utils import trim_release_name
from kpops.component_handlers.helm_wrapper.utils import (
create_helm_release_name,
)


def test_trim_release_name_with_suffix():
name = trim_release_name(
"example-component-name-too-long-fake-fakefakefakefakefake-clean",
suffix="-clean",
)
assert name == "example-component-name-too-long-fake-fakefakef-clean"
assert len(name) == 52
def test_helm_release_name_for_long_names():
long_release_name = "example-component-name-too-long-fake-fakefakefakefakefake"

actual_release_name = create_helm_release_name(long_release_name)

def test_trim_release_name_without_suffix():
name = trim_release_name(
"example-component-name-too-long-fake-fakefakefakefakefake"
)
assert name == "example-component-name-too-long-fake-fakefakefakefak"
assert len(name) == 52
expected_helm_release_name = "example-component-name-too-long-fake-fakefakef-0a7fc"
assert expected_helm_release_name == actual_release_name
assert len(expected_helm_release_name) == 52


def test_no_trim_release_name():
assert (
trim_release_name("normal-name-with-no-need-of-trim-clean", suffix="-clean")
== "normal-name-with-no-need-of-trim-clean"
)
assert (
trim_release_name("normal-name-with-no-need-of-trim")
== "normal-name-with-no-need-of-trim"
def test_helm_release_name_for_install_and_clean_must_be_different():
long_release_name = "example-component-name-too-long-fake-fakefakefakefakefake"

helm_clean_release_name = create_helm_release_name(long_release_name, "-clean")
expected_helm_release_name = (
"example-component-name-too-long-fake-fakefakef-0a7fc-clean"
)

assert expected_helm_release_name != helm_clean_release_name


def test_helm_release_name_for_short_names():
short_release_name = "example-component-name"

actual_helm_release_name = create_helm_release_name(short_release_name)

assert actual_helm_release_name == short_release_name
assert len(actual_helm_release_name) < 53
3 changes: 2 additions & 1 deletion tests/components/test_kafka_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
HelmRepoConfig,
HelmUpgradeInstallFlags,
)
from kpops.component_handlers.helm_wrapper.utils import create_helm_release_name
from kpops.components.base_components import KafkaApp
from kpops.config import KpopsConfig

Expand Down Expand Up @@ -92,7 +93,7 @@ def test_should_deploy_kafka_app(

print_helm_diff.assert_called_once()
helm_upgrade_install.assert_called_once_with(
"${pipeline_name}-example-name",
create_helm_release_name("${pipeline_name}-example-name"),
"test/test-chart",
True,
"test-namespace",
Expand Down
19 changes: 18 additions & 1 deletion tests/components/test_kafka_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
DEFAULTS_PATH = Path(__file__).parent / "resources"
CONNECTOR_NAME = "test-connector-with-long-name-0123456789abcdefghijklmnop"
CONNECTOR_FULL_NAME = "${pipeline_name}-" + CONNECTOR_NAME
CONNECTOR_CLEAN_FULL_NAME = "${pipeline_name}-test-connector-with-long-name-clean"
CONNECTOR_CLEAN_FULL_NAME = CONNECTOR_FULL_NAME + "-clean"
CONNECTOR_CLEAN_RELEASE_NAME = "${pipeline_name}-test-connector-with-lon-449ec-clean"
CONNECTOR_CLASS = "com.bakdata.connect.TestConnector"


Expand Down Expand Up @@ -111,3 +112,19 @@ def test_connector_config_name_override(
app={"connector.class": CONNECTOR_CLASS, "name": ""}, # type: ignore[reportGeneralTypeIssues]
namespace="test-namespace",
)

def test_resetter_release_name(
self,
config: KpopsConfig,
handlers: ComponentHandlers,
connector_config: KafkaConnectorConfig,
):
connector = KafkaConnector(
name=CONNECTOR_NAME,
config=config,
handlers=handlers,
app=connector_config,
namespace="test-namespace",
)
assert connector.app.name == CONNECTOR_FULL_NAME
assert connector._resetter_release_name == CONNECTOR_CLEAN_RELEASE_NAME
Loading