Skip to content

TRT-2647: rhcos10 default for 5.0#3498

Open
neisw wants to merge 2 commits intoopenshift:mainfrom
neisw:trt-2647-5.0-rhcos10-default
Open

TRT-2647: rhcos10 default for 5.0#3498
neisw wants to merge 2 commits intoopenshift:mainfrom
neisw:trt-2647-5.0-rhcos10-default

Conversation

@neisw
Copy link
Copy Markdown
Contributor

@neisw neisw commented May 5, 2026

Staging for when the default changes over to rhcos10 for 5.0

Open question, should we remove OS: from the 5.0 main view so they are all picked up by default or explicitly specify rhcos9 and rhcos10?

Summary by CodeRabbit

  • New Features

    • Extended RHCOS 10 compatibility to version 5 and main-branch jobs.
  • Chores

    • Updated scenario comparisons and analysis settings (including direction and feature set changes).
    • Disabled certain regression/metrics tracking and adjusted thresholds for a comparison scenario.
    • Removed an obsolete comparison scenario.
    • Updated tests to reflect the new OS variant defaults.

@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: automatic mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 5, 2026
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented May 5, 2026

@neisw: This pull request references TRT-2647 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Staging for when the default changes over to rhcos10 for 5.0

Open question, should we remove OS: from the 5.0 main view so they are all picked up by default or explicitly specify rhcos9 and rhcos10?

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
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

Walkthrough

Updates default OS variant for OpenShift 5.x/main to rhcos10 in variant registry fallback logic; config scenarios are adjusted to include rhcos10 (and a 5.0 comparison scenario is reworked). Tests updated to expect rhcos10 for affected jobs; one 4.22 techpreview comparison scenario removed.

Changes

OS Variant Default Update for 5.0

Layer / File(s) Summary
Variant Registry Logic
pkg/variantregistry/ocp.go
setOS fallback assigns VariantOS = "rhcos10" when VariantReleaseMajor == "5" or job indicates main branch (previously "rhcos9").
Component Readiness Configuration
config/views.yaml (lines ~49–50, ~139–141, ~226–227)
component_readiness scenarios 5.0-main, 5.0-rosa, and 5.0-main-mass-failure now include rhcos10 alongside rhcos9 in their OS lists.
Comparison Scenario Rework
config/views.yaml (lines ~411–491)
Replaced 5.0-techpreview-rhcos9-vs-rhcos10 with 5.0-rhcos10-vs-rhcos9; swapped include_variants.OS/compare_variants.OS, changed include_variants.FeatureSet from techpreview to default, adjusted advanced_options thresholds, and set include_multi_release_analysis, metrics.enabled, and regression_tracking.enabled to false.
Scenario Removal
config/views.yaml (line ~1433)
Removed 4.22-techpreview-rhcos9-vs-rhcos10 scenario.
Test Updates
pkg/variantregistry/ocp_test.go
TestVariantSyncer expectations updated: several jobs now expect VariantOS = "rhcos10" instead of "rhcos9".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 15 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Test Structure And Quality ❓ Inconclusive Check requires Ginkgo test code, but sippy uses Go's standard testing with table-driven patterns. No Ginkgo patterns found in codebase. Check targets Ginkgo tests. Sippy uses Go standard testing. Apply to Ginkgo tests or adapt for table-driven patterns.
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective: making rhcos10 the default for 5.0 releases, which is directly supported by changes across all three files.
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.
Go Error Handling ✅ Passed PR maintains proper Go error handling patterns. No panic(), error ignoring, or nil dereference issues introduced.
Sql Injection Prevention ✅ Passed The SQL Injection check is not applicable. Changes involve configuration updates and variant logic modifications with no SQL query construction, parameterization, or user input handling involved.
Excessive Css In React Should Use Styles ✅ Passed PR modifies only Go code and YAML configuration files with no React/JSX components. Custom check targets React inline CSS styling, which is not present.
Test Coverage For New Features ✅ Passed The setOS pure function was updated to return rhcos10 for 5.x or main jobs. Tests cover changes with 8 cases expecting rhcos10 and 59 expecting rhcos9, validating logic.
Single Responsibility And Clear Naming ✅ Passed No violations of single responsibility principle. Changes are minimal and focused: type refinement, clear method names, no generic naming patterns. Struct scope remains appropriate.
Stable And Deterministic Test Names ✅ Passed Project does not use Ginkgo framework. Modified test file uses standard Go testing with testify. No Ginkgo test declarations found. Check is not applicable.
Microshift Test Compatibility ✅ Passed This PR modifies the sippy CI analysis tool and does not add any Ginkgo e2e tests. The project uses standard Go testing, not Ginkgo. The custom check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests added. Changes are YAML config, variant registry logic, and unit test updates only. SNO check only applies to new Ginkgo e2e tests.
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies test infrastructure (config/views.yaml and pkg/variantregistry/) only. No deployment manifests, operator code, or controllers are changed. The check does not apply.
Ote Binary Stdout Contract ✅ Passed PR modifies only library code (pkg/variantregistry/) and configuration files. No process-level code, binary entry points, or stdout writes present. Check not applicable.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No Ginkgo e2e tests are added in this PR. It modifies configuration, Go source code, and Go unit tests using testing.T framework, not Ginkgo. Check not applicable.

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

