Add CNO secondary encap CI lanes for 4.20-4.22 and 5.0 - #83663
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: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughRelease 4.20 through 5.0 CI configurations add optional intranet-capable metal end-to-end jobs for OVN secondary encapsulation. Each job uses targeted OVN path filters and the ChangesOVN secondary encapsulation CI
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR adds CI coverage for OVN encapsulation override changes on four release branches. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.) Full details: Stable And Deterministic Test NamesExplanation PASS. The pull request changes only CI YAML and generated presubmit YAML. The added identifiers are fixed strings such as Full details: Test Structure And QualityExplanation PASS: The pull request changes only eight YAML CI configuration/generated job files. The commit adds no Ginkgo test files, Go test code, or Ginkgo DSL. Therefore the stated requirements for It-block responsibility, fixture cleanup, cluster-operation timeouts, assertion messages, and Ginkgo test patterns are not applicable to this change. Full details: Microshift Test CompatibilityExplanation PASS: The pull request adds only eight YAML CI configuration files. The parent-to-HEAD diff contains no Go files, Ginkgo test declarations, MicroShift checks, or OpenShift API references in test code. The check applies only when new Ginkgo e2e tests are added, so it is not applicable. Full details: Single Node Openshift (Sno) Test CompatibilityExplanation PASS: The pull request adds only CI configuration and generated Prow presubmit YAML. The commit diff contains no Go or other test-source changes and no new Ginkgo declarations such as It(), Describe(), Context(), or When(). Therefore, the SNO compatibility check is not applicable. The referenced secondary-encap lane mirrors existing master/5.1 configuration and does not introduce a new test body or a new multi-node assumption. Full details: Topology-Aware Scheduling CompatibilityExplanation PASS. The commit changes only CI test declarations and generated Prow presubmit jobs. The added entries select an Equinix cluster profile, mark the job optional, filter changed files, and reference an existing workflow. The diff introduces no deployment manifests, operator/controller code, replicas, anti-affinity, topology spread constraints, node selectors or affinities, tolerations, or PDBs. Therefore the custom topology-aware scheduling failure conditions do not apply. Full details: Ote Binary Stdout ContractExplanation PASS. The pull request changes only eight YAML files: four CI configurations and four generated presubmit job definitions. The added content defines optional Full details: Ipv6 And Disconnected Network Test CompatibilityExplanation PASS: The pull request adds only YAML CI job declarations and generated presubmit entries. It adds no Ginkgo test code, Go files, or shell test files, and no added lines contain the listed IPv4 or external-connectivity patterns. The referenced secondary-encap workflow already existed and contains its own IPv4 configuration; that unchanged behavior is not introduced by this pull request. The presubmit image uses the internal Full details: No-Weak-CryptoExplanation PASS. The pull request adds CI YAML and generated presubmit job metadata only. The added content contains no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret/token comparison logic. The referenced secondary-encapsulation workflow also contains no flagged weak-crypto usage. Full details: Container-PrivilegesExplanation The pull request adds CI lane declarations and generated Prow jobs only. Added lines contain no Full details: No-Sensitive-Data-In-LogsExplanation The pull request adds four Prow jobs that invoke the existing secondary-encapsulation workflow. That workflow logs the Kubernetes node name and pod name in Resolution Redact node names and pod names before logging, and remove the raw ConfigMap dump from the pre step. Log only sanitized validation results. Store any required diagnostic mapping in access-controlled artifacts instead of public CI logs, or do not enable these jobs until the workflow output is sanitized.
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/pj-rehearse skip We can't test CI until the 5.0 backport PR (openshift/cluster-network-operator#3124) and 4.x backport PRs merge first. |
|
@cgoncalves: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse ack |
|
@cgoncalves: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
dee130a to
6937ae9
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/lgtm |
|
Needs pj-rehearse to be run again and the ack ? across all releases? |
|
/pj-rehearse ack |
|
@cgoncalves: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@tssurya PR just needs approval label now. |
|
@cgoncalves where are the rehearsal job links that show they are running but failing at the encap step? I think in the original PR @SchSeba had a link from rehearse job we looked at to ensure it works as per our expectations |
|
/pj-rehearse |
|
@tssurya: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
| steps: | ||
| cluster_profile: equinix-ocp-metal | ||
| workflow: cluster-network-operator-ovn-secondary-encap | ||
| - as: e2e-aws-ovn-fdp-qe |
There was a problem hiding this comment.
where is the always_run: false for fdp-qe gone?
is it me or does the diff look weird? please insert the new block between two jobs instead of inserting after the always-run block of the fdp-qe job for both 5.0 and 4.22
4.21 and 4.20 look better
There was a problem hiding this comment.
afaict Carlos addressed this
|
@cgoncalves I don't get why rehearsals weren't triggered for 5.0 secondary-encap lane :P its like the fdp-qe lane is getting triggered lol which has nothng to do with this PR the diff looks strange as if we updated the fdp-qe instead of just inserting the new lane |
Mirror the existing master/5.1 optional metal IPI job so OVN encap override changes are covered on those release branches. Co-authored-by: Cursor <cursoragent@cursor.com Signed-off-by: Carlos Goncalves <cgoncalves@redhat.com>
6937ae9 to
e6fe4af
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/pj-rehearse |
|
@cgoncalves: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@cgoncalves please make sure rehearsals run, and then check they fail at the encap-ip step and then add the ack, that's the point of rehearsals to ensure the lanes run like we want them to - I understand they will fail w/o code - but that's not the point, its to make sure config is correct etc |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
Rehearsals are running now. |
|
@cgoncalves: 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. |
|
/pj-rehearse ack |
|
@andreaskaris: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
checked the 4 runs they fail at the expected step: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_release/83663/rehearse-83663-pull-ci-openshift-cluster-network-operator-release-4.20-e2e-metal-ipi-ovn-secondary-encap/2092533755037093888 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgoncalves, SchSeba, tssurya, zeeke 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 |
97bee72
into
openshift:main
Mirror the existing master/5.1 optional metal IPI job so OVN encap override changes are covered on those release branches.
Summary by CodeRabbit
Adds optional OVN secondary encapsulation metal E2E CI lanes for Cluster Network Operator release branches 4.20, 4.21, 4.22, and 5.0. Each lane uses the
cluster-network-operator-ovn-secondary-encapworkflow and runs only when relevant OVN or OVN-Kubernetes files change.