Skip to content

Add a CNO secondary encap CI lane - #81614

Open
SchSeba wants to merge 1 commit into
openshift:mainfrom
SchSeba:ovn-k_encap_lane
Open

Add a CNO secondary encap CI lane#81614
SchSeba wants to merge 1 commit into
openshift:mainfrom
SchSeba:ovn-k_encap_lane

Conversation

@SchSeba

@SchSeba SchSeba commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Create a baremetal cluster-network-operator workflow that provisions a secondary interface, injects node-specific OVN_ENCAP_IP overrides, and prints the resolved override mapping before validating OVS state.

Summary by CodeRabbit

This PR adds a bare-metal cluster-network-operator CI lane for OVN secondary encapsulation. The workflow provisions a secondary interface, derives each node’s secondary IP, injects node-specific OVN_ENCAP_IP values through an env-overrides ConfigMap, restarts ovnkube-node, and validates the resulting OVS Geneve state.

It adds pre-validation and validation steps, workflow metadata, and ownership configuration. The validation step checks each node’s configured ovn-encap-ip, Geneve ports, and peer remote IPs against the expected secondary-interface mapping.

The CI configuration runs this workflow on equinix-ocp-metal when relevant OVN-Kubernetes or networking test files change.

@openshift-ci
openshift-ci Bot requested review from arghosh93 and arkadeepsen July 8, 2026 14:18
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR adds an optional Metal IPI E2E job for OVN secondary encapsulation. It defines the workflow, discovers node secondary-interface IPs, injects OVN_ENCAP_IP overrides, and validates the resulting OVS state.

Changes

OVN secondary encapsulation CI flow

Layer / File(s) Summary
Job and workflow wiring
ci-operator/config/openshift/cluster-network-operator/openshift-cluster-network-operator-master.yaml, ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/*, ci-operator/step-registry/cluster-network-operator/{OWNERS,ovn/OWNERS}
Registers the optional E2E job, defines the Metal IPI workflow, and adds workflow and step ownership metadata.
Secondary interface discovery and override injection
ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/*
Discovers per-node secondary-interface IPs, creates OVN_ENCAP_IP entries in the env-overrides ConfigMap, restarts ovnkube-node, and verifies mounted overrides.
Encapsulation IP validation
ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/*
Reads OVS state for each node, compares external_ids:ovn-encap-ip with the expected IP, validates Geneve peers, stores command output, and returns an aggregate validation status.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant JobConfig
  participant Workflow
  participant PreScript
  participant NodePod
  participant ConfigMap
  participant ValidateScript
  participant OVS

  JobConfig->>Workflow: starts e2e-metal-ipi-ovn-secondary-encap
  Workflow->>PreScript: runs pre step
  PreScript->>NodePod: reads secondary interface and IP
  PreScript->>ConfigMap: writes per-node OVN_ENCAP_IP overrides
  PreScript->>NodePod: restarts ovnkube-node and verifies mounted overrides
  Workflow->>ValidateScript: runs validation step
  ValidateScript->>NodePod: locates each ready node pod
  ValidateScript->>OVS: reads ovs-vsctl state and external_ids
  ValidateScript->>ValidateScript: compares ovn-encap-ip and Geneve peer data
Loading

Possibly related PRs

  • openshift/release#82957: Both modify the Cluster Network Operator E2E job configuration, but they target different tests and settings.

Suggested labels: lgtm

Suggested reviewers: arkadeepsen, arghosh93


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors)

Check name Status Explanation Resolution
Container-Privileges ❌ Error The new workflow sets EXTRA_MG_ARGS: --host-network; the gather step passes this value to oc adm must-gather, requesting a host-networked pod. Remove --host-network from EXTRA_MG_ARGS, or provide an approved design that avoids hostNetwork for must-gather.
No-Sensitive-Data-In-Logs ❌ Error The pre script logs Kubernetes node and pod names and the validator streams full OVS external_ids/show output, which can expose internal hostnames and network endpoints. Redact or replace node/pod identifiers, and filter OVS output to only the required Geneve type, local_ip, remote_ip, and encap-IP fields before logging or uploading artifacts.
✅ Passed checks (13 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: adding a Cluster Network Operator secondary encapsulation CI lane.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 changes only CI YAML, metadata, OWNERS, and shell scripts; it adds no Ginkgo It/Describe/Context/When declarations or dynamic test titles.
Test Structure And Quality ✅ Passed The PR changes only CI YAML/JSON and shell workflow scripts; no Ginkgo test code or It/BeforeEach/AfterEach/Eventually constructs are added or modified.
Microshift Test Compatibility ✅ Passed The patch adds CI YAML and Bash steps only; it changes no Go files and adds no Ginkgo tests, so MicroShift API compatibility checks do not apply.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The pull request adds CI YAML, shell scripts, metadata, and OWNERS files only; it adds no Go files or Ginkgo It/Describe/Context/When tests requiring SNO review.
Topology-Aware Scheduling Compatibility ✅ Passed The PR adds CI workflow/scripts and a ConfigMap, then restarts the existing OVN DaemonSet; it adds no replicas, affinity, topology spread, selectors, tolerations, or PDB constraints.
Ote Binary Stdout Contract ✅ Passed The diff adds only shell, YAML, JSON, and OWNERS files; no Go OTE binary or suite setup. Shell stdout belongs to CI steps, outside this contract.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The patch adds CI YAML and Bash workflow steps, not new Ginkgo e2e declarations; the IPv6/disconnected Ginkgo compatibility check is not applicable.
No-Weak-Crypto ✅ Passed The changed files add only CI YAML, metadata, ownership, and shell orchestration; strict scans found no MD5, SHA1, DES, RC4, 3DES, Blowfish, or ECB usage, and no secret/token comparisons.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@SchSeba
SchSeba force-pushed the ovn-k_encap_lane branch from 722fe53 to c32399f Compare July 8, 2026 15:17

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.yaml (1)

5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Same subnet value hardcoded twice.

192.168.221.0/24 appears both as the standalone EXTRANET_NETWORK_SUBNET_V4 env var (line 10) and again inline inside the EXTRA_NETWORK_CONFIG heredoc (line 13). Since these are two separate literals, an update to one without the other would silently desync the extranet provisioning config from what the pre-step/validate steps expect.

🤖 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/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.yaml`
around lines 5 - 13, The extranet subnet value is duplicated in the workflow env
setup, so keep it defined from a single source instead of hardcoding it twice.
Update the cluster-network-operator-ovn-secondary-encap workflow’s env block so
EXTRANET_NETWORK_SUBNET_V4 and the EXTRA_NETWORK_CONFIG content both reference
the same subnet value consistently, using the existing extranet config symbols
to avoid future drift.
🤖 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/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-commands.sh`:
- Around line 15-26: Tighten the EXTRANET_NETWORK_SUBNET_V4 validation in
cluster-network-operator-ovn-secondary-encap-pre-commands.sh: the current */24
glob plus %0/24 stripping in the subnet/prefix setup is too loose and can
produce an empty secondary_prefix. Update the validation to use a single
anchored format check in the existing shell logic around secondary_prefix and
escaped_secondary_prefix, then derive the prefix only after the value is
confirmed to be in the expected x.x.x.0/24 form. Also ensure the value
interpolated into the remote bash -c awk match is always safely escaped before
use.
- Around line 119-124: The pod lookup in the secondary-encap pre-commands script
is still causing the shell to exit under set -e when no matching ovnkube-node
pod exists. Update the oc get call inside the while loop that assigns pod_name
to tolerate the empty-result case without failing, so the subsequent if [[ -z
"${pod_name}" ]] fallback can run and print pod=<missing>.

