feat(ci): run OpenShift e2e against merge queue commits by squizzi · Pull Request #287 · openshift-online/hypershell

squizzi · 2026-09-15T20:14:43Z

What

Add dedicated OpenShift e2e testing workflow for merge queue commits, filling the gap where the merge gate currently only exercises Kind, not OpenShift.

Highlights

New e2e-openshift-merge-queue.yml workflow brings up ephemeral OpenShift environment per merge queue entry
Intelligently determines which Konflux builds to await based on changed files
Mirrors e2e-openshift-main.yml pattern (bring-up-test-tear-down) for consistency
Namespace keyed by commit SHA to support concurrent merge queue entries without clobbering
Uses admin/admin auth (traditional path, not GitHub OAuth)
Updated e2e.yml comments to clarify relationship between three workflows

Scope

CI and workflows only; no changes to application code or test logic. The workflow is self-contained and follows existing OpenShift e2e patterns in the codebase.

The E2E OpenShift job was silently skipped on merge_group events: Tests / E2E / OpenShift (e2e.yml) only runs on pull_request because it polls the per-PR ephemeral environment from pr-environment.yml, which never deploys for a merge queue entry. This let merge queue commits land without OpenShift coverage, only Kind. Add e2e-openshift-merge-queue.yml, a bring-up-test-tear-down workflow triggered on merge_group that mirrors e2e-openshift-main.yml's push job, but builds each entry's on-merge-queue-<sha> component images and uses a namespace keyed by commit SHA so concurrent queue entries cannot collide or tear down each other's environment. It lives in its own workflow file rather than as a job in e2e.yml so it does not show up as a permanently skipped check on pull_request and push runs. Assisted-by: Claude Sonnet 5

coderabbitai · 2026-09-15T20:14:50Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (2)

do-not-merge/work-in-progress
do-not-merge/hold

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

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

Review profile: CHILL

Plan: Advanced

Run ID: 0b3a7693-ec9e-4a5e-93a3-41e02bb62f42

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

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

github-actions · 2026-09-15T20:15:03Z

HyperShell environment deploying

Deploying commit 7238ebe to an ephemeral OpenShift environment. This
comment will update in place once the environment is ready.

amber-review-bot · 2026-09-15T20:19:37Z

Amber review

Status: Complete

View the submitted review.

…flows e2e.yml, e2e-openshift-main.yml, and e2e-openshift-merge-queue.yml each carried their own copy of the "map changed files to api-server/control-plane/web-console flags for a Konflux pipeline suffix" grep block. Each copy's comment flagged that it MUST mirror the matching .tekton CEL trigger, so a future watched-path change had to be replayed by hand in three places -- a missed one would silently fall back to a stale baseline image instead of the freshly built one. Extract that mapping into a composite action, .github/actions/detect-konflux-components, parameterized by changed-files and pipeline-suffix. Each workflow keeps its own event-specific "how do I compute changed files" step (pull_request merge-base diff, merge_group base diff, push diff-or-show-for-first- push) and calls the shared action to turn the result into flags. e2e.yml's pull_request and merge_group/push detection paths, which previously duplicated the grep inline in two places, now both go through the same step. Behavior is unchanged; this only removes the duplicated pattern list. Assisted-by: Claude Sonnet 5

… action pr-environment.yml still carried its own inline copy of the changed-files-to-component-flags grep block, left over from before detect-konflux-components was extracted for e2e.yml, e2e-openshift-main.yml, and e2e-openshift-merge-queue.yml. Route it through the same composite action so the pull-request Konflux CEL pattern list exists in exactly one place repo-wide. Behavior is unchanged. Assisted-by: Claude Sonnet 5

amber-review-bot

Verdict

This head keeps the sound, behavior-preserving CI refactor (Konflux component detection centralized in .github/actions/detect-konflux-components) and now also routes pr-environment.yml through it - a good response to the earlier duplication finding. However, the new pr-environment.yml commit was written on a pre-rebase revision and drops the should_run / detect-components deploy gating that main already carries, so as-authored it regresses merged behavior and the PR is currently in a merge-conflicting state; that needs to be reconciled before merge.

What I verified at this head

