Skip to content

Tour and logs#1947

Open
JoaoFula wants to merge 5 commits into
openshift:release-4.19from
JoaoFula:tour-and-logs
Open

Tour and logs#1947
JoaoFula wants to merge 5 commits into
openshift:release-4.19from
JoaoFula:tour-and-logs

Conversation

@JoaoFula
Copy link
Copy Markdown
Contributor

@JoaoFula JoaoFula commented May 15, 2026

Summary by CodeRabbit

  • Tests

    • Enhanced test infrastructure with improved cluster artifact collection for diagnostics and troubleshooting
    • Refined test error handling for more stable test execution
  • Chores

    • Updated CI/CD pipeline configuration for better integration testing and artifact retention

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Warning

Rate limit exceeded

@JoaoFula has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 51 minutes and 16 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8e0b9fc2-01e8-4647-bb0d-6e0db4bd068c

📥 Commits

Reviewing files that changed from the base of the PR and between 991dc33 and f5ce6c7.

📒 Files selected for processing (3)
  • .tekton/integration-tests/lightspeed-console-pre-commit.yaml
  • cypress.config.ts
  • tests/tests/lightspeed-install.cy.ts
📝 Walkthrough

Walkthrough

This PR enhances observability infrastructure across CI/CD and test environments. The Tekton pipeline is updated to select the correct component for metadata extraction and export logs to Quay via a finally block. Cypress tests gain cluster artifact collection utilities that gather OpenShift resources and pod logs when tests complete, plus exception handling to suppress known plugin-loading timing errors.

Changes

Artifact Collection and Log Export Infrastructure

Layer / File(s) Summary
Pipeline metadata extraction and log export
.tekton/integration-tests/lightspeed-console-pre-commit.yaml
Snapshot metadata extraction queries lightspeed-console-4-19 component for console-image and commit. Pipeline finally block runs export-logs-for-retention task to export logs to quay.io/openshift-lightspeed/ols-console-artifacts using bot credentials.
Cypress cluster artifact gathering
cypress.config.ts
Adds runOC synchronous oc command executor and gatherClusterArtifacts function to create directory structure, fetch cluster resource YAML, and write per-pod-container logs. Updated after:spec hook calls artifact gathering using config.env.KUBECONFIG_PATH when tests complete without failures.
Test exception handler for plugin loading
cypress/support/e2e.js
Global Cypress uncaught:exception listener suppresses exceptions whose messages include __load_plugin_entry__ while allowing other uncaught exceptions to propagate.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • openshift/lightspeed-console#1948: Directly addresses the same snapshot metadata extraction change to use lightspeed-console-4-19 component for console image exporting.

Suggested labels

lgtm, approved

Suggested reviewers

  • joshuawilson

Poem

🐰 A log in the shadows now finds its way,
Cluster artifacts gather throughout the day,
Plugin errors fade with a gentle hand,
Test observation across the land!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Tour and logs' is vague and does not clearly describe the main changes in the pull request, which involve both pipeline artifact export and Cypress test logging/artifact collection. Consider a more descriptive title that captures the primary changes, such as 'Add cluster artifact collection and pipeline log export' or 'Improve test logging with cluster artifacts and pipeline exports'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested review from joshuawilson and kyoto May 15, 2026 09:10
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 15, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign syedriko for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
cypress/support/e2e.js (1)

7-15: ⚡ Quick win

Consider logging suppressed exceptions for observability.

While the handler correctly suppresses the known timing issue, suppressed errors leave no trace in test output. Adding a log statement would help track how often this occurs and aid debugging if the pattern inadvertently matches other errors.

📊 Suggested enhancement for observability
 Cypress.on('uncaught:exception', (err) => {
   // The console defines __load_plugin_entry__ via webpack module federation
   // before loading plugin bundles, but race conditions during page load can
   // cause the plugin script to execute before the global is set.
   // This is a console-internal timing issue, not a test failure.
   if (err.message?.includes('__load_plugin_entry__')) {
+    console.log('[Suppressed] Known plugin loading timing error:', err.message);
     return false;
   }
 });
🤖 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 `@cypress/support/e2e.js` around lines 7 - 15, The uncaught exception handler
(Cypress.on("uncaught:exception", ...)) in e2e.js currently suppresses a known
timing error with a bare return false; update that handler to emit a log entry
(e.g., console.warn or Cypress.log) before returning false that includes the
error message and stack (and optional context like the test title) so suppressed
exceptions are observable and countable for debugging.
🤖 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 @.tekton/integration-tests/lightspeed-console-pre-commit.yaml:
- Around line 132-133: The jq extraction for the component
"lightspeed-console-4-19" can produce empty strings (from SNAPHOT) and later
cause failures; update the script that writes to step.results.console-image.path
and step.results.commit.path to first assign the jq results to variables (e.g.,
CONSOLE_IMAGE and COMMIT), validate both are non-empty, and if either is missing
emit a clear error and exit non‑zero; keep the same jq queries (using SNAPSHOT
and the component name) and only write to the result files after the existence
checks succeed.

---

Nitpick comments:
In `@cypress/support/e2e.js`:
- Around line 7-15: The uncaught exception handler
(Cypress.on("uncaught:exception", ...)) in e2e.js currently suppresses a known
timing error with a bare return false; update that handler to emit a log entry
(e.g., console.warn or Cypress.log) before returning false that includes the
error message and stack (and optional context like the test title) so suppressed
exceptions are observable and countable for debugging.
🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c3947880-3879-4fe6-83f8-90ec5157a1a7

📥 Commits

Reviewing files that changed from the base of the PR and between 16e82d2 and 991dc33.

📒 Files selected for processing (3)
  • .tekton/integration-tests/lightspeed-console-pre-commit.yaml
  • cypress.config.ts
  • cypress/support/e2e.js

