Set "-aws-ovn-dualstack" jobs to candidate tier#3396
Set "-aws-ovn-dualstack" jobs to candidate tier#3396openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
Add pattern to classify jobs with "-aws-ovn-dualstack" substring as candidate tier. Currently, this should include 2 jobs `e2e-aws-ovn-dualstack-ipv4-primary-techpreview` and `e2e-aws-ovn-dualstack-ipv6-primary-techpreview`. This allows these techpreview jobs to be tracked in Sippy while they stabilize before being promoted to standard tier.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughAdded a new job-tier classification rule to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
There was a problem hiding this comment.
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 `@pkg/variantregistry/ocp.go`:
- Around line 810-812: The rule entry that currently matches
[]string{"-aws-ovn-dualstack"} and forces "candidate" is too broad; narrow the
matcher so it only targets techpreview dualstack jobs (e.g., change the pattern
to only match "-aws-ovn-dualstack-techpreview" or add a suffix/regex that
requires "techpreview"), or add an explicit techpreview condition alongside the
existing matcher; update the rule that maps []string{"-aws-ovn-dualstack"} ->
"candidate" so it only triggers for techpreview variants and adjust the comment
to accurately describe the scope.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 7cb3f90b-7439-49e6-a032-9da219179762
📒 Files selected for processing (1)
pkg/variantregistry/ocp.go
| // AWS DualStack Techpreview jobs - candidate tier to collect data while stabilizing | ||
| {[]string{"-aws-ovn-dualstack"}, "candidate"}, | ||
|
|
There was a problem hiding this comment.
Scope mismatch: this rule matches all -aws-ovn-dualstack jobs, not just techpreview
The comment says techpreview-only, but the matcher is broader. This can unintentionally force future non-techpreview dualstack jobs to remain candidate and hide promotable signal.
Suggested fix
- {[]string{"-aws-ovn-dualstack"}, "candidate"},
+ {[]string{"-aws-ovn-dualstack", "-techpreview"}, "candidate"},As per coding guidelines, "-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // AWS DualStack Techpreview jobs - candidate tier to collect data while stabilizing | |
| {[]string{"-aws-ovn-dualstack"}, "candidate"}, | |
| // AWS DualStack Techpreview jobs - candidate tier to collect data while stabilizing | |
| {[]string{"-aws-ovn-dualstack", "-techpreview"}, "candidate"}, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/variantregistry/ocp.go` around lines 810 - 812, The rule entry that
currently matches []string{"-aws-ovn-dualstack"} and forces "candidate" is too
broad; narrow the matcher so it only targets techpreview dualstack jobs (e.g.,
change the pattern to only match "-aws-ovn-dualstack-techpreview" or add a
suffix/regex that requires "techpreview"), or add an explicit techpreview
condition alongside the existing matcher; update the rule that maps
[]string{"-aws-ovn-dualstack"} -> "candidate" so it only triggers for
techpreview variants and adjust the comment to accurately describe the scope.
There was a problem hiding this comment.
There are no non-techpreview aws dual stack jobs. We are adding two jobs and while we collect e2e tests that need to be fixed we don't want the results to degrade component readiness:
e2e-aws-ovn-dualstack-ipv4-primary-techpreview
e2e-aws-ovn-dualstack-ipv6-primary-techpreview
There was a problem hiding this comment.
@patrickdillon, understood — since both existing jobs include -techpreview in their names, the current broad matcher works correctly today. The only residual risk is if a non-techpreview -aws-ovn-dualstack job is ever added in the future, it would silently be forced to candidate. If that ever happens, the pattern would need to be narrowed. Acknowledged for now.
📝 Learnings were identified and not saved (knowledge base disabled). Enable
There was a problem hiding this comment.
I was going to add my nit about the comment being out of sync with the code but I see the rabbit has beat me to it.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
/lgtm |
|
Scheduling required tests: |
|
/test e2e |
1 similar comment
|
/test e2e |
|
/test e2e Weird... |
|
e2e test failing with: Not sure what that means. |
|
/test e2e |
1 similar comment
|
/test e2e |
|
/lgtm |
|
@stbenjam we are looking for an approval for this PR. We want to run AWS DualStack jobs in the cadidate tier until the feature stabilizes. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gpei, patrickdillon, sadasu, stbenjam, tthvo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test lint |
|
@sadasu: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Add pattern to classify jobs with "-aws-ovn-dualstack" substring as candidate tier. Currently, this should include 2 jobs
e2e-aws-ovn-dualstack-ipv4-primary-techpreviewande2e-aws-ovn-dualstack-ipv6-primary-techpreview.This allows these techpreview jobs to be tracked
in Sippy while they stabilize before being promoted to standard tier.
Summary by CodeRabbit