Commit 1dc7b758 authored by Konstantinos Togias's avatar Konstantinos Togias
Browse files

Merge branch 'fix/helm-release-name' into 'refactor/tfsdk-architecture-alignment'

fix(kubernetes): derive Helm names accepted by Kubernetes

See merge request !201
parents 1c4dcfa2 b282d0a2
Loading
Loading
Loading
Loading
+1 −0
Changes for src/openop_tfsdk/_diagnostics.py: 1 added line, 0 removed lines.
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ _HELM_REASONS = {
    "forbidden": "authorization_denied",
    "release: not found": "release_not_found",
    "cannot re-use a name": "release_name_in_use",
    "is invalid": "invalid_kubernetes_object",
}


+22 −3
Changes for src/openop_tfsdk/contract/adapters/kubernetes.py: 22 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ What a kubernetes control path can execute. Not topology, so not read from
the bootstrap YAML.
"""
import json
import re
import subprocess
from pathlib import Path
from typing import Optional
@@ -82,6 +83,24 @@ def _chart_args(unit) -> list:
    ]


# helm's own limit; charts name their objects after the release, often with a suffix
_RELEASE_NAME_MAX = 53


def release_name(deployment_unit_ref: str) -> str:
    """A helm release name that charts can build Kubernetes object names from.

    Charts name Services after the release, and a Service name is a DNS-1035 label:
    lowercase letters, digits and '-', starting with a letter. A deployment unit ref is
    often a UUID or an instance name with '_', so it is made into one. A ref that already is
    one is used as it is.
    """
    name = re.sub(r"[^a-z0-9-]+", "-", deployment_unit_ref.lower()).strip("-")
    if not name[:1].isalpha():
        name = f"r-{name}" if name else "r"
    return name[:_RELEASE_NAME_MAX].rstrip("-")


class KubernetesCapabilityAdapter:
    """Declares the kubernetes execution surface and executes it through helm."""

@@ -100,13 +119,13 @@ class KubernetesCapabilityAdapter:
        unit = request.deployment_unit
        if unit is None or not unit.artifact_ref:
            raise ValidationError("activate needs a deployment_unit with an artifact_ref")
        release_name = unit.deployment_unit_ref
        release = release_name(unit.deployment_unit_ref)
        namespace = self._infrastructure.get("namespace", "default")

        result = _helm(
            [
                "install",
                release_name,
                release,
                *_chart_args(unit),
                "--namespace",
                namespace,
@@ -128,7 +147,7 @@ class KubernetesCapabilityAdapter:
            operation_id=request.operation_id,
            capability_kind=request.capability_kind,
            state=LifecycleState.ACTIVE,
            external_id=release_name,
            external_id=release,
        )

    def read_runtime_state(self, request: CapabilityRuntimeStateRequest) -> CapabilityRuntimeState:
+73 −0
Changes for tests/contract/test_kubernetes_release_name.py: 73 added lines, 0 removed lines.
Original line number Diff line number Diff line
"""Helm release names must work as Kubernetes object names, whatever the unit ref is."""

import re
import subprocess
from types import SimpleNamespace

import pytest

from openop_tfsdk._diagnostics import helm_diagnostics
from openop_tfsdk.contract.adapters.kubernetes import (
    KubernetesCapabilityAdapter,
    release_name,
)

DNS_1035 = re.compile(r"^[a-z]([-a-z0-9]*[a-z0-9])?$")


@pytest.mark.parametrize(
    ("ref", "expected"),
    [
        ("podinfobcn01", "podinfobcn01"),
        ("podinfo-bcn-01", "podinfo-bcn-01"),
        ("261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f", "r-261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f"),
        ("podinfo_bcn_01", "podinfo-bcn-01"),
        ("Video_Athens_01", "video-athens-01"),
        ("__", "r"),
    ],
)
def test_release_name_is_a_dns_1035_label(ref, expected):
    name = release_name(ref)

    assert name == expected
    assert DNS_1035.match(name)


def test_release_name_fits_helm_limit():
    name = release_name("a" * 40 + "_" + "b" * 40)

    assert len(name) <= 53
    assert DNS_1035.match(name)


def test_activate_installs_under_the_safe_name_and_reports_it(monkeypatch):
    calls = []

    def run(args, **kwargs):
        calls.append(args)
        return subprocess.CompletedProcess(args, 0, stdout="", stderr="")

    monkeypatch.setattr(subprocess, "run", run)
    unit = SimpleNamespace(
        deployment_unit_ref="261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f",
        runtime_type="helm",
        artifact_ref="https://charts.example/podinfo-6.7.1.tgz",
    )
    request = SimpleNamespace(operation_id="op-1", capability_kind="workload", deployment_unit=unit)

    result = KubernetesCapabilityAdapter({"namespace": "oop-apps"}).activate(request)

    assert calls[0][:3] == ["helm", "install", "r-261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f"]
    # terminate, update and status use the external id, so they follow the same name
    assert result.external_id == "r-261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f"


def test_an_invalid_object_name_is_named_in_the_diagnostics():
    stderr = (
        'Error: INSTALLATION FAILED: 1 error occurred:\n\t* Service "1abc-podinfo" is invalid: '
        "metadata.name: Invalid value"
    )

    diagnostics = helm_diagnostics(subprocess.CompletedProcess([], 1, stderr=stderr))

    assert diagnostics["reason"] == "invalid_kubernetes_object"