Comment on lines +132 to +133
echo -n "$(jq -r --arg component_name "lightspeed-console-4-19" '.components[] | select(.name == $component_name) | .containerImage' <<< "$SNAPSHOT")" > $(step.results.console-image.path)
echo -n "$(jq -r --arg component_name "lightspeed-console-4-19" '.components[] | select(.name == $component_name) | .source.git.revision' <<< "$SNAPSHOT")" > $(step.results.commit.path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify component naming consistency

# Search for any component name references in the codebase
echo "=== Searching for component name references ==="
rg -n "lightspeed-console" --type yaml -g '!node_modules/**' -C 2

# Check for other release-specific component patterns
echo -e "\n=== Checking for other version-specific naming patterns ==="
rg -n "\"components\"\s*:\s*\[" .tekton/ -A 10

Repository: openshift/lightspeed-console

Length of output: 1333


🏁 Script executed:

# Read the specific lines under review
sed -n '130,135p' .tekton/integration-tests/lightspeed-console-pre-commit.yaml

Repository: openshift/lightspeed-console

Length of output: 580


🏁 Script executed:

# Search for SNAPSHOT references and how components are defined
rg -n "SNAPSHOT" .tekton/ -A 3 -B 3 | head -50

Repository: openshift/lightspeed-console

Length of output: 4544


🏁 Script executed:

# Look for any hardcoded component names in pipeline definitions
rg -n "component.*name.*=" .tekton/ -A 2 -B 2

Repository: openshift/lightspeed-console

Length of output: 970


🏁 Script executed:

# Check for snapshot/component configurations
fd -e json -e yaml | xargs rg -l "components" | head -10

Repository: openshift/lightspeed-console

Length of output: 85


🏁 Script executed:

# Search for Konflux release configuration and component definitions
find . -name "*release*" -type f | head -20

Repository: openshift/lightspeed-console

Length of output: 117


🏁 Script executed:

# Check for .tekton directory structure and any release configs
ls -la .tekton/ 2>/dev/null || echo "No .tekton at root"
fd -t f -H "release\|konflux\|snapshot" --max-depth 3

Repository: openshift/lightspeed-console

Length of output: 661


🏁 Script executed:

# Look for any documentation about components or snapshots
fd -t f -e md -e txt -e rst | xargs rg -l "component.*snapshot\|SNAPSHOT.*component" 2>/dev/null | head -10

Repository: openshift/lightspeed-console

Length of output: 54


🏁 Script executed:

# Search for how component names are generated or referenced in scripts
rg -n "component.*name\|name.*component" .tekton/ --type yaml -A 1 -B 1

Repository: openshift/lightspeed-console

Length of output: 54


🏁 Script executed:

# Check release-branches.md for documentation about components and snapshots
cat .cursor/skills/release/release-branches.md

Repository: openshift/lightspeed-console

Length of output: 110


🏁 Script executed:

# Look at the main pull-request and push pipeline files to see how SNAPSHOT is referenced
sed -n '1,50p' .tekton/lightspeed-console-4-19-pull-request.yaml | grep -A 10 -B 5 "SNAPSHOT\|component"

Repository: openshift/lightspeed-console

Length of output: 598


🏁 Script executed:

# Check if there are any Konflux-specific files that define components
find . -type f \( -name "*konflux*" -o -name "*component*" \) 2>/dev/null

Repository: openshift/lightspeed-console

Length of output: 54


🏁 Script executed:

# Check how the integration test pipeline receives SNAPSHOT and what triggers it
grep -n "SNAPSHOT" .tekton/lightspeed-console-4-19-pull-request.yaml | head -20

Repository: openshift/lightspeed-console

Length of output: 54


🏁 Script executed:

# Look for task that calls the integration test or passes SNAPSHOT
grep -n "integration-test\|extract-snapshot" .tekton/lightspeed-console-4-19-pull-request.yaml -A 5 -B 5

Repository: openshift/lightspeed-console

Length of output: 54


🏁 Script executed:

# Check if there's any documentation on SNAPSHOT structure in the repo
rg -n "\.components\[\]" . --type yaml -A 2 -B 2

Repository: openshift/lightspeed-console

Length of output: 54


🏁 Script executed:

# Look at the full context of the extract-snapshot-metadata task usage
sed -n '100,200p' .tekton/integration-tests/lightspeed-console-pre-commit.yaml

Repository: openshift/lightspeed-console

Length of output: 3967


🏁 Script executed:

# Check if there's any validation or error handling for empty results
sed -n '155,180p' .tekton/integration-tests/lightspeed-console-pre-commit.yaml

Repository: openshift/lightspeed-console

Length of output: 1262


🏁 Script executed:

# Check the git clone step in lint task to see error handling
sed -n '145,165p' .tekton/integration-tests/lightspeed-console-pre-commit.yaml | grep -A 5 "set -e"

Repository: openshift/lightspeed-console

Length of output: 337


Hardcoded component name "lightspeed-console-4-19" lacks validation for empty results.

The component name is extracted from SNAPSHOT using jq (lines 132-133). If the component doesn't exist, jq returns empty strings for both console-image and commit. The lint task has set -euo pipefail, so an empty COMMIT_SHA will cause git fetch origin "" to fail, but the e2e-tests task may not fail as explicitly. Add validation to ensure both fields are non-empty and provide clear error messaging:

script: |
  dnf -y install jq
  CONSOLE_IMAGE="$(jq -r --arg component_name "lightspeed-console-4-19" '.components[] | select(.name == $component_name) | .containerImage' <<< "$SNAPSHOT")"
  COMMIT="$(jq -r --arg component_name "lightspeed-console-4-19" '.components[] | select(.name == $component_name) | .source.git.revision' <<< "$SNAPSHOT")"
  
  if [ -z "$CONSOLE_IMAGE" ] || [ -z "$COMMIT" ]; then
    echo "Error: Component lightspeed-console-4-19 not found in SNAPSHOT or missing required fields"
    exit 1
  fi
  
  echo -n "$CONSOLE_IMAGE" > $(step.results.console-image.path)
  echo -n "$COMMIT" > $(step.results.commit.path)
🤖 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 @.tekton/integration-tests/lightspeed-console-pre-commit.yaml around lines
132 - 133, The jq extraction for the component "lightspeed-console-4-19" can
produce empty strings (from SNAPHOT) and later cause failures; update the script
that writes to step.results.console-image.path and step.results.commit.path to
first assign the jq results to variables (e.g., CONSOLE_IMAGE and COMMIT),
validate both are non-empty, and if either is missing emit a clear error and
exit non‑zero; keep the same jq queries (using SNAPSHOT and the component name)
and only write to the result files after the existence checks succeed.

@JoaoFula
Copy link
Copy Markdown
Contributor Author

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant