{Dataprotection} Isolate AKS backup vault discovery - #10309
Conversation
|
Hi FumingZhang, |
There was a problem hiding this comment.
🟢 Approval recommended
The functional changes and regression tests align with the PR’s race-condition goals, with only a minor docstring contract clarification suggested.
Pull request overview
This PR updates the az dataprotection enable-backup trigger AKS flow to avoid cross-run races by scoping backup vault discovery to the caller’s resolved backup resource group and by waiting for a newly created vault’s service-visible provisioningState before proceeding with dependent operations.
Changes:
- Scope tag-based backup vault discovery to the explicit backup resource group (instead of subscription-wide search).
- Add bounded polling for backup vault
provisioningStateafter create to reduce eventual-consistency failures. - Bump extension version to
1.11.5and add matching release notes, plus regression unit tests for the new behaviors.
File summaries
| File | Description |
|---|---|
| src/dataprotection/setup.py | Bumps dataprotection extension version to 1.11.5. |
| src/dataprotection/HISTORY.rst | Adds 1.11.5 release note describing the vault discovery + readiness wait changes. |
| src/dataprotection/azext_dataprotection/manual/aks/aks_helper.py | Implements resource-group-scoped vault discovery and bounded vault readiness polling. |
| src/dataprotection/azext_dataprotection/tests/latest/test_dataprotection_enable_backup.py | Adds unit coverage for RG scoping, tag matching, polling success/failure, and bounded retries. |
Review details
- Files reviewed: 4/4 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.
| Scoping the ``list`` call to ``backup_resource_group_name`` (derived from | ||
| the caller-supplied ``backupResourceGroupId``, or the per-cluster | ||
| resource group we just created/validated) is required: without it, every | ||
| parallel run/test that happens to omit ``backupResourceGroupId`` shares | ||
| the same subscription-wide, tag-matched vault, so one run's | ||
| ``aks delete``/vault cleanup can race another run's discovery and lookup | ||
| (``ResourceGroupBeingDeleted``/404 on the shared vault). Restricting | ||
| discovery to the caller's own resource group keeps each run isolated. |
Honor backupResourceGroupId when locating reusable vaults so concurrent AKS backup runs cannot race over unrelated subscription-level matches. Poll newly created vaults for a service-visible terminal state before dependent role and backup operations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e21903a to
fc9782b
Compare
|
dataprotection |
|
[Release] Update index.json for extension [ dataprotection-1.11.5 ] : https://dev.azure.com/msazure/One/_build/results?buildId=180062234&view=results |
🤖 PR Validation — ️✔️ All clear
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az dataprotection enable-backup triggerWhat / Why
Fixes a race in AKS backup setup when multiple runs operate in the same subscription:
backupResourceGroupIdinstead of searching the entire subscription for a location-tagged vault. This prevents concurrent runs from sharing a vault that another run may delete during cleanup.provisioningStatewith bounded retries before starting dependent role-assignment and backup-instance operations.Validation
pytest src/dataprotection/azext_dataprotection/tests/latest/test_dataprotection_enable_backup.py -q- 48 passed.flake8on the changed helper and test files using the extension's configured ignores - passed.python -m py_compilefor all changed Python files - passed.git diff --check upstream/main...HEAD- passed.Scope / Index / History
1.11.4to1.11.5.HISTORY.rstcontains the matching1.11.5release note.src/index.jsonis not modified; it will be updated by the post-merge automation.General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired) -azdevis unavailable locally; targetedflake8, syntax, and unit checks passed, and the PR's style workflow provides the repository check.python scripts/ci/test_index.py -qlocally? (pip install azdevrequired) - N/A,src/index.jsonis unchanged.For new extensions: