[KFLUXINFRA-3003] Add DR sanity pipeline with staging-only Triggers and Kueue LocalQueue#13186
Open
vsokolen-redhat wants to merge 1 commit into
Open
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vsokolen-redhat The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
Kustomize Render DiffComparing
Total: 3 components, +2466 -180 lines 📋 Full diff available in the workflow summary and as a downloadable artifact. |
PR Summary by QodoAdd DR sanity Tekton pipeline; stage-only Triggers and Kueue LocalQueue
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
3 rules 1. Snapshot unchecked post-restore
|
![qodo-for-redhat-appstudio[bot]](https://avatars.githubusercontent.com/u/89927105?s=60&v=4){kind=small}
Comment on lines
+1
to
+2
| apiVersion: kueue.x-k8s.io/v1beta1 | ||
| kind: LocalQueue |
There was a problem hiding this comment.
1. Localqueue api mismatch 🐞 Bug ≡ Correctness
The DR LocalQueue is defined as kueue.x-k8s.io/v1beta1, but the repo’s development Kueue CRD test resource serves only v1beta2, so this manifest can fail to apply/sync on dev/staging clusters that match that CRD versioning.
Agent Prompt
## Issue description
`components/disaster-recovery/development/localqueue.yaml` uses `apiVersion: kueue.x-k8s.io/v1beta1`, but the repo’s development Kueue CRD test manifest for LocalQueue only serves `v1beta2`. If the dev/staging clusters follow that versioning, ArgoCD sync/apply will fail with “no matches for kind LocalQueue in version …/v1beta1”.
## Issue Context
Development Kueue config in this repo uses `kueue.x-k8s.io/v1beta2`, and the dev chainsaw LocalQueue CRD serves only `v1beta2`.
## Fix Focus Areas
- components/disaster-recovery/development/localqueue.yaml[1-6]
- components/policies/development/kueue/queue-config/.chainsaw-test/resources/localqueue-crd.yaml[13-16]
- components/kueue/development/queue-config/cluster-queue.yaml[1-10]
## Suggested fix
- Change `apiVersion` in `localqueue.yaml` to `kueue.x-k8s.io/v1beta2` (or, if you truly must support both, split by overlay so dev/staging use v1beta2 and only environments that serve v1beta1 apply v1beta1).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+120
to
+155
| echo "Waiting for build PipelineRun $BUILD_PR to complete..." | ||
| for i in $(seq 1 120); do | ||
| STATUS=$(oc get pipelinerun "$BUILD_PR" -n "$NS" \ | ||
| -o jsonpath='{.status.conditions[0].reason}' 2>/dev/null || echo "Pending") | ||
| if [ "$STATUS" = "Completed" ] || [ "$STATUS" = "Succeeded" ]; then | ||
| echo "OK: Build completed" | ||
| break | ||
| elif [ "$STATUS" = "Failed" ]; then | ||
| echo "FAIL: Build PipelineRun $BUILD_PR failed" | ||
| exit 1 | ||
| fi | ||
| echo " Build status: $STATUS (attempt $i/120)" | ||
| sleep 15 | ||
| done | ||
|
|
||
| echo "Waiting for integration test PipelineRun..." | ||
| for i in $(seq 1 60); do | ||
| INT_PR=$(oc get pipelineruns -n "$NS" \ | ||
| -l "appstudio.openshift.io/application=$APP,test.appstudio.openshift.io/scenario" \ | ||
| --sort-by=.metadata.creationTimestamp -o jsonpath='{.items[-1].metadata.name}' 2>/dev/null || echo "") | ||
| if [ -n "$INT_PR" ]; then | ||
| INT_STATUS=$(oc get pipelinerun "$INT_PR" -n "$NS" \ | ||
| -o jsonpath='{.status.conditions[0].reason}' 2>/dev/null || echo "Pending") | ||
| if [ "$INT_STATUS" = "Completed" ] || [ "$INT_STATUS" = "Succeeded" ]; then | ||
| echo "OK: Integration test completed ($INT_PR)" | ||
| break | ||
| elif [ "$INT_STATUS" = "Failed" ]; then | ||
| echo "FAIL: Integration test PipelineRun $INT_PR failed" | ||
| exit 1 | ||
| fi | ||
| echo " Integration status: $INT_STATUS (attempt $i/60)" | ||
| else | ||
| echo " Waiting for integration PipelineRun to appear (attempt $i/60)" | ||
| fi | ||
| sleep 15 | ||
| done |
There was a problem hiding this comment.
2. Timeout polls never fail 🐞 Bug ☼ Reliability
Several polling loops in the DR Tasks break on success but never exit non-zero when the loop limit is exhausted, so the Task can continue and print “PASSED” even when build/integration/release/backup/restore never completed.
Agent Prompt
## Issue description
Multiple bounded polling loops (build completion, integration completion, release completion, backup completion, restore completion) have no post-loop “timed out” failure check. If the awaited condition never becomes true and never enters an explicit failure phase, the loop ends and the script proceeds as if it succeeded, causing false-positive DR results and potentially operating on incomplete state.
## Issue Context
Example patterns:
- `dr-baseline-run`: waits for build/integration/release but doesn’t fail if any of those never reach success before the loop ends.
- `dr-backup`: writes `backup-name` and prints `Backup PASSED` even if backup phase never becomes `Completed`.
- `dr-disaster-and-restore`: prints `PASSED` even if restore never becomes `Completed` before the loop ends.
## Fix Focus Areas
- components/disaster-recovery/development/pipeline.yaml[120-156]
- components/disaster-recovery/development/pipeline.yaml[258-277]
- components/disaster-recovery/development/pipeline.yaml[383-409]
## Suggested fix
For each polling loop:
- Track a `success=false` flag; set it to true on success and `break`.
- After the loop, if `success != true`, print a clear timeout error and `exit 1`.
- For the disaster-and-restore task, ensure NetworkPolicy cleanup occurs on timeout paths as well (and rely on `finally` only as a last resort).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
🤖 Pipeline Failure AnalysisCategory: Infrastructure The pipeline failed because critical ArgoCD applications, including backup (OADP) and Kueue components, timed out during deployment and reconciliation due to missing resources and a degraded 📋 Technical DetailsImmediate CauseThe pipeline failed due to an Contributing FactorsThe failure to sync was primarily caused by critical resources for these applications, such as ConfigMaps, Secrets, Jobs, and Deployments, remaining in a 'Missing' state. This underlying issue stemmed from a degraded ImpactThe inability to deploy and reconcile essential backup and Kueue components rendered the disaster recovery setup incomplete, directly blocking the successful execution of the 🔍 Evidenceappstudio-konflux-disaster-recovery/redhat-appstudio-konflux-disaster-recoveryCategory: Logs:
|
vsokolen-redhat
force-pushed
the
dr-sanity-pipeline-v2
branch
from
July 24, 2026 07:21
6baf24c to
0bc47df
Compare
Comment on lines
+25
to
+30
| echo "Checking Velero deployment..." | ||
| READY=$(oc get deploy velero -n openshift-adp -o jsonpath='{.status.readyReplicas}' 2>/dev/null || echo "0") | ||
| if [ "$READY" != "1" ]; then | ||
| echo "FAIL: Velero deployment not ready (readyReplicas=$READY)" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
1. Velero replica check brittle 🐞 Bug ≡ Correctness
dr-preflight exits unless the Velero Deployment has exactly readyReplicas==1, so a healthy Velero deployment scaled to 2+ replicas will incorrectly fail the DR pipeline.
Agent Prompt
### Issue description
`dr-preflight` treats Velero as ready only when `.status.readyReplicas` equals the string `"1"`. This is brittle: a healthy Velero Deployment can legitimately run with more than one replica.
### Issue Context
This check is part of the new DR sanity Tekton Task `dr-preflight` and blocks the entire pipeline.
### Fix Focus Areas
- components/disaster-recovery/development/pipeline.yaml[25-30]
### Suggested fix
Update the check to accept `readyReplicas >= 1` (or compare `readyReplicas` against the Deployment's desired replica count, e.g. `.spec.replicas`, and require them to match).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+518
to
+533
| echo "Finding post-restore Snapshot..." | ||
| SNAPSHOT=$(oc get snapshots.appstudio.redhat.com -n "$NS" \ | ||
| -l "appstudio.openshift.io/application=$APP" \ | ||
| --sort-by=.metadata.creationTimestamp -o jsonpath='{.items[-1].metadata.name}') | ||
| echo "Creating post-restore Release..." | ||
| RELEASE_NAME="dr-sanity-post-restore-release-$(date +%Y%m%d%H%M%S)" | ||
| cat <<RELEASE_EOF | oc apply -f - | ||
| apiVersion: appstudio.redhat.com/v1alpha1 | ||
| kind: Release | ||
| metadata: | ||
| name: $RELEASE_NAME | ||
| namespace: $NS | ||
| spec: | ||
| releasePlan: $RP | ||
| snapshot: $SNAPSHOT | ||
| RELEASE_EOF |
There was a problem hiding this comment.
2. Snapshot unchecked post-restore 🐞 Bug ≡ Correctness
dr-post-restore-verify creates a Release using $SNAPSHOT without verifying the snapshot lookup returned a name, so Release creation can fail (or be invalid) when no Snapshot exists.
Agent Prompt
### Issue description
In `dr-post-restore-verify`, the script queries for the latest Snapshot and then immediately interpolates it into a Release manifest without checking for an empty result.
### Issue Context
`dr-baseline-run` already contains an explicit `if [ -z "$SNAPSHOT" ]; then exit 1` guard; the post-restore path should be consistent to avoid attempting to create a Release with an empty snapshot reference.
### Fix Focus Areas
- components/disaster-recovery/development/pipeline.yaml[518-533]
### Suggested fix
Add an emptiness check after computing `SNAPSHOT` (mirroring `dr-baseline-run`), and `exit 1` with a clear error message if no Snapshot is found.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+566
to
+600
| results: | ||
| - name: test-result | ||
| description: Overall test result (PASSED or FAILED) | ||
| steps: | ||
| - name: cleanup | ||
| image: quay.io/konflux-ci/tools@sha256:b846eb68ab06f4c61c987bd1053e0e2dda13e18ecd5a75c945a18f7d78743bd8 | ||
| command: ["bash", "-c"] | ||
| args: | ||
| - 'sleep 5' | ||
| - | | ||
| echo "=== DR Cleanup ===" | ||
| NS="$(params.TENANT_NAMESPACE)" | ||
|
|
||
| # Remove NetworkPolicy if it was left behind | ||
| oc delete networkpolicy dr-block-image-controller -n image-controller --ignore-not-found 2>/dev/null || true | ||
|
|
||
| # Delete test Release CRs | ||
| for REL in $(oc get releases.appstudio.redhat.com -n "$NS" -o name 2>/dev/null | grep "dr-sanity"); do | ||
| oc delete "$REL" -n "$NS" --ignore-not-found 2>/dev/null || true | ||
| echo "Deleted $REL" | ||
| done | ||
|
|
||
| # Delete test Backup CRs | ||
| for BACKUP in $(oc get backups.velero.io -n openshift-adp -o name 2>/dev/null | grep "dr-sanity"); do | ||
| oc delete "$BACKUP" -n openshift-adp --ignore-not-found 2>/dev/null || true | ||
| echo "Deleted $BACKUP" | ||
| done | ||
|
|
||
| # Delete test Restore CRs | ||
| for RESTORE in $(oc get restores.velero.io -n openshift-adp -o name 2>/dev/null | grep "dr-sanity"); do | ||
| oc delete "$RESTORE" -n openshift-adp --ignore-not-found 2>/dev/null || true | ||
| echo "Deleted $RESTORE" | ||
| done | ||
|
|
||
| printf "COMPLETED" > "$(results.test-result.path)" | ||
| echo "=== Cleanup Done ===" |
There was a problem hiding this comment.
3. Cleanup result misleading value 🐞 Bug ⚙ Maintainability
dr-cleanup documents its test-result as "PASSED or FAILED" but always writes COMPLETED, making the published result inconsistent with its declared meaning.
Agent Prompt
### Issue description
The cleanup task's `test-result` result is described as `PASSED or FAILED`, but the implementation always writes `COMPLETED`.
### Issue Context
Even if the result is not currently consumed, it becomes a footgun for future use and is misleading during debugging.
### Fix Focus Areas
- components/disaster-recovery/development/pipeline.yaml[566-600]
### Suggested fix
Either:
- Change the result description to match the actual value (`COMPLETED`), or
- Write `PASSED`/`FAILED` based on an explicit input/param that indicates pipeline outcome (or remove the result entirely if it is not intended to be consumed).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 0bc47df |
…RBAC, and LocalQueue Replace the sleep pipeline with a full DR sanity test pipeline (6 Tasks: preflight, baseline-run, backup, disaster-and-restore, post-restore-verify, cleanup). Cluster-scoped and CRD-dependent resources are placed in staging/base/ only: - Tekton Triggers (CronJob, EventListener, TriggerBinding, TriggerTemplate) - ClusterRole/ClusterRoleBinding for dr-sanity-runner - Kueue LocalQueue - NetworkPolicy and cron-trigger RBAC CI ephemeral clusters do not have Tekton Triggers CRDs and the ArgoCD app is namespace-scoped, so cluster-scoped resources break sync (see revert redhat-appstudio#13183 of original redhat-appstudio#13074). CI clusters (development/): Namespace + ServiceAccount + Pipeline + Tasks. Staging clusters (staging/base/): adds CronJob + Triggers + RBAC + LocalQueue.
vsokolen-redhat
force-pushed
the
dr-sanity-pipeline-v2
branch
from
July 24, 2026 10:30
0bc47df to
1e11373
Compare
Comment on lines
+97
to
+110
| # Record PipelineRun count before triggering | ||
| BEFORE_COUNT=$(oc get pipelineruns -n "$NS" --no-headers 2>/dev/null | wc -l) | ||
|
|
||
| echo "Triggering build on $COMP..." | ||
| oc annotate component/"$COMP" -n "$NS" \ | ||
| build.appstudio.openshift.io/request=trigger-pac-build --overwrite | ||
|
|
||
| echo "Waiting for new build PipelineRun..." | ||
| for i in $(seq 1 120); do | ||
| CURRENT_COUNT=$(oc get pipelineruns -n "$NS" --no-headers 2>/dev/null | wc -l) | ||
| if [ "$CURRENT_COUNT" -gt "$BEFORE_COUNT" ]; then | ||
| BUILD_PR=$(oc get pipelineruns -n "$NS" \ | ||
| --sort-by=.metadata.creationTimestamp -o jsonpath='{.items[-1].metadata.name}') | ||
| echo "Found build PipelineRun: $BUILD_PR" |
There was a problem hiding this comment.
1. Pipelinerun correlation race 🐞 Bug ≡ Correctness
dr-baseline-run (and dr-post-restore-verify) identifies the build/release PipelineRun by watching the *total* PipelineRun count and then selecting the newest PipelineRun in the namespace, so concurrent unrelated PipelineRuns can be misidentified and the DR check can wait on/validate the wrong run.
Agent Prompt
## Issue description
The pipeline currently “guesses” the PipelineRun it should wait for by (a) counting all PipelineRuns in a namespace and (b) selecting the newest one. This can associate the DR sanity pipeline with an unrelated PipelineRun created concurrently, producing incorrect pass/fail results.
## Issue Context
This pattern appears in both the baseline run path and the post-restore verification path, and similarly for the release PipelineRun in the managed namespace.
## Fix Focus Areas
- components/disaster-recovery/development/pipeline.yaml[97-114]
- components/disaster-recovery/development/pipeline.yaml[181-200]
- components/disaster-recovery/development/pipeline.yaml[458-477]
## Suggested fix approach
- Correlate by a deterministic selector instead of “newest PipelineRun”, e.g.:
- Prefer label/field filtering that uniquely identifies the intended PipelineRun (if there is a known label for component/application/scenario/release), OR
- Capture the set of PipelineRun names *before* triggering, then after triggering compute the new names and apply additional checks (age window, expected labels/params/ownerReferences) before choosing, OR
- Avoid the indirect “annotate component -> controller creates PipelineRun” correlation by creating a PipelineRun/Run object directly (if acceptable for this DR test).
- Apply the same correlation approach for release PipelineRuns in `$MANAGED_NS` (don’t just pick the newest PipelineRun in that namespace).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+33
to
+36
| echo "Checking BackupStorageLocation..." | ||
| BSL_PHASE=$(oc get backupstoragelocations -n openshift-adp -o jsonpath='{.items[0].status.phase}' 2>/dev/null || echo "Unknown") | ||
| if [ "$BSL_PHASE" != "Available" ]; then | ||
| echo "FAIL: BSL not Available (phase=$BSL_PHASE)" |
There was a problem hiding this comment.
2. Arbitrary bsl selection 🐞 Bug ☼ Reliability
dr-preflight and dr-backup select the BackupStorageLocation via .items[0], which is an implicit selection policy; on clusters with multiple BSLs this can validate one BSL but create backups against a different (possibly unavailable/unintended) BSL.
Agent Prompt
## Issue description
The pipeline selects BackupStorageLocation using the first returned list item (`.items[0]`). That’s an implicit policy and can point to the wrong BSL when multiple locations exist.
## Issue Context
Other Velero-related manifests in this repo use an explicit `storageLocation` name, which avoids ambiguity.
## Fix Focus Areas
- components/disaster-recovery/development/pipeline.yaml[33-38]
- components/disaster-recovery/development/pipeline.yaml[228-255]
## Suggested fix approach
- Add a pipeline/task param like `VELERO_BSL_NAME` (with a sensible default for the target cluster) and:
- In preflight: check phase for that specific BSL by name.
- In backup: set `spec.storageLocation` to that same explicit name.
- Alternatively: select the BSL by filtering for `.status.phase==Available` and then ensure the *same chosen name* is used for both the preflight check and the Backup creation (don’t compute it twice independently).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 1e11373 |
|
@vsokolen-redhat: The following tests 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. |
Contributor
Author
|
/retest |
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
2 participants
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.
Summary
staging/base/— CI clusters lack Triggers CRDs, which broke ArgoCD sync at 43/44 apps ([KFLUXINFRA-3003] Replace DR sleep-pipeline with nightly sanity test pipeline #13074 → reverted in Revert "[KFLUXINFRA-3003] Address review feedback" (#13074) #13183)pipelines-queueLocalQueue — without it, PipelineRuns stay stuck in PipelineRunPending on staging clustersStructure
CI clusters (development/): Pipeline + 6 Tasks + dr-sanity-runner RBAC + Namespace + LocalQueue
Staging clusters (staging/base/ on top): CronJob (daily 02:00 UTC) + EventListener + TriggerBinding + TriggerTemplate + NetworkPolicy + cron-trigger RBAC
Note
~130 stale
sleep-pipeline-runPipelineRuns on stone-stg-rh01 should be cleaned up before or after this merges to prevent them from running when the LocalQueue becomes active.Risk Assessment
Jira: KFLUXINFRA-3003