Modified vSphere config provider to not lose AddressesFromPools when …

…applying Failure Domains.

go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/onsi/gomega v1.30.0
 	github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e
 	github.com/openshift/client-go v0.0.0-20240125160436-aa5df63097c4
-	github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20231130130825-ea989e248004
+	github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20240201120052-f5a6a5f74055
 	github.com/openshift/library-go v0.0.0-20240115112243-470c096a1ca9
 	github.com/spf13/pflag v1.0.5
 	go.uber.org/zap v1.26.0

go.sum

@@ -460,8 +460,8 @@ github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e h1:cxgCNo/R769CO23AK
 github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
 github.com/openshift/client-go v0.0.0-20240125160436-aa5df63097c4 h1:Ct+/c9d5rjZudN+VBLxRJIQfPy1gJZier1P1KdpvCaM=
 github.com/openshift/client-go v0.0.0-20240125160436-aa5df63097c4/go.mod h1:ZS5cpA+0zI/ziDQxAKKvGkRKn9BJppqDYdAph+OUTlo=
-github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20231130130825-ea989e248004 h1:z5MmF35DnAa+HOX6AHMrGhkm7YXaKZAxrNL3JsYHJ1I=
-github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20231130130825-ea989e248004/go.mod h1:8U97lU5NyyrNKkVcG3zRXFbI33Z5D+GZYp6k4oHoT9k=
+github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20240201120052-f5a6a5f74055 h1:tDyJ73kGqi2RbS5bgpDXfKxckR2JbVOX4sMR4yf46mE=
+github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20240201120052-f5a6a5f74055/go.mod h1:osVq9/R6qKHBQxDP4cYTvkgXVBKOMs1SOfPLFfn0m7A=
 github.com/openshift/library-go v0.0.0-20240115112243-470c096a1ca9 h1:dQmXUWo3Ko+8K/fxKPF01NuTrD92d/ABBQ4NfxkwbAY=
 github.com/openshift/library-go v0.0.0-20240115112243-470c096a1ca9/go.mod h1:82B0gt8XawdXWRtKMrm3jSMTeRsiOSYKCi4F0fvPjG0=
 github.com/otiai10/copy v1.2.0 h1:HvG945u96iNadPoG2/Ja2+AUJeW5YuFQMixq9yirC+k=

pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/vsphere.go

@@ -17,6 +17,7 @@ limitations under the License.
 package providerconfig
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -33,6 +34,7 @@ import (
 type VSphereProviderConfig struct {
 	providerConfig machinev1beta1.VSphereMachineProviderSpec
 	infrastructure *configv1.Infrastructure
+	logger         logr.Logger
 }
 
 func (v VSphereProviderConfig) getFailureDomainFromInfrastructure(fd machinev1.VSphereFailureDomain) (*configv1.VSpherePlatformFailureDomainSpec, error) {
@@ -90,16 +92,29 @@ func (v VSphereProviderConfig) InjectFailureDomain(fd machinev1.VSphereFailureDo
 	newVSphereProviderConfig.providerConfig.Workspace = newVSphereProviderConfig.getWorkspaceFromFailureDomain(failureDomain)
 	topology := failureDomain.Topology
 
+	logNetworkInfo(newVSphereProviderConfig.providerConfig.Network, "control plane machine set network before failure domain: %v", v.logger)
+
 	if len(topology.Networks) > 0 {
-		newVSphereProviderConfig.providerConfig.Network = machinev1beta1.NetworkSpec{
-			Devices: []machinev1beta1.NetworkDeviceSpec{
-				{
-					NetworkName: topology.Networks[0],
-				},
-			},
+		networkSpec := machinev1beta1.NetworkSpec{}
+		// If original has AddressesFromPools, that means static IP is desired for the CPMS.  Keep that and just add the FD info.
+		if len(newVSphereProviderConfig.providerConfig.Network.Devices[0].AddressesFromPools) > 0 {
+			networkSpec.Devices = newVSphereProviderConfig.providerConfig.Network.Devices
+		}
+
+		// Set the network name for the device from FD.
+		for index, network := range topology.Networks {
+			if len(networkSpec.Devices) <= index {
+				networkSpec.Devices = append(networkSpec.Devices, machinev1beta1.NetworkDeviceSpec{})
+			}
+
+			networkSpec.Devices[index].NetworkName = network
 		}
+
+		newVSphereProviderConfig.providerConfig.Network = networkSpec
 	}
 
+	logNetworkInfo(newVSphereProviderConfig.providerConfig.Network, "control plane machine set network after failure domain: %v", v.logger)
+
 	if len(topology.Template) > 0 {
 		newVSphereProviderConfig.providerConfig.Template = topology.Template[strings.LastIndex(topology.Template, "/")+1:]
 	} else if len(v.infrastructure.Spec.PlatformSpec.VSphere.FailureDomains) > 0 {
@@ -181,6 +196,7 @@ func newVSphereProviderConfig(logger logr.Logger, raw *runtime.RawExtension, inf
 	VSphereProviderConfig := VSphereProviderConfig{
 		providerConfig: vsphereMachineProviderSpec,
 		infrastructure: infrastructure,
+		logger:         logger,
 	}
 
 	// For networking, we only need to compare the network name.  For static IPs, we can ignore all ip configuration;
@@ -204,3 +220,16 @@ func newVSphereProviderConfig(logger logr.Logger, raw *runtime.RawExtension, inf
 
 	return config, nil
 }
+
+// logNetworkInfo log network info as json for better debugging of data being processed.
+func logNetworkInfo(network machinev1beta1.NetworkSpec, msg string, logger logr.Logger) {
+	// To limit marshalling to only when log level > 4.
+	if logger.GetV() >= 4 {
+		jsonOutput, err := json.Marshal(network)
+		if err != nil {
+			logger.Error(err, "Got error Marshalling NetworkSpec")
+		}
+
+		logger.V(4).Info(msg, string(jsonOutput))
+	}
+}

pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/vsphere_test.go

@@ -94,6 +94,31 @@ var _ = Describe("VSphere Provider Config", Label("vSphereProviderConfig"), func
 		})
 	})
 
+	Context("StaticIP", func() {
+
+		BeforeEach(func() {
+			machineProviderConfig := machinev1beta1resourcebuilder.VSphereProviderSpec().
+				WithZone(usCentral1a).WithIPPool().
+				Build()
+
+			providerConfig = VSphereProviderConfig{
+				providerConfig: *machineProviderConfig,
+				infrastructure: configv1resourcebuilder.Infrastructure().AsVSphereWithFailureDomains("vsphere-test", nil).Build(),
+			}
+		})
+
+		It("contains an AddressesFromPools block", func() {
+			Expect(providerConfig.providerConfig.Network.Devices[0].AddressesFromPools).To(Not(BeEmpty()))
+		})
+
+		It("returns networking with AddressesFromPools and the configured network name from failure domain", func() {
+			expected, err := providerConfig.InjectFailureDomain(providerConfig.ExtractFailureDomain())
+			Expect(err).To(BeNil())
+			Expect(expected.providerConfig.Network.Devices[0].AddressesFromPools).To(Not(BeNil()))
+			Expect(expected.providerConfig.Network.Devices[0].NetworkName).To(Equal(providerConfig.infrastructure.Spec.PlatformSpec.VSphere.FailureDomains[0].Topology.Networks[0]))
+		})
+	})
+
 	Context("newVSphereProviderConfig", func() {
 		var providerConfig ProviderConfig
 		var expectedVSphereConfig machinev1beta1.VSphereMachineProviderSpec

vendor/modules.txt

@@ -639,8 +639,8 @@ github.com/openshift/client-go/config/listers/config/v1alpha1
 github.com/openshift/client-go/machine/applyconfigurations/internal
 github.com/openshift/client-go/machine/applyconfigurations/machine/v1
 github.com/openshift/client-go/machine/applyconfigurations/machine/v1beta1
-# github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20231130130825-ea989e248004
-## explicit; go 1.20
+# github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20240201120052-f5a6a5f74055
+## explicit; go 1.21
 github.com/openshift/cluster-api-actuator-pkg/testutils
 github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder
 github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/config/v1