Skip to content

Commit

Permalink
Refactor Helm nameOverride (#397)
Browse files Browse the repository at this point in the history 
Fixes #327 

Remove previous workaround for setting Helm app `nameOverride` in favor
of a more robust implementation in `HelmApp`
  • Loading branch information
@disrupted
disrupted authored Jan 2, 2024
1 parent 4a41594 commit c256a03
Show file tree
Hide file tree
Showing 18 changed files with 166 additions and 171 deletions.
104 changes: 59 additions & 45 deletions docs/docs/schema/pipeline.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@
"app": {
"allOf": [
{
"$ref": "#/$defs/KubernetesAppConfig"
"$ref": "#/$defs/HelmAppValues"
}
],
"description": "Application-specific settings"
"description": "Helm app values"
},
"from": {
"anyOf": [
Expand Down Expand Up @@ -146,6 +146,27 @@
"title": "HelmApp",
"type": "object"
},
"HelmAppValues": {
"additionalProperties": true,
"description": "Helm app values.",
"properties": {
"nameOverride": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": null,
"description": "Override name with this value",
"title": "Nameoverride"
}
},
"title": "HelmAppValues",
"type": "object"
},
"HelmRepoConfig": {
"description": "Helm repository configuration.",
"properties": {
Expand Down Expand Up @@ -420,13 +441,6 @@
"title": "KafkaSourceConnector",
"type": "object"
},
"KubernetesAppConfig": {
"additionalProperties": true,
"description": "Settings specific to Kubernetes apps.",
"properties": {},
"title": "KubernetesAppConfig",
"type": "object"
},
"OutputTopicTypes": {
"description": "Types of output topic.\n\nOUTPUT (output topic), ERROR (error topic)",
"enum": [
Expand All @@ -443,7 +457,7 @@
"app": {
"allOf": [
{
"$ref": "#/$defs/ProducerValues"
"$ref": "#/$defs/ProducerAppValues"
}
],
"description": "Application-specific settings"
Expand Down Expand Up @@ -523,6 +537,38 @@
"title": "ProducerApp",
"type": "object"
},
"ProducerAppValues": {
"additionalProperties": true,
"description": "Settings specific to producers.",
"properties": {
"nameOverride": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": null,
"description": "Override name with this value",
"title": "Nameoverride"
},
"streams": {
"allOf": [
{
"$ref": "#/$defs/ProducerStreamsConfig"
}
],
"description": "Kafka Streams settings"
}
},
"required": [
"streams"
],
"title": "ProducerAppValues",
"type": "object"
},
"ProducerStreamsConfig": {
"additionalProperties": true,
"description": "Kafka Streams settings specific to Producer.",
Expand Down Expand Up @@ -574,38 +620,6 @@
"title": "ProducerStreamsConfig",
"type": "object"
},
"ProducerValues": {
"additionalProperties": true,
"description": "Settings specific to producers.",
"properties": {
"nameOverride": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": null,
"description": "Override name with this value",
"title": "Nameoverride"
},
"streams": {
"allOf": [
{
"$ref": "#/$defs/ProducerStreamsConfig"
}
],
"description": "Kafka Streams settings"
}
},
"required": [
"streams"
],
"title": "ProducerValues",
"type": "object"
},
"RepoAuthFlags": {
"description": "Authorisation-related flags for `helm repo`.",
"properties": {
Expand Down Expand Up @@ -680,7 +694,7 @@
"app": {
"allOf": [
{
"$ref": "#/$defs/StreamsAppConfig"
"$ref": "#/$defs/StreamsAppValues"
}
],
"description": "Application-specific settings"
Expand Down Expand Up @@ -847,7 +861,7 @@
"title": "StreamsAppAutoScaling",
"type": "object"
},
"StreamsAppConfig": {
"StreamsAppValues": {
"additionalProperties": true,
"description": "StreamsBoostrap app configurations.\nThe attributes correspond to keys and values that are used as values for the streams bootstrap helm chart.",
"properties": {
Expand Down Expand Up @@ -888,7 +902,7 @@
"required": [
"streams"
],
"title": "StreamsAppConfig",
"title": "StreamsAppValues",
"type": "object"
},
"StreamsConfig": {
Expand Down
15 changes: 4 additions & 11 deletions kpops/component_handlers/kafka_connect/model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from pydantic.json_schema import SkipJsonSchema
from typing_extensions import override

from kpops.components.base_components.helm_app import HelmAppValues
from kpops.utils.pydantic import (
CamelCaseConfigModel,
DescConfigModel,
Expand Down Expand Up @@ -92,21 +93,13 @@ class KafkaConnectConfigErrorResponse(BaseModel):
configs: list[KafkaConnectConfigDescription]


class KafkaConnectResetterConfig(CamelCaseConfigModel):
class KafkaConnectorResetterConfig(CamelCaseConfigModel):
brokers: str
connector: str
delete_consumer_group: bool | None = None
offset_topic: str | None = None


class KafkaConnectResetterValues(CamelCaseConfigModel):
class KafkaConnectorResetterValues(HelmAppValues):
connector_type: Literal["source", "sink"]
config: KafkaConnectResetterConfig
name_override: str

# TODO(Ivan Yordanov): Replace with a function decorated with `@model_serializer`
# BEWARE! All default values are enforced, hard to replicate without
# access to ``model_dump``
@override
def model_dump(self, **_) -> dict[str, Any]:
return super().model_dump(by_alias=True, exclude_none=True)
config: KafkaConnectorResetterConfig
40 changes: 35 additions & 5 deletions kpops/components/base_components/helm_app.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,47 @@
HelmUpgradeInstallFlags,
)
from kpops.component_handlers.helm_wrapper.utils import create_helm_release_name
from kpops.components.base_components.kubernetes_app import KubernetesApp
from kpops.components.base_components.kubernetes_app import (
KubernetesApp,
KubernetesAppValues,
)
from kpops.utils.colorify import magentaify
from kpops.utils.docstring import describe_attr
from kpops.utils.pydantic import exclude_by_name

log = logging.getLogger("HelmApp")


class HelmAppValues(KubernetesAppValues):
"""Helm app values.
:param name_override: Override name with this value
"""

name_override: str | None = Field(
default=None,
title="Nameoverride",
description=describe_attr("name_override", __doc__),
)

# TODO(Ivan Yordanov): Replace with a function decorated with `@model_serializer`
# BEWARE! All default values are enforced, hard to replicate without
# access to ``model_dump``
@override
def model_dump(self, **_) -> dict[str, Any]:
return super().model_dump(
by_alias=True, exclude_none=True, exclude_defaults=True
)


class HelmApp(KubernetesApp):
"""Kubernetes app managed through Helm with an associated Helm chart.
:param repo_config: Configuration of the Helm chart repo to be used for
deploying the component, defaults to None this means that the command "helm repo add" is not called and Helm
expects a path to local Helm chart.
:param version: Helm chart version, defaults to None
:param app: Helm app values
"""

repo_config: HelmRepoConfig | None = Field(
Expand All @@ -42,6 +68,10 @@ class HelmApp(KubernetesApp):
default=None,
description=describe_attr("version", __doc__),
)
app: HelmAppValues = Field(
default=...,
description=describe_attr("app", __doc__),
)

@cached_property
def helm(self) -> Helm:
Expand Down Expand Up @@ -74,7 +104,7 @@ def helm_release_name(self) -> str:
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)
return create_helm_release_name(self.full_name + suffix, suffix)

@property
def helm_chart(self) -> str:
Expand Down Expand Up @@ -149,9 +179,9 @@ def to_helm_values(self) -> dict:
:returns: Thte values to be used by Helm
"""
return self.app.model_dump(
by_alias=True, exclude_none=True, exclude_defaults=True
)
if self.app.name_override is None:
self.app.name_override = self.full_name
return self.app.model_dump()

def print_helm_diff(self, stdout: str) -> None:
"""Print the diff of the last and current release of this component.
Expand Down
15 changes: 3 additions & 12 deletions kpops/components/base_components/kafka_app.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@
HelmRepoConfig,
HelmUpgradeInstallFlags,
)
from kpops.components.base_components.helm_app import HelmApp
from kpops.components.base_components.kubernetes_app import (
KubernetesAppConfig,
)
from kpops.components.base_components.helm_app import HelmApp, HelmAppValues
from kpops.utils.docstring import describe_attr
from kpops.utils.pydantic import CamelCaseConfigModel, DescConfigModel

Expand All @@ -37,21 +34,15 @@ class KafkaStreamsConfig(CamelCaseConfigModel, DescConfigModel):
)


class KafkaAppConfig(KubernetesAppConfig):
class KafkaAppValues(HelmAppValues):
"""Settings specific to Kafka Apps.
:param streams: Kafka streams config
:param name_override: Override name with this value
"""

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


class KafkaApp(HelmApp, ABC):
Expand All @@ -66,7 +57,7 @@ class KafkaApp(HelmApp, ABC):
:param version: Helm chart version, defaults to "2.9.0"
"""

app: KafkaAppConfig = Field(
app: KafkaAppValues = Field(
default=...,
description=describe_attr("app", __doc__),
)
Expand Down
10 changes: 5 additions & 5 deletions kpops/components/base_components/kafka_connector.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
from kpops.component_handlers.helm_wrapper.utils import create_helm_release_name
from kpops.component_handlers.kafka_connect.model import (
KafkaConnectorConfig,
KafkaConnectorResetterConfig,
KafkaConnectorResetterValues,
KafkaConnectorType,
KafkaConnectResetterConfig,
KafkaConnectResetterValues,
)
from kpops.components.base_components.base_defaults_component import deduplicate
from kpops.components.base_components.models.from_section import FromTopic
Expand Down Expand Up @@ -175,7 +175,7 @@ def _run_connect_resetter(
:param dry_run: If the cleanup should be run in dry run mode or not
:param retain_clean_jobs: If the cleanup job should be kept
:param kwargs: Other values for the KafkaConnectResetter
:param kwargs: Other values for the KafkaConnectorResetter
"""
log.info(
magentaify(
Expand Down Expand Up @@ -236,8 +236,8 @@ def _get_kafka_connect_resetter_values(
:return: The Helm chart values of the connector resetter
"""
return {
**KafkaConnectResetterValues(
config=KafkaConnectResetterConfig(
**KafkaConnectorResetterValues(
config=KafkaConnectorResetterConfig(
connector=self.full_name,
brokers=self.config.kafka_brokers,
**kwargs,
Expand Down
4 changes: 2 additions & 2 deletions kpops/components/base_components/kubernetes_app.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
)


class KubernetesAppConfig(CamelCaseConfigModel, DescConfigModel):
class KubernetesAppValues(CamelCaseConfigModel, DescConfigModel):
"""Settings specific to Kubernetes apps."""

model_config = ConfigDict(
Expand All @@ -39,7 +39,7 @@ class KubernetesApp(PipelineComponent, ABC):
default=...,
description=describe_attr("namespace", __doc__),
)
app: KubernetesAppConfig = Field(
app: KubernetesAppValues = Field(
default=...,
description=describe_attr("app", __doc__),
)
Expand Down
4 changes: 2 additions & 2 deletions kpops/components/streams_bootstrap/producer/model.py
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from pydantic import ConfigDict, Field

from kpops.components.base_components.kafka_app import (
KafkaAppConfig,
KafkaAppValues,
KafkaStreamsConfig,
)
from kpops.utils.docstring import describe_attr
Expand All @@ -22,7 +22,7 @@ class ProducerStreamsConfig(KafkaStreamsConfig):
)


class ProducerValues(KafkaAppConfig):
class ProducerAppValues(KafkaAppValues):
"""Settings specific to producers.
:param streams: Kafka Streams settings
Expand Down
Loading

0 comments on commit c256a03

Please sign in to comment.