Skip to content

Check all hypershift variants when the featuregate is not platform specific#2951

Open
JoelSpeed wants to merge 1 commit into
openshift:masterfrom
JoelSpeed:fix-hypershift-promotion
Open

Check all hypershift variants when the featuregate is not platform specific#2951
JoelSpeed wants to merge 1 commit into
openshift:masterfrom
JoelSpeed:fix-hypershift-promotion

Conversation

@JoelSpeed

Copy link
Copy Markdown
Contributor

This now copies the logic directly below for self managed and ensures that we check all variants when the featuregate is otherwise not filtered to a specific platform

@JoelSpeed

Copy link
Copy Markdown
Contributor Author

/verified by @JoelSpeed

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jul 24, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@JoelSpeed: This PR has been marked as verified by @JoelSpeed.

Details

In response to this:

/verified by @JoelSpeed

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 openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The feature gate test analyzer now falls back to all required Hypershift job variants when variant filtering returns no results. Existing verifyJobPassRate return paths and the Sippy job-run lookback duration were reformatted without changing their behavior.

Possibly related PRs

  • openshift/api#2943: Modifies the same analyzer functions for related variant-handling and pass-rate logic.

Suggested reviewers: everettraven

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: checking all HyperShift variants when no platform-specific filter applies.
Description check ✅ Passed The description is directly related to the change and matches the updated HyperShift variant selection logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 The PR only changes a codegen helper; no Ginkgo test titles were added or modified, and no dynamic test-name strings appear in the changed file.
Test Structure And Quality ✅ Passed Only tools/codegen/cmd/featuregate-test-analyzer.go changed; no Ginkgo test code or test-related constructs were touched.
Microshift Test Compatibility ✅ Passed Only featuregate analyzer logic changed; no new Ginkgo e2e tests or MicroShift-relevant APIs were added.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR only changes featuregate analyzer code; no new Ginkgo e2e tests or SNO-sensitive test assumptions were added.
Topology-Aware Scheduling Compatibility ✅ Passed This PR only changes the featuregate test analyzer and Sippy query logic; it adds no manifests, controllers, or scheduling constraints.
Ote Binary Stdout Contract ✅ Passed The PR only changes variant selection and formatting; it adds no stdout writes in main/init/TestMain/suite setup paths.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed Only tools/codegen/cmd/featuregate-test-analyzer.go changed; no Ginkgo test additions or IPv4/external-connectivity assumptions were introduced.
No-Weak-Crypto ✅ Passed Diff only changes variant selection/formatting; no weak-crypto algorithms or custom crypto were added, and the only crypto use is pre-existing tls config.
Container-Privileges ✅ Passed PR only changes Go logic in tools/codegen/cmd/featuregate-test-analyzer.go; no container/K8s manifests or privilege settings were modified.
No-Sensitive-Data-In-Logs ✅ Passed No new logging was added; the diff only changes HyperShift variant fallback and formatting, with no sensitive data exposure.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented
The command is terminated due to an error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented


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

@openshift-ci

openshift-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Hello @JoelSpeed! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci openshift-ci Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 24, 2026
@openshift-ci
openshift-ci Bot requested review from deads2k and everettraven July 24, 2026 11:16
@qodo-for-rh-openshift

Copy link
Copy Markdown

PR Summary by Qodo

Ensure Hypershift promotion checks all variants for non-platform featuregates

🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

AI Description

• Ensure Hypershift profiles check all required variants when featuregates aren’t platform-specific.
• Align Hypershift variant-selection fallback behavior with the existing SelfManagedHA logic.
• Apply minor formatting cleanups in Sippy query and pass-rate helpers.
Diagram

graph TD
  A["featuregate-test-analyzer"] --> B["listTestResultFor()"] --> C["Hypershift variants (filter+fallback)"] --> E["listTestResultForVariant()"] --> F["verifyJobPassRate()"] --> G{{"Sippy API"}}
  B --> D["SelfManagedHA variants (filter+fallback)"] --> E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Move fallback behavior into filterVariants() via a defaulting flag
  • ➕ Centralizes the “if none matched, use all required” behavior
  • ➕ Reduces duplication across Hypershift and SelfManagedHA selection blocks
  • ➖ Slightly obscures intent at call sites (behavior becomes less explicit)
  • ➖ Requires touching a shared helper used by multiple variant lists
2. Extract a shared helper for “profile variant selection”
  • ➕ Eliminates repeated filter+fallback patterns across cluster profiles
  • ➕ Easier to add additional cluster profiles consistently
  • ➖ More refactor than necessary for a targeted bug fix
  • ➖ May require additional test coverage to justify structural change

Recommendation: The current approach is the best tradeoff for a promotion-gating bug: it keeps the Hypershift fix explicit and mirrors the proven SelfManagedHA pattern. Consider a shared helper only if more cluster profiles/variant lists are added soon.

Files changed (1) +17 / -12

Bug fix (1) +17 / -12
featuregate-test-analyzer.goAdd Hypershift variant fallback when featuregate isn’t platform-specific +17/-12

Add Hypershift variant fallback when featuregate isn’t platform-specific

• Updates Hypershift handling in listTestResultFor to fall back to all required Hypershift variants when filtering yields no platform-specific match, mirroring existing SelfManagedHA behavior. Includes minor formatting-only adjustments in pass-rate evaluation and Sippy request helpers.

tools/codegen/cmd/featuregate-test-analyzer.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tools/codegen/cmd/featuregate-test-analyzer.go (1)

746-753: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression test for the Hypershift fallback.

The existing tests cover filterVariants, but not the new listTestResultFor behavior. Add coverage proving that a generic feature gate selects every requiredHypershiftJobVariants entry, while a platform-specific gate selects only its matching variants before Sippy calls are made.