✨ 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.1)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


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

@openshift-ci openshift-ci Bot requested review from deads2k and sosiouxme May 5, 2026 19:25
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 5, 2026
@openshift-merge-bot openshift-merge-bot Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label May 5, 2026
Copy link
Copy Markdown
Contributor

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

🧹 Nitpick comments (1)
pkg/variantregistry/ocp_test.go (1)

1139-1161: ⚡ Quick win

Consider adding a no-release -main- sibling case for branch-fallback coverage.

You now validate -master- with no release (rhcos10). Adding a matching -main- no-release case would close the parity gap for isMainBranch.

✅ Suggested test addition
 		{
 			job: "periodic-ci-openshift-master-no-release-version-e2e-aws-ovn",
 			expected: map[string]string{
 				VariantArch:           "amd64",
 				VariantInstaller:      "ipi",
 				VariantPlatform:       "aws",
 				VariantNetwork:        "ovn",
 				VariantNetworkStack:   "ipv4",
 				VariantOwner:          "eng",
 				VariantSuite:          "unknown",
 				VariantTopology:       "ha",
 				VariantUpgrade:        VariantNoValue,
 				VariantAggregation:    VariantNoValue,
 				VariantProcedure:      "none",
 				VariantJobTier:        "candidate",
 				VariantFeatureSet:     VariantDefaultValue,
 				VariantNetworkAccess:  VariantDefaultValue,
 				VariantScheduler:      VariantDefaultValue,
 				VariantSecurityMode:   VariantDefaultValue,
 				VariantCGroupMode:     "v2",
 				VariantLayeredProduct: VariantNoValue,
 				VariantOS:             "rhcos10",
 			},
 		},
+		{
+			job: "periodic-ci-openshift-main-no-release-version-e2e-aws-ovn",
+			expected: map[string]string{
+				VariantArch:           "amd64",
+				VariantInstaller:      "ipi",
+				VariantPlatform:       "aws",
+				VariantNetwork:        "ovn",
+				VariantNetworkStack:   "ipv4",
+				VariantOwner:          "eng",
+				VariantSuite:          "unknown",
+				VariantTopology:       "ha",
+				VariantUpgrade:        VariantNoValue,
+				VariantAggregation:    VariantNoValue,
+				VariantProcedure:      "none",
+				VariantJobTier:        "candidate",
+				VariantFeatureSet:     VariantDefaultValue,
+				VariantNetworkAccess:  VariantDefaultValue,
+				VariantScheduler:      VariantDefaultValue,
+				VariantSecurityMode:   VariantDefaultValue,
+				VariantCGroupMode:     "v2",
+				VariantLayeredProduct: VariantNoValue,
+				VariantOS:             "rhcos10",
+			},
+		},
🤖 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 `@pkg/variantregistry/ocp_test.go` around lines 1139 - 1161, Add a sibling test
case to the table in ocp_test.go that mirrors the existing
"periodic-ci-openshift-master-no-release-version-e2e-aws-ovn" case but uses
"periodic-ci-openshift-main-no-release-version-e2e-aws-ovn" as the job name to
exercise the isMainBranch path; copy the expected map (VariantArch,
VariantInstaller, VariantPlatform, VariantNetwork, VariantNetworkStack,
VariantOwner, VariantSuite, VariantTopology, VariantUpgrade, VariantAggregation,
VariantProcedure, VariantJobTier, VariantFeatureSet, VariantNetworkAccess,
VariantScheduler, VariantSecurityMode, VariantCGroupMode, VariantLayeredProduct,
VariantOS) from the master case so the assertion logic in the test (table-driven
test around the job field) verifies parity for main-branch fallback.
🤖 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 `@pkg/variantregistry/ocp_test.go`:
- Around line 1139-1161: Add a sibling test case to the table in ocp_test.go
that mirrors the existing
"periodic-ci-openshift-master-no-release-version-e2e-aws-ovn" case but uses
"periodic-ci-openshift-main-no-release-version-e2e-aws-ovn" as the job name to
exercise the isMainBranch path; copy the expected map (VariantArch,
VariantInstaller, VariantPlatform, VariantNetwork, VariantNetworkStack,
VariantOwner, VariantSuite, VariantTopology, VariantUpgrade, VariantAggregation,
VariantProcedure, VariantJobTier, VariantFeatureSet, VariantNetworkAccess,
VariantScheduler, VariantSecurityMode, VariantCGroupMode, VariantLayeredProduct,
VariantOS) from the master case so the assertion logic in the test (table-driven
test around the job field) verifies parity for main-branch fallback.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 683d6765-f56e-45e2-9847-79272263d51c

📥 Commits

Reviewing files that changed from the base of the PR and between 54af8d3 and 72e7de3.

📒 Files selected for processing (2)
  • pkg/variantregistry/ocp_test.go
  • pkg/variantregistry/snapshot.yaml

@neisw
Copy link
Copy Markdown
Contributor Author

neisw commented May 5, 2026

/hold
until we are ready to switch over

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 5, 2026
@openshift-merge-bot
Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 5, 2026

@neisw: 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.

@petr-muller
Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 5, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 5, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neisw, petr-muller

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

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants