Skip to content

Commit 140583c

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #279 from mkowalski/OCPBUGS-18641
OCPBUGS-18641: Set dual-stack IPFamilyPriority for vSphere
2 parents 30371bd + 428d639 commit 140583c

File tree

3 files changed

+181
-19
lines changed

3 files changed

+181
-19
lines changed

‎pkg/cloud/vsphere/assets/cloud-controller-manager-deployment.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ spec:
6666
value: {{ .globalCredsSecretNamespace }}
6767
- name: VSPHERE_SECRET_NAME
6868
value: {{ .globalCredsSecretName }}
69+
- name: ENABLE_ALPHA_DUAL_STACK
70+
value: "true"
6971
resources:
7072
requests:
7173
cpu: 200m

‎pkg/cloud/vsphere/vsphere_config_transformer.go‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"strings"
66

77
configv1 "github.com/openshift/api/config/v1"
8+
"k8s.io/utils/net"
89

910
ccmConfig "github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/vsphere/vsphere_cloud_config"
1011
)
@@ -26,7 +27,7 @@ const (
2627
// Currently, CloudConfigTransformer is responsible to populate vcenters, labels, and node networking parameters from
2728
// the Infrastructure resource.
2829
// Also, this function converts legacy deprecated INI configuration format to a YAML-based one.
29-
func CloudConfigTransformer(source string, infra *configv1.Infrastructure, _ *configv1.Network) (string, error) {
30+
func CloudConfigTransformer(source string, infra *configv1.Infrastructure, network *configv1.Network) (string, error) {
3031
if infra.Status.PlatformStatus == nil ||
3132
infra.Status.PlatformStatus.Type != configv1.VSpherePlatformType {
3233
return "", fmt.Errorf("invalid platform, expected to be %s", configv1.VSpherePlatformType)
@@ -42,6 +43,7 @@ func CloudConfigTransformer(source string, infra *configv1.Infrastructure, _ *co
4243
// https://github.com/openshift/enhancements/blob/f6b33eb0cd4ba060af71fee6192297cf6bc31e5a/enhancements/installer/vsphere-ipi-zonal.md
4344
// https://github.com/openshift/api/pull/1278
4445
if infra.Spec.PlatformSpec.VSphere != nil {
46+
setDualStack(cpiCfg, infra.Status.PlatformStatus.VSphere, &infra.Spec.PlatformSpec.VSphere.NodeNetworking, network)
4547
setNodes(cpiCfg, &infra.Spec.PlatformSpec.VSphere.NodeNetworking)
4648
setVirtualCenters(cpiCfg, infra.Spec.PlatformSpec.VSphere)
4749

@@ -99,3 +101,47 @@ func setVirtualCenters(cfg *ccmConfig.CPIConfig, vSphereSpec *configv1.VSpherePl
99101
}
100102
}
101103
}
104+
105+
// setDualStack updates the configuration required by the cloud-provider-vsphere to explicitly set
106+
// value of IPFamilyPriority instead of using the default which is IPv4. This is needed by the
107+
// cloud provider in order to properly filter IP addresses that feed the instance metadata.
108+
//
109+
// We rely on the Service Networks configuration that initially comes from o/installer and later
110+
// from the Cluster Network Operator as those two components take care of validating that clusters
111+
// with dual-stack configuration have exactly 2 of them and that they match the required order.
112+
//
113+
// We are mangling with the ExcludeNetworkSubnetCIDR param here because VM agent by default detects
114+
// also IP addresses that are used by us internally and which should never be exposed as node IPs
115+
// (i.e. API VIP and Ingress VIP for IPI installations and fd69::2 which is internal to OVN-K8s).
116+
//
117+
// Ref.: https://issues.redhat.com/browse/OCPBUGS-18641
118+
func setDualStack(cfg *ccmConfig.CPIConfig, status *configv1.VSpherePlatformStatus, nodeNetworking *configv1.VSpherePlatformNodeNetworking, network *configv1.Network) {
119+
if network != nil && len(network.Spec.ServiceNetwork) == 2 {
120+
// Extensive validations are performed by o/installer so that here we already know that
121+
// if the configuration is dual-stack, we will have exactly 2 service networks and if
122+
// single-stack then 1 service network. Simplified logic here is applied to avoid code
123+
// duplication.
124+
//
125+
// Ref.: https://github.com/openshift/installer/blob/6471b31/pkg/types/validation/installconfig.go#L241
126+
if net.IsIPv4CIDRString(network.Spec.ServiceNetwork[0]) {
127+
cfg.Global.IPFamilyPriority = []string{"ipv4", "ipv6"}
128+
} else {
129+
cfg.Global.IPFamilyPriority = []string{"ipv6", "ipv4"}
130+
}
131+
132+
if status != nil {
133+
for _, addr := range append(status.APIServerInternalIPs, status.IngressIPs...) {
134+
if net.IsIPv4String(addr) {
135+
addr = addr + "/32"
136+
} else {
137+
addr = addr + "/128"
138+
}
139+
nodeNetworking.External.ExcludeNetworkSubnetCIDR = append(nodeNetworking.External.ExcludeNetworkSubnetCIDR, addr)
140+
nodeNetworking.Internal.ExcludeNetworkSubnetCIDR = append(nodeNetworking.Internal.ExcludeNetworkSubnetCIDR, addr)
141+
}
142+
}
143+
144+
nodeNetworking.External.ExcludeNetworkSubnetCIDR = append(nodeNetworking.External.ExcludeNetworkSubnetCIDR, "fd69::2/128")
145+
nodeNetworking.Internal.ExcludeNetworkSubnetCIDR = append(nodeNetworking.Internal.ExcludeNetworkSubnetCIDR, "fd69::2/128")
146+
}
147+
}

‎pkg/cloud/vsphere/vsphere_config_transformer_test.go‎

Lines changed: 132 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,20 @@ const (
1818

1919
func newVsphereInfraBuilder() infraBuilder {
2020
return infraBuilder{
21-
platform: configv1.VSpherePlatformType,
2221
platformSpec: configv1.PlatformSpec{
2322
Type: configv1.VSpherePlatformType,
2423
VSphere: &configv1.VSpherePlatformSpec{},
2524
},
25+
platformStatus: configv1.PlatformStatus{
26+
Type: configv1.VSpherePlatformType,
27+
VSphere: &configv1.VSpherePlatformStatus{},
28+
},
2629
}
2730
}
2831

2932
type infraBuilder struct {
30-
platform configv1.PlatformType
31-
platformSpec configv1.PlatformSpec
33+
platformSpec configv1.PlatformSpec
34+
platformStatus configv1.PlatformStatus
3235
}
3336

3437
func (b infraBuilder) Build() *configv1.Infrastructure {
@@ -37,9 +40,7 @@ func (b infraBuilder) Build() *configv1.Infrastructure {
3740
Name: "cluster",
3841
},
3942
Status: configv1.InfrastructureStatus{
40-
PlatformStatus: &configv1.PlatformStatus{
41-
Type: b.platform,
42-
},
43+
PlatformStatus: &b.platformStatus,
4344
},
4445
Spec: configv1.InfrastructureSpec{
4546
CloudConfig: configv1.ConfigMapFileReference{
@@ -52,7 +53,7 @@ func (b infraBuilder) Build() *configv1.Infrastructure {
5253
}
5354

5455
func (b infraBuilder) withPlatform(platform configv1.PlatformType) infraBuilder {
55-
b.platform = platform
56+
b.platformStatus.Type = platform
5657
return b
5758
}
5859

@@ -126,10 +127,54 @@ func (b infraBuilder) withVSphereZones() infraBuilder {
126127
return b
127128
}
128129

130+
func (b infraBuilder) withPrimaryIPv4VIP() infraBuilder {
131+
b.platformStatus.VSphere.APIServerInternalIPs = []string{"192.168.96.3", "fd65:a1a8:60ad:271c::200"}
132+
b.platformStatus.VSphere.IngressIPs = []string{"192.168.96.4", "fd65:a1a8:60ad:271c::201"}
133+
return b
134+
}
135+
136+
func (b infraBuilder) withPrimaryIPv6VIP() infraBuilder {
137+
b.platformStatus.VSphere.APIServerInternalIPs = []string{"fd65:a1a8:60ad:271c::200", "192.168.96.3"}
138+
b.platformStatus.VSphere.IngressIPs = []string{"fd65:a1a8:60ad:271c::201", "192.168.96.4"}
139+
return b
140+
}
141+
129142
func makeDummyNetworkConfig() *configv1.Network {
130143
return &configv1.Network{}
131144
}
132145

146+
func withDualStackPrimaryIPv4NetworkConfig() *configv1.Network {
147+
return &configv1.Network{
148+
Spec: configv1.NetworkSpec{
149+
ClusterNetwork: []configv1.ClusterNetworkEntry{
150+
{CIDR: "10.128.0.0/14", HostPrefix: 14},
151+
{CIDR: "fd65:10:128::/56", HostPrefix: 64},
152+
},
153+
NetworkType: "OVNKubernetes",
154+
ServiceNetwork: []string{
155+
"172.30.0.0/16",
156+
"fd65:172:16::/112",
157+
},
158+
},
159+
}
160+
}
161+
162+
func withDualStackPrimaryIPv6NetworkConfig() *configv1.Network {
163+
return &configv1.Network{
164+
Spec: configv1.NetworkSpec{
165+
ClusterNetwork: []configv1.ClusterNetworkEntry{
166+
{CIDR: "fd65:10:128::/56", HostPrefix: 64},
167+
{CIDR: "10.128.0.0/14", HostPrefix: 14},
168+
},
169+
NetworkType: "OVNKubernetes",
170+
ServiceNetwork: []string{
171+
"fd65:172:16::/112",
172+
"172.30.0.0/16",
173+
},
174+
},
175+
}
176+
}
177+
133178
const iniConfigWithWorkspace = `
134179
[Global]
135180
secret-name = "vsphere-creds"
@@ -213,6 +258,48 @@ nodes:
213258
excludeInternalNetworkSubnetCidr: 192.0.2.0/24,fe80::1/128
214259
excludeExternalNetworkSubnetCidr: 192.1.2.0/24,fe80::2/128`
215260

261+
const yamlConfigNodeNetworkingDualStackPrimaryIPv4 = `
262+
global:
263+
insecureFlag: true
264+
secretName: vsphere-creds
265+
secretNamespace: kube-system
266+
vcenter:
267+
test-server:
268+
server: test-server
269+
datacenters:
270+
- DC1
271+
ipFamily:
272+
- ipv4
273+
- ipv6
274+
nodes:
275+
internalNetworkSubnetCidr: 192.0.3.0/24,fe80::4/128
276+
externalNetworkSubnetCidr: 198.51.100.0/24,fe80::3/128
277+
internalVmNetworkName: internal-network
278+
externalVmNetworkName: external-network
279+
excludeInternalNetworkSubnetCidr: 192.0.2.0/24,fe80::1/128,192.168.96.3/32,fd65:a1a8:60ad:271c::200/128,192.168.96.4/32,fd65:a1a8:60ad:271c::201/128,fd69::2/128
280+
excludeExternalNetworkSubnetCidr: 192.1.2.0/24,fe80::2/128,192.168.96.3/32,fd65:a1a8:60ad:271c::200/128,192.168.96.4/32,fd65:a1a8:60ad:271c::201/128,fd69::2/128`
281+
282+
const yamlConfigNodeNetworkingDualStackPrimaryIPv6 = `
283+
global:
284+
insecureFlag: true
285+
secretName: vsphere-creds
286+
secretNamespace: kube-system
287+
vcenter:
288+
test-server:
289+
server: test-server
290+
datacenters:
291+
- DC1
292+
ipFamily:
293+
- ipv6
294+
- ipv4
295+
nodes:
296+
internalNetworkSubnetCidr: 192.0.3.0/24,fe80::4/128
297+
externalNetworkSubnetCidr: 198.51.100.0/24,fe80::3/128
298+
internalVmNetworkName: internal-network
299+
externalVmNetworkName: external-network
300+
excludeInternalNetworkSubnetCidr: 192.0.2.0/24,fe80::1/128,fd65:a1a8:60ad:271c::200/128,192.168.96.3/32,fd65:a1a8:60ad:271c::201/128,192.168.96.4/32,fd69::2/128
301+
excludeExternalNetworkSubnetCidr: 192.1.2.0/24,fe80::2/128,fd65:a1a8:60ad:271c::200/128,192.168.96.3/32,fd65:a1a8:60ad:271c::201/128,192.168.96.4/32,fd69::2/128`
302+
216303
const iniConfigZonal = `
217304
[Global]
218305
secret-name = "vsphere-creds"
@@ -247,87 +334,114 @@ func TestCloudConfigTransformer(t *testing.T) {
247334
testcases := []struct {
248335
name string
249336
infraBuilder infraBuilder
337+
networkBuilder *configv1.Network
250338
inputConfig string
251339
equivalentConfig string
252340
errMsg string
253341
}{
254342
{
255343
name: "in-tree to external with empty infra",
256344
infraBuilder: newVsphereInfraBuilder(),
345+
networkBuilder: makeDummyNetworkConfig(),
257346
inputConfig: iniConfigWithWorkspace,
258347
equivalentConfig: iniConfigWithoutWorkspace,
259348
},
260349
{
261350
name: "in-tree to external with node networking",
262351
infraBuilder: newVsphereInfraBuilder().withVSphereNodeNetworking(),
352+
networkBuilder: makeDummyNetworkConfig(),
263353
inputConfig: iniConfigWithWorkspace,
264354
equivalentConfig: iniConfigNodeNetworking,
265355
},
266356
{
267357
name: "populating labels datacenters from zones config",
268358
infraBuilder: newVsphereInfraBuilder().withVSphereZones(),
359+
networkBuilder: makeDummyNetworkConfig(),
269360
inputConfig: iniConfigWithWorkspace,
270361
equivalentConfig: iniConfigZonal,
271362
},
272363
{
273364
name: "replacing existing labels with openshift specific",
274365
infraBuilder: newVsphereInfraBuilder().withVSphereZones(),
366+
networkBuilder: makeDummyNetworkConfig(),
275367
inputConfig: iniConfigWithExistingLabels,
276368
equivalentConfig: iniConfigZonal,
277369
},
278370
{
279371
name: "yaml and ini config parsing results should be the same",
280372
infraBuilder: newVsphereInfraBuilder(),
373+
networkBuilder: makeDummyNetworkConfig(),
281374
inputConfig: yamlConfig,
282375
equivalentConfig: iniConfigWithoutWorkspace,
283376
},
284377
{
285378
name: "yaml and ini config parsing results should be the same, with zones",
286379
infraBuilder: newVsphereInfraBuilder().withVSphereZones(),
380+
networkBuilder: makeDummyNetworkConfig(),
287381
inputConfig: yamlConfigZonal,
288382
equivalentConfig: iniConfigZonal,
289383
},
290384
{
291385
name: "yaml and ini config parsing results should be the same, node networking",
292386
infraBuilder: newVsphereInfraBuilder().withVSphereNodeNetworking(),
387+
networkBuilder: makeDummyNetworkConfig(),
293388
inputConfig: yamlConfigNodeNetworking,
294389
equivalentConfig: iniConfigNodeNetworking,
295390
},
296391
{
297392
name: "yaml config should contain node networking if it's specified in infra",
298393
infraBuilder: newVsphereInfraBuilder().withVSphereNodeNetworking(),
394+
networkBuilder: makeDummyNetworkConfig(),
299395
inputConfig: yamlConfig,
300396
equivalentConfig: yamlConfigNodeNetworking,
301397
},
302398
{
303399
name: "yaml config should be populated with datacenters and labels if failure domains specified",
304400
infraBuilder: newVsphereInfraBuilder().withVSphereZones(),
401+
networkBuilder: makeDummyNetworkConfig(),
305402
inputConfig: yamlConfig,
306403
equivalentConfig: yamlConfigZonal,
307404
},
308405
{
309-
name: "empty input",
310-
infraBuilder: newVsphereInfraBuilder(),
311-
errMsg: "failed to read the cloud.conf: vSphere config is empty",
406+
name: "yaml config should contain ipv4-primary dual-stack config and correct excluded subnets",
407+
infraBuilder: newVsphereInfraBuilder().withVSphereNodeNetworking().withPrimaryIPv4VIP(),
408+
networkBuilder: withDualStackPrimaryIPv4NetworkConfig(),
409+
inputConfig: yamlConfig,
410+
equivalentConfig: yamlConfigNodeNetworkingDualStackPrimaryIPv4,
411+
},
412+
{
413+
name: "yaml config should contain ipv6-primary dual-stack config and correct excluded subnets",
414+
infraBuilder: newVsphereInfraBuilder().withVSphereNodeNetworking().withPrimaryIPv6VIP(),
415+
networkBuilder: withDualStackPrimaryIPv6NetworkConfig(),
416+
inputConfig: yamlConfig,
417+
equivalentConfig: yamlConfigNodeNetworkingDualStackPrimaryIPv6,
418+
},
419+
{
420+
name: "empty input",
421+
infraBuilder: newVsphereInfraBuilder(),
422+
networkBuilder: makeDummyNetworkConfig(),
423+
errMsg: "failed to read the cloud.conf: vSphere config is empty",
312424
},
313425
{
314-
name: "incorrect platform",
315-
infraBuilder: newVsphereInfraBuilder().withPlatform(configv1.NonePlatformType),
316-
errMsg: "invalid platform, expected to be VSphere",
426+
name: "incorrect platform",
427+
infraBuilder: newVsphereInfraBuilder().withPlatform(configv1.NonePlatformType),
428+
networkBuilder: makeDummyNetworkConfig(),
429+
errMsg: "invalid platform, expected to be VSphere",
317430
},
318431
{
319-
name: "invalid ini input",
320-
infraBuilder: newVsphereInfraBuilder(),
321-
inputConfig: ":",
322-
errMsg: "failed to read the cloud.conf",
432+
name: "invalid ini input",
433+
infraBuilder: newVsphereInfraBuilder(),
434+
networkBuilder: makeDummyNetworkConfig(),
435+
inputConfig: ":",
436+
errMsg: "failed to read the cloud.conf",
323437
},
324438
}
325439

326440
for _, tc := range testcases {
327441
t.Run(tc.name, func(t *testing.T) {
328442
g := gmg.NewWithT(t)
329443
infraResouce := tc.infraBuilder.Build()
330-
transformedConfig, err := CloudConfigTransformer(tc.inputConfig, infraResouce, makeDummyNetworkConfig())
444+
transformedConfig, err := CloudConfigTransformer(tc.inputConfig, infraResouce, tc.networkBuilder)
331445
if tc.errMsg != "" {
332446
g.Expect(err).To(gmg.MatchError(gmg.ContainSubstring(tc.errMsg)))
333447
return

0 commit comments

Comments
 (0)