In
`@ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh`:
- Around line 39-61: The node validation loop in
cluster-network-operator-ovn-secondary-encap-validate-commands.sh relies on
command substitutions that can exit early under set -e, so failures from oc or
get_ovnkube_container stop the script before validation_failed is updated.
Update the logic around pod_name, container_name, and actual_ip to capture
failures explicitly and mark validation_failed instead of aborting the loop,
while preserving the per-node validation flow in this script.

---

Nitpick comments:
In
`@ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.yaml`:
- Around line 5-13: The extranet subnet value is duplicated in the workflow env
setup, so keep it defined from a single source instead of hardcoding it twice.
Update the cluster-network-operator-ovn-secondary-encap workflow’s env block so
EXTRANET_NETWORK_SUBNET_V4 and the EXTRA_NETWORK_CONFIG content both reference
the same subnet value consistently, using the existing extranet config symbols
to avoid future drift.
🪄 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: a4b9d3e1-a0de-41d5-aebb-1879caac1620

📥 Commits

Reviewing files that changed from the base of the PR and between 0c8b4cf and c32399f.

⛔ Files ignored due to path filters (1)
  • ci-operator/jobs/openshift/cluster-network-operator/openshift-cluster-network-operator-master-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (14)
  • ci-operator/config/openshift/cluster-network-operator/openshift-cluster-network-operator-master.yaml
  • ci-operator/step-registry/cluster-network-operator/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-commands.sh
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.yaml

@SchSeba

SchSeba commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@SchSeba: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@SchSeba

SchSeba commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-metal-ipi-ovn-secondary-encap

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@SchSeba: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@zeeke

zeeke commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

