Skip to content

fix(operator): a Reconcile adopts the live objects the record renders before helm upgrades them - #3775

Merged
meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/operator-adopts-record-rendered-resources
Sep 9, 2026
Merged

meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/operator-adopts-record-rendered-resources

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

A Reconcile adopts the live objects the record renders before helm upgrades them

Measured 2026-09-09 01:20Z on memex (Deployments/memex-reconcile-20260909-operator-0b79490, through the API, on operator image 0b79490 from the record): steps 1–3 at size, namespace memex exists (#3770 works), then the first record-driven helm upgrade this instance ever reached:

UPGRADE FAILED: Unable to continue with update: SecretProviderClass "memex-kv" in namespace "memex"
exists and cannot be imported into the current release: invalid ownership metadata

memex-kv (keyVaultSecretClasses[0] on the record) was hand-applied in 2026-08 and never in any release. The committed overlay never renders it, so the config repo's adopt action (2026-09-06) never stamped it; the record does render it. --atomic rolled back; memex untouched.

What changes

hosting-deploy, after ensure-namespace and before helm upgrade:

  1. helm template from the record values → the rendered resource names (kubectl create --dry-run=client -o name).
  2. Per resource: absent → helm creates it; owned by this release → skip; live with no ownership → annotate release-name/release-namespace + label managed-by=Helm (metadata only, no spec change, nothing restarted); owned by another release → refuse, helm never runs (two releases claiming one object is how a teardown deletes the wrong instance's Secret).
  3. Logged per resource; ::hosting:: adopted=<n> for the mesh.

Tests (stubs/deploy, fixtures/deploy: a three-object estate — absent, owned, unowned)

adopted=1 on the memex shape · the three counts logged · ownership stamped on exactly the SPC · owned/absent never touched · adoption BEFORE helm upgrade · a second run adopts 0 (the stub rewrites the live object) · takeover refused with no helm upgrade in the log. 144 passed, 0 failed. RBAC coverage: 0 missing (annotate/label = patch on secretproviderclasses, granted).

After this lands

Operator image → Deployments/memex.operator.image (API) → the Reconcile again; expected: adopted=1, then helm upgrade from the record succeeds. This is the last known gap on memex's record→cluster path. ClientB is separately blocked on its image (cr.meshweaver.cloud holds none — Roland's call between ACR 8131 and #3760).

… before helm upgrades them

Measured 2026-09-09 01:20Z on memex (Deployments/memex-reconcile-20260909-operator-0b79490): the
first record-driven Reconcile to reach helm failed —

  UPGRADE FAILED: Unable to continue with update: SecretProviderClass "memex-kv" in namespace
  "memex" exists and cannot be imported into the current release: invalid ownership metadata

memex-kv (keyVaultSecretClasses[0] on the record) was hand-applied in 2026-08 and never in any
release; the committed overlay never renders it, so the config repo's adopt action (2026-09-06)
never stamped it; the record does render it. --atomic rolled back.

hosting-deploy, before helm upgrade: `helm template` from the record values → the rendered
resource names → for each: absent (helm creates), owned by this release (skip), live without
ownership (annotate release-name/release-namespace + label managed-by=Helm — metadata only, no
spec change, nothing restarted), owned by ANOTHER release (refuse; helm never runs). Logged per
resource, `::hosting:: adopted=<n>`.

Tests (stubs/deploy + fixtures/deploy): adopted=1 on the memex shape, counts logged, ownership
stamped on exactly the SPC, owned/absent untouched, adoption BEFORE helm, second run adopted=0,
takeover refused with no helm upgrade. 144 passed, 0 failed. RBAC coverage: 0 missing (annotate/
label = patch on secretproviderclasses, granted). What's New (Fix).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 9, 2026 01:27
@meshweaver-cloud
meshweaver-cloud Bot enabled auto-merge September 9, 2026 01:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new adoption logic currently misclassifies non-NotFound kubectl get failures as “absent” and it doesn’t validate meta.helm.sh/release-namespace when deciding an object is already owned, both of which can cause misleading logs or continued Helm ownership failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a pre-helm upgrade “adoption” step to the AKS hosting operator so a Reconcile can take ownership of already-existing, record-rendered Kubernetes objects (e.g., a previously hand-applied SecretProviderClass) by stamping Helm ownership metadata, preventing helm upgrade from failing on “invalid ownership metadata”.

Changes:

  • hosting-deploy now templates the chart from record-derived values, enumerates rendered resources, and adopts unowned live resources by applying Helm ownership annotations/label (refusing conflicts).
  • Adds an operator test harness (helm/kubectl stubs + fixtures) and new run-tests.sh cases covering absent/owned/unowned/conflict estates and ordering (adopt before upgrade).
  • Adds a What’s New entry documenting the incident and the new behavior.
File summaries
File Description
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-09-a-reconcile-adopts-what-the-record-renders.md Documents the reconcile failure mode and the new adoption behavior.
deploy/aks/operator/bin/hosting-deploy Implements the “adopt unowned live objects rendered by the record” step before helm upgrade.
deploy/aks/operator/test/run-tests.sh Adds end-to-end stubbed tests asserting adoption behavior, idempotency, refusal on conflict, and ordering vs helm upgrade.
deploy/aks/operator/test/stubs/deploy/kubectl Stub kubectl implementation for adoption tests (get/create/annotate/label + live-object rewrites).
deploy/aks/operator/test/stubs/deploy/helm Stub helm implementation for adoption tests (template + upgrade logging).
deploy/aks/operator/test/fixtures/deploy/rendered.yaml Fixture manifest representing what the record renders for adoption enumeration.
deploy/aks/operator/test/fixtures/deploy/names.txt Fixture list of rendered resource names returned by the kubectl stub.
deploy/aks/operator/test/fixtures/deploy/live/secretproviderclass.secrets-store.csi.x-k8s.io_memex-kv.json Fixture live unowned SPC object to be adopted.
deploy/aks/operator/test/fixtures/deploy/live/configmap_memex-portal-config.json Fixture live already-owned object (control case for “skip”).
Review details
  • Files reviewed: 8/9 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +3 to +4
# $HOSTING_DEPLOY_STUB_LOG. The live estate is $HOSTING_DEPLOY_FIXTURE/live/<name>.json, one file
# per rendered resource name (kind.group/name with '/' → '__'); absent file → NotFound.
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

254 tests   254 ✅  2m 20s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 4a9086e.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

496 tests   496 ✅  38s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 4a9086e.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

389 tests   389 ✅  44s ⏱️
  2 suites    0 💤
  2 files      0 ❌

Results for commit 4a9086e.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 857 tests   1 857 ✅  2m 18s ⏱️
    4 suites      0 💤
    4 files        0 ❌

Results for commit 4a9086e.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 141 tests   1 139 ✅  3m 38s ⏱️
    3 suites      2 💤
    3 files        0 ❌

Results for commit 4a9086e.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    6 files      6 suites   4m 12s ⏱️
2 473 tests 2 281 ✅ 192 💤 0 ❌
2 474 runs  2 282 ✅ 192 💤 0 ❌

Results for commit 4a9086e.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Test Results

   17 files     17 suites   13m 51s ⏱️
6 610 tests 6 416 ✅ 194 💤 0 ❌
6 611 runs  6 417 ✅ 194 💤 0 ❌

Results for commit 4a9086e.

@meshweaver-cloud
meshweaver-cloud Bot added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit 60a5964 Sep 9, 2026
34 checks passed
@rbuergi
rbuergi deleted the fix/operator-adopts-record-rendered-resources branch October 10, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants