Skip to content

[DO NOT MERGE] Test: Revert "Remove dead OVN_MULTI_NETWORK_ENABLE template variable" #3032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
weliang1 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[DO NOT MERGE] Test: Revert "Remove dead OVN_MULTI_NETWORK_ENABLE template variable" #3032

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce08349
Revert "Remove dead OVN_MULTI_NETWORK_ENABLE template variable"
weliang1 Jun 18, 2026
ce8cdfd
Revert "NVIDIA-554: DPU-host mode: use ConfigMap for OVN feature enab…
weliang1 Jun 18, 2026
a086a1e
Revert "OCPBUGS-78731: Move enable-multicast from config maps to CLI …
weliang1 Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions bindata/network/ovn-kubernetes/common/008-script-lib.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,17 @@ data:
node_mgmt_port_netdev_flags="$node_mgmt_port_netdev_flags --ovnkube-node-mgmt-port-dp-resource-name ${OVNKUBE_NODE_MGMT_PORT_DP_RESOURCE_NAME}"
fi

multi_network_enabled_flag=
if [[ "{{.OVN_MULTI_NETWORK_ENABLE}}" == "true" && "${OVN_NODE_MODE}" != "dpu-host" ]]; then
multi_network_enabled_flag="--enable-multi-network"
fi

network_segmentation_enabled_flag=
if [[ "${OVN_NODE_MODE}" != "dpu-host" ]]; then
multi_network_enabled_flag="--enable-multi-network"
network_segmentation_enabled_flag="--enable-network-segmentation"
fi
Comment thread
coderabbitai[bot] marked this conversation as resolved.

route_advertisements_enable_flag=
if [[ "{{.OVN_ROUTE_ADVERTISEMENTS_ENABLE}}" == "true" ]]; then
route_advertisements_enable_flag="--enable-route-advertisements"
Expand Down
7 changes: 7 additions & 0 deletions bindata/network/ovn-kubernetes/managed/004-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ data:
{{- if .ReachabilityNodePort }}
egressip-node-healthcheck-port={{.ReachabilityNodePort}}
{{- end }}
{{- if not .OVN_MULTI_NETWORK_ENABLE }}
enable-multi-network=true
{{- end }}
Comment on lines +39 to +41

@coderabbitai coderabbitai Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

enable-multi-network rendering currently defeats the disable override

These template branches render enable-multi-network=true when .OVN_MULTI_NETWORK_ENABLE is false (and in hostedcluster, effectively in both branches), so DisableMultiNetwork=true does not actually disable the feature.

Suggested fix 
-    {{- if not .OVN_MULTI_NETWORK_ENABLE }}
-    enable-multi-network=true
-    {{- end }}
+    enable-multi-network={{.OVN_MULTI_NETWORK_ENABLE}}
...
-{{- if .OVN_MULTI_NETWORK_ENABLE }}
-    enable-multi-network=true
-{{- end }}
-    {{- if not .OVN_MULTI_NETWORK_ENABLE }}
-    enable-multi-network=true
-    {{- end }}
+    enable-multi-network={{.OVN_MULTI_NETWORK_ENABLE}}

Also applies to: 133-138

🤖 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 `@bindata/network/ovn-kubernetes/managed/004-config.yaml` around lines 43 - 45,
The conditional logic for enable-multi-network is inverted, causing the feature
to be enabled when it should be disabled. In the template section at lines 43-45
and the similar section at lines 133-138, remove the `not` operator from the
condition `{{- if not .OVN_MULTI_NETWORK_ENABLE }}` so it becomes `{{- if
.OVN_MULTI_NETWORK_ENABLE }}`. This ensures that `enable-multi-network=true` is
only rendered when the feature is actually intended to be enabled, allowing
DisableMultiNetwork to properly override the setting.

enable-network-segmentation=true
enable-preconfigured-udn-addresses=true

Expand Down Expand Up @@ -128,7 +130,12 @@ data:
{{- if .ReachabilityNodePort }}
egressip-node-healthcheck-port={{.ReachabilityNodePort}}
{{- end }}
{{- if .OVN_MULTI_NETWORK_ENABLE }}
enable-multi-network=true
{{- end }}
{{- if not .OVN_MULTI_NETWORK_ENABLE }}
enable-multi-network=true
{{- end }}
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
{{- if .OVN_MULTI_NETWORK_POLICY_ENABLE }}
Expand Down
2 changes: 2 additions & 0 deletions bindata/network/ovn-kubernetes/self-hosted/004-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ data:
{{- if .ReachabilityNodePort }}
egressip-node-healthcheck-port={{.ReachabilityNodePort}}
{{- end }}
{{- if not .OVN_MULTI_NETWORK_ENABLE }}
enable-multi-network=true
{{- end }}
Comment on lines +46 to +48

@coderabbitai coderabbitai Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Conditional is inverted for multi-network enablement

