diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index a22268f..8585026 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -35,19 +35,23 @@ jobs: run: bats tests/unit.bats # Layer 2 — real kind cluster, one mode per matrix leg (parallel, isolated). + # CRD-conflict legs run on Helm v4 so the server-side-apply default codepath + # (the version where the cert-manager-cainjector caBundle conflict surfaces) + # is actually exercised; the rest stay on v3 for broad version coverage. layer2-kind: runs-on: ubuntu-latest needs: layer1-orchestration strategy: fail-fast: false matrix: - mode: - - operator-only - - logs-only - - monitoring-only - - events-only - - crd-conflict - - context + include: + - { mode: operator-only, helm: v3.14.0 } + - { mode: logs-only, helm: v3.14.0 } + - { mode: monitoring-only, helm: v3.14.0 } + - { mode: events-only, helm: v3.14.0 } + - { mode: crd-conflict, helm: v4.1.3 } + - { mode: operator-crd-conflict, helm: v4.1.3 } + - { mode: context, helm: v3.14.0 } steps: - uses: actions/checkout@v4 with: @@ -61,9 +65,9 @@ jobs: - name: Install helm uses: azure/setup-helm@v4 with: - version: v3.14.0 + version: ${{ matrix.helm }} - - name: Run kind e2e (${{ matrix.mode }}) + - name: Run kind e2e (${{ matrix.mode }}, helm ${{ matrix.helm }}) run: bash tests/integration/run.sh layer2 ${{ matrix.mode }} - name: Dump diagnostics on failure diff --git a/last9-otel-setup.sh b/last9-otel-setup.sh index b65aaf3..ffe5933 100755 --- a/last9-otel-setup.sh +++ b/last9-otel-setup.sh @@ -1738,6 +1738,59 @@ install_operator() { # Adopt any pre-existing OTel CRDs before Helm tries to manage them adopt_otel_crds + # When OTel CRDs already exist (e.g. a prior cert-manager-based install), + # cert-manager-cainjector owns .spec.conversion.webhook.clientConfig.caBundle + # via server-side apply and re-injects it continuously. Helm v4's SSA does + # NOT pass --force-conflicts, so its CRD apply fails fatally: + # conflict with "cert-manager-cainjector" ... .caBundle + # The adoption migration above cannot win that race against an active + # controller. + # + # Unlike kube-prometheus-stack (CRDs in the chart's crds/ dir, so + # `helm show crds` + --skip-crds work), the opentelemetry-operator chart + # ships its CRDs as TEMPLATES gated by crds.create — `helm show crds` is + # empty and --skip-crds has NO effect. So instead: render the CRDs from the + # chart template and force-apply them out-of-band (upgrades schema to the + # chart version and forcibly takes field ownership from cainjector), then + # disable Helm's own CRD rendering with crds.create=false so Helm never + # applies — and never re-conflicts on — the CRDs. + # Probe for ANY pre-existing *.opentelemetry.io CRD — a partial set (e.g. + # instrumentations present but collectors absent) hits the same cainjector + # conflict, so gate on any match rather than one well-known name. The + # out-of-band force-apply below is idempotent and safe to run whenever any + # OTel CRD exists. This mirrors what adopt_otel_crds already iterates. + local disable_crds="" + if kubectl get crd -o name 2>/dev/null | grep -q '\.opentelemetry\.io$'; then + log_info "Pre-existing OpenTelemetry CRDs detected — force-applying chart CRDs and installing with crds.create=false" + + # NOTE: this script runs under `set -e` but NOT `set -o pipefail`, so a + # piped `helm template | awk | kubectl apply` would mask an upstream + # failure (a broken render or a failed apply) and still proceed to + # crds.create=false — leaving stale/missing CRDs with no error. Capture + # each stage and check it explicitly instead. + local rendered_crds + if ! rendered_crds=$(helm template opentelemetry-operator open-telemetry/opentelemetry-operator \ + --version "$OPERATOR_VERSION" --include-crds --set crds.create=true \ + $HELM_SCHEMA_FLAG 2>&1); then + log_error "Failed to render opentelemetry-operator CRDs via 'helm template'. Output: $rendered_crds" + fi + + # Isolate only the CustomResourceDefinition documents from the chart. + local crd_manifests + crd_manifests=$(printf '%s\n' "$rendered_crds" \ + | awk 'BEGIN{RS="\n---\n"} /(^|\n)kind: CustomResourceDefinition/ {print "---"; print $0}') + + if ! printf '%s' "$crd_manifests" | grep -q 'kind: CustomResourceDefinition'; then + log_error "Rendered opentelemetry-operator chart contained no CustomResourceDefinition documents; refusing to install with crds.create=false." + fi + + if ! printf '%s\n' "$crd_manifests" | kubectl apply --server-side --force-conflicts \ + --field-manager=helm -f -; then + log_error "Failed to force-apply opentelemetry-operator CRDs (kubectl apply --server-side --force-conflicts)." + fi + disable_crds="yes" + fi + # Build helm command as array for proper argument handling local helm_args=( "upgrade" "--install" @@ -1751,6 +1804,11 @@ install_operator() { "--set" "admissionWebhooks.autoGenerateCert.enabled=true" ) + # Disable Helm's CRD rendering when CRDs were pre-existing (handled above) + if [ -n "$disable_crds" ]; then + helm_args+=("--set" "crds.create=false") + fi + # Add tolerations and nodeSelector if provided if [ -n "$TOLERATIONS_FILE_PATH" ]; then log_info "Adding tolerations to operator installation..." diff --git a/tests/integration/kind-e2e.sh b/tests/integration/kind-e2e.sh index 1b795f5..441510a 100755 --- a/tests/integration/kind-e2e.sh +++ b/tests/integration/kind-e2e.sh @@ -16,6 +16,7 @@ # monitoring-only kube-prometheus-stack (metrics) # events-only Kubernetes events agent # crd-conflict Pre-seed Terraform-owned Prometheus CRDs, then monitoring-only +# operator-crd-conflict Pre-seed foreign-owned OTel CRDs, then operator-only # context monitoring-only pinned to an explicit kubectl context # all Run every mode above, each in its own cluster # @@ -188,6 +189,50 @@ test_crd_conflict() { info "✓ crd-conflict passed" } +# Reproduce the customer failure on the OTel operator path: OTel CRDs already on +# the cluster, owned by a non-Helm field manager (cert-manager-cainjector, which +# in a live cluster owns .spec.conversion.webhook.clientConfig.caBundle). The +# opentelemetry-operator chart ships CRDs as templates gated by crds.create (NOT +# the chart's crds/ dir), so the script must render them with +# `helm template --include-crds`, force-apply them out-of-band, and install with +# crds.create=false so Helm never re-applies — and never conflicts on — them. +# +# Note: this is a PATH-ASSERTION test, not a live-race test. With no running +# cainjector re-asserting ownership, adopt_otel_crds would itself resolve a +# static conflict — so the guard here is that the crds.create=false mitigation +# path is taken (catches removal of the fix), not that an active-controller race +# is won. The live race only manifests with real cert-manager + Helm v4; that is +# verified by a manual repro (cert-manager + seed operator -> cainjector owns +# caBundle -> the unpatched script reproduces the exact customer conflict and the +# patched script installs cleanly). +test_operator_crd_conflict() { + create_cluster "${CLUSTER_PREFIX}-operator-crdconflict" + + info "Pre-seeding OTel CRDs as field-manager cert-manager-cainjector" + # Pin to the operator tag matching the chart the script installs, so the + # seeded CRD schema is the one the mitigation upgrades from — not a moving + # `main` that can drift. Chart OPERATOR_VERSION=0.92.1 (last9-otel-setup.sh) + # has appVersion 0.129.1, i.e. operator tag v0.129.1. Bump both together. + local crd_base="https://raw.githubusercontent.com/open-telemetry/opentelemetry-operator/v0.129.1/config/crd/bases" + for crd in \ + opentelemetry.io_opentelemetrycollectors \ + opentelemetry.io_instrumentations \ + opentelemetry.io_opampbridges; do + kubectl apply --server-side --field-manager=cert-manager-cainjector \ + -f "${crd_base}/${crd}.yaml" \ + || warn "could not pre-seed $crd (continuing)" + done + + local out + out=$(run_setup operator-only token="$DUMMY_TOKEN" endpoint="$DUMMY_OTLP" 2>&1) \ + || { echo "$out"; fail "operator-only failed with pre-existing CRDs"; } + + echo "$out" | grep -q -- "crds.create=false\|Pre-existing OpenTelemetry CRDs detected" \ + || fail "script did not take the crds.create=false path with pre-existing OTel CRDs" + assert_rollout deployment opentelemetry-operator + info "✓ operator-crd-conflict passed" +} + test_context() { create_cluster "${CLUSTER_PREFIX}-context" # Use the existing context when provided, else the kind- context @@ -226,17 +271,19 @@ main() { monitoring-only) test_monitoring_only ;; events-only) test_events_only ;; crd-conflict) test_crd_conflict ;; + operator-crd-conflict) test_operator_crd_conflict ;; context) test_context ;; all) - test_operator_only; cleanup_cluster - test_logs_only; cleanup_cluster - test_monitoring_only; cleanup_cluster - test_events_only; cleanup_cluster - test_crd_conflict; cleanup_cluster + test_operator_only; cleanup_cluster + test_logs_only; cleanup_cluster + test_monitoring_only; cleanup_cluster + test_events_only; cleanup_cluster + test_crd_conflict; cleanup_cluster + test_operator_crd_conflict; cleanup_cluster test_context ;; *) - echo "Usage: $0 " >&2 + echo "Usage: $0 " >&2 exit 1 ;; esac diff --git a/tests/test_install_operator.py b/tests/test_install_operator.py new file mode 100644 index 0000000..b2bf996 --- /dev/null +++ b/tests/test_install_operator.py @@ -0,0 +1,231 @@ +#!/usr/bin/env python3 +""" +Tests for install_operator CRD-conflict handling in last9-otel-setup.sh + +Reproduces the cert-manager-cainjector SSA field-conflict on +.spec.conversion.webhook.clientConfig.caBundle: when OTel CRDs already exist, +Helm's server-side apply collides with cert-manager-cainjector and the install +fails. Unlike kube-prometheus-stack (CRDs in the chart's crds/ dir, where +--skip-crds works), opentelemetry-operator ships CRDs as templates gated by +crds.create, so --skip-crds is a no-op. The fix instead renders the chart CRDs +out-of-band, force-applies them (kubectl apply --server-side --force-conflicts), +and passes --set crds.create=false to Helm so it never re-applies the CRDs. + +Uses mock kubectl/helm/sleep binaries (PATH override) — no cluster needed. +Run: python3 tests/test_install_operator.py +""" + +import os +import stat +import subprocess +import tempfile +import textwrap +import unittest + +SCRIPT = os.path.join(os.path.dirname(__file__), "..", "last9-otel-setup.sh") +SCRIPT = os.path.abspath(SCRIPT) + +# Mock kubectl: drives adopt_otel_crds + the pre-existing-CRD detection. +# SCENARIO "crds_exist" -> `get crd opentelemetrycollectors...` exits 0 (present) +# SCENARIO "no_crds" -> that probe exits 1 (absent), and `get crd -o name` empty +MOCK_KUBECTL = textwrap.dedent("""\ + #!/bin/bash + MOCK_DIR="$(cd "$(dirname "$0")" && pwd)" + echo "kubectl $*" >> "$MOCK_DIR/calls.log" + SCENARIO="$(cat "$MOCK_DIR/scenario")" + case "$*" in + "get crd opentelemetrycollectors.opentelemetry.io") + [ "$SCENARIO" = "crds_exist" ] && exit 0 || exit 1 + ;; + "get crd -o name") + [ "$SCENARIO" = "no_crds" ] && exit 0 + printf 'crd/opentelemetrycollectors.opentelemetry.io\\n' + printf 'crd/instrumentations.opentelemetry.io\\n' + ;; + "apply --server-side"*) + # The out-of-band force-apply reads the filtered manifests on stdin. + # Capture them so a test can assert the awk filter passed CRDs only. + cat > "$MOCK_DIR/applied.yaml" + ;; + *"metadata.labels.app"*) echo "" ;; + *"metadata.annotations.meta"*) echo "" ;; + *"-o yaml"*) + echo "apiVersion: apiextensions.k8s.io/v1" + echo "kind: CustomResourceDefinition" + ;; + *) exit 0 ;; + esac +""") + +# Mock helm: record every invocation, succeed. `template` emits Helm-realistic +# multi-doc output — each doc prefixed with a `# Source:` comment, no leading +# `---` on the first doc — so the awk CRD-only filter in install_operator is +# tested against the actual shape `helm template --include-crds` produces, not a +# sanitized one. Two CRDs + a Deployment, to prove the filter keeps both CRDs +# and drops the Deployment. +MOCK_HELM = textwrap.dedent("""\ + #!/bin/bash + MOCK_DIR="$(cd "$(dirname "$0")" && pwd)" + echo "helm $*" >> "$MOCK_DIR/calls.log" + case "$*" in + "template"*) + printf -- '# Source: opentelemetry-operator/crds/crd-opentelemetrycollector.yaml\\n' + printf 'apiVersion: apiextensions.k8s.io/v1\\n' + printf 'kind: CustomResourceDefinition\\n' + printf 'metadata:\\n name: opentelemetrycollectors.opentelemetry.io\\n' + printf -- '---\\n' + printf '# Source: opentelemetry-operator/crds/crd-instrumentation.yaml\\n' + printf 'apiVersion: apiextensions.k8s.io/v1\\n' + printf 'kind: CustomResourceDefinition\\n' + printf 'metadata:\\n name: instrumentations.opentelemetry.io\\n' + printf -- '---\\n' + printf '# Source: opentelemetry-operator/templates/deployment.yaml\\n' + printf 'apiVersion: apps/v1\\n' + printf 'kind: Deployment\\n' + printf 'metadata:\\n name: opentelemetry-operator\\n' + ;; + *) exit 0 ;; + esac +""") + +MOCK_SLEEP = "#!/bin/bash\nexit 0\n" + +RUNNER = textwrap.dedent("""\ + #!/bin/bash + source '{script}' + install_operator +""") + + +def run_install(scenario: str) -> tuple[list[str], str]: + """Run install_operator with mocked kubectl/helm/sleep. + + Returns (calls, applied) where `calls` is every recorded CLI invocation and + `applied` is the manifest stream the out-of-band force-apply received on + stdin (empty string if no force-apply ran).""" + with tempfile.TemporaryDirectory() as mock_dir: + with open(f"{mock_dir}/scenario", "w") as f: + f.write(scenario) + open(f"{mock_dir}/calls.log", "w").close() + + for name, body in ( + ("kubectl", MOCK_KUBECTL), + ("helm", MOCK_HELM), + ("sleep", MOCK_SLEEP), + ): + p = f"{mock_dir}/{name}" + with open(p, "w") as f: + f.write(body) + os.chmod(p, os.stat(p).st_mode | stat.S_IEXEC) + + runner_path = f"{mock_dir}/run.sh" + with open(runner_path, "w") as f: + f.write(RUNNER.format(script=SCRIPT)) + os.chmod(runner_path, os.stat(runner_path).st_mode | stat.S_IEXEC) + + env = os.environ.copy() + env["PATH"] = f"{mock_dir}:{env['PATH']}" + env["NAMESPACE"] = "last9" + env["OPERATOR_VERSION"] = "0.90.0" + env["TOLERATIONS_FILE_PATH"] = "" + env["HELM_SCHEMA_FLAG"] = "" + + result = subprocess.run( + ["bash", runner_path], env=env, capture_output=True, text=True + ) + + with open(f"{mock_dir}/calls.log") as f: + calls = f.read().splitlines() + + applied = "" + applied_path = f"{mock_dir}/applied.yaml" + if os.path.exists(applied_path): + with open(applied_path) as f: + applied = f.read() + + if result.returncode != 0: + raise RuntimeError( + f"install_operator failed (exit {result.returncode})\n" + f"stderr: {result.stderr[:500]}" + ) + return calls, applied + + +class TestInstallOperator(unittest.TestCase): + + def test_preexisting_crds_rendered_from_template(self): + """The opentelemetry-operator chart ships CRDs as templates (not crds/), + so they must be rendered with `helm template --include-crds`, NOT + `helm show crds` (which is empty for this chart).""" + calls, _ = run_install("crds_exist") + tmpl = [c for c in calls if c.startswith("helm template") and "--include-crds" in c] + self.assertTrue(tmpl, f"Expected 'helm template --include-crds': {calls}") + self.assertFalse( + [c for c in calls if c.startswith("helm show crds")], + f"Must NOT use 'helm show crds' (empty for this chart): {calls}", + ) + + def test_preexisting_crds_applied_with_force_conflicts(self): + """Rendered CRDs are force-applied out-of-band to steal caBundle ownership + from cert-manager-cainjector and upgrade schema before Helm runs.""" + calls, _ = run_install("crds_exist") + force_applies = [ + c + for c in calls + if c.startswith("kubectl apply --server-side") and "--force-conflicts" in c + ] + self.assertTrue( + force_applies, f"Expected an out-of-band force-conflicts apply: {calls}" + ) + + def test_preexisting_crds_helm_disables_crd_creation(self): + """When OTel CRDs pre-exist, the helm install must pass crds.create=false + (NOT --skip-crds, which is a no-op for template-rendered CRDs) so Helm + does not re-conflict with cert-manager-cainjector on caBundle.""" + calls, _ = run_install("crds_exist") + install = [c for c in calls if c.startswith("helm upgrade --install")] + self.assertEqual(len(install), 1, f"Expected one helm install: {install}") + self.assertIn( + "crds.create=false", install[0], f"Helm install must disable CRDs: {install[0]}" + ) + self.assertNotIn( + "--skip-crds", install[0], + f"--skip-crds is ineffective for this chart, must not be used: {install[0]}", + ) + + def test_no_crds_helm_owns_crds(self): + """When no OTel CRDs pre-exist, Helm installs CRDs itself — no + crds.create=false and no out-of-band CRD render/apply.""" + calls, _ = run_install("no_crds") + install = [c for c in calls if c.startswith("helm upgrade --install")] + self.assertEqual(len(install), 1, f"Expected one helm install: {install}") + self.assertNotIn( + "crds.create=false", install[0], + f"Fresh install should let Helm create CRDs: {install[0]}", + ) + self.assertFalse( + [c for c in calls if c.startswith("helm template")], + f"No out-of-band CRD render when CRDs absent: {calls}", + ) + + def test_force_apply_receives_crds_only(self): + """The awk filter must isolate CRD docs from realistic `helm template` + output (each doc carries a `# Source:` comment) — every + CustomResourceDefinition is force-applied and the Deployment is dropped, + so the out-of-band apply never touches non-CRD manifests.""" + _, applied = run_install("crds_exist") + self.assertTrue(applied, "Expected the force-apply to receive manifests on stdin") + self.assertEqual( + applied.count("kind: CustomResourceDefinition"), 2, + f"Both rendered CRDs must be force-applied:\n{applied}", + ) + self.assertIn("opentelemetrycollectors.opentelemetry.io", applied) + self.assertIn("instrumentations.opentelemetry.io", applied) + self.assertNotIn( + "kind: Deployment", applied, + f"Non-CRD docs must be filtered out before force-apply:\n{applied}", + ) + + +if __name__ == "__main__": + unittest.main(verbosity=2)