OCPBUGS-88063: ovn-kubernetes: Move MNP from ConfigMap to CLI flags - #3072
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/severity-critical
|
@weliang1: This pull request references Jira Issue OCPBUGS-88063, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (weliang@redhat.com), skipping review request. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
WalkthroughOVN multi-network policy enablement moves from rendered feature configuration into conditional control-plane startup flags for managed and self-hosted deployments. Tests update rendered configuration expectations to use separate feature flags, and documentation describes the two configuration mechanisms. ChangesOVN multi-network policy enablement
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant OVN_MULTI_NETWORK_POLICY_ENABLE
participant ControlPlaneTemplate
participant ovnkube
OVN_MULTI_NETWORK_POLICY_ENABLE->>ControlPlaneTemplate: render conditional startup flag
ControlPlaneTemplate->>ovnkube: exec with --enable-multi-networkpolicy
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24✅ Passed checks (24 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@bindata/network/ovn-kubernetes/managed/ovnkube-control-plane.yaml`:
- Around line 203-206: Remove the enable-multi-networkpolicy feature setting
from the managed ConfigMap template in 004-config.yaml, leaving feature
enablement exclusively through the multi_network_policy_enabled_flag CLI path
shown in the ovnkube control-plane template.
🪄 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: 913a6a21-578f-44bb-971a-99927cd8918a
📒 Files selected for processing (5)
bindata/network/ovn-kubernetes/managed/004-config.yamlbindata/network/ovn-kubernetes/managed/ovnkube-control-plane.yamlbindata/network/ovn-kubernetes/self-hosted/004-config.yamlbindata/network/ovn-kubernetes/self-hosted/ovnkube-control-plane.yamlpkg/network/ovn_kubernetes_test.go
💤 Files with no reviewable changes (3)
- bindata/network/ovn-kubernetes/self-hosted/004-config.yaml
- bindata/network/ovn-kubernetes/managed/004-config.yaml
- pkg/network/ovn_kubernetes_test.go
ce89aab to
7772bb8
Compare
weliang1
changed the title
openshift-ci-robot
removed
jira/severity-critical
|
@weliang1: No Jira issue is referenced in the title of this pull request. DetailsIn response to this:
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. |
Re: Container-Privileges Pre-merge CheckThe pre-merge check flagged This is pre-existing configuration, not introduced by this PR.
The diff shows no modifications to the hostNetwork line: $ git diff origin/master -- bindata/network/ovn-kubernetes/self-hosted/ovnkube-control-plane.yaml | grep -A2 -B2 hostNetwork
(no output - hostNetwork line unchanged)This check should be considered a false positive for this PR. |
RCA: Azure Job Failure - Infrastructure Issue ❌The Root Cause: Azure VM Quota ExceededWhat HappenedThe failure occurred during cluster installation (pre-phase), before any PR code was executed:
Why This Is NOT a Code Issue
The cluster installation failed before any OpenShift components (including cluster-network-operator) were deployed. The upgrade test never started, so the PR code (multi-networkpolicy CLI flags) was never executed. Impact on This PR✅ No action required - This failure does NOT block the PR:
Recommendation✅ Safe to proceed with review and approval This is a CI infrastructure team issue (Azure quota management). The PR code changes are correct and can proceed to merge pending:
Full RCA: See build log at https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-network-operator/3072/pull-ci-openshift-cluster-network-operator-master-5.0-upgrade-from-stable-4.22-e2e-azure-ovn-upgrade/2079224013493637120 |
|
@coderabbitai review The issue you identified has been fixed. All instances have been removed from both ConfigMaps in (there were two ConfigMaps - one for HyperShift nodes and one for control plane). Please re-review the latest commit. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Changed files and latest PR diff stat ==='
git diff --stat origin/master...HEAD
git diff --name-only origin/master...HEAD
echo
echo '=== All current occurrences of the option ==='
rg -n -C 5 --fixed-strings 'enable-multi-networkpolicy' \
bindata/network/ovn-kubernetes pkg/network/ovn_kubernetes_test.go || true
echo
echo '=== Relevant current template sections ==='
for file in \
bindata/network/ovn-kubernetes/managed/004-config.yaml \
bindata/network/ovn-kubernetes/managed/ovnkube-control-plane.yaml \
bindata/network/ovn-kubernetes/self-hosted/004-config.yaml \
bindata/network/ovn-kubernetes/self-hosted/ovnkube-control-plane.yaml \
pkg/network/ovn_kubernetes_test.go
do
echo "--- $file ---"
rg -n -C 12 --fixed-strings 'OVN_MULTI_NETWORK_POLICY_ENABLE' "$file" || true
rg -n -C 12 --fixed-strings 'enable-multi-networkpolicy' "$file" || true
done
echo
echo '=== Complete PR patch for changed target files ==='
git diff --unified=40 origin/master...HEAD -- \
bindata/network/ovn-kubernetes/managed/004-config.yaml \
bindata/network/ovn-kubernetes/managed/ovnkube-control-plane.yaml \
bindata/network/ovn-kubernetes/self-hosted/004-config.yaml \
bindata/network/ovn-kubernetes/self-hosted/ovnkube-control-plane.yaml \
pkg/network/ovn_kubernetes_test.go \
docs/ovn_node_mode.mdLength of output: 35268
No further issue found regarding the previously reported ConfigMap entry. ✅ Action performedReview finished.
|
|
Pre merge testing passed: ┌───────────────┬─────────────┬─────────────┬───────────────┐ Key Observations:
═══════════════════════════════════════════════════════════════ Conclusion Both test runs passed successfully, confirming that PR #3072 is working correctly and consistently: ✅ Moving MultiNetworkPolicy from ConfigMap to CLI flags works as designed The fix is stable and ready for merge.` |
|
|
||
| - **ConfigMap-based** (`004-config.yaml`): Most features (egress IP, multi-network, network segmentation, admin network policy, etc.) are configured in the cluster-wide ConfigMap which is passed to ovnkube via `--config-file`. | ||
|
|
||
| - **CLI flags** (`ovnkube-control-plane.yaml`): Features that require pod restarts on configuration changes (multicast, multi-networkpolicy) are enabled via CLI flags (e.g., `--enable-multicast`, `--enable-multi-networkpolicy`) to ensure ovnkube-control-plane pods restart automatically when the feature is toggled. |
There was a problem hiding this comment.
"features that require control-plane restarts" right? Features that only require ovnkube-node restarts can be handled via the ConfigMap, because we restart ovnkube-node when the configmap changes. (Right?)
There was a problem hiding this comment.
@danwinship Yes, exactly right!
- ovnkube-node: ConfigMap changes → automatic restart
- ovnkube-control-plane: ConfigMap changes → NO restart
There was a problem hiding this comment.
can you update the new docs to make that clearer?
|
/lgtm |
|
Scheduling required tests: Scheduling tests matching the |
openshift-ci
Bot
added
the
approved
MultiNetworkPolicy is not enforced on UDN secondary interfaces in OCP 5.0 because ovnkube-control-plane pods do not restart when the ConfigMap is updated with enable-multi-networkpolicy=true. Root cause: PR openshift#2944 moved MNP enablement to ConfigMap, but: 1. The ConfigMap hash only includes 008-script-lib.yaml, not 004-config.yaml 2. ovnkube-control-plane has no hash annotation to trigger restarts Solution: Follow the same pattern as commit f4734c5 (OCPBUGS-78731): move MNP back to CLI flags where pod restart happens automatically on spec changes. This creates consistency with multicast (also a CLI flag) and avoids the ConfigMap hash timing issues that caused the original NVIDIA-554 fix to be reverted. How to verify: 1. Deploy cluster with UseMultiNetworkPolicy=false - Verify ovnkube-control-plane pods lack --enable-multi-networkpolicy flag 2. Set UseMultiNetworkPolicy=true via network.operator/cluster - Verify ovnkube-control-plane pods restart with --enable-multi-networkpolicy 3. Test MNP enforcement on UDN Layer2/Layer3 networks 4. Verify DPU-host mode still works correctly 5. CI lanes: e2e-aws-ovn, e2e-gcp-ovn, e2e-metal-ipi-ovn-dualstack Tested: - MNP works on UDN Layer2 and Layer3 networks - Pods restart when UseMultiNetworkPolicy changes - DPU-host mode still works correctly - Other ConfigMap features (egress-ip, etc.) unaffected Jira: OCPBUGS-88063 docs: Clarify CLI flags are for control-plane pod restarts Update ovn_node_mode.md to specify that CLI flags are used for features requiring ovnkube-control-plane pod restarts, not just any pod restarts. This clarifies that ovnkube-node pods already restart when the ConfigMap changes, so CLI flags are only needed for control-plane-specific features. Addresses review feedback from danwinship. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
314b6d7 to
17f08a7
Compare
|
/lgtm |
|
Scheduling required tests: Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, weliang1 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 |
|
/retest-required |
|
/override ci/prow/e2e-aws-ovn-upgrade-ipsec ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw ci/prow/e2e-metal-ipi-ovn-dualstack-bgp ci/prow/e2e-ovn-ipsec-step-registry ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec ci/prow/e2e-aws-ovn-fdp-qe ci/prow/hypershift-e2e-aks ci/prow/e2e-aws-ovn-hypershift-conformance |
|
@danwinship: Overrode contexts on behalf of danwinship: ci/prow/e2e-aws-ovn-fdp-qe, ci/prow/e2e-aws-ovn-hypershift-conformance, ci/prow/e2e-aws-ovn-upgrade-ipsec, ci/prow/e2e-metal-ipi-ovn-dualstack-bgp, ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw, ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec, ci/prow/e2e-ovn-ipsec-step-registry, ci/prow/hypershift-e2e-aks DetailsIn response to this:
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. |
|
/retitle OCPBUGS-88063: ovn-kubernetes: Move MNP from ConfigMap to CLI flags |
openshift-ci
Bot
changed the title
openshift-ci-robot
added
jira/severity-critical
|
@weliang1: This pull request references Jira Issue OCPBUGS-88063, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (weliang@redhat.com), skipping review request. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
|
/verified by @weliang1 |
openshift-ci-robot
added
the
verified
|
@danwinship: This PR has been marked as verified by DetailsIn response to this:
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. |
|
/test e2e-aws-ovn-upgrade |
|
/test e2e-aws-ovn-windows |
|
/hold Revision 17f08a7 was retested 3 times: holding |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/test e2e-aws-ovn-upgrade |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
@weliang1: 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. |
00e6cc5
into
openshift:master
|
@weliang1: Jira Issue Verification Checks: Jira Issue OCPBUGS-88063 Jira Issue OCPBUGS-88063 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
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. |
3 participants
Summary
MultiNetworkPolicy is not enforced on UDN secondary interfaces in OCP 5.0 because ovnkube-control-plane pods do not restart when the ConfigMap is updated with
enable-multi-networkpolicy=true.Root Cause
PR #2944 moved MNP enablement to ConfigMap, but:
Solution
Follow the same pattern as commit f4734c5 (OCPBUGS-78731): move MNP back to CLI flags where pod restart happens automatically on spec changes.
This creates consistency with multicast (also a CLI flag) and avoids the ConfigMap hash timing issues.
Changes
--enable-multi-networkpolicyCLI flag--enable-multi-networkpolicyCLI flagHow to Verify
Manual Testing
Deploy cluster with
UseMultiNetworkPolicy=falseinnetwork.operator/cluster--enable-multi-networkpolicyflagoc get pod -n openshift-ovn-kubernetes -l app=ovnkube-control-plane -o yaml | grep enable-multi-networkpolicy(should return nothing)Set
UseMultiNetworkPolicy=trueviaoc edit network.operator cluster--enable-multi-networkpolicyflag is present after restartoc get pod -n openshift-ovn-kubernetes -l app=ovnkube-control-plane -o yaml | grep enable-multi-networkpolicy(should show flag)Test MNP enforcement on UDN Layer2 and Layer3 networks
Verify DPU-host mode still works correctly
CI Verification
Platforms Tested
Rollback/Upgrade Considerations
Upgrade Path (4.x → 5.0 with this fix)
UseMultiNetworkPolicy=truebefore upgrade, pods restart with--enable-multi-networkpolicyflagOVN_MULTI_NETWORK_POLICY_ENABLEunchanged, works in both modesRollback Path (5.0 with this fix → 4.x)
In-place Update (5.0 without fix → 5.0 with fix)
oc get pods -n openshift-ovn-kubernetes -wRisks & Mitigations
Risk: Pod restart during upgrade
Risk: DPU-host compatibility
Risk: ConfigMap hash mechanism side effects
Testing
Jira
https://redhat.atlassian.net/browse/OCPBUGS-88063
Related