Repository navigation
[k8s-extension] ChaosStudio built-in role migration and namespace retention (1.9.5) - #10542
Open
Frank Li (franklixuefei) wants to merge 7 commits into
Open
Frank Li (franklixuefei) wants to merge 7 commits into
Frank Li (franklixuefei) wants to merge 7 commits into
Conversation
AB#40054123 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Promotion source: AzureArcForKubernetes#35 Original commit: 268fa2d Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf68851-0503-4249-a0ca-464fb560d529
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Promotion source: AzureArcForKubernetes#35 Original commit: a6855bd Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf68851-0503-4249-a0ca-464fb560d529
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Promotion source: AzureArcForKubernetes#35 Original commit: 240b73d Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf68851-0503-4249-a0ca-464fb560d529
Keep learned-key state protected; document the chart 0.1.13+ live-proof dependency. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Promotion source: AzureArcForKubernetes#35 Original commit: 1becc7c Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf68851-0503-4249-a0ca-464fb560d529
The CLI progress helper can suppress a failed 404 delete poller result. Require the raw result to succeed before entering PostDelete and exercise the actual helper in a focused regression. Model an already-completed poller without recording a second operation completion in the existing ordering test. The regression fails on the original source and passes with this guard. The normal Chaos unit module passes all 62 tests. Promotion source: AzureArcForKubernetes#35 Original commit: e9d1a24 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf68851-0503-4249-a0ca-464fb560d529
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Outstanding issues include namespace cleanup paths that can delete customer resources, leave role grants active, or fail to complete safely on retry.
5 open issues
What changed in this PR
Promotes the accepted ChaosStudio changes from the Arc fork into k8s-extension 1.9.5, covering built-in role migration and stricter uninstall behavior.
Changes:
- Assigns the built-in operator role while retaining owned legacy assignments until uninstall.
- Requires confirmed synchronous deletion before ownership-checked namespace cleanup, with regression tests and updated documentation.
- Updates the package version and uses
$GITHUB_WORKSPACEfor CI repository setup.
| File | Description |
|---|---|
src/k8s-extension/setup.py |
Bumps the package version to 1.9.5. |
src/k8s-extension/README.rst |
Documents migration, cleanup safeguards, prerequisites and pending live validation. |
src/k8s-extension/HISTORY.rst |
Records the 1.9.5 behavior changes. |
src/k8s-extension/azext_k8s_extension/tests/latest/test_chaos_studio.py |
Adds role-migration, namespace-inventory and deletion regression coverage. |
src/k8s-extension/azext_k8s_extension/partner_extensions/ChaosStudio.py |
Implements built-in role reconciliation and ownership-gated namespace cleanup. |
src/k8s-extension/azext_k8s_extension/custom.py |
Propagates lookup failures and verifies deletion before namespace cleanup. |
.github/workflows/VersionCalPRComment.yml |
Resolves the extension checkout through $GITHUB_WORKSPACE. |
.github/actions/env-setup/action.yml |
Uses $GITHUB_WORKSPACE in shared environment setup. |
Review effort: Balanced
Retain root CA ConfigMaps with customer metadata, allowing only the known controller description annotation. Validate legacy cleanup role IDs before persistence or teardown, reject updates that add/change the original ID, and preserve the original value for equivalent input. Reconcile the existing built-in workspace role after a pending active install successfully completes. Keep failed/no-wait paths non-mutating and retain the legacy assignment. Addresses approved review comments 4428934501, 4428934560 and 4428934634 on Azure/azure-cli-extensions PR 10542. Seven focused regression methods expose 16 failures on the old source; the normal module now passes 69 tests. Namespace inventory/delete concurrency and post-delete recovery remain open design decisions (4428934530 and 4428934588). Their flows are unchanged and the PR remains Draft. No drain, checkpoint, new cleanup authority or live resource operation is introduced. All four changed files were already in the PR; no generated/vendor/ripple files or duplicate predecessor patch set. Client behavior only: no EV2 producer, dependency, package version, index or publication changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf68851-0503-4249-a0ca-464fb560d529
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
Apply the explicitly approved uninstall ownership boundary: delete only the ARM extension and its owned connection/grants. PostDelete now reports namespace retention without acquiring clients, reading cluster state, deleting namespaces or waiting for finalization. Keep strict raw delete-poller success checks. Remove the unused namespace inventory/default-classifier/finalization helpers and tests for the withdrawn automatic-removal feature. Add regressions proving no post-delete API access, no cleanup mutation to resume, and preservation of customer resources created during ARM deletion, including an idempotent 404 retry. Retain chart installation ownership inputs and earlier role-state fixes. Update README/HISTORY to state the narrower 1.9.5 contract. Automatic namespace removal is deferred; Task 40054123 and Bug 40051963 remain open. No admission drain, durable checkpoint, new cleanup authority or GA cleanup proof is claimed. This supersedes review blockers 4428934530 and 4428934588 by removing the automatic mutation, not by pretending namespace resourceVersion is a write fence. All five files were already in upstream PR 10542. No duplicate predecessor patch set, generator/vendor output, dependency/version/index, EV2 or live-resource changes. No merge, sync or publication is performed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf68851-0503-4249-a0ca-464fb560d529
Frank Li (franklixuefei)
changed the title
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
Frank Li (franklixuefei)
marked this pull request as ready for review
October 31, 2026 00:04
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
Comment on lines
+124
to
+126
| self.client = config.new_client_from_config_dict( | ||
| yaml.safe_load(base64.b64decode(configs[0]["value"]).decode("utf-8")) | ||
| ) |
Comment on lines
+133
to
+148
| def request(self, method, path, body=None, allow_not_found=False): | ||
| remaining = self.deadline - time.monotonic() | ||
| if remaining <= 0: | ||
| raise AzureResponseError("Namespace inspection exceeded its 210-second API budget.") | ||
| try: | ||
| return self.client.call_api( | ||
| path, method, body=body, response_type="object", | ||
| auth_settings=["BearerToken"], _return_http_data_only=True, | ||
| _request_timeout=min(30, remaining), | ||
| ) | ||
| except ApiException as error: | ||
| if allow_not_found and error.status == 404: | ||
| return None | ||
| raise AzureResponseError( | ||
| "Target-cluster {} {} failed; namespace inspection was not confirmed: {}".format(method, path, error) | ||
| ) from error |
| * Microsoft.ChaosStudio: intentionally retain release namespaces and their remaining Kubernetes resources on uninstall. Automatic namespace removal and its fenced/recoverable cleanup contract are deferred, not claimed as clean-namespace or GA cleanup acceptance. | ||
| * Microsoft.ChaosStudio: validate and retain immutable legacy-role cleanup IDs before any teardown, and reconcile the built-in assignment after a pending active installation completes successfully. | ||
| * Microsoft.ChaosStudio: assign the built-in Chaos Kubernetes Extension Operator role instead of provisioning or validating custom roles. Create and update add a separate built-in assignment; existing owned legacy assignments stay until uninstall to avoid interrupting authorization during migration. Delete removes both owned assignments but never role definitions. | ||
| * Microsoft.ChaosStudio: require the persisted namespaceLifecycle.createdByInstall assertion as well as the matching namespace marker before post-uninstall namespace removal. |
Comment on lines
+191
to
+201
| It exposed the create-only ``bindings`` discovery gap fixed here. Retrying | ||
| namespace inspection preserved an unmarked ``durable-broker-learned-keys`` | ||
| ConfigMap and Kubernetes Events rather than erasing them. AKS also returned a | ||
| pristine default ServiceAccount without managedFields, now covered by a regression. | ||
| This wasn't live proof of clean namespace removal. This CLI version now | ||
| intentionally retains namespaces; a chart release alone does not provide the | ||
| concurrent-writer fence or durable recovery contract required for automatic | ||
| removal. The compatible chart's learned-key owner marker and pre-delete cleanup | ||
| remain separate from that deferred capability. The CLI doesn't | ||
| special-case learned-key state; preserving the unmarked ConfigMap on 0.1.12 is | ||
| correct. Events no longer block namespace removal. The built-in role |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 PR Validation — ️✔️ All clear
Summary and explicit scope decision
Promote AzureArcForKubernetes#35 once, with the explicitly approved review corrections below. Its duplicate predecessor, AzureArcForKubernetes#34, is not applied as a second patch set.
Version 1.9.5 intentionally retains namespaces. This PR delivers built-in role migration and safe extension uninstall, not automatic clean-namespace removal. Namespace garbage collection and its fenced/recoverable protocol are deferred.
Task 40054123 and Bug 40051963 remain Active. This change does not complete their clean-namespace acceptance or establish GA cleanup proof. Ready for review is not approval to merge, publish a wheel or close those items.
Related commands:
az k8s-extension create,update, anddeleteforMicrosoft.ChaosStudio.Current uninstall and ownership contract
--no-waitand--forcedo not run namespace cleanup either.namespaceLifecycle.createdByInstallinputs and chart ownership compatibility are preserved. Ownership is not treated as proof that all namespace writers are stopped.Review disposition
Current source:
b776f112317f0ae0eca687cc12582867bedc19e6.The inventory/delete race and post-delete recovery gap are addressed by withdrawing automatic namespace mutation, not by claiming namespace resourceVersion is a write fence or an ARM 404 reconstructs ownership.
The intermediate root-CA classifier is removed with the now-unused inventory/default/finalization helpers. All namespace contents are retained, so no customized ConfigMap is classified as disposable. Earlier legacy-ID and pending-role reconciliation fixes remain intact.
This adds no admission drain, checkpoint, new recovery command, cluster-wide permission or cleanup authority. The documentation and tests now assert the explicit retention contract.
Provenance and overlap
Accepted team-fork head:
e9d1a249291e61903bd968c75e39b45c7d31a92f; merge:901ca85f39c28272090fe295f240f83148910d93.Initial promotion
e778d669replayed only #35's five commits onto upstreamd79d5bc724e7701a03ad7a74c064d06c260ad417, preserving original authors/dates and all eight accepted blobs. The two explicitly approved review follow-ups modify only original PR paths. The fork's 37 pre-existingtesting/assets are excluded and unrelated upstream changes retained.Open upstream PR #10528 overlaps the initial non-404 lookup guard and version
1.9.5. That guard was already in accepted #35; no commits or extra files from #10528 are imported. Maintainers should coordinate merge/version order rather than publish the same version twice.Validation and release boundary
1.9.5wheel source parity and packaged logging-only PostDelete verified. Wheel SHA-256:b6b15433ac83f577abb50020bcbef67392dafbb2ff6eb5e8b6c1732a2bbb3863.src/index.jsonremains unchanged.azdev styleremains unverified because the pinned tool rejects Git worktree registration. Prior-head upstream style/linter results are not transferred to this source; new-head CI/review remain required.No live uninstall, Azure/cluster-policy operation, upstream merge, fork sync, generated-index edit or wheel publication was performed. No EV2 producer or dependency/version change is required.
General guidelines
azdev style k8s-extensionlocally (limitation disclosed above).1.9.5) and update only its unreleased history.