[DNM] Test EVPN E2Es in v4 cluster#78801
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughA new test job ChangesNew BGP Local-Gateway Test Job
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pperiyasamy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse |
|
@pperiyasamy: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@ci-operator/config/openshift/ovn-kubernetes/openshift-ovn-kubernetes-master.yaml`:
- Around line 351-354: The job is overriding TEST_SKIPS with a narrow value
which hides the broader skip list; in the job definition that contains
TEST_SKIPS and workflow: baremetalds-e2e-ovn-bgp-ipv4-local-gw remove the
TEST_SKIPS override (or expand it to include the original broader skip entries)
so it does not replace the skip list defined in
baremetalds-e2e-ovn-bgp-ipv4-local-gw-workflow.yaml; edit the job to either
delete the TEST_SKIPS key entirely or merge the original skip patterns with the
CPU Partitioning entry to preserve all intended skips.
🪄 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), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e6c82fa1-db53-450c-a9aa-da4df25a6f71
⛔ Files ignored due to path filters (4)
ci-operator/jobs/openshift/ovn-kubernetes/openshift-ovn-kubernetes-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/ovn-kubernetes/openshift-ovn-kubernetes-release-4.22-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/ovn-kubernetes/openshift-ovn-kubernetes-release-4.23-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/ovn-kubernetes/openshift-ovn-kubernetes-release-5.0-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (7)
ci-operator/config/openshift/ovn-kubernetes/openshift-ovn-kubernetes-master.yamlci-operator/config/openshift/ovn-kubernetes/openshift-ovn-kubernetes-release-4.22.yamlci-operator/config/openshift/ovn-kubernetes/openshift-ovn-kubernetes-release-4.23.yamlci-operator/config/openshift/ovn-kubernetes/openshift-ovn-kubernetes-release-5.0.yamlci-operator/step-registry/baremetalds/e2e/ovn/bgp/ipv4-local-gw/OWNERSci-operator/step-registry/baremetalds/e2e/ovn/bgp/ipv4-local-gw/baremetalds-e2e-ovn-bgp-ipv4-local-gw-workflow.metadata.jsonci-operator/step-registry/baremetalds/e2e/ovn/bgp/ipv4-local-gw/baremetalds-e2e-ovn-bgp-ipv4-local-gw-workflow.yaml
| FEATURE_SET: TechPreviewNoUpgrade | ||
| TEST_SKIPS: CPU Partitioning cluster platform workloads should be annotated | ||
| correctly for Deployments | ||
| workflow: baremetalds-e2e-ovn-bgp-ipv4-local-gw |
There was a problem hiding this comment.
Avoid overriding workflow TEST_SKIPS with a narrower value.
At Lines 351-353, this job sets TEST_SKIPS to only the CPU partitioning case, which overrides the broader skip list already defined in ci-operator/step-registry/baremetalds/e2e/ovn/bgp/ipv4-local-gw/baremetalds-e2e-ovn-bgp-ipv4-local-gw-workflow.yaml (Lines 16-18). That can unintentionally re-enable known-skipped tests and make the job flaky.
Suggested fix
steps:
cluster_profile: equinix-ocp-metal
env:
FEATURE_SET: TechPreviewNoUpgrade
- TEST_SKIPS: CPU Partitioning cluster platform workloads should be annotated
- correctly for Deployments
workflow: baremetalds-e2e-ovn-bgp-ipv4-local-gw📝 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.
| FEATURE_SET: TechPreviewNoUpgrade | |
| TEST_SKIPS: CPU Partitioning cluster platform workloads should be annotated | |
| correctly for Deployments | |
| workflow: baremetalds-e2e-ovn-bgp-ipv4-local-gw | |
| FEATURE_SET: TechPreviewNoUpgrade | |
| workflow: baremetalds-e2e-ovn-bgp-ipv4-local-gw |
🤖 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
`@ci-operator/config/openshift/ovn-kubernetes/openshift-ovn-kubernetes-master.yaml`
around lines 351 - 354, The job is overriding TEST_SKIPS with a narrow value
which hides the broader skip list; in the job definition that contains
TEST_SKIPS and workflow: baremetalds-e2e-ovn-bgp-ipv4-local-gw remove the
TEST_SKIPS override (or expand it to include the original broader skip entries)
so it does not replace the skip list defined in
baremetalds-e2e-ovn-bgp-ipv4-local-gw-workflow.yaml; edit the job to either
delete the TEST_SKIPS key entirely or merge the original skip patterns with the
CPU Partitioning entry to preserve all intended skips.
Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
3c79a5b to
1b247a5
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse |
|
@pperiyasamy: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@pperiyasamy: The following tests failed, say
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. |
This PR adds a new end-to-end test job for validating OVN BGP (Border Gateway Protocol) functionality in IPv4-only clusters. The PR is marked [DNM] (Do Not Merge) as a test run to validate EVPN testing infrastructure.
Changes to OVN-Kubernetes CI
The PR introduces the
e2e-metal-ipi-ovn-ipv4-bgp-local-gw-techpreviewtest job to the OVN-Kubernetes 4.22 release CI configuration. This test:TechPreviewNoUpgradefeature set)Supporting Infrastructure
A new workflow
baremetalds-e2e-ovn-bgp-ipv4-local-gwwas created to orchestrate the testing:IP_STACK=v4)The new test complements existing BGP tests in the release branches, which focus on dualstack environments, by providing IPv4-specific BGP validation coverage.