Skip to content

fixes small issue with launching gangway jobs when supporting multiple OpenShift versions#203

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift-eng:mainfrom
dhensel-rh:launch-sh-fix
Jun 25, 2026
Merged

fixes small issue with launching gangway jobs when supporting multiple OpenShift versions#203
openshift-merge-bot[bot] merged 2 commits into
openshift-eng:mainfrom
dhensel-rh:launch-sh-fix

Conversation

@dhensel-rh

@dhensel-rh dhensel-rh commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix release detection order in launch.sh so an explicit version tag takes priority over job file lookup
  • Skip release image verification during --refresh operations
  • Remove all generated .txt files under plugins/edge-ocp-rc/jobs/ from git tracking
  • Add plugins/edge-ocp-rc/jobs/*.txt to .gitignore to keep generated job files out of future commits

Details

The job files (tnf.txt, sno.txt, tna.txt and their z/y-stream variants) are generated at runtime by launch.sh and should not be tracked in the repository. They have been removed from the git index but remain on disk.

Summary by CodeRabbit

  • Bug Fixes

    • Improved release detection so an explicit version tag is used first when available.
    • Skips release image validation during refresh flows, reducing unnecessary checks and delays.
  • Chores

    • Cleaned up ignored paths and removed several outdated job entries from release job listings.

dhensel-rh and others added 2 commits June 24, 2026 06:08
Move version tag regex match before job file lookup so an explicit
version tag takes priority. Skip release image verification during
refresh. Remove generated job txt files from tracking and add them
to .gitignore.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove sno and tna job txt files from the git index. These are
generated at runtime by launch.sh and already covered by the
.gitignore rule added in the previous commit.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 24, 2026
@openshift-ci

openshift-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci

openshift-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhensel-rh

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

The pull request process is described 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2026
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Removes all nightly-4.22 periodic CI job entries from sno, tna, and tnf variant job list files under plugins/edge-ocp-rc/jobs/. Adds .gitignore rules for eval/runs/ and plugins/edge-ocp-rc/jobs/*.txt. Fixes detect_release in launch.sh to prioritize version_tag parsing before job-file inspection, and skips release image verification when --refresh is set.

Changes

OCP RC Job Cleanup and Script Fixes

Layer / File(s) Summary
4.22 job list removal and .gitignore rules
.gitignore, plugins/edge-ocp-rc/jobs/sno.txt, plugins/edge-ocp-rc/jobs/sno-z-stream.txt, plugins/edge-ocp-rc/jobs/tna*.txt, plugins/edge-ocp-rc/jobs/tnf*.txt
All nightly-4.22 periodic CI job identifiers are removed from sno, tna (y-stream, z-stream, base), and tnf (z-stream, base) job list files, leaving them empty. .gitignore adds rules to ignore eval/runs/ and all plugins/edge-ocp-rc/jobs/*.txt files.
detect_release precedence and --refresh image-verify skip
plugins/edge-ocp-rc/scripts/launch.sh
detect_release is reordered so a matching version_tag (X.Y) is echoed and returned immediately, before any job-file inspection. The quay.io release image verification block is now skipped when --refresh is active.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 9 | ❌ 2

❌ Failed checks (2 warnings)

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.
Ai-Attribution ⚠️ Warning HEAD contains Co-Authored-By: Claude Opus 4.6, but no Assisted-by or Generated-by Red Hat trailer was found. Replace the AI co-author line with the appropriate Red Hat Assisted-by: or Generated-by: trailer, and avoid Co-Authored-By for AI tools.
✅ Passed checks (9 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is clearly related to the main launch.sh fix for multi-version OpenShift job handling, though it omits the generated job-file cleanup.
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.
No-Weak-Crypto ✅ Passed Changed files only adjust job lists, .gitignore, and release detection/refresh logic; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB or custom crypto found.
Container-Privileges ✅ Passed The PR only changes .gitignore, job text files, and a shell script; no container/K8s manifests or privileged settings are present.
No-Sensitive-Data-In-Logs ✅ Passed The diff only reorders release detection and skips image verification on --refresh; it adds no logging of secrets, PII, or internal data.
No-Hardcoded-Secrets ✅ Passed Touched files add ignore rules and shell logic only; scans found no hardcoded keys/passwords or embedded-cred URLs, just MY_APPCI_TOKEN used at runtime.
No-Injection-Vectors ✅ Passed PASS: Touched files only adjust release detection/refresh gating and ignore generated txts; no eval/exec, shell=True, yaml.load, or other listed injection patterns found.

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

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@plugins/edge-ocp-rc/scripts/launch.sh`:
- Line 148: The launch condition in launch.sh uses REFRESH as an unquoted
command-style boolean, which violates the shell quoting guideline and can fail
shellcheck validation. Update the conditional around the RELEASE_IMAGE check to
use an explicit quoted test for REFRESH in the relevant launch logic, keeping
the behavior the same while ensuring all variable references in the shell script
are quoted.
🪄 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: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 7cf9fbd0-dfd3-409d-bbe1-bc105a165a6d

📥 Commits

Reviewing files that changed from the base of the PR and between 089dd10 and 68cd5d3.

📒 Files selected for processing (9)
  • .gitignore
  • plugins/edge-ocp-rc/jobs/sno-z-stream.txt
  • plugins/edge-ocp-rc/jobs/sno.txt
  • plugins/edge-ocp-rc/jobs/tna-y-stream.txt
  • plugins/edge-ocp-rc/jobs/tna-z-stream.txt
  • plugins/edge-ocp-rc/jobs/tna.txt
  • plugins/edge-ocp-rc/jobs/tnf-z-stream.txt
  • plugins/edge-ocp-rc/jobs/tnf.txt
  • plugins/edge-ocp-rc/scripts/launch.sh
💤 Files with no reviewable changes (7)
  • plugins/edge-ocp-rc/jobs/sno-z-stream.txt
  • plugins/edge-ocp-rc/jobs/tnf-z-stream.txt
  • plugins/edge-ocp-rc/jobs/tnf.txt
  • plugins/edge-ocp-rc/jobs/tna-y-stream.txt
  • plugins/edge-ocp-rc/jobs/tna-z-stream.txt
  • plugins/edge-ocp-rc/jobs/tna.txt
  • plugins/edge-ocp-rc/jobs/sno.txt

fi

if [[ -n "$RELEASE_IMAGE" ]]; then
if [[ -n "$RELEASE_IMAGE" ]] && ! $REFRESH; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Quote REFRESH in the condition (severity: medium).

Line 148 uses ! $REFRESH, which leaves the variable unquoted and relies on command-style boolean evaluation. Prefer an explicit quoted comparison.

Suggested fix
-if [[ -n "$RELEASE_IMAGE" ]] && ! $REFRESH; then
+if [[ -n "$RELEASE_IMAGE" && "$REFRESH" != true ]]; then

As per coding guidelines: **/*.sh requires “Quote all variables in shell scripts” and “Shell scripts must pass shellcheck validation”; and per path instructions/CONTRIBUTING.md, Bash scripts must quote all variables.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [[ -n "$RELEASE_IMAGE" ]] && ! $REFRESH; then
if [[ -n "$RELEASE_IMAGE" && "$REFRESH" != true ]]; then
🤖 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 `@plugins/edge-ocp-rc/scripts/launch.sh` at line 148, The launch condition in
launch.sh uses REFRESH as an unquoted command-style boolean, which violates the
shell quoting guideline and can fail shellcheck validation. Update the
conditional around the RELEASE_IMAGE check to use an explicit quoted test for
REFRESH in the relevant launch logic, keeping the behavior the same while
ensuring all variable references in the shell script are quoted.

Sources: Coding guidelines, Path instructions

@dhensel-rh
dhensel-rh marked this pull request as ready for review June 24, 2026 21:23
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 24, 2026
@lucaconsalvi

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit 392b62e into openshift-eng:main Jun 25, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants