Skip to content
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

Jump to bottom

Make egress IP and ICNI mutually exclusive when bootstrapping OVN-kube #1145

Merged
merged 1 commit into from Jul 17, 2021
Merged

Make egress IP and ICNI mutually exclusive when bootstrapping OVN-kube #1145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion bindata/network/ovn-kubernetes/004-config.yaml
View file
Open in desktop
Expand Up @@ -22,11 +22,13 @@ data:
{{- end }}

[ovnkubernetesfeature]
{{- if .OVN_ENABLE_EGRESS_IP}}
enable-egress-ip=true
{{- end}}
enable-egress-firewall=true

[gateway]
mode=local
mode={{.OVN_GATEWAY_MODE}}
nodeport=true
{{ if .OVNHybridOverlayEnable }}
[hybridoverlay]
Expand Down
3 changes: 3 additions & 0 deletions bindata/network/ovn-kubernetes/ovnkube-master.yaml
View file
Open in desktop
Expand Up @@ -837,6 +837,9 @@ spec:
--nbctl-daemon-mode \
--nb-cert-common-name "{{.OVN_CERT_CN}}" \
--enable-multicast \
{{- if .OVN_DISABLE_SNAT_MULTIPLE_GWS }}
--disable-snat-multiple-gws \
{{- end }}
--acl-logging-rate-limit "{{.OVNPolicyAuditRateLimit}}"
lifecycle:
preStop:
Expand Down
3 changes: 3 additions & 0 deletions bindata/network/ovn-kubernetes/ovnkube-node.yaml
View file
Open in desktop
Expand Up @@ -269,6 +269,9 @@ spec:
${gateway_mode_flags} \
--metrics-bind-address "127.0.0.1:29103" \
--metrics-enable-pprof \
{{- if .OVN_DISABLE_SNAT_MULTIPLE_GWS }}
--disable-snat-multiple-gws \
{{- end }}
${export_network_flows_flags}
env:
# for kubectl
Expand Down
8 changes: 7 additions & 1 deletion pkg/bootstrap/types.go
View file
Open in desktop
Expand Up @@ -27,13 +27,19 @@ type KuryrBootstrapResult struct {
UserCACert string
}

type OVNConfigBoostrapResult struct {
GatewayMode string
EnableEgressIP bool
DisableSNATMutlipleGWs bool
}

type OVNBootstrapResult struct {
MasterIPs []string
ClusterInitiator string
ExistingMasterDaemonset *appsv1.DaemonSet
ExistingNodeDaemonset *appsv1.DaemonSet
GatewayMode string
Platform configv1.PlatformType
OVNKubernetesConfig *OVNConfigBoostrapResult
}