The only change since my previous review (7238ebe -> 73bf3fe) is pr-environment.yml (30 insertions / 12 deletions). The composite action and the three e2e workflows are unchanged from the previously reviewed, behavior-preserving refactor.
No inline Konflux grep -qE detection remains in any workflow; all four (e2e.yml, e2e-openshift-main.yml, e2e-openshift-merge-queue.yml, pr-environment.yml) now consume uses: ./.github/actions/detect-konflux-components, and the composite patterns still match the .tekton/hypershell-*-main-{pull-request,merge-queue,push}.yaml triggers.
The pr-environment.yml refactor is grep-pattern-equivalent to the block it replaces (api-server / control-plane+Dockerfile / web-console+packages/gateway-management-ui/, suffix pull-request), and PR_HEAD_SHA is still available in the plan step for the on-pr-${PR_HEAD_SHA} tag.

New finding: `pr-environment.yml` is refactored on a stale base and drops the deploy gate

Main's pr-environment.yml plan-images job now emits a should_run output and its deploy job is gated ... && needs.plan-images.outputs.should_run == 'true', so e2e-irrelevant (e.g. docs-only) PRs skip the baseline deploy. This PR's rewrite of the same plan-images step is based on the older revision: it removes the Detect changed components (detect-components.sh) step, emits no should_run, and leaves the deploy gate at just head.repo.full_name == github.repository. Consequences:

The PR is currently CONFLICTING with main precisely on this step.
Resolved naively, it would revert the merged "skip deploy when no e2e-relevant paths changed" behavior and re-deploy a baseline environment on every PR.

Please rebase onto main and re-apply the composite-action routing on top of the current should_run / detect-components version of the step, keeping both the should_run output and the deploy gate. (Inline note added.)

Cross-PR coordination

#237 adds a release_version_changed / VERSION rule directly into the inline Konflux grep block in e2e.yml and adds "VERSION".pathChanged() to the .tekton push CEL triggers. This PR deletes that inline block in favor of the composite action whose patterns "MUST mirror" the .tekton triggers. Whoever merges second must re-express the other's change in the new location: if this PR lands first, #237's VERSION rule has to be threaded through detect-konflux-components (e.g. a new input) rather than the now-removed inline grep, and the action's patterns kept in sync with the new VERSION trigger. Maintainers should pick a merge order.
#284 restructures change detection more broadly (component-paths.json, detect-components.sh, a new push OpenShift workflow) and edits the same files (e2e.yml, e2e-openshift-main.yml, pr-environment.yml) while adding fresh inline copies of the same Konflux-CEL grep. That is a competing consolidation approach for the exact concern this PR centralizes. Maintainers should align on a single detection architecture and a merge order before both land, so one does not silently reintroduce the duplication the other removes.

Previous concerns

Branch-protection wiring (discussion) - still present, needs a maintainer decision (config, outside the diff). e2e.yml's e2e-openshift job is gated if: github.event_name == 'pull_request' && ... (.github/workflows/e2e.yml L669-672), so on merge_group it is skipped and contributes nothing to Tests CI Gate; the new E2E OpenShift (merge queue) job lives in a standalone workflow that tests.yml never invokes, so its result is not part of that gate either. Unless E2E OpenShift (merge queue) is itself a required status check for main, a failing OpenShift merge-queue run will not block the merge. I already replied inline to the committer's Tests CI Gate objection with this evidence (reply); that reply still applies, so no new reply was posted.
Duplication / drift risk (discussion) - addressed. The detection grep is now a single copy in .github/actions/detect-konflux-components/action.yml, and all four workflows consume it via uses: ./.github/actions/detect-konflux-components (grep -rn "grep -qE" .github/workflows returns nothing; grep -rln detect-konflux-components .github lists e2e.yml, e2e-openshift-main.yml, e2e-openshift-merge-queue.yml, pr-environment.yml). This head additionally closed the pr-environment.yml copy that my previous review flagged as remaining.

Both prior findings are tracked in the existing Amber threads linked above; no duplicate inline comments were added for them.

Findings Summary (ordered by severity, highest first):

[Major] pr-environment.yml refactor is based on a stale revision: it drops the should_run output and the detect-components-driven deploy gate that main already has, causing the current merge conflict and, if merged as-is, reverting the "skip deploy for e2e-irrelevant PRs" behavior (.github/workflows/pr-environment.yml L77-115, L131) - Regression / Change Order
[Minor] E2E OpenShift (merge queue) must be added to branch-protection required checks; Tests CI Gate does not cover it because the merge_group OpenShift job in e2e.yml is skipped (.github/workflows/e2e.yml L669-672) - Spec Completeness

