🐛 Fix CI E2E: clean up orphaned cluster-scoped WVA resources - #715
🐛 Fix CI E2E: clean up orphaned cluster-scoped WVA resources#715clubanderson wants to merge 1 commit into
Conversation
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
There was a problem hiding this comment.
Pull request overview
Updates the OpenShift CI E2E workflow to proactively remove stale, cluster-scoped Workload Variant Autoscaler (WVA) RBAC resources that can block subsequent Helm installs when their owning namespace has been deleted.
Changes:
- Add a pre-deploy scan that deletes WVA-labeled ClusterRoles/ClusterRoleBindings whose
meta.helm.sh/release-namespaceno longer exists. - Extend post-test cleanup to also delete cluster-scoped WVA resources annotated as owned by this PR’s namespaces (in addition to the existing instance-label-based deletion).
| echo "Checking for orphaned cluster-scoped WVA resources..." | ||
| for kind in clusterrole clusterrolebinding; do | ||
| for resource in $(kubectl get "$kind" -l app.kubernetes.io/name=workload-variant-autoscaler -o jsonpath='{range .items[*]}{.metadata.name}={.metadata.annotations.meta\.helm\.sh/release-namespace}{"\n"}{end}' 2>/dev/null); do | ||
| name="${resource%%=*}" | ||
| ns="${resource##*=}" | ||
| if [ -n "$ns" ] && ! kubectl get namespace "$ns" &>/dev/null; then | ||
| echo " Deleting orphaned $kind/$name (owning namespace '$ns' no longer exists)" | ||
| kubectl delete "$kind" "$name" --ignore-not-found || true | ||
| fi | ||
| done | ||
| done | ||
|
|
There was a problem hiding this comment.
This orphan/namespace-owned cluster-scope cleanup logic is duplicated (same kubectl get ... jsonpath + parsing) in both pre-cleanup and post-cleanup sections. To reduce drift and make future changes safer, consider factoring it into a small helper script or a reusable step/function within the workflow.
| echo "Checking for orphaned cluster-scoped WVA resources..." | |
| for kind in clusterrole clusterrolebinding; do | |
| for resource in $(kubectl get "$kind" -l app.kubernetes.io/name=workload-variant-autoscaler -o jsonpath='{range .items[*]}{.metadata.name}={.metadata.annotations.meta\.helm\.sh/release-namespace}{"\n"}{end}' 2>/dev/null); do | |
| name="${resource%%=*}" | |
| ns="${resource##*=}" | |
| if [ -n "$ns" ] && ! kubectl get namespace "$ns" &>/dev/null; then | |
| echo " Deleting orphaned $kind/$name (owning namespace '$ns' no longer exists)" | |
| kubectl delete "$kind" "$name" --ignore-not-found || true | |
| fi | |
| done | |
| done | |
| cleanup_orphaned_wva_cluster_resources() { | |
| echo "Checking for orphaned cluster-scoped WVA resources..." | |
| for kind in clusterrole clusterrolebinding; do | |
| for resource in $(kubectl get "$kind" -l app.kubernetes.io/name=workload-variant-autoscaler -o jsonpath='{range .items[*]}{.items[*]}{.metadata.name}={.metadata.annotations.meta\.helm\.sh/release-namespace}{"\n"}{end}' 2>/dev/null); do | |
| name="${resource%%=*}" | |
| ns="${resource##*=}" | |
| if [ -n "$ns" ] && ! kubectl get namespace "$ns" &>/dev/null; then | |
| echo " Deleting orphaned $kind/$name (owning namespace '$ns' no longer exists)" | |
| kubectl delete "$kind" "$name" --ignore-not-found || true | |
| fi | |
| done | |
| done | |
| } | |
| cleanup_orphaned_wva_cluster_resources |
| for kind in clusterrole clusterrolebinding; do | ||
| for resource in $(kubectl get "$kind" -l app.kubernetes.io/name=workload-variant-autoscaler -o jsonpath='{range .items[*]}{.metadata.name}={.metadata.annotations.meta\.helm\.sh/release-namespace}{"\n"}{end}' 2>/dev/null); do | ||
| name="${resource%%=*}" | ||
| ns="${resource##*=}" | ||
| if [ "$ns" = "$LLMD_NAMESPACE" ] || [ "$ns" = "$LLMD_NAMESPACE_B" ] || [ "$ns" = "$WVA_NAMESPACE" ]; then | ||
| echo " Deleting $kind/$name (owned by PR namespace '$ns')" | ||
| kubectl delete "$kind" "$name" --ignore-not-found || true | ||
| fi |
There was a problem hiding this comment.
Here too, kubectl get stderr is fully suppressed (2>/dev/null), which can mask listing failures and lead to incomplete cleanup while still printing success messages. Consider preserving errors (or checking exit status) so troubleshooting doesn’t require re-running with extra logging.
| for kind in clusterrole clusterrolebinding; do | ||
| for resource in $(kubectl get "$kind" -l app.kubernetes.io/name=workload-variant-autoscaler -o jsonpath='{range .items[*]}{.metadata.name}={.metadata.annotations.meta\.helm\.sh/release-namespace}{"\n"}{end}' 2>/dev/null); do | ||
| name="${resource%%=*}" | ||
| ns="${resource##*=}" | ||
| if [ -n "$ns" ] && ! kubectl get namespace "$ns" &>/dev/null; then | ||
| echo " Deleting orphaned $kind/$name (owning namespace '$ns' no longer exists)" | ||
| kubectl delete "$kind" "$name" --ignore-not-found || true | ||
| fi |
There was a problem hiding this comment.
Both loops silence all kubectl get stderr (2>/dev/null), which can hide real failures (e.g., RBAC/auth issues or API errors) and make the orphan-cleanup appear to run while doing nothing. Consider handling the empty-result case without suppressing errors, or explicitly checking the command exit code and logging a warning/error when the list operation fails.
2e022e0 to
f25d301
Compare
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
f25d301 to
923aa70
Compare
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
| # Search by name pattern (not labels — helmfile deployments may use different labels) | ||
| # Use jq to reliably extract annotation keys containing dots/slashes | ||
| kubectl get "$kind" -o json 2>/dev/null | \ | ||
| jq -r '.items[] | select(.metadata.name | contains("workload-variant-autoscaler")) | "\(.metadata.name)\t\(.metadata.annotations["meta.helm.sh/release-namespace"] // "")"' 2>/dev/null | \ |
There was a problem hiding this comment.
The jq filter uses contains("workload-variant-autoscaler") which will match any resource name containing this substring, potentially including unrelated resources if they happen to have similar names. Consider using a more specific filter such as startswith("workload-variant-autoscaler") or matching against specific known prefixes to reduce the risk of false positives.
| jq -r '.items[] | select(.metadata.name | contains("workload-variant-autoscaler")) | "\(.metadata.name)\t\(.metadata.annotations["meta.helm.sh/release-namespace"] // "")"' 2>/dev/null | \ | |
| jq -r '.items[] | select(.metadata.name | startswith("workload-variant-autoscaler")) | "\(.metadata.name)\t\(.metadata.annotations["meta.helm.sh/release-namespace"] // "")"' 2>/dev/null | \ |
| # (covers helmfile-created resources whose instance label differs from WVA_RELEASE_NAME) | ||
| for kind in clusterrole clusterrolebinding; do | ||
| kubectl get "$kind" -o json 2>/dev/null | \ | ||
| jq -r '.items[] | select(.metadata.name | contains("workload-variant-autoscaler")) | "\(.metadata.name)\t\(.metadata.annotations["meta.helm.sh/release-namespace"] // "")"' 2>/dev/null | \ |
There was a problem hiding this comment.
The jq filter uses contains("workload-variant-autoscaler") which will match any resource name containing this substring, potentially including unrelated resources if they happen to have similar names. Consider using a more specific filter such as startswith("workload-variant-autoscaler") or matching against specific known prefixes to reduce the risk of false positives.
| jq -r '.items[] | select(.metadata.name | contains("workload-variant-autoscaler")) | "\(.metadata.name)\t\(.metadata.annotations["meta.helm.sh/release-namespace"] // "")"' 2>/dev/null | \ | |
| jq -r '.items[] | select(.metadata.name | startswith("workload-variant-autoscaler-")) | "\(.metadata.name)\t\(.metadata.annotations["meta.helm.sh/release-namespace"] // "")"' 2>/dev/null | \ |
| kubectl get "$kind" -o json 2>/dev/null | \ | ||
| jq -r '.items[] | select(.metadata.name | contains("workload-variant-autoscaler")) | "\(.metadata.name)\t\(.metadata.annotations["meta.helm.sh/release-namespace"] // "")"' 2>/dev/null | \ | ||
| while IFS=$'\t' read -r name ns; do | ||
| if [ "$ns" = "$LLMD_NAMESPACE" ] || [ "$ns" = "$LLMD_NAMESPACE_B" ] || [ "$ns" = "$WVA_NAMESPACE" ]; then | ||
| echo " Deleting $kind/$name (owned by PR namespace '$ns')" | ||
| kubectl delete "$kind" "$name" --ignore-not-found || true | ||
| fi | ||
| done |
There was a problem hiding this comment.
The jq command and loop logic at lines 806-813 are identical to lines 571-578 except for the namespace checking condition. Consider extracting this into a shell function to reduce code duplication and improve maintainability. For example, define a function cleanup_wva_resources() that takes the kind and a namespace-matching function as parameters.
The helmfile uses a generic release name "workload-variant-autoscaler" which produces non-unique ClusterRole names. On shared clusters, these may be owned by another namespace's release, causing Helm ownership conflicts. Pre-cleanup: adopt shared resources by patching Helm release annotations to our namespace. Post-cleanup: delete resources owned by our namespaces. Uses jq (not jsonpath) to handle annotation keys with dots/slashes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Andrew Anderson <andy@clubanderson.com>
923aa70 to
e3edd2c
Compare
GPU Pre-flight Check ✅GPUs are available for e2e-openshift tests. Proceeding with deployment.
|
|
@lionelvillard @ev-shindin — This PR adds orphan ClusterRole cleanup to the CI E2E workflow, preventing Helm ownership conflicts when running on shared OpenShift clusters. Would appreciate a review/merge to unblock CI. Thanks! |
|
Consolidated into #719 |
- Fix CI E2E: clean up orphaned cluster-scoped WVA resources - Fix Helm chart: make ClusterRole names unique per namespace - Fix broken reusable workflow references (@2b273d6 → @main) Consolidates #715, #716, and #718. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Andrew Anderson <andy@clubanderson.com>
- Fix CI E2E: clean up orphaned cluster-scoped WVA resources - Fix Helm chart: make ClusterRole names unique per namespace - Fix broken reusable workflow references (@2b273d6 → @main) Consolidates #715, #716, and #718. Signed-off-by: Andrew Anderson <andy@clubanderson.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…-d#719) - Fix CI E2E: clean up orphaned cluster-scoped WVA resources - Fix Helm chart: make ClusterRole names unique per namespace - Fix broken reusable workflow references (@2b273d6 → @main) Consolidates llm-d#715, llm-d#716, and llm-d#718. Signed-off-by: Andrew Anderson <andy@clubanderson.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Root Cause
When a WVA installation's namespace is deleted (e.g., failed CI run cleanup, manual deletion), cluster-scoped resources like ClusterRoles are left behind. The next CI run tries to create the same ClusterRole, but Helm rejects it:
Changes
Pre-cleanup (before deploy):
meta.helm.sh/release-namespace) still existsPost-cleanup (after tests):
app.kubernetes.io/instancelabel (matches WVA controller release)Test plan