enable-multi-network=true is emitted only when .OVN_MULTI_NETWORK_ENABLE is false, which inverts the intended behavior and breaks DisableMultiNetwork.

Suggested fix 
-    {{- if not .OVN_MULTI_NETWORK_ENABLE }}
-    enable-multi-network=true
-    {{- end }}
+    enable-multi-network={{.OVN_MULTI_NETWORK_ENABLE}}
📝 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.

Suggested change
{{- if not .OVN_MULTI_NETWORK_ENABLE }}
enable-multi-network=true
{{- end }}
enable-multi-network={{.OVN_MULTI_NETWORK_ENABLE}}
🤖 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 `@bindata/network/ovn-kubernetes/self-hosted/004-config.yaml` around lines 49 -
51, The conditional logic is inverted in the template around the
enable-multi-network configuration. The current condition `{{- if not
.OVN_MULTI_NETWORK_ENABLE }}` causes enable-multi-network=true to be set only
when the feature flag is false, which breaks the intended behavior. Remove the
`not` operator from the if statement so that enable-multi-network=true is
applied when .OVN_MULTI_NETWORK_ENABLE is true.

enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
{{- if .OVN_MULTI_NETWORK_POLICY_ENABLE }}
Expand Down
7 changes: 6 additions & 1 deletion pkg/network/ovn_kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,13 @@ func renderOVNKubernetes(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.Bo
data.Data["NorthdThreads"] = 1
data.Data["IsSNO"] = bootstrapResult.OVN.ControlPlaneReplicaCount == 1

data.Data["OVN_MULTI_NETWORK_ENABLE"] = true
data.Data["OVN_MULTI_NETWORK_POLICY_ENABLE"] = false
if conf.UseMultiNetworkPolicy != nil && *conf.UseMultiNetworkPolicy {
if conf.DisableMultiNetwork != nil && *conf.DisableMultiNetwork {
data.Data["OVN_MULTI_NETWORK_ENABLE"] = false
} else if conf.UseMultiNetworkPolicy != nil && *conf.UseMultiNetworkPolicy {
// Multi-network policy support requires multi-network support to be
// enabled
data.Data["OVN_MULTI_NETWORK_POLICY_ENABLE"] = true
}

Expand Down
60 changes: 42 additions & 18 deletions pkg/network/ovn_kubernetes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -353,7 +352,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -413,7 +411,6 @@ enable-egress-qos=true
enable-egress-service=true
egressip-reachability-total-timeout=3
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -475,7 +472,6 @@ enable-egress-qos=true
enable-egress-service=true
egressip-reachability-total-timeout=0
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -536,7 +532,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -597,7 +592,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -647,7 +641,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -700,7 +693,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand All @@ -718,6 +710,47 @@ logfile-maxage=0`,
controlPlaneReplicaCount: 2,
disableGRO: true,
},
{
desc: "disabled multi-network",
expected: `
[default]
mtu="1500"
cluster-subnets="10.128.0.0/15/23,10.0.0.0/14/24"
encap-port="8061"
enable-lflow-cache=true
lflow-cache-limit-kb=1048576
enable-udp-aggregation=true
udn-allowed-default-services="default/kubernetes,openshift-dns/dns-default"

[kubernetes]
service-cidrs="172.30.0.0/16"
ovn-config-namespace="openshift-ovn-kubernetes"
apiserver="https://testing.test:8443"
host-network-namespace="openshift-host-network"
platform-type="GCP"
healthz-bind-address="0.0.0.0:10256"
dns-service-namespace="openshift-dns"
dns-service-name="dns-default"

[ovnkubernetesfeature]
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true

[gateway]
mode=shared
nodeport=true

[logging]
libovsdblogfile=/var/log/ovnkube/libovsdb.log
logfile-maxsize=100
logfile-maxbackups=5
logfile-maxage=0`,
controlPlaneReplicaCount: 2,

disableMultiNet: true,
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
{
desc: "enable multi-network policies and admin network policies",
expected: `
Expand Down Expand Up @@ -746,7 +779,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-multi-networkpolicy=true
Expand Down Expand Up @@ -795,7 +827,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand All @@ -814,7 +845,7 @@ logfile-maxage=0`,
enabledFeatureGates: []configv1.FeatureGateName{},
},
{
desc: "enable multi-network policies with DisableMultiNetwork",
desc: "enable multi-network policies without multi-network support",
expected: `
[default]
mtu="1500"
Expand All @@ -841,12 +872,8 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-multi-networkpolicy=true
enable-admin-network-policy=true
enable-multi-external-gateway=true

[gateway]
mode=shared
Expand Down Expand Up @@ -890,7 +917,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -937,7 +963,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down Expand Up @@ -983,7 +1008,6 @@ enable-egress-firewall=true
enable-egress-qos=true
enable-egress-service=true
egressip-node-healthcheck-port=9107
enable-multi-network=true
enable-network-segmentation=true
enable-preconfigured-udn-addresses=true
enable-admin-network-policy=true
Expand Down