🤖 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 `@tools/codegen/cmd/featuregate-test-analyzer.go` around lines 746 - 753, Add
regression coverage around listTestResultFor for Hypershift variant selection:
verify a generic feature gate falls back to every requiredHypershiftJobVariants
entry, while a platform-specific gate includes only matching variants. Stub or
inspect Sippy calls to confirm the selected variants are used before requests
are made, while preserving existing filterVariants tests.
🤖 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.

Nitpick comments:
In `@tools/codegen/cmd/featuregate-test-analyzer.go`:
- Around line 746-753: Add regression coverage around listTestResultFor for
Hypershift variant selection: verify a generic feature gate falls back to every
requiredHypershiftJobVariants entry, while a platform-specific gate includes
only matching variants. Stub or inspect Sippy calls to confirm the selected
variants are used before requests are made, while preserving existing
filterVariants tests.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: b9d83a8e-4c96-4de7-b3f8-96a5f6847053

📥 Commits

Reviewing files that changed from the base of the PR and between 18550f1 and ffbad1d.

📒 Files selected for processing (1)
  • tools/codegen/cmd/featuregate-test-analyzer.go

@qodo-for-rh-openshift

qodo-for-rh-openshift Bot commented Jul 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 29 rules
✅ Skills: api-review

Grey Divider


Remediation recommended

1. Wrong hypershift fallback ✗ Dismissed 🐞 Bug ≡ Correctness
Description
When Hypershift is enabled and no Hypershift variants match the feature gate name, the new fallback
forces checking all requiredHypershiftJobVariants (currently AWS/amd64/external). This can cause
platform-specific feature gates (e.g., PowerVS/IBMCloud) to be evaluated against unrelated AWS
Hypershift jobs, producing misleading pass/fail results (and potentially failing promotion depending
on downstream handling).
Code

tools/codegen/cmd/featuregate-test-analyzer.go[R746-753]

+		// See if the feature gate is specific to any platform
+		hypershiftPlatformVariants := filterVariants(featureGate, requiredHypershiftJobVariants)
+
+		if len(hypershiftPlatformVariants) == 0 {
+			hypershiftPlatformVariants = requiredHypershiftJobVariants
+		}
+
+		jobVariantsToCheck = append(jobVariantsToCheck, hypershiftPlatformVariants...)
Relevance

⭐⭐⭐ High

Repo tends to accept fixes preventing wrong variant/Sippy matching and misleading promotion outcomes
in this analyzer.

PR-#2899
PR-#2445
PR-#2763

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new code explicitly replaces an empty filtered hypershift variant list with
requiredHypershiftJobVariants, but that required list currently only includes AWS/amd64/external.
Hypershift feature gate manifests include platform-specific names (e.g., IBMCloud/PowerVS) that do
not match AWS/amd64/external and are not excluded by nonHypershiftPlatforms, so they will follow
the fallback path and be checked against AWS Hypershift jobs.

tools/codegen/cmd/featuregate-test-analyzer.go[744-753]
tools/codegen/cmd/featuregate-test-analyzer.go[604-612]
tools/codegen/cmd/featuregate-test-analyzer.go[785-801]
payload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yaml[61-100]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`listTestResultFor` now falls back to `requiredHypershiftJobVariants` when `filterVariants(featureGate, requiredHypershiftJobVariants)` returns no matches. This makes a Hypershift feature gate that is platform-specific but not represented in `requiredHypershiftJobVariants` get analyzed as if it were generic, triggering unrelated AWS Hypershift Sippy queries.

## Issue Context
- `requiredHypershiftJobVariants` currently only contains AWS/amd64/external.
- Hypershift FeatureGate manifests include platform-specific gate names like `ClusterAPIInstallIBMCloud` and `ClusterAPIMachineManagementPowerVS`.
- `nonHypershiftPlatforms` does not match `IBMCloud` or `PowerVS`, so these names enter the Hypershift block and can hit the new fallback.

## Fix Focus Areas
- tools/codegen/cmd/featuregate-test-analyzer.go[744-754]

### Suggested approach
1. Detect whether `featureGate` is platform/arch-specific (e.g., matches a broader keyword set than `nonHypershiftPlatforms`, including `ibmcloud`/`powervs`, etc.).
2. Only apply the “check all required hypershift variants” fallback when the feature gate is truly generic (no platform/arch keywords).
3. If the feature gate appears platform-specific but no hypershift variants match, return an explicit error (or emit a validation warning/error) prompting to add the missing hypershift variant(s) rather than checking AWS by default.
4. Add/adjust a unit test around the variant-selection decision (ideally by extracting a helper that is testable without live Sippy calls).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread tools/codegen/cmd/featuregate-test-analyzer.go

@everettraven everettraven 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.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification

No second-stage tests were triggered for this PR.

This can happen when:

  • The changed files don't match any pipeline_run_if_changed patterns
  • All files match pipeline_skip_if_only_changed patterns
  • No pipeline-controlled jobs are defined for the master branch

Use /test ? to see all available tests.

@openshift-ci

openshift-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: everettraven

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 Jul 24, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification

No second-stage tests were triggered for this PR.

This can happen when:

  • The changed files don't match any pipeline_run_if_changed patterns
  • All files match pipeline_skip_if_only_changed patterns
  • No pipeline-controlled jobs are defined for the master branch

Use /test ? to see all available tests.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 18550f1 and 2 for PR HEAD ffbad1d in total

@openshift-ci

openshift-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

@JoelSpeed: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/verify-hypershift-integration ffbad1d link true /test verify-hypershift-integration

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

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. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants