Skip to content

fix: improve catalog release script for OCP version lifecycle#2386

Open
yanmxa wants to merge 2 commits intostolostron:mainfrom
yanmxa:fix/catalog-release-script
Open

fix: improve catalog release script for OCP version lifecycle#2386
yanmxa wants to merge 2 commits intostolostron:mainfrom
yanmxa:fix/catalog-release-script

Conversation

@yanmxa
Copy link
Copy Markdown
Contributor

@yanmxa yanmxa commented Apr 8, 2026

Summary

  • Remove filter_catalog.py lines from new OCP platform Containerfiles (new platforms have no existing catalog entries to filter)
  • Clean up all OCP version directories and pipelines below OCP_MIN (not just the single previous version)
  • Add README.md version update step (Step 2.7)
  • Skip Containerfile creation if it already exists on the branch (avoids spurious trailing newline diffs)

Test plan

  • Ran 04-catalog.sh with RELEASE_BRANCH=release-2.17 and verified:
    • v4.22 Containerfile created without filter_catalog.py lines
    • v4.16 directory and pipelines cleaned up
    • README.md updated with correct version references
    • Existing v4.22 Containerfile not touched on re-run

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Prefer existing catalog output and skip regenerating when present; otherwise copy and adapt from the previous version.
    • Normalize auxiliary catalog content only when specific filters are detected, and log normalization or pre-existing outputs.
    • Perform README/version replacements only when exact mismatches are found; otherwise log that references are already correct.

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 8, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yanmxa

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 label Apr 8, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

04-catalog.sh now prefers an existing NEW_CONTAINERFILE, otherwise copies PREV_CONTAINERFILE and applies substitutions; newly created Containerfiles are normalized by removing filter_catalog.py-related blocks when detected; README tag/branch replacements are performed only if exact source strings are present (grep -qF) and logged per replacement.

Changes

Cohort / File(s) Summary
Catalog Script
.claude/skills/new-release/scripts/04-catalog.sh
Prefer NEW_CONTAINERFILE if present; else copy PREV_CONTAINERFILE and apply tag/path substitutions. After creation, detect filter_catalog.py and remove its comment/copy/remove/run blocks before staging. README updates are conditional: replacements (PREV_CATALOG_TAGCATALOG_TAG, ${BASE_BRANCH}${CATALOG_BRANCH}) only run when exact strings are present (grep -qF), with detailed per-replacement logging and an info message if no changes occurred.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nudged the Containerfile, sniffed for the new,
Copied the old, then trimmed traces askew,
Filtered the filter with a tidy sweep,
README swaps only where the old tags sleep,
A bunny's hop to keep the release neat 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description includes a clear summary of changes and test plan, but is missing required template sections: no emoji/text prefix, no explicit 'Related issue(s)' section, and no test checkboxes (unit/integration/E2E). Add the emoji/text prefix (e.g., ':bug: 🐛' for bug fix), include a 'Related issue(s)' section with issue references, and add the test checkboxes with appropriate status indicators.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: improve catalog release script for OCP version lifecycle' clearly and specifically describes the main change—improvements to the catalog release script for managing OCP version lifecycle.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 and usage tips.

@yanmxa yanmxa force-pushed the fix/catalog-release-script branch 2 times, most recently from 8001f79 to 5c7c923 Compare April 8, 2026 08:00
@yanmxa yanmxa force-pushed the fix/catalog-release-script branch from 5c7c923 to 5ad1a2b Compare April 8, 2026 08:02
@yanmxa yanmxa force-pushed the fix/catalog-release-script branch from 5ad1a2b to 3805820 Compare April 8, 2026 08:06
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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/skills/new-release/scripts/04-catalog.sh:
- Around line 680-698: Resolve the unresolved git merge markers by removing the
conflict lines (<<<<<<<, =======, >>>>>>>) and keep the incoming logic that
checks and updates the README: keep the grep -q "$PREV_CATALOG_TAG" conditional,
the sed calls that use SED_INPLACE to replace PREV_CATALOG_TAG → CATALOG_TAG and
BASE_BRANCH → CATALOG_BRANCH, and the associated echo messages; ensure variables
README_FILE, PREV_CATALOG_TAG, CATALOG_TAG, BASE_BRANCH, CATALOG_BRANCH and
SED_INPLACE are referenced exactly as in the incoming block so the script is
idempotent and parses correctly.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8f414982-88ea-43c5-9408-9b2b096aa695

📥 Commits

Reviewing files that changed from the base of the PR and between a77c35a and 5ad1a2b.

📒 Files selected for processing (1)
  • .claude/skills/new-release/scripts/04-catalog.sh

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/skills/new-release/scripts/04-catalog.sh:
- Around line 680-689: The current conditional gates the
BASE_BRANCH→CATALOG_BRANCH sed replacement behind a grep for PREV_CATALOG_TAG,
so if the tag was already updated but the branch string is stale it will be
skipped; change the logic so the sed "${SED_INPLACE[@]}"
"s/${BASE_BRANCH}/${CATALOG_BRANCH}/g" on README_FILE always runs
(unconditionally or in its own check) regardless of whether PREV_CATALOG_TAG
exists, and adjust the echo messages accordingly to reflect whether each
replacement actually occurred; locate the block referencing PREV_CATALOG_TAG,
CATALOG_TAG, BASE_BRANCH, CATALOG_BRANCH, README_FILE and move or duplicate the
branch-rewrite sed out of the grep-guarded branch so branch updates are applied
and reported independently.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 64de85e2-7437-4961-89e7-391fbd8c8caa

📥 Commits

Reviewing files that changed from the base of the PR and between 5ad1a2b and 3805820.

📒 Files selected for processing (1)
  • .claude/skills/new-release/scripts/04-catalog.sh

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/skills/new-release/scripts/04-catalog.sh:
- Around line 549-551: The early-exit branch skips running the cleanup script
when NEW_CONTAINERFILE already exists; change the logic so the copy-from-PREV
step only runs when PREV_CONTAINERFILE exists and NEW_CONTAINERFILE does not,
but always invoke the filter step (filter_catalog.py) against NEW_CONTAINERFILE
even if it already existed; update the conditional around
NEW_CONTAINERFILE/PREV_CONTAINERFILE to only guard copying (leave the echo/info
message) and ensure the filter_catalog.py invocation and any sanitization logic
for Containerfile.catalog always runs for NEW_CONTAINERFILE; apply the same fix
to the similar block that currently lives around the other conditional (the one
referenced at lines 571-581).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 669f1e62-6b7c-4f48-9ec4-be3293d3ecda

📥 Commits

Reviewing files that changed from the base of the PR and between 3805820 and 685b75d.

📒 Files selected for processing (1)
  • .claude/skills/new-release/scripts/04-catalog.sh

@yanmxa yanmxa force-pushed the fix/catalog-release-script branch from a2e9f54 to d6826c8 Compare April 8, 2026 09:51
yanmxa and others added 2 commits April 8, 2026 18:04
- Remove filter_catalog.py lines from new OCP platform Containerfiles
  (new platforms have no existing catalog entries to filter)
- Clean up all OCP version directories and pipelines below OCP_MIN
  (not just the single previous version)
- Add README.md version update step
- Skip Containerfile creation if it already exists on the branch

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Meng Yan <myan@redhat.com>
- Separate catalog tag and branch replacements into independent grep checks
  so stale branch references are updated even if the tag was already correct
- Always sanitize filter_catalog.py lines from new Containerfile regardless
  of whether the file was just created or already existed from a previous run

Signed-off-by: Meng Yan <myan@redhat.com>
@yanmxa yanmxa force-pushed the fix/catalog-release-script branch from d6826c8 to 4424ac7 Compare April 8, 2026 10:04
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 8, 2026

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant