Skip to content

fix: prevent duplicate Tekton file PRs in multi-fastforward#79634

Open
dislbenn wants to merge 1 commit into
openshift:mainfrom
dislbenn:fix-tekton-duplicate-prs
Open

fix: prevent duplicate Tekton file PRs in multi-fastforward#79634
dislbenn wants to merge 1 commit into
openshift:mainfrom
dislbenn:fix-tekton-duplicate-prs

Conversation

@dislbenn
Copy link
Copy Markdown
Contributor

@dislbenn dislbenn commented May 21, 2026

Summary

Fixes duplicate PR spam for Tekton files already merged to main (e.g., stolostron/siteconfig#1106).

Problem

The ocm-ci-fastforward-multiple script was creating duplicate PRs for Tekton files that were already on the default branch. For example, siteconfig repo kept getting PRs to add acm-50 and acm-51 files even though they were already merged to main.

Root cause: Script checked for existing files on the PR branch instead of the default branch. When a stale PR branch existed with old files (e.g., acm-217), the script would:

  1. Check out the PR branch
  2. Find acm-50/51 exist on that branch
  3. Skip creating new files (files_created=false)
  4. But still create a PR because the branch existed

Changes

Fix 1: Check default branch first (lines 547-599)

  • Before touching PR branch, check if all requested Tekton file versions already exist on the default branch
  • If all exist: close any stale PR, delete the branch, exit early
  • Prevents creating PRs when files already merged

Fix 2: Reset PR branch to default when needed (lines 605-627)

  • When some files are missing and PR branch exists, hard reset PR branch to default branch first
  • Removes stale version files (e.g., old acm-217) before adding new files
  • Prevents accumulating garbage on PR branches

Test plan

  • Verified script logic in review
  • Monitor next periodic run for siteconfig repo - should delete existing stale PR branch
  • Verify no new duplicate PRs created for repos with Tekton files already on main

🤖 Generated with Claude Code

Summary by CodeRabbit

This PR fixes a bug in the OpenShift CI infrastructure's ocm-ci-fastforward-multiple script that was generating duplicate pull requests for Tekton workflow files already merged to the default branch.

Root Cause: The script was checking whether Tekton files existed on the PR branch instead of the target repository's default branch. When a stale PR branch lingered with outdated files, the script would detect those old files, skip creating new ones, but still generate a PR—resulting in duplicate PRs for changes already merged.

The Solution: The fix implements three key safeguards:

  1. Early existence check on default branch (lines 546–599): Before attempting any file creation, the script now verifies whether all requested Tekton file versions already exist on the default branch. If they do, it cleans up by closing any lingering PR associated with the stale branch, deletes that remote branch, and exits early—preventing the creation of unnecessary duplicate PRs.

  2. Branch reset to remove stale content (lines 609–627): When a PR branch already exists on the remote, the script now hard-resets it to origin/${default_branch} instead of just checking out the existing branch and attempting DCO sign-off amendments. This eliminates stale versions and prevents the accumulation of old content.

  3. PR creation guard (lines 809–820): When handling the case where a PR branch exists remotely but no PR exists, the script now fetches the default branch and uses git diff --quiet to verify the branch actually contains changes relative to the default branch. It only creates a PR if substantive changes are present.

These changes ensure the Tekton file fastforward process remains idempotent and prevents infrastructure PRs from proliferating when changes have already been merged to the main branch.

Script was creating duplicate PRs for Tekton files already merged to main.

Root cause: Script checked for existing files on PR branch instead of default branch, then created PRs even when main already had all files.

Fixes:
1. Check default branch for existing files BEFORE touching PR branch. If all requested versions exist on default branch, delete stale PR branch and exit.

2. Reset PR branch to default branch when files are needed. Prevents accumulating stale version files (e.g., acm-217) on PR branch.

Impact: Stops spam PRs like stolostron/siteconfig#1106 where acm-50/51 files already exist on main.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 93a9f3e1-d6d4-4576-9fbc-c2d2733d0960

📥 Commits

Reviewing files that changed from the base of the PR and between 847c9fb and 49ef3e1.

📒 Files selected for processing (1)
  • ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh

Walkthrough

This PR refines the create_tekton_files function in a CI operator step script to reduce redundant work and improve branch hygiene. It adds early-exit logic when Tekton files already exist, resets branches to a clean state before changes, and creates pull requests only when branch content differs from the default branch.

Changes

Tekton File and PR Branch Handling

Layer / File(s) Summary
Early-exit check for existing Tekton files
ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh
When all requested Tekton files are already present on default_branch, the function closes any stale PR tied to the automation branch via gh pr close, deletes the remote branch, and exits without further processing.
Branch reset to clean state
ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh
When the PR branch already exists locally, it is reset to origin/${default_branch} using a hard reset before Tekton file generation or commits, removing any stale content.
Conditional PR creation with diff validation
ci-operator/step-registry/ocm/ci/fastforward-multiple/ocm-ci-fastforward-multiple-commands.sh
When a remote branch exists without a corresponding PR, the function fetches default_branch and skips PR creation if the branch is identical to origin/${default_branch}, proceeding only when actual differences are detected.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 12
✅ Passed checks (12 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main fix: preventing duplicate Tekton file PRs in the multi-fastforward process, which aligns with the changeset's primary objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Stable And Deterministic Test Names ✅ Passed PR contains no Ginkgo tests (It/Describe/Context/When/By patterns). Changes are to shell scripts and standard Go testing.T tests, making this check not applicable.
Test Structure And Quality ✅ Passed The custom check for Ginkgo test quality is not applicable to this PR. The PR modifies only a shell script and configuration files, with no Ginkgo tests, Go test files, or test code added or modified.
Microshift Test Compatibility ✅ Passed PR modifies only a bash shell script for CI/CD infrastructure automation; no Ginkgo e2e tests are added, so the MicroShift compatibility check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR modifies only shell scripts (ocm-ci-fastforward-multiple-commands.sh), not Go test files. No Ginkgo e2e tests (It, Describe, Context, When) are added, so SNO compatibility check is not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed Pull request modifies a CI automation shell script and step configuration files; does not introduce deployment manifests, operator code, or scheduling constraints.
Ote Binary Stdout Contract ✅ Passed The PR modifies only a shell script, not a Go/Ginkgo test. The OTE Binary Stdout Contract check applies only to Go programs and is not applicable.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No Ginkgo e2e tests are added in this PR; all added test files are standard Go unit tests using testing.T, not Ginkgo patterns.

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

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

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 neisw and sosiouxme May 21, 2026 19:21
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 21, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dislbenn
Once this PR has been reviewed and has the lgtm label, please assign mark-nc 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

@openshift-merge-bot
Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@dislbenn: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
periodic-ci-stolostron-acm-config-main-fast-forward N/A periodic Registry content changed

Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals.

Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@dislbenn
Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-stolostron-acm-config-main-fast-forward

@openshift-merge-bot
Copy link
Copy Markdown
Contributor

@dislbenn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 21, 2026

@dislbenn: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions 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.

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