/lgtm

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

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: SchSeba, zeeke
Once this PR has been reviewed and has the lgtm label, please assign danwinship for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 12, 2026
@SchSeba
SchSeba force-pushed the ovn-k_encap_lane branch from c32399f to d73c04d Compare July 14, 2026 14:24
@openshift-ci openshift-ci Bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 14, 2026
@openshift-ci

openshift-ci Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

New changes are detected. LGTM label has been removed.

@SchSeba
SchSeba force-pushed the ovn-k_encap_lane branch from d73c04d to 45cd87c Compare July 14, 2026 18:08
@SchSeba

SchSeba commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse max

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@SchSeba: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@SchSeba
SchSeba force-pushed the ovn-k_encap_lane branch from 45cd87c to c7c8dc5 Compare July 27, 2026 15:24
@tssurya

tssurya commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@SchSeba did we run the test-with-pr option and is there a link to a successful run of this job that I can look at?

@tssurya

tssurya commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

oh and you would need to address coderabbit comments

@tssurya

tssurya commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Create a baremetal cluster-network-operator workflow that provisions a secondary interface, injects node-specific OVN_ENCAP_IP overrides, and prints the resolved override mapping before validating OVS state.

Summary by CodeRabbit

This PR adds a new baremetal CI lane for cluster-network-operator to validate OVN secondary encapsulation behavior. It introduces a new step-registry workflow that provisions a secondary interface, injects per-node OVN_ENCAP_IP overrides through an env-overrides ConfigMap, restarts ovnkube-node, and verifies the live OVS encapsulation IP matches the expected secondary interface address.

I assume we are having a e2e test that actually tests east-west traffic over the configured geneve encap IP right? I don't see that detail in PR describe - that's what we promise through doing this so hope that's getting added..

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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.

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

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/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh`:
- Around line 71-84: Extend the validation workflow after
cluster-network-operator-ovn-secondary-encap-pre with a cross-node Geneve
traffic test that generates traffic between pods on different nodes and verifies
it uses the configured encapsulation IPs. Keep the existing
external_ids:ovn-encap-ip validation unchanged, and fail the validation when
cross-node Geneve traffic cannot be established or observed.
🪄 Autofix

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: 9f29ef11-d5b2-49fe-a075-b29aa1e3f644

📥 Commits

Reviewing files that changed from the base of the PR and between b8ce723 and 4973dfd.

⛔ Files ignored due to path filters (1)
  • ci-operator/jobs/openshift/cluster-network-operator/openshift-cluster-network-operator-master-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (14)
  • ci-operator/config/openshift/cluster-network-operator/openshift-cluster-network-operator-master.yaml
  • ci-operator/step-registry/cluster-network-operator/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-commands.sh
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (13)
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/OWNERS
  • ci-operator/config/openshift/cluster-network-operator/openshift-cluster-network-operator-master.yaml
  • ci-operator/step-registry/cluster-network-operator/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-commands.sh
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/OWNERS

Comment on lines +71 to +84
if ! actual_ip=$(oc exec -n "${namespace}" "${pod_name}" -c "${container_name}" -- ovs-vsctl get Open_vSwitch . external_ids:ovn-encap-ip | tr -d '"'); then
echo "unable to read ovn-encap-ip for node ${node_name}" >&2
validation_failed=1
continue
fi
if [[ "${actual_ip}" != "${expected_ip}" ]]; then
echo "encap IP mismatch on ${node_name}: expected ${expected_ip}, got ${actual_ip}" >&2
validation_failed=1
continue
fi

if ! grep -Fq "${expected_ip}" "${show_file}"; then
echo "ovs-vsctl show output for ${node_name} did not contain ${expected_ip}; using external_ids:ovn-encap-ip as the source of truth"
fi

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect this workflow's commands for a post-override data-plane assertion.
fd -t f -e yaml -e sh . \
  ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap |
  sort |
  xargs -r rg -n -i -C 4 \
    'ping|iperf|netperf|curl|nc |geneve|encap|east.?west|cross.?node'

Repository: openshift/release

Length of output: 27093


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- workflow ---'
cat -n ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.yaml

printf '%s\n' '--- validation script ---'
sed -n '45,95p' ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh

printf '%s\n' '--- candidate reusable traffic steps ---'
rg -n -i -g '*.yaml' -g '*.sh' \
  'east.?west|cross.?node|iperf|netperf|geneve|ovn.*traffic|traffic.*ovn|connectivity|network.*test' \
  ci-operator/step-registry |
  head -n 300

Repository: openshift/release

Length of output: 50373


Add a cross-node Geneve traffic test after cluster-network-operator-ovn-secondary-encap-pre.

The workflow only runs the validation step, which checks external_ids:ovn-encap-ip. It does not prove that east-west traffic uses the configured encapsulation IP.

🤖 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/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh`
around lines 71 - 84, Extend the validation workflow after
cluster-network-operator-ovn-secondary-encap-pre with a cross-node Geneve
traffic test that generates traffic between pods on different nodes and verifies
it uses the configured encapsulation IPs. Keep the existing
external_ids:ovn-encap-ip validation unchanged, and fail the validation when
cross-node Geneve traffic cannot be established or observed.

