Skip to content

AzureBackup live tests: clean up vault dependencies before resource group removal - #3336

Open
Shraddha Jain (shrja-ms) wants to merge 3 commits into
microsoft:mainfrom
shrja-ms:user/azurebackup-cleanup-vault-dependencies
Open

AzureBackup live tests: clean up vault dependencies before resource group removal#3336
Shraddha Jain (shrja-ms) wants to merge 3 commits into
microsoft:mainfrom
shrja-ms:user/azurebackup-cleanup-vault-dependencies

Conversation

@shrja-ms

Copy link
Copy Markdown
Contributor

Description

Fixes #3312.

Backup Vault resources (Recovery Services Vaults and Data Protection Backup Vaults) cannot be deleted while they still contain backup instances, protected items, registered containers, backup policies, or soft-deleted items. The Azure.Mcp.Tools.AzureBackup live tests intentionally leave soft-deleted items behind (they exist so that the undelete live tests have real soft-deleted items to restore). When the pipeline then invokes Remove-TestResources.ps1, Azure refuses to delete the vault, which in turn blocks resource group deletion — leaving orphaned resource groups behind and requiring manual cleanup.

Fix

Introduces a generic remove-test-resources-pre.ps1 hook that runs before remove-test-resources.yml, so toolsets that own resources with dependency chains can perform ordered cleanup.

New file

  • tools/Azure.Mcp.Tools.AzureBackup/tests/remove-test-resources-pre.ps1
    • Idempotent PowerShell script that walks each RSV and DPP vault in the test resource group and performs the required cleanup sequence:
      1. Disable soft delete on each vault (RSV via Set-AzRecoveryServicesVaultProperty, DPP via Update-AzDataProtectionBackupVault).
      2. Undelete + hard-delete all backup instances / protected items (RSV backup items and DPP backup instances, including soft-deleted ones).
      3. Unregister backup containers (RSV storage containers).
      4. Delete all backup policies (both RSV and DPP).
    • Uses -ErrorAction SilentlyContinue on discovery calls and try/catch on per-item mutations so a single failure does not halt cleanup.
    • Exits cleanly when the resource group, vaults, instances, containers, or policies are already gone.

Pipeline changes

  • eng/pipelines/templates/jobs/live-test.yml
    • Adds an AzurePowerShell@5 step before each remove-test-resources.yml invocation (one for the stdio leg, one for the http leg) that:
      • Looks for $(TestResourcesPath)/remove-test-resources-pre.ps1.
      • Skips silently if the script is not present (no-op for other toolsets).
      • Runs the script against $env:RESOURCE_GROUP (set by New-TestResources.ps1).
      • Uses condition: succeededOrFailed() gated on CI_HAS_DEPLOYED_RESOURCES = true, and continueOnError: true, so it runs even after test failures and never blocks the downstream remove-test-resources.yml.

eng/common/** is intentionally left untouched (it is auto-synced from the Azure SDK common repo).

Acceptance criteria mapping

Requirement How it's satisfied
Cleanup logic disables soft delete before resource deletion Step 1 in remove-test-resources-pre.ps1
All backup instances are removed before policy deletion Steps 2–3 run before step 4
All backup policies are removed before vault deletion Step 4 completes before pipeline hands off to remove-test-resources.yml
Resource group deletion succeeds without manual intervention Vault has no dependencies by the time Remove-AzResourceGroup runs
Re-running cleanup is idempotent Every step is guarded (missing RG / vaults / items / containers / policies all short-circuit cleanly)

Testing

  • YAML parsed with PyYAML (16 steps, valid structure).
  • PowerShell script parsed with [System.Management.Automation.Language.Parser]::ParseFile (no syntax errors).
  • Cmdlet names and parameters validated against local Az.RecoveryServices.Backup and Az.DataProtection modules.
  • Spelling clean (Invoke-Cspell.ps1).
  • Live validation will run when this PR is merged and the next Azure.Mcp.Tools.AzureBackup live-test build executes; a follow-up run against the same subscription should show the previously orphaned resource groups being cleaned up automatically.

Invoking Livetests

Copilot submitted PRs are not trustworthy by default. Users with write access to the repo need to validate the contents of this PR before leaving a comment with the text /azp run mcp - pullrequest - live. This will trigger the necessary livetest workflows to complete required validation.

…roup removal (microsoft#3312)

Backup Vault resources (RSV and DPP) block resource group deletion when
they still contain backup instances, protected items, registered
containers, backup policies, or soft-deleted items. The live test pipeline
previously invoked Remove-TestResources.ps1 directly and left orphaned
resource groups behind.

Changes:
- Add tools/Azure.Mcp.Tools.AzureBackup/tests/remove-test-resources-pre.ps1
  which disables soft delete on each vault, undeletes and hard-deletes all
  backup instances/protected items, unregisters storage containers, and
  removes all backup policies. The script is idempotent and tolerant of
  missing resources.
- Update eng/pipelines/templates/jobs/live-test.yml to invoke
  $(TestResourcesPath)/remove-test-resources-pre.ps1 when present, in
  the correct sequence relative to remove-test-resources.yml. The hook is
  a no-op for toolsets that do not need it.

Fixes microsoft#3312
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

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.

Pull request overview

This PR addresses AzureBackup live-test resource-group leaks by introducing a pre-cleanup hook that removes Azure Backup vault dependency chains (soft-delete items, backup instances, containers, policies) before the standard Remove-TestResources.ps1 resource-group deletion runs.

Changes:

  • Added an AzureBackup-specific remove-test-resources-pre.ps1 script to disable soft delete and purge vault dependencies in the correct order.
  • Updated the live-test pipeline job template to optionally invoke remove-test-resources-pre.ps1 (if present) before remove-test-resources.yml for both stdio and http legs.

Invoking Livetests

Copilot submitted PRs are not trustworthy by default. Users with write access to the repo need to validate the contents of this PR before leaving a comment with the text /azp run mcp - pullrequest - live. This will trigger the necessary livetest workflows to complete required validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tools/Azure.Mcp.Tools.AzureBackup/tests/remove-test-resources-pre.ps1 Adds ordered, best-effort cleanup logic to remove vault dependencies that block vault/RG deletion.
eng/pipelines/templates/jobs/live-test.yml Invokes an optional pre-cleanup script before the standard test-resource removal steps (stdio + http).

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

Comment thread tools/Azure.Mcp.Tools.AzureBackup/tests/remove-test-resources-pre.ps1 Outdated
- Drop `SupportsShouldProcess` from `[CmdletBinding()]`. The script does
  not call `$PSCmdlet.ShouldProcess`, so advertising -WhatIf / -Confirm
  was misleading.
- Omit `-SoftDeleteRetentionDurationInDay 0` when disabling DPP soft
  delete. The service validates retention against a 14-180 day range and
  rejects `0`, which would leave soft delete enabled and block vault
  cleanup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] AzureBackup MCP cleanup fails because Backup Vault resources are deleted in the wrong order

2 participants