Convention Checklist:

Convention	Result
No secrets in logs or responses	Pass
Minimal workflow permissions	Pass
Pinned action SHAs	Pass
Image/tag references consistent with `.tekton` sources	Pass
Refactor behavior-preserving (detection outputs unchanged)	Pass
Rebased on `main` / no functional regression	Fail
Conventional commit message	Pass

amber-review-bot · 2026-09-15T21:12:41Z

+          changed-files: ${{ steps.diff.outputs.changed_files }}
+          pipeline-suffix: pull-request
+
      - name: Plan images and required Konflux builds


[Major - Regression / Change Order] This refactor is a good direction (routing the PR-environment Konflux detection through the shared detect-konflux-components action, closing the last inline copy), but it is written on a revision that predates main's current plan-images step. On main this job also emits a should_run output and the deploy job below is gated ... && needs.plan-images.outputs.should_run == 'true', so e2e-irrelevant (docs-only) PRs skip the baseline deploy. This head removes the detect-components step, emits no should_run, and leaves the deploy gate at just head.repo.full_name == github.repository (L131).

Result: the PR is currently merge-CONFLICTING on exactly this step, and if resolved by taking this side it would revert the merged "skip deploy when nothing e2e-relevant changed" behavior. Please rebase onto main and re-apply the composite-action routing on top of the current should_run / detect-components version - keep the Detect changed components step, the should_run output, and the should_run == 'true' deploy gate, while still calling detect-konflux-components for the Konflux wait/image plan.

squizzi · 2026-09-15T21:35:30Z

I'm parking this for now, this is just going to make the insufficient resources issues worse, we can return to this if we want in the future but we may just want to run these more expensive tests on main and in PRs and skip merge queue intentionally since it's yet another test, kind should validate enough for merge queue specific issues I think.

This comment was marked as outdated.

Sign in to view

amber-review-bot mentioned this pull request Sep 15, 2026

Enable automerge for dependency updates #288

Merged

This comment was marked as outdated.

Sign in to view

amber-review-bot reviewed Sep 15, 2026

View reviewed changes

amber-review-bot added the amber/changes-requested Amber requested changes on this PR label Sep 15, 2026

squizzi marked this pull request as draft September 15, 2026 21:35

This was referenced Sep 16, 2026

[HYPERSHELL-343] feat(ci): ephemeral-by-default PR environments #302

Merged

[HYPERSHELL-240][HYPERSHELL-343] refactor(ci): consolidate OpenShift e2e #306

Merged

docs(platform): specify ephemeral test credentials and CI secrets #337

Open

Navigation Menu

feat(ci): run OpenShift e2e against merge queue commits - #287

feat(ci): run OpenShift e2e against merge queue commits#287
squizzi wants to merge 3 commits into
mainfrom
squizzi/e2e-openshift-merge-queue

squizzi commented Sep 15, 2026

Uh oh!

coderabbitai Bot commented Sep 15, 2026 •

edited

Loading

Review skipped

Uh oh!

github-actions Bot commented Sep 15, 2026 •

edited

Loading

Uh oh!

amber-review-bot commented Sep 15, 2026 •

edited

Loading

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

amber-review-bot left a comment

Uh oh!

amber-review-bot Sep 15, 2026

Uh oh!

squizzi commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

squizzi commented Sep 15, 2026

What

Highlights

Scope

Uh oh!

coderabbitai Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

github-actions Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

HyperShell environment deploying

Uh oh!

amber-review-bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Amber review

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

amber-review-bot left a comment

Choose a reason for hiding this comment

Verdict

What I verified at this head

New finding: pr-environment.yml is refactored on a stale base and drops the deploy gate

Cross-PR coordination

Previous concerns

Uh oh!

amber-review-bot Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

squizzi commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Sep 15, 2026 •

edited

Loading

github-actions Bot commented Sep 15, 2026 •

edited

Loading

amber-review-bot commented Sep 15, 2026 •

edited

Loading

New finding: `pr-environment.yml` is refactored on a stale base and drops the deploy gate