Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions bindata/network/ovn-kubernetes/common/008-script-lib.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,9 @@ data:
# Ensure ovn_advertised_udn_isolation_mode_flag is always defined
ovn_advertised_udn_isolation_mode_flag=

# Ensure openflow_probe_flag is always defined
openflow_probe_flag=

if [[ $# -ne 3 ]]; then
echo "Expected three arguments but got $#"
exit 1
Expand Down Expand Up @@ -669,6 +672,10 @@ data:
ovn_advertised_udn_isolation_mode_flag="--advertised-udn-isolation-mode={{.AdvertisedUDNIsolationMode}}"
fi

if [[ "{{.OpenFlowProbe}}" != "" ]]; then
openflow_probe_flag="--openflow-probe={{.OpenFlowProbe}}"
fi

NETWORK_NODE_IDENTITY_ENABLE=
if [[ "{{.NETWORK_NODE_IDENTITY_ENABLE}}" == "true" ]]; then
NETWORK_NODE_IDENTITY_ENABLE="
Expand Down Expand Up @@ -738,6 +745,7 @@ data:
${gw_interface_flag} \
${ip_forwarding_flag} \
${ovn_advertised_udn_isolation_mode_flag} \
${openflow_probe_flag} \
${NETWORK_NODE_IDENTITY_ENABLE} \
${ovn_v4_join_subnet_opt} \
${ovn_v6_join_subnet_opt} \
Expand Down
11 changes: 11 additions & 0 deletions pkg/network/ovn_kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,17 @@ func renderOVNKubernetes(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.Bo
data.Data["NETWORK_NODE_IDENTITY_ENABLE"] = bootstrapResult.Infra.NetworkNodeIdentityEnabled
data.Data["NodeIdentityCertDuration"] = OVN_NODE_IDENTITY_CERT_DURATION
data.Data["AdvertisedUDNIsolationMode"] = bootstrapResult.OVN.OVNKubernetesConfig.ConfigOverrides["advertised-udn-isolation-mode"]
data.Data["OpenFlowProbe"] = ""
if raw, ok := bootstrapResult.OVN.OVNKubernetesConfig.ConfigOverrides["openflow-probe"]; ok {
probe := strings.TrimSpace(raw)
if probe != "" {
if _, err := strconv.ParseUint(probe, 10, 32); err != nil {
klog.Warningf("Ignoring invalid openflow-probe override %q: expected non-negative integer", raw)
} else {
data.Data["OpenFlowProbe"] = probe
}
}
}

if conf.Migration != nil {
if conf.Migration.MTU != nil {
Expand Down
46 changes: 46 additions & 0 deletions pkg/network/ovn_kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4257,6 +4257,52 @@ func TestRenderOVNKubernetes_AdvertisedUDNIsolationModeOverride(t *testing.T) {
})
}

func TestRenderOVNKubernetes_OpenFlowProbeOverride(t *testing.T) {
g := NewGomegaWithT(t)

crd := OVNKubernetesConfig.DeepCopy()
config := &crd.Spec
fillDefaults(config, nil)

renderWithOverrides := func(overrides map[string]string) string {
bootstrapResult := fakeBootstrapResult()
bootstrapResult.OVN = bootstrap.OVNBootstrapResult{
ControlPlaneReplicaCount: 3,
OVNKubernetesConfig: &bootstrap.OVNConfigBoostrapResult{
DpuHostModeLabel: OVN_NODE_SELECTOR_DEFAULT_DPU_HOST,
DpuModeLabel: OVN_NODE_SELECTOR_DEFAULT_DPU,
SmartNicModeLabel: OVN_NODE_SELECTOR_DEFAULT_SMART_NIC,
MgmtPortResourceName: "",
HyperShiftConfig: &bootstrap.OVNHyperShiftBootstrapResult{
Enabled: false,
},
ConfigOverrides: overrides,
},
}
featureGatesCNO := getDefaultFeatureGates()
fakeClient := cnofake.NewFakeClient()

objs, _, err := renderOVNKubernetes(config, bootstrapResult, manifestDirOvn, fakeClient, featureGatesCNO)
g.Expect(err).NotTo(HaveOccurred())
return extractOVNScriptLib(g, objs)
}

t.Run("with openflow-probe override", func(t *testing.T) {
ovnkubeScriptLib := renderWithOverrides(map[string]string{"openflow-probe": "60"})
g.Expect(ovnkubeScriptLib).To(ContainSubstring(`--openflow-probe=60"`))
})

t.Run("without openflow-probe override", func(t *testing.T) {
ovnkubeScriptLib := renderWithOverrides(nil)
g.Expect(ovnkubeScriptLib).To(ContainSubstring(`--openflow-probe="`))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't this g.Expect(ovnkubeScriptLib).NotTo(ContainSubstring(--openflow-probe=")) ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This will replace the OpenFlowProbevariable in the if block of the go template with empty string. Since the if block condition won't be true, this value will not be used.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok, so this is just validating rendered script and not passing a value into ovnkube script for openflow_probe_flag

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.

This is confusing but appears to be consistent with most of the rest of 008-script-lib.yaml...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When OpenFlowProbe is an empty string and when the go template is replaced its value, then the resultant script will look like this:

      if [[ "" != "" ]]; then
        openflow_probe_flag="--openflow-probe="
      fi

The test checks the string inside the if condition of the script. When the script is actually executed, the assignment inside the if condition will not happen as the if condition will not be true.

})

t.Run("with invalid openflow-probe override", func(t *testing.T) {
ovnkubeScriptLib := renderWithOverrides(map[string]string{"openflow-probe": "-60"})
g.Expect(ovnkubeScriptLib).To(ContainSubstring(`--openflow-probe="`))
})
}

func TestOVNKubernetesControlPlaneFlags(t *testing.T) {
g := NewGomegaWithT(t)

Expand Down