@SchSeba

SchSeba commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse --pull=openshift/cluster-network-operator#2998 pull-ci-openshift-cluster-network-operator-master-e2e-metal-ipi-ovn-secondary-encap

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@SchSeba: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@SchSeba: job(s): --pull=openshift/cluster-network-operator#2998 either don't exist or were not found to be affected, and cannot be rehearsed

Create a baremetal cluster-network-operator workflow that provisions a secondary interface,
injects node-specific OVN_ENCAP_IP overrides, and prints the resolved override mapping
before validating OVS state.

Signed-off-by: Sebastian Sch <sebassch@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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.

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

♻️ Duplicate comments (1)
ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh (1)

101-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle a zero-match port count without exiting the validation loop.

At Line 101, grep returns status 1 when no local_ip matches. With pipefail and errexit, the assignment exits the script before Lines 102-105 report the failed count or validate remaining nodes.

This has the same failure mode as the previously fixed command substitutions, but it remains at this count operation. Treat grep status 1 as a zero count. Preserve other grep failures.

Proposed fix
-  local_geneve_ports=$(grep -F "local_ip=\"${expected_ip}\"" "${show_file}" | wc -l | tr -d ' ')
+  if local_geneve_ports=$(grep -Fc -- "local_ip=\"${expected_ip}\"" "${show_file}"); then
+    :
+  else
+    grep_status=$?
+    if [[ "${grep_status}" -eq 1 ]]; then
+      local_geneve_ports=0
+    else
+      echo "unable to count local Geneve ports for node ${node_name}" >&2
+      validation_failed=1
+      continue
+    fi
+  fi

Rehearse a case with no matching local_ip entry. The script should report the node mismatch and continue with the remaining mappings.

🤖 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/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh`
around lines 101 - 105, Update the local_geneve_ports count operation in the
validation loop to treat grep’s status 1 (no matching local_ip) as a zero count,
allowing the existing mismatch reporting and remaining-node validation to
continue. Preserve propagation of other grep failures while retaining the
current count behavior for matches.

Source: Linters/SAST tools

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

Duplicate comments:
In
`@ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh`:
- Around line 101-105: Update the local_geneve_ports count operation in the
validation loop to treat grep’s status 1 (no matching local_ip) as a zero count,
allowing the existing mismatch reporting and remaining-node validation to
continue. Preserve propagation of other grep failures while retaining the
current count behavior for matches.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 11723d1e-0863-46f1-9d95-38441cc0c4fc

📥 Commits

Reviewing files that changed from the base of the PR and between eb58676 and 0f0798d.

⛔ Files ignored due to path filters (1)
  • ci-operator/jobs/openshift/cluster-network-operator/openshift-cluster-network-operator-master-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (14)
  • ci-operator/config/openshift/cluster-network-operator/openshift-cluster-network-operator-master.yaml
  • ci-operator/step-registry/cluster-network-operator/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-commands.sh
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-commands.sh
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (12)
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.yaml
  • ci-operator/config/openshift/cluster-network-operator/openshift-cluster-network-operator-master.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/cluster-network-operator-ovn-secondary-encap-validate-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/cluster-network-operator-ovn-secondary-encap-workflow.metadata.json
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.metadata.json
  • ci-operator/step-registry/cluster-network-operator/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-ref.yaml
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/validate/OWNERS
  • ci-operator/step-registry/cluster-network-operator/ovn/secondary-encap/pre/cluster-network-operator-ovn-secondary-encap-pre-commands.sh

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@SchSeba: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-openshift-cluster-network-operator-master-e2e-metal-ipi-ovn-secondary-encap openshift/cluster-network-operator presubmit Presubmit changed
pull-ci-openshift-cluster-network-operator-master-e2e-aws-ovn-fdp-qe openshift/cluster-network-operator presubmit Ci-operator config changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@SchSeba

SchSeba commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-metal-ipi-ovn-secondary-encap

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@SchSeba: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-ci

openshift-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@SchSeba: 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/rehearse/openshift/cluster-network-operator/master/e2e-metal-ipi-ovn-secondary-encap 0f0798d link unknown /pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-metal-ipi-ovn-secondary-encap

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants