Add Azure IPI resource garbage collection periodic - #83200
Conversation
Add periodic-ipi-deprovision-azure to clean up leaked Azure IPI cluster resources, matching the existing AWS and GCP GC patterns. Uses a 6-hour TTL to avoid disrupting active tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe image now installs Azure CLI with repository signature checks. The new ChangesAzure cluster deprovisioning
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant azure_sh
participant AzureCLI
participant AzureResourceManager
participant openshift_install
azure_sh->>AzureCLI: Authenticate and select subscription
azure_sh->>AzureResourceManager: Query paginated resource groups
AzureResourceManager-->>azure_sh: Return expired CI resource groups
azure_sh->>openshift_install: Destroy clusters concurrently
openshift_install-->>azure_sh: Return cleanup status
azure_sh->>AzureCLI: Delete resource groups after failures
azure_sh-->>azure_sh: Report warnings and failures
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hlipsig The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@clusters/app.ci/ipi-deprovision/ipi-deprovision-bc.yaml`:
- Around line 50-52: Update the package-installation command in the image build
to remove the --nogpg option and retain RPM signature verification. Ensure GPG
keys are imported for every configured repository, including the Google Cloud
SDK repository and the Azure CLI repository before dnf install runs.
In `@core-services/ipi-deprovision/azure.sh`:
- Around line 100-101: Update the marker-file searches in the deprovisioning
flow around the `clusters` loop and lines 124–133 to use `${logdir}` explicitly
as the `find` start path, including when `clusters` is empty. Ensure no command
falls back to searching the current working directory, while preserving the
existing filtering and processing behavior.
- Around line 114-117: Update the fallback deletion command in the azure
deprovisioning flow to wait until the resource group deletion completes before
removing ${workdir}/failure and creating ${workdir}/warning. Remove --no-wait
from az group delete, or poll for resource group removal and only update the
markers after confirmed completion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e8565343-1514-45e9-9f3a-006c917315b0
⛔ Files ignored due to path filters (1)
ci-operator/jobs/infra-build-farm-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (2)
clusters/app.ci/ipi-deprovision/ipi-deprovision-bc.yamlcore-services/ipi-deprovision/azure.sh
Remove --no-wait from the fallback az group delete so the script only marks a resource group as cleaned up after deletion completes, avoiding false success when async deletion fails. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use ${logdir} instead of ${clusters} for marker-file searches to
avoid searching cwd when no clusters are found. Enable GPG signature
verification for all repos by adding gpgcheck to the Google Cloud SDK
repo config, importing both GPG keys, and removing --nogpg from dnf.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core-services/ipi-deprovision/azure.sh`:
- Around line 124-130: Update the WARNINGS and FAILED find commands to search
only marker files with -type f and print each marker’s containing cluster
directory using %h instead of %H. Preserve the existing sorting and summary
behavior, and apply the same correction to both warning and failure collection.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f715a536-58e7-4593-bdc9-42ec30ecb94c
📒 Files selected for processing (2)
clusters/app.ci/ipi-deprovision/ipi-deprovision-bc.yamlcore-services/ipi-deprovision/azure.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- clusters/app.ci/ipi-deprovision/ipi-deprovision-bc.yaml
Add securityContext with runAsNonRoot and runAsUser 65534 (nobody). None of the tools (az, openshift-install, jq) require root, and HOME=/tmp ensures config writes go to world-writable paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/pj-rehearse |
|
@hlipsig: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ipi-deprovision-azure |
|
@hlipsig: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/retest |
The warning and failure marker find commands used %H (starting-point directory) instead of %h (containing directory), causing basename to always print the logdir name rather than individual cluster IDs. Also add -type f to restrict matches to regular files only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core-services/ipi-deprovision/azure.sh (1)
127-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winQuote
WARNINGSin the here-string.Line 127 expands
WARNINGSwithout quotes. Use<<< "$WARNINGS"to prevent unintended shell expansion and resolve ShellCheck SC2086.Proposed fix
- xargs --max-args 1 basename <<< $WARNINGS + xargs --max-args 1 basename <<< "$WARNINGS"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core-services/ipi-deprovision/azure.sh` at line 127, Update the here-string in the warning-processing command around basename to quote the WARNINGS expansion, using the existing WARNINGS variable while preventing unintended shell expansion and satisfying ShellCheck SC2086.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core-services/ipi-deprovision/azure.sh`:
- Line 127: Update the here-string in the warning-processing command around
basename to quote the WARNINGS expansion, using the existing WARNINGS variable
while preventing unintended shell expansion and satisfying ShellCheck SC2086.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 523edb18-254a-4f11-8bb3-6e17a4a1beea
⛔ Files ignored due to path filters (1)
ci-operator/jobs/infra-build-farm-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
core-services/ipi-deprovision/azure.sh
This job requires real Azure credentials and resources, so it cannot be meaningfully rehearsed in PR context. Marking it as non-rehearsable prevents the rehearsal system from failing on missing build history. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@hlipsig: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Summary
periodic-ipi-deprovision-azurejob to clean up leaked Azure IPI cluster resources, matching the existing AWS and GCP GC patternscore-services/ipi-deprovision/azure.shscript that authenticates viacluster-secrets-azure4, discovers stale CI resource groups (ci-op-*older than 6 hours), runsopenshift-install destroy clusterin parallel, and force-deletes any remaining resource groupsazure-clito theci_ipi-deprovision_latestcontainer image (already has AWS CLI and GCP SDK)Test plan
ipi-deprovisioncontainer image builds successfully withazure-cliaddedperiodic-ipi-deprovision-azurejob to validate script execution#ops-testplatformon failure🤖 Generated with Claude Code
Summary by CodeRabbit
This PR adds periodic Azure IPI cluster garbage collection to OpenShift CI. The new job finds
ci-op-*Azure resource groups older than six hours, destroys clusters in parallel, and force-deletes remaining resource groups.The
ci_ipi-deprovision_latestimage now includes GPG-verifiedazure-clipackages. The Azure cleanup runs as a non-root user withHOME=/tmp, authenticates withcluster-secrets-azure4, and reports cleanup failures for Slack alerting.