From 445cd489e9513689ced67453c8e6a00f1a1cf708 Mon Sep 17 00:00:00 2001 From: Ishwar Kanse Date: Tue, 22 Sep 2026 16:01:30 +0530 Subject: [PATCH 1/6] Add suppressed-exception diagnostic to tracing-ui QE agent skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The qe-agent's tracing-ui skill misdiagnosed a real product bug as FLAKY on openshift/distributed-tracing-console-plugin#306: Tempo omits the `traces` field from `/api/search` on zero-match queries, crashing `@perses-dev/tempo-plugin`'s response parser. Cypress support/e2e.js suppresses that exact crash signature (Cypress.on('uncaught:exception', ...) returning false for 'Cannot read prop'/etc.), so the test only ever times out waiting for an element that never renders, and the real error — logged via browser console, not the Node process — never reaches qe-agent-commands.log or the JUnit XML. A single passing rerun on a fresh cluster then looked like flakiness, and the agent recommended cy.intercept()-based waits that would not have fixed anything. Adds a "Suppressed exception check" to Step 4 pointing at this mechanism and a concrete way to unmask it, plus a guard against calling FLAKY off an incomplete rerun loop. Other lines are wording trims to stay under skillsaw's 6,000-token budget; verified with `skillsaw lint` (0 errors). Co-Authored-By: Claude Sonnet 5 --- .../resources/skills/tracing-ui/SKILL.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md index bf9325ab94af..397a13e69ec3 100644 --- a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md +++ b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md @@ -157,7 +157,7 @@ Read the rerun JUnit XML: ### Flakiness confirmation loop -Run the rerun block 3 more times with `RUN=2`, `3` and `4` (one JUnit file each) and record the pass/fail pattern (e.g. `PFPP`). Look for missing `cy.intercept()` or condition-based waits before asserting UI state, `cy.get()` without a visibility wait, or a changed URL path. A failure in even 1 of 4 runs is `FLAKY` → Step 5c. +Run the rerun block 3 more times with `RUN=2`, `3` and `4` (one JUnit file each) and record the pass/fail pattern (e.g. `PFPP`). Look for missing `cy.intercept()` or condition-based waits before asserting UI state, `cy.get()` without a visibility wait, or a changed URL path. A failure in even 1 of 4 runs is `FLAKY` → Step 5c. An incomplete loop is tentative, never `FLAKY`. --- @@ -168,7 +168,7 @@ Run the diagnostics below before classifying — logs and resource status, read ### Cluster Observability Operator Diagnostics ```bash -# Auto-detect COO namespace (depends on install mode) +# Auto-detect COO namespace COO_NS="$(oc get pods --all-namespaces -l app.kubernetes.io/name=observability-operator -o jsonpath='{.items[0].metadata.namespace}' 2>/dev/null)" COO_NS="${COO_NS:-openshift-cluster-observability-operator}" @@ -177,7 +177,7 @@ oc get pods -n "${COO_NS}" oc logs -n "${COO_NS}" deploy/observability-operator --tail=150 2>/dev/null || true oc logs -n "${COO_NS}" deploy/observability-operator --previous --tail=50 2>/dev/null || true -# UIPlugins (cluster-scoped; control console plugin registration) and MonitoringStacks +# UIPlugins and MonitoringStacks oc get uiplugins -o jsonpath='{range .items[*]}{.metadata.namespace}/{.metadata.name}: {.status.conditions[*].type}={.status.conditions[*].status} {.status.conditions[*].message}{"\n"}{end}' 2>/dev/null || true oc get monitoringstacks --all-namespaces -o wide 2>/dev/null || true @@ -185,7 +185,7 @@ oc get monitoringstacks --all-namespaces -o wide 2>/dev/null || true oc get consoleplugin distributed-tracing-console-plugin -o jsonpath='{.status}{"\n"}' 2>/dev/null || true oc get consoles.operator.openshift.io cluster -o jsonpath='{.spec.plugins}{"\n"}' 2>/dev/null || true -# Events and CSV status in the COO namespace +# Events and CSV status oc get events -n "${COO_NS}" --sort-by='.lastTimestamp' | tail -20 oc get csv -n "${COO_NS}" -o jsonpath='{range .items[*]}{.metadata.name}: {.status.phase} — {.status.message}{"\n"}{end}' ``` @@ -200,7 +200,7 @@ oc api-resources | grep observability ### Operator catalog availability check -The `before` hook installs COO, OpenTelemetry, Tempo and Lightspeed from `redhat-operators`. Pre-GA OCP versions may not ship all of them yet; OperatorHub then never renders the install form (`[data-test="install-operator"]` times out). Check before blaming the test or product: +The `before` hook installs COO, OpenTelemetry, Tempo and Lightspeed from `redhat-operators`. Pre-GA OCP versions may lack one, so OperatorHub never renders the install form (`[data-test="install-operator"]` times out). Check before blaming the test or product: ```bash oc get clusterversion version -o jsonpath='{.status.desired.version}{"\n"}' @@ -211,6 +211,10 @@ done A package missing from the catalog on a pre-GA OCP version is `JOB_CONFIG` (Step 5e): do not add catalog auto-detection or skip logic to the test (it would silently skip the capability where the operator must exist), and do not file a product bug. +### Suppressed exception check + +`e2e.js`'s `uncaught:exception` handler swallows crashes like `'Cannot read prop'` — they surface as a timeout, invisible to `qe-agent-commands.log`/JUnit; a clean rerun doesn't rule it out. Before `FLAKY`/`TEST_ISSUE`, re-run with the filter commented out, or diff the API response against what the frontend expects. + ### Product Bug indicators Classify as `PRODUCT_BUG` when the operator, plugin, or console itself misbehaved: - COO operator pod in `CrashLoopBackOff` or `OOMKilled` @@ -226,7 +230,7 @@ Classify as `TEST_ISSUE` when the test itself is wrong or stale: ### Cluster Instability indicators -Before classifying as `CLUSTER_INSTABILITY`, rule out a tight COO reconciliation loop with debug logging: +Before `CLUSTER_INSTABILITY`, rule out a tight COO reconciliation loop with debug logging: ```bash COO_NS="$(oc get pods --all-namespaces -l app.kubernetes.io/name=observability-operator \ From 5b727f4c0032353c43802f15dd65fab1a15ca06e Mon Sep 17 00:00:00 2001 From: Ishwar Kanse Date: Tue, 22 Sep 2026 16:08:41 +0530 Subject: [PATCH 2/6] Route flaky-loop failures through Step 4 before classifying FLAKY CodeRabbit review on PR #85649: "A failure in even 1 of 4 runs is FLAKY -> Step 5c" sent the agent straight to the fix step, bypassing Step 4 entirely -- including the suppressed-exception check just added there. That defeats the point of the check: the real incident this skill update targets was exactly a rerun-loop result being called FLAKY without ever checking for a swallowed crash. Route through Step 4 first; only classify FLAKY if it finds no other explanation. Also trims a few more words elsewhere to stay within skillsaw's 6,000-token budget after the addition; re-verified with `skillsaw lint` (0 errors). Co-Authored-By: Claude Sonnet 5 --- .../qe-agent/resources/skills/tracing-ui/SKILL.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md index 397a13e69ec3..beb631f53d02 100644 --- a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md +++ b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md @@ -145,7 +145,7 @@ npx cypress run --browser chrome --headless --spec "e2e/dt-plugin-tests.cy.ts" \ ### Selecting what to rerun -- Keep `CYPRESS_SKIP_TESTS` in the grep (`;` separates patterns, `-` excludes): the `before` hook reads it too, e.g. `-Lightspeed` skips the Lightspeed install on OCP versions where Lightspeed is not published. +- Keep `CYPRESS_SKIP_TESTS` in the grep (`;` separates, `-` excludes): the `before` hook reads it too, e.g. `-Lightspeed` skips the Lightspeed install where it's not published. - Tests are order-dependent: `Capability:RBAC` creates the Tempo instances (`chainsaw-rbac / simplst`, `chainsaw-mmo-rbac / mmo-rbac`) and traces that every later test uses except `Capability:TLSCertRotation` and `Capability:Installation`. Include it for those tests (`GREP="Capability:RBAC; Capability:TraceLimits"`), otherwise the rerun fails on `input[placeholder="Select a Tempo instance"]`. - For a `before` hook failure, set `GREP` to only the `CYPRESS_SKIP_TESTS` value (empty runs every test). - `CYPRESS_SKIP_COO_INSTALL=true` skips the OperatorHub install path (`CYPRESS_COO_UI_INSTALL`, the job default). A passing rerun does not verify an install-path fix — mark it "not re-verified" in `CHANGES.md`. @@ -157,7 +157,7 @@ Read the rerun JUnit XML: ### Flakiness confirmation loop -Run the rerun block 3 more times with `RUN=2`, `3` and `4` (one JUnit file each) and record the pass/fail pattern (e.g. `PFPP`). Look for missing `cy.intercept()` or condition-based waits before asserting UI state, `cy.get()` without a visibility wait, or a changed URL path. A failure in even 1 of 4 runs is `FLAKY` → Step 5c. An incomplete loop is tentative, never `FLAKY`. +Run the rerun block 3 more times with `RUN=2`, `3` and `4` (one JUnit file each) and record the pass/fail pattern (e.g. `PFPP`). Look for missing `cy.intercept()` or condition-based waits before asserting UI state, `cy.get()` without a visibility wait, or a changed URL path. A failure in even 1 of 4 runs still goes through Step 4 first — call it `FLAKY` → Step 5c only if that finds no other explanation. An incomplete loop is tentative, never `FLAKY`. --- @@ -193,7 +193,7 @@ oc get csv -n "${COO_NS}" -o jsonpath='{range .items[*]}{.metadata.name}: {.stat ### CRD and API availability check ```bash -# Missing CRDs cause plugin registration failures +# Missing CRDs cause registration failures oc get crd | grep -E 'observability|uiplugin|monitoringstack' oc api-resources | grep observability ``` @@ -213,7 +213,7 @@ A package missing from the catalog on a pre-GA OCP version is `JOB_CONFIG` (Step ### Suppressed exception check -`e2e.js`'s `uncaught:exception` handler swallows crashes like `'Cannot read prop'` — they surface as a timeout, invisible to `qe-agent-commands.log`/JUnit; a clean rerun doesn't rule it out. Before `FLAKY`/`TEST_ISSUE`, re-run with the filter commented out, or diff the API response against what the frontend expects. +`e2e.js`'s `uncaught:exception` handler swallows crashes like `'Cannot read prop'` — they surface as a timeout, invisible to `qe-agent-commands.log`/JUnit. Before `FLAKY`/`TEST_ISSUE`, re-run with the filter commented out, or diff the API response against what the frontend expects. ### Product Bug indicators Classify as `PRODUCT_BUG` when the operator, plugin, or console itself misbehaved: From f007fb55316dd79f72f72283ab506dd3d03bd4d7 Mon Sep 17 00:00:00 2001 From: Ishwar Kanse Date: Tue, 22 Sep 2026 16:14:30 +0530 Subject: [PATCH 3/6] Fold in operational gaps the QE agent found running on PR #306 The completed qe-agent-analysis.md from build 2100954650738954240 self-reported two operational gaps in Step 0b, both directly hit during that run (confirmed from its command audit log): - Operator installs/OperatorGroups: the skill assumed "already installed by the original run", but the suite's after() hook had actually deleted COO/OTel/Tempo, so the agent had to install them via CLI before it could rerun anything. - Cypress binary: npx cypress version succeeded while the binary only existed at /root/.cache/Cypress/, not $CYPRESS_CACHE_FOLDER (/tmp/Cypress); the agent had to discover and copy it manually. Both are pod-image/environment facts that will recur on every run, not one-off flakiness, so they're worth encoding directly rather than re-discovering each time. Left out the agent's third suggestion (Bash timeout margin) since the skill already tells the agent to use run_in_background for reruns -- that recommendation didn't point at a real gap. A few more wording trims elsewhere to stay under skillsaw's 6,000-token budget; verified with `skillsaw lint` (0 errors). Co-Authored-By: Claude Sonnet 5 --- .../qe-agent/resources/skills/tracing-ui/SKILL.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md index beb631f53d02..7e4b8c3c7743 100644 --- a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md +++ b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md @@ -94,12 +94,13 @@ Export the `env` vars, then run the script's setup section (up to the first `npx | Script pattern | Adaptation | |---|---| -| `cp -R /tmp/` (image mount) | `git clone ` — repo URL from the step script or CI config | -| `kubectl create -f ` (CRDs) | `kubectl apply -f ` — `create` fails if the CRD exists | +| `cp -R /tmp/` (image mount) | `git clone ` — URL from the step script | +| `kubectl create -f ` (CRDs) | `kubectl apply -f ` — `create` fails if it exists | | `oc patch csv ...` | Skip — already patched; verify with `oc get csv -n openshift-cluster-observability-operator` | | `CYPRESS_SKIP_TESTS` block | Keep it; reruns use it (Step 3) | -| htpasswd / oauth setup | Create the secret only if `oc get secret htpass-secret -n openshift-config` fails; patch oauth only if the htpasswd IDP is missing | -| Operator installs / OperatorGroups | Already installed by the original run — check `oc get csv -A`, do not reinstall. Check `oc get operatorgroup -n ` before creating one; a second OperatorGroup fails the CSV ("csv created in namespace with multiple operatorgroups") | +| htpasswd / oauth setup | Create the secret only if `oc get secret htpass-secret -n openshift-config` fails; patch oauth only if the IDP is missing | +| Operator installs / OperatorGroups | Verify — check `oc get csv -A`; install only if missing (`after()` can delete COO/OTel/Tempo/Lightspeed). Check `oc get operatorgroup -n ` first; a second one fails the CSV | +| Cypress binary | `npx cypress version` can pass while the binary is only under `/root/.cache/Cypress/`, not `$CYPRESS_CACHE_FOLDER`. If empty: `cp -r /root/.cache/Cypress/*/ /tmp/Cypress/` | Then continue with Steps 1–6 in the cloned repo. If `qe-agent-context.json` is missing, infer the suite from the JUnit prefix, skip the rerun, and diagnose from the JUnit content and cluster state. @@ -109,7 +110,7 @@ Read `${SHARED_DIR}/qe-agent-junit-*.xml`. For each file extract the suite name ### High-failure triage: more than 5 failures total -More than 5 failures usually share one root cause (console plugin not loaded, auth failure, UI not responding, network error, or a failed `before` hook). Look for a common pattern: the same error string (`Cannot read properties of null`, `element not found`, `401 Unauthorized`, `plugin not enabled`), the same failing Cypress command (`cy.visit`, `cy.get`, `cy.findByText`), or tightly clustered failure times. +More than 5 failures usually share one root cause (plugin not loaded, auth failure, UI not responding, network error, or a failed `before` hook). Look for a common pattern: the same error string (`Cannot read properties of null`, `401 Unauthorized`), the same failing Cypress command (`cy.visit`, `cy.get`), or tightly clustered failure times. - **Clear pattern**: pick the simplest failing test as the representative and run Steps 2–5 for it only. - **No clear pattern**: process failures individually, cap at 3 tests, and note this in the summary. @@ -120,7 +121,7 @@ Write the pattern conclusion near the top of `${ARTIFACT_DIR}/qe-agent-analysis. ## Step 2 — Locate Test Source Files -The repo root is the clone destination from the step script — do not scan `/tmp/` broadly. All tests are in one spec, `tests/e2e/dt-plugin-tests.cy.ts`: a single `describe` whose `before` hook installs/verifies the operators, sets up Lightspeed and creates the UIPlugin, then one `it` per capability. A `before` hook failure skips every test. Locate the `it` block with `grep -n ""`. Supporting files under `tests/`: Cypress config, `cypress/support/` custom commands (e.g. `cy.runChainsawTest`), `views/` page objects, `fixtures/` chainsaw tests. +The repo root is the clone destination — do not scan `/tmp/` broadly. All tests are in one spec, `tests/e2e/dt-plugin-tests.cy.ts`: a single `describe` whose `before` hook installs/verifies the operators, sets up Lightspeed and creates the UIPlugin, then one `it` per capability. A `before` hook failure skips every test. Locate the `it` block with `grep -n ""`. Supporting files under `tests/`: Cypress config, `cypress/support/` custom commands (e.g. `cy.runChainsawTest`), `views/` page objects, `fixtures/` chainsaw tests. --- @@ -163,7 +164,7 @@ Run the rerun block 3 more times with `RUN=2`, `3` and `4` (one JUnit file each) ## Step 4 — Diagnose: Product Bug vs Test Issue -Run the diagnostics below before classifying — logs and resource status, read with the failure message and test source, are the primary evidence. +Run the diagnostics below first; logs and resource status are the primary evidence alongside the failure message and test source. ### Cluster Observability Operator Diagnostics From c81d95909763f1d362cf49ffe3df5f549a3737f7 Mon Sep 17 00:00:00 2001 From: Ishwar Kanse Date: Tue, 22 Sep 2026 16:25:27 +0530 Subject: [PATCH 4/6] Address second CodeRabbit review: pin clone branch, check CSV phase Two of the review's actionable comments were valid: - Preserve the step script's branch when cloning. Verified against the real script (distributed-tracing-tests-tracing-ui-upstream- commands.sh:198): it clones with `--branch main --single-branch` deliberately, into a separate directory from the PR-under-test's own pre-populated checkout, because the e2e specs are meant to run from main regardless of what the PR under test changes. The skill's adaptation row dropped that pin; restored it. - Check CSV readiness, not just presence, before skipping operator install. Existence alone doesn't rule out a Failed or Terminating CSV -- which after() (already known to delete operators, per the prior commit) could plausibly leave behind instead of a clean absence. Now requires phase Succeeded. Left out the finding's other half -- verifying the htpasswd secret "contains the expected data": the credentials are freshly randomized every run (tr < /dev/urandom), so there's no fixed "expected data" to check, and a broken login surfaces immediately as a `before` hook failure Step 4 already diagnoses. More wording trims to stay under skillsaw's 6,000-token budget; verified with `skillsaw lint` (0 errors). Co-Authored-By: Claude Sonnet 5 --- .../resources/skills/tracing-ui/SKILL.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md index 7e4b8c3c7743..9490c54921fa 100644 --- a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md +++ b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md @@ -29,10 +29,10 @@ Read `${SHARED_DIR}/qe-agent-context.json`, written by the test step at exit: } ``` -- `step_script_ref` — path relative to `ci-operator/step-registry/` in openshift/release +- `step_script_ref` — path relative to `ci-operator/step-registry/` - `env` — job-time env values needed to reproduce setup; `CYPRESS_SKIP_TESTS` is the job's `@cypress/grep` pattern (empty = all tests), applied to reruns in Step 3 -Fetch `https://raw.githubusercontent.com/openshift/release/main/ci-operator/step-registry/`. Everything before the first `npx cypress run` / `npm run` is **setup** (repo clone, IDP/htpasswd, env vars, `npm install`); the rest is **test execution**. +Fetch `https://raw.githubusercontent.com/openshift/release/main/ci-operator/step-registry/`. Everything before the first `npx cypress run` / `npm run` is **setup** (clone, IDP/htpasswd, env vars, `npm install`); the rest is **test execution**. ## Step 0a — Verify Cluster Stability @@ -94,12 +94,12 @@ Export the `env` vars, then run the script's setup section (up to the first `npx | Script pattern | Adaptation | |---|---| -| `cp -R /tmp/` (image mount) | `git clone ` — URL from the step script | +| `cp -R /tmp/` (image mount) | `git clone --branch main --single-branch ` — URL/branch from the step script | | `kubectl create -f ` (CRDs) | `kubectl apply -f ` — `create` fails if it exists | | `oc patch csv ...` | Skip — already patched; verify with `oc get csv -n openshift-cluster-observability-operator` | | `CYPRESS_SKIP_TESTS` block | Keep it; reruns use it (Step 3) | | htpasswd / oauth setup | Create the secret only if `oc get secret htpass-secret -n openshift-config` fails; patch oauth only if the IDP is missing | -| Operator installs / OperatorGroups | Verify — check `oc get csv -A`; install only if missing (`after()` can delete COO/OTel/Tempo/Lightspeed). Check `oc get operatorgroup -n ` first; a second one fails the CSV | +| Operator installs / OperatorGroups | Verify readiness, not just presence — require CSV phase `Succeeded`; (re)install if missing or not Succeeded (`after()` can delete or fail COO/OTel/Tempo/Lightspeed). Check `oc get operatorgroup -n ` first; a second one fails the CSV | | Cypress binary | `npx cypress version` can pass while the binary is only under `/root/.cache/Cypress/`, not `$CYPRESS_CACHE_FOLDER`. If empty: `cp -r /root/.cache/Cypress/*/ /tmp/Cypress/` | Then continue with Steps 1–6 in the cloned repo. If `qe-agent-context.json` is missing, infer the suite from the JUnit prefix, skip the rerun, and diagnose from the JUnit content and cluster state. @@ -134,7 +134,7 @@ Before rerunning, inspect the RBAC test's `chainsaw-*` namespaces and `verify-tr ```bash cd "/tests" export NO_COLOR=1 CYPRESS_CACHE_FOLDER=/tmp/Cypress CYPRESS_SKIP_COO_INSTALL=true -# Fresh shell: also re-export the CYPRESS_* vars from the step script setup (base URL, login, kubeconfig, Lightspeed) +# Fresh shell: also re-export the CYPRESS_* vars from Step 0b setup CYPRESS_SKIP_TESTS=$(jq -r '.env.CYPRESS_SKIP_TESTS // ""' "${SHARED_DIR}/qe-agent-context.json" 2>/dev/null) GREP="" [[ -n "${CYPRESS_SKIP_TESTS}" ]] && GREP="${GREP}; ${CYPRESS_SKIP_TESTS}" @@ -147,9 +147,9 @@ npx cypress run --browser chrome --headless --spec "e2e/dt-plugin-tests.cy.ts" \ ### Selecting what to rerun - Keep `CYPRESS_SKIP_TESTS` in the grep (`;` separates, `-` excludes): the `before` hook reads it too, e.g. `-Lightspeed` skips the Lightspeed install where it's not published. -- Tests are order-dependent: `Capability:RBAC` creates the Tempo instances (`chainsaw-rbac / simplst`, `chainsaw-mmo-rbac / mmo-rbac`) and traces that every later test uses except `Capability:TLSCertRotation` and `Capability:Installation`. Include it for those tests (`GREP="Capability:RBAC; Capability:TraceLimits"`), otherwise the rerun fails on `input[placeholder="Select a Tempo instance"]`. -- For a `before` hook failure, set `GREP` to only the `CYPRESS_SKIP_TESTS` value (empty runs every test). -- `CYPRESS_SKIP_COO_INSTALL=true` skips the OperatorHub install path (`CYPRESS_COO_UI_INSTALL`, the job default). A passing rerun does not verify an install-path fix — mark it "not re-verified" in `CHANGES.md`. +- Tests are order-dependent: `Capability:RBAC` creates the Tempo instances (`chainsaw-rbac / simplst`, `chainsaw-mmo-rbac / mmo-rbac`) and traces every later test uses except `Capability:TLSCertRotation`/`Capability:Installation`. Include it for others (`GREP="Capability:RBAC; Capability:TraceLimits"`), otherwise the rerun fails on `input[placeholder="Select a Tempo instance"]`. +- For a `before` hook failure, set `GREP` to just `CYPRESS_SKIP_TESTS` (empty runs every test). +- `CYPRESS_SKIP_COO_INSTALL=true` skips the OperatorHub install path (`CYPRESS_COO_UI_INSTALL`, the job default). A passing rerun doesn't verify an install-path fix — mark it "not re-verified" in `CHANGES.md`. Read the rerun JUnit XML: - **Same failure** → Step 4 From 1bceb9a44809d5a3cfe89f64f926c3a0f6e88ba6 Mon Sep 17 00:00:00 2001 From: Ishwar Kanse Date: Tue, 22 Sep 2026 16:36:08 +0530 Subject: [PATCH 5/6] Address third CodeRabbit review: real secret/IDP names, persist CYPRESS_* vars Both actionable comments were valid, and both are pre-existing gaps (not introduced by this branch) surfaced because this PR happened to touch the same lines: - The setup script creates `uiauto-htpass-secret` and `uiauto-htpasswd-idp` (verified: distributed-tracing-tests- tracing-ui-upstream-commands.sh:139-140), not the generic `htpass-secret` the table checked for. That guard would always report the secret missing and recreate it every time. - Step 3's rerun command ran in "a fresh shell" per its own comment, but only *said* to re-export CYPRESS_BASE_URL/KUBECONFIG_PATH/ LOGIN_IDP/LOGIN_USERS without showing how, and the Bash tool this agent runs on does not persist shell state between commands (true of this very session, too) -- so a rerun could easily execute without them. Added the literal export lines (matching what the original step script does, and what the one completed qe-agent run actually had to reconstruct by hand) directly into the rerun block. More wording trims across Step 5c/5d/5e and the closing notes to stay under skillsaw's 6,000-token budget; verified with `skillsaw lint` (0 errors). Co-Authored-By: Claude Sonnet 5 --- .../resources/skills/tracing-ui/SKILL.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md index 9490c54921fa..9094a20cdfb2 100644 --- a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md +++ b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md @@ -98,7 +98,7 @@ Export the `env` vars, then run the script's setup section (up to the first `npx | `kubectl create -f ` (CRDs) | `kubectl apply -f ` — `create` fails if it exists | | `oc patch csv ...` | Skip — already patched; verify with `oc get csv -n openshift-cluster-observability-operator` | | `CYPRESS_SKIP_TESTS` block | Keep it; reruns use it (Step 3) | -| htpasswd / oauth setup | Create the secret only if `oc get secret htpass-secret -n openshift-config` fails; patch oauth only if the IDP is missing | +| htpasswd / oauth setup | Create `uiauto-htpass-secret` only if `oc get secret uiauto-htpass-secret -n openshift-config` fails; patch oauth only if `uiauto-htpasswd-idp` is missing | | Operator installs / OperatorGroups | Verify readiness, not just presence — require CSV phase `Succeeded`; (re)install if missing or not Succeeded (`after()` can delete or fail COO/OTel/Tempo/Lightspeed). Check `oc get operatorgroup -n ` first; a second one fails the CSV | | Cypress binary | `npx cypress version` can pass while the binary is only under `/root/.cache/Cypress/`, not `$CYPRESS_CACHE_FOLDER`. If empty: `cp -r /root/.cache/Cypress/*/ /tmp/Cypress/` | @@ -134,7 +134,11 @@ Before rerunning, inspect the RBAC test's `chainsaw-*` namespaces and `verify-tr ```bash cd "/tests" export NO_COLOR=1 CYPRESS_CACHE_FOLDER=/tmp/Cypress CYPRESS_SKIP_COO_INSTALL=true -# Fresh shell: also re-export the CYPRESS_* vars from Step 0b setup +# Fresh shell — these don't persist, set them in every rerun command: +export CYPRESS_KUBECONFIG_PATH="${KUBECONFIG}" +export CYPRESS_BASE_URL="https://$(oc get route console -n openshift-console -o jsonpath='{.spec.host}')" +export CYPRESS_LOGIN_IDP="kube:admin" +export CYPRESS_LOGIN_USERS="kubeadmin:$(cat /tmp/secret/kubeadmin-password)" CYPRESS_SKIP_TESTS=$(jq -r '.env.CYPRESS_SKIP_TESTS // ""' "${SHARED_DIR}/qe-agent-context.json" 2>/dev/null) GREP="" [[ -n "${CYPRESS_SKIP_TESTS}" ]] && GREP="${GREP}; ${CYPRESS_SKIP_TESTS}" @@ -364,19 +368,19 @@ The description must NOT contain raw credentials, tokens, passwords, or SHA-256 ## Step 5c — If FLAKY: Fix and Export -Fix the race itself, not with blanket retries. Typical fixes: `cy.intercept()` plus `cy.wait('@alias')` before asserting UI state after an API call, `.should('be.visible')` with a timeout instead of an immediate assertion, and condition-based waits instead of `cy.wait()`. Export the changed files and `CHANGES.md` as in Step 5a, with the 4-run pass/fail pattern as evidence. +Fix the race itself, not with blanket retries: `cy.intercept()` + `cy.wait('@alias')` before asserting post-API UI state, `.should('be.visible')` with a timeout, condition-based waits instead of `cy.wait()`. Export as in Step 5a, with the 4-run pattern as evidence. --- ## Step 5d — If CLUSTER_INSTABILITY: Write Incident Note -Write `${ARTIFACT_DIR}/cluster-instability-report.md` with: a one-sentence summary; a table of affected tests (suite / test case / original duration / rerun duration); root cause (MCP updates, node evictions, COO pod restarts — include the MCP status snapshot from Step 0a); evidence (MCP output, relevant pod events); and a recommendation to rerun the CI job. Begin it with the same AI-Generated Content banner as the other reports. +Write `${ARTIFACT_DIR}/cluster-instability-report.md`: a one-sentence summary; affected-tests table (suite / test case / original vs rerun duration); root cause (MCP updates, node evictions, COO restarts — include the Step 0a MCP snapshot); evidence (MCP output, pod events); and a recommendation to rerun the job. Same AI-Generated Content banner as the other reports. --- ## Step 5e — If JOB_CONFIG: Recommend a Job Configuration Change -The test and product are fine, but the job needs something this cluster cannot provide (typically an operator not yet published for this OCP version). Do not modify the tests or write `jira-payload.json`. In `qe-agent-analysis.md`, give the missing package, OCP version and catalog evidence, and recommend the config change. For Lightspeed, add `CYPRESS_SKIP_TESTS: -Lightspeed` to the e2e `env` in `ci-operator/config/openshift/distributed-tracing-console-plugin/.yaml`; the spec then skips the Lightspeed install, setup and test. If COO, OpenTelemetry or Tempo is missing, recommend disabling or re-pointing the job instead. +The test and product are fine, but the job needs something the cluster can't provide (typically an operator unpublished for this OCP version). Don't modify tests or write `jira-payload.json`. In `qe-agent-analysis.md`, give the missing package, OCP version and catalog evidence, and recommend the config change. For Lightspeed, add `CYPRESS_SKIP_TESTS: -Lightspeed` to the e2e `env` in `ci-operator/config/openshift/distributed-tracing-console-plugin/.yaml`. If COO, OpenTelemetry or Tempo is missing, recommend disabling or re-pointing the job. --- @@ -438,7 +442,7 @@ Write `${ARTIFACT_DIR}/qe-agent-analysis.md` after each diagnosis and overwrite - The cluster is already provisioned with COO and the Tracing UI console plugin installed — do not reinstall them - The qe-agent runs in a fresh pod, so `/tmp/` is empty at start; Step 0b clones the test repo there - `$KUBECONFIG` points to the test cluster; `oc`, `kubectl`, `jq` and `npm`/`npx` are in PATH -- Do **not** copy Cypress screenshots or videos to `$SHARED_DIR` (1 MiB Secret limit); only JUnit XML is safe there. Write all output to `$ARTIFACT_DIR` (uploaded to GCS) or `$SHARED_DIR` (shared with other steps) +- Do **not** copy screenshots/videos to `$SHARED_DIR` (1 MiB Secret limit); only JUnit XML is safe there. Write output to `$ARTIFACT_DIR` (GCS) or `$SHARED_DIR` (shared with other steps) - **Namespace restriction**: You MUST NOT access, read, list, or modify any resource in the `kube-system` namespace (cloud provider credentials, platform-critical components) — no `oc` or `kubectl` command may target it. Filter `kube-system` out of all-namespace output (e.g. `oc get pods -A`) before analysis -- Do not call external APIs (Jira, GitHub API, Slack, etc.); the wrapper script handles integrations after the agent exits +- Do not call external APIs (Jira, GitHub, Slack); the wrapper script handles integrations after exit - This step runs `best_effort: true` — always exit 0 even if analysis is incomplete From efa5ab21c89268d9a8b5816641ebf3bc8fc73fbf Mon Sep 17 00:00:00 2001 From: Ishwar Kanse Date: Tue, 22 Sep 2026 16:45:20 +0530 Subject: [PATCH 6/6] Address fourth CodeRabbit review: close the kube-system fetch loophole Two actionable comments this round; one valid, one not: - Invalid: "export CYPRESS_SKIP_TESTS so Cypress/before() can read it." Checked the actual test source (tests/e2e/dt-plugin-tests.cy.ts:35): SKIP_LIGHTSPEED comes from `Cypress.env('grep')`, which the skill already passes correctly via `--env grep="${GREP}"`. Cypress never reads CYPRESS_SKIP_TESTS itself -- it's purely a shell-side value folded into GREP. Adding export would be a no-op; skipped. - Valid: the "Namespace restriction" bullet said to fetch all-namespaces output and filter kube-system out "before analysis" -- which still executes the request against kube-system first. This is exactly what the one completed qe-agent run did in practice (`oc get csv -A ... | grep -v kube-system`, visible in its own command log), contradicting the bullet's own "MUST NOT access" opening sentence. Replaced the guidance with what the skill's own COO diagnostics already do correctly elsewhere: scope by namespace or a label selector that structurally excludes kube-system, never -A piped to grep. More wording trims to close out under skillsaw's 6,000-token budget; verified with `skillsaw lint` (0 errors). Co-Authored-By: Claude Sonnet 5 --- .../qe-agent/resources/skills/tracing-ui/SKILL.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md index 9094a20cdfb2..9078bada603a 100644 --- a/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md +++ b/ci-operator/step-registry/openshift-observability/qe-agent/resources/skills/tracing-ui/SKILL.md @@ -440,9 +440,9 @@ Write `${ARTIFACT_DIR}/qe-agent-analysis.md` after each diagnosis and overwrite ## Notes for CI context - The cluster is already provisioned with COO and the Tracing UI console plugin installed — do not reinstall them -- The qe-agent runs in a fresh pod, so `/tmp/` is empty at start; Step 0b clones the test repo there -- `$KUBECONFIG` points to the test cluster; `oc`, `kubectl`, `jq` and `npm`/`npx` are in PATH +- The qe-agent runs in a fresh pod — `/tmp/` is empty at start; Step 0b clones the test repo there +- `$KUBECONFIG` points to the test cluster; `oc`, `kubectl`, `jq`, `npm`/`npx` are in PATH - Do **not** copy screenshots/videos to `$SHARED_DIR` (1 MiB Secret limit); only JUnit XML is safe there. Write output to `$ARTIFACT_DIR` (GCS) or `$SHARED_DIR` (shared with other steps) -- **Namespace restriction**: You MUST NOT access, read, list, or modify any resource in the `kube-system` namespace (cloud provider credentials, platform-critical components) — no `oc` or `kubectl` command may target it. Filter `kube-system` out of all-namespace output (e.g. `oc get pods -A`) before analysis +- **Namespace restriction**: MUST NOT access, read, list, or modify any `kube-system` resource — an all-namespaces query filtered afterward still fetches it, so scope every command to named namespaces or a label selector that excludes it (e.g. `-l app.kubernetes.io/name=...`, as used above), not `-A` piped to `grep -v kube-system` - Do not call external APIs (Jira, GitHub, Slack); the wrapper script handles integrations after exit -- This step runs `best_effort: true` — always exit 0 even if analysis is incomplete +- This step runs `best_effort: true` — always exit 0, even incomplete