type BootstrapResult struct {
Expand Down
48 changes: 32 additions & 16 deletions pkg/network/ovn_kubernetes.go
View file
Open in desktop
Expand Up @@ -40,6 +40,8 @@ const CLUSTER_CONFIG_NAMESPACE = "kube-system"
const OVN_CERT_CN = "ovn"
const OVN_MASTER_DISCOVERY_POLL = 5
const OVN_MASTER_DISCOVERY_BACKOFF = 120
const OVN_LOCAL_GW_MODE = "local"
const OVN_SHARED_GW_MODE = "shared"

var OVN_MASTER_DISCOVERY_TIMEOUT = 250

Expand Down Expand Up @@ -67,7 +69,9 @@ func renderOVNKubernetes(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.Bo
data.Data["GenevePort"] = c.GenevePort
data.Data["CNIConfDir"] = pluginCNIConfDir(conf)
data.Data["CNIBinDir"] = CNIBinDir
data.Data["OVN_GATEWAY_MODE"] = bootstrapResult.OVN.GatewayMode
data.Data["OVN_GATEWAY_MODE"] = bootstrapResult.OVN.OVNKubernetesConfig.GatewayMode
data.Data["OVN_ENABLE_EGRESS_IP"] = bootstrapResult.OVN.OVNKubernetesConfig.EnableEgressIP
data.Data["OVN_DISABLE_SNAT_MULTIPLE_GWS"] = bootstrapResult.OVN.OVNKubernetesConfig.DisableSNATMutlipleGWs
data.Data["OVN_NB_PORT"] = OVN_NB_PORT
data.Data["OVN_SB_PORT"] = OVN_SB_PORT
data.Data["OVN_NB_RAFT_PORT"] = OVN_NB_RAFT_PORT
Expand Down Expand Up @@ -194,28 +198,40 @@ func renderOVNKubernetes(conf *operv1.NetworkSpec, bootstrapResult *bootstrap.Bo
return objs, nil
}

// returns the value of mode found in the openshift-ovn-kubernetes/gateway-mode-config configMap
// if it exists, otherwise returns whatever the global OVN_GATEWAY_MODE is set to (shared)
func GetGatewayMode(kubeClient client.Client) (string, error) {
defaultGatewayMode := "shared"
// bootstrapOVNConfig returns the value of mode found in the openshift-ovn-kubernetes/gateway-mode-config configMap
// if it exists, otherwise returns default configuration for OCP clusters using OVN-Kubernetes
func bootstrapOVNConfig(kubeClient client.Client) (*bootstrap.OVNConfigBoostrapResult, error) {
ovnConfigResult := &bootstrap.OVNConfigBoostrapResult{
GatewayMode: OVN_SHARED_GW_MODE,
EnableEgressIP: true,
DisableSNATMutlipleGWs: false,
}

cm := &corev1.ConfigMap{}
nsn := types.NamespacedName{Namespace: "openshift-network-operator", Name: "gateway-mode-config"}
err := kubeClient.Get(context.TODO(), nsn, cm)

if err != nil {
if apierrors.IsNotFound(err) {
klog.Warningf("Did not find gateway-mode-config. Using default mode: \"%s\"", defaultGatewayMode)
return defaultGatewayMode, nil
klog.Infof("Did not find gateway-mode-config. Using default OVN configuration: %+v", ovnConfigResult)
return ovnConfigResult, nil
} else {
return "", fmt.Errorf("Could not determine gateway mode: %w", err)
return nil, fmt.Errorf("Could not determine gateway mode: %w", err)
}
}
if cm.Data["mode"] != "shared" && cm.Data["mode"] != "local" {
klog.Warningf("Ignoring gateway-mode-config %s. Does not match \"shared\" or \"local\"", cm.Data["mode"])
return defaultGatewayMode, nil
modeOverride := cm.Data["mode"]
_, disableSNATMultipleGWsOverride := cm.Data["disable-snat-multiple-gws"]
if modeOverride != OVN_SHARED_GW_MODE && modeOverride != OVN_LOCAL_GW_MODE {
klog.Warningf("gateway-mode-config does not match %q or %q, is: %q. Using default OVN configuration: %+v", OVN_LOCAL_GW_MODE, OVN_SHARED_GW_MODE, modeOverride, ovnConfigResult)
return ovnConfigResult, nil
}
ovnConfigResult.GatewayMode = modeOverride
if disableSNATMultipleGWsOverride {
Copy link
Contributor

Choose a reason for hiding this comment

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

disableSNATMultipleGWs with local gateway mode doesn't do anything, it is assumed to always be enabled for that mode. That being said I dont think it hurts to pass it as enabled to ovnkube with local gw mode + I dont see why anyone would set this with local gateway mode so safe to ignore checking gateway mode here.

ovnConfigResult.EnableEgressIP = false
ovnConfigResult.DisableSNATMutlipleGWs = true
}
klog.Infof("Overriding OVN gateway mode to %s", cm.Data["mode"])
return cm.Data["mode"], nil
klog.Infof("Overriding OVN configuration to %+v", ovnConfigResult)
return ovnConfigResult, nil
}

// validateOVNKubernetes checks that the ovn-kubernetes specific configuration
Expand Down Expand Up @@ -378,9 +394,9 @@ func bootstrapOVN(conf *operv1.Network, kubeClient client.Client) (*bootstrap.Bo
return nil, fmt.Errorf("Unable to bootstrap OVN, unable to unmarshal install-config: %s", err)
}

gatewayMode, err := GetGatewayMode(kubeClient)
ovnConfigResult, err := bootstrapOVNConfig(kubeClient)
if err != nil {
return nil, fmt.Errorf("Unable to bootstrap OVN, undetermined gateway-mode: '%s'", err)
return nil, fmt.Errorf("Unable to bootstrap OVN config, err: %v", err)
}

controlPlaneReplicaCount, _ := strconv.Atoi(rcD.ControlPlane.Replicas)
Expand Down Expand Up @@ -483,7 +499,7 @@ func bootstrapOVN(conf *operv1.Network, kubeClient client.Client) (*bootstrap.Bo
ClusterInitiator: clusterInitiator,
ExistingMasterDaemonset: masterDS,
ExistingNodeDaemonset: nodeDS,
GatewayMode: gatewayMode,
OVNKubernetesConfig: ovnConfigResult,
},
}
return &res, nil
Expand Down
38 changes: 34 additions & 4 deletions pkg/network/ovn_kubernetes_test.go
View file
Open in desktop
Expand Up @@ -61,6 +61,11 @@ func TestRenderOVNKubernetes(t *testing.T) {
bootstrapResult := &bootstrap.BootstrapResult{
OVN: bootstrap.OVNBootstrapResult{
MasterIPs: []string{"1.2.3.4", "5.6.7.8", "9.10.11.12"},
OVNKubernetesConfig: &bootstrap.OVNConfigBoostrapResult{
GatewayMode: "shared",
EnableEgressIP: true,
DisableSNATMutlipleGWs: false,
},
},
}

Expand Down Expand Up @@ -124,6 +129,11 @@ func TestRenderOVNKubernetesIPv6(t *testing.T) {
bootstrapResult := &bootstrap.BootstrapResult{
OVN: bootstrap.OVNBootstrapResult{
MasterIPs: []string{"1.2.3.4", "5.6.7.8", "9.10.11.12"},
OVNKubernetesConfig: &bootstrap.OVNConfigBoostrapResult{
GatewayMode: "shared",
EnableEgressIP: true,
DisableSNATMutlipleGWs: false,
},
},
}
objs, err := renderOVNKubernetes(config, bootstrapResult, manifestDirOvn)
Expand All @@ -137,6 +147,11 @@ func TestRenderOVNKubernetesIPv6(t *testing.T) {
bootstrapResult = &bootstrap.BootstrapResult{
OVN: bootstrap.OVNBootstrapResult{
MasterIPs: []string{"fd01::1", "fd01::2", "fd01::3"},
OVNKubernetesConfig: &bootstrap.OVNConfigBoostrapResult{
GatewayMode: "shared",
EnableEgressIP: true,
DisableSNATMutlipleGWs: false,
},
},
}
objs, err = renderOVNKubernetes(config, bootstrapResult, manifestDirOvn)
Expand Down Expand Up @@ -174,7 +189,7 @@ enable-egress-ip=true
enable-egress-firewall=true

[gateway]
mode=local
mode=shared
nodeport=true`,
},

Expand All @@ -198,7 +213,7 @@ enable-egress-ip=true
enable-egress-firewall=true

[gateway]
mode=local
mode=shared
nodeport=true

[hybridoverlay]
Expand Down Expand Up @@ -230,7 +245,7 @@ enable-egress-ip=true
enable-egress-firewall=true

[gateway]
mode=local
mode=shared
nodeport=true

[hybridoverlay]
Expand Down Expand Up @@ -265,7 +280,7 @@ enable-egress-ip=true
enable-egress-firewall=true

[gateway]
mode=local
mode=shared
nodeport=true

[hybridoverlay]
Expand Down Expand Up @@ -298,6 +313,11 @@ enabled=true`,
bootstrapResult := &bootstrap.BootstrapResult{
OVN: bootstrap.OVNBootstrapResult{
MasterIPs: []string{"1.2.3.4", "5.6.7.8", "9.10.11.12"},
OVNKubernetesConfig: &bootstrap.OVNConfigBoostrapResult{
GatewayMode: "shared",
EnableEgressIP: true,
DisableSNATMutlipleGWs: false,
},
},
}
objs, err := renderOVNKubernetes(config, bootstrapResult, manifestDirOvn)
Expand Down Expand Up @@ -1048,6 +1068,11 @@ metadata:
MasterIPs: []string{"1.2.3.4", "5.6.7.8", "9.10.11.12"},
ExistingMasterDaemonset: master,
ExistingNodeDaemonset: node,
OVNKubernetesConfig: &bootstrap.OVNConfigBoostrapResult{
GatewayMode: "shared",
EnableEgressIP: true,
DisableSNATMutlipleGWs: false,
},
},
}

Expand Down Expand Up @@ -1336,6 +1361,11 @@ func TestRenderOVNKubernetesDualStackPrecedenceOverUpgrade(t *testing.T) {
},
},
},
OVNKubernetesConfig: &bootstrap.OVNConfigBoostrapResult{
GatewayMode: "shared",
EnableEgressIP: true,
DisableSNATMutlipleGWs: false,
},
},
}
usNode, err := k8s.ToUnstructured(bootstrapResult.OVN.ExistingNodeDaemonset)
Expand Down