Make NodeAddresses Configuration optional (#44)

pkg/cloudprovider/metal/cloud.go

@@ -80,7 +80,12 @@ func (o *cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder,
 		log.Fatalf("Failed to create new cluster: %v", err)
 	}
 
-	o.instancesV2 = newMetalInstancesV2(o.targetCluster.GetClient(), o.metalCluster.GetClient(), o.metalNamespace, o.cloudConfig.ClusterName)
+	o.instancesV2 = newMetalInstancesV2(
+		o.targetCluster.GetClient(),
+		o.metalCluster.GetClient(),
+		o.metalNamespace,
+		o.cloudConfig,
+	)
 
 	if err := o.metalCluster.GetFieldIndexer().IndexField(ctx, &metalv1alpha1.ServerClaim{}, serverClaimMetadataUIDField, func(object client.Object) []string {
 		serverClaim := object.(*metalv1alpha1.ServerClaim)

pkg/cloudprovider/metal/cloud_test.go

@@ -9,7 +9,13 @@ import (
 )
 
 var _ = Describe("Cloud", func() {
-	_, cp, _ := SetupTest()
+	cloudConfig := CloudConfig{
+		ClusterName: clusterName,
+		Networking: Networking{
+			ConfigureNodeAddresses: true,
+		},
+	}
+	_, cp, _ := SetupTest(cloudConfig)
 
 	It("should ensure the correct cloud provider setup", func() {
 		Expect((*cp).HasClusterID()).To(BeTrue())

pkg/cloudprovider/metal/config.go

@@ -22,8 +22,13 @@ type CloudProviderConfig struct {
 	cloudConfig CloudConfig
 }
 
+type Networking struct {
+	ConfigureNodeAddresses bool `json:"configureNodeAddresses"`
+}
+
 type CloudConfig struct {
-	ClusterName string `json:"clusterName"`
+	ClusterName string     `json:"clusterName"`
+	Networking  Networking `json:"networking"`
 }
 
 var (

pkg/cloudprovider/metal/instances_v2.go

@@ -20,15 +20,15 @@ type metalInstancesV2 struct {
 	targetClient   client.Client
 	metalClient    client.Client
 	metalNamespace string
-	clusterName    string
+	cloudConfig    CloudConfig
 }
 
-func newMetalInstancesV2(targetClient client.Client, metalClient client.Client, namespace, clusterName string) cloudprovider.InstancesV2 {
+func newMetalInstancesV2(targetClient client.Client, metalClient client.Client, namespace string, cloudConfig CloudConfig) cloudprovider.InstancesV2 {
 	return &metalInstancesV2{
 		targetClient:   targetClient,
 		metalClient:    metalClient,
 		metalNamespace: namespace,
-		clusterName:    clusterName,
+		cloudConfig:    cloudConfig,
 	}
 }
 
@@ -98,7 +98,7 @@ func (o *metalInstancesV2) InstanceMetadata(ctx context.Context, node *corev1.No
 	if serverClaim.Labels == nil {
 		serverClaim.Labels = make(map[string]string)
 	}
-	serverClaim.Labels[LabelKeyClusterName] = o.clusterName
+	serverClaim.Labels[LabelKeyClusterName] = o.cloudConfig.ClusterName
 	klog.V(2).InfoS("Adding cluster name label to server claim object", "ServerClaim", client.ObjectKeyFromObject(serverClaim), "Node", node.Name)
 	if err := o.metalClient.Patch(ctx, serverClaim, client.MergeFrom(serverClaimBase)); err != nil {
 		return nil, fmt.Errorf("failed to patch server claim for Node %s: %w", node.Name, err)
@@ -139,13 +139,16 @@ func (o *metalInstancesV2) InstanceMetadata(ctx context.Context, node *corev1.No
 		klog.V(2).InfoS("No region label found for node instance", "Node", node.Name)
 	}
 
-	return &cloudprovider.InstanceMetadata{
-		ProviderID:    providerID,
-		InstanceType:  instanceType,
-		NodeAddresses: addresses,
-		Zone:          zone,
-		Region:        region,
-	}, nil
+	metaData := &cloudprovider.InstanceMetadata{
+		ProviderID:   providerID,
+		InstanceType: instanceType,
+		Zone:         zone,
+		Region:       region,
+	}
+	if o.cloudConfig.Networking.ConfigureNodeAddresses {
+		metaData.NodeAddresses = addresses
+	}
+	return metaData, nil
 }
 
 func (o *metalInstancesV2) getServerClaimForNode(ctx context.Context, node *corev1.Node) (*metalv1alpha1.ServerClaim, error) {

pkg/cloudprovider/metal/instances_v2_test.go

@@ -15,11 +15,19 @@ import (
 	. "sigs.k8s.io/controller-runtime/pkg/envtest/komega"
 )
 
+var (
+	instancesProvider cloudprovider.InstancesV2
+	clusterName       = "test"
+)
+
 var _ = Describe("InstancesV2", func() {
-	var (
-		instancesProvider cloudprovider.InstancesV2
-	)
-	ns, cp, clusterName := SetupTest()
+	cloudConfig := CloudConfig{
+		ClusterName: clusterName,
+		Networking: Networking{
+			ConfigureNodeAddresses: true,
+		},
+	}
+	ns, cp, clusterName := SetupTest(cloudConfig)
 
 	BeforeEach(func() {
 		By("Instantiating the instances v2 provider")
@@ -252,6 +260,103 @@ var _ = Describe("InstancesV2", func() {
 	})
 })
 
+var _ = Describe("InstancesV2 with modified CloudConfig", func() {
+	cloudConfig := CloudConfig{
+		ClusterName: clusterName,
+		Networking: Networking{
+			ConfigureNodeAddresses: false,
+		},
+	}
+	ns, cp, clusterName := SetupTest(cloudConfig)
+
+	BeforeEach(func() {
+		By("Instantiating the instances v2 provider")
+		var ok bool
+		instancesProvider, ok = (*cp).InstancesV2()
+		Expect(ok).To(BeTrue())
+	})
+
+	It("Should not configure node addresses if ConfigureNodeAddresses is false", func(ctx SpecContext) {
+		By("Creating a Server")
+		server := &metalv1alpha1.Server{
+			ObjectMeta: metav1.ObjectMeta{
+				GenerateName: "test-",
+				Labels: map[string]string{
+					"instance-type": "foo",
+					"zone":          "a",
+					"region":        "bar",
+				},
+			},
+			Spec: metalv1alpha1.ServerSpec{
+				UUID:  "12345",
+				Power: "On",
+			},
+		}
+		Expect(k8sClient.Create(ctx, server)).To(Succeed())
+		DeferCleanup(k8sClient.Delete, server)
+
+		By("Patching the Server object to have a valid network interface status")
+		Eventually(UpdateStatus(server, func() {
+			server.Status.PowerState = metalv1alpha1.ServerOnPowerState
+			server.Status.NetworkInterfaces = []metalv1alpha1.NetworkInterface{{
+				Name: "my-nic",
+				IP:   metalv1alpha1.MustParseIP("10.0.0.1"),
+			}}
+		})).Should(Succeed())
+
+		By("Creating a ServerClaim for a Node")
+		serverClaim := &metalv1alpha1.ServerClaim{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace:    ns.Name,
+				GenerateName: "test-",
+			},
+			Spec: metalv1alpha1.ServerClaimSpec{
+				Power:     "On",
+				ServerRef: &corev1.LocalObjectReference{Name: server.Name},
+			},
+		}
+		Expect(k8sClient.Create(ctx, serverClaim)).To(Succeed())
+		DeferCleanup(k8sClient.Delete, serverClaim)
+
+		By("Creating a Node object with a provider ID referencing the machine")
+		node := &corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{
+				GenerateName: "test-",
+			},
+			Spec: corev1.NodeSpec{
+				ProviderID: getProviderID(serverClaim.Namespace, serverClaim.Name),
+			},
+		}
+		Expect(k8sClient.Create(ctx, node)).To(Succeed())
+		DeferCleanup(k8sClient.Delete, node)
+
+		By("Ensuring that an instance for a Node exists")
+		ok, err := instancesProvider.InstanceExists(ctx, node)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(ok).To(BeTrue())
+
+		By("Ensuring that the instance is not shut down")
+		ok, err = instancesProvider.InstanceShutdown(ctx, node)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(ok).To(BeFalse())
+
+		By("Ensuring that the instance meta data has empty addresses")
+		instanceMetadata, err := instancesProvider.InstanceMetadata(ctx, node)
+		Expect(err).NotTo(HaveOccurred())
+		Eventually(instanceMetadata).Should(SatisfyAll(
+			HaveField("ProviderID", getProviderID(serverClaim.Namespace, serverClaim.Name)),
+			HaveField("InstanceType", "foo"),
+			HaveField("NodeAddresses", BeEmpty()),
+			HaveField("Zone", "a"),
+			HaveField("Region", "bar")))
+
+		By("Ensuring cluster name label is added to ServerClaim object")
+		Eventually(Object(serverClaim)).Should(SatisfyAll(
+			HaveField("Labels", map[string]string{LabelKeyClusterName: clusterName}),
+		))
+	})
+})
+
 func getProviderID(namespace, serverClaimName string) string {
 	return fmt.Sprintf("%s://%s/%s", ProviderName, namespace, serverClaimName)
 }

pkg/cloudprovider/metal/suite_test.go

@@ -97,11 +97,10 @@ var _ = BeforeSuite(func() {
 	SetClient(k8sClient)
 })
 
-func SetupTest() (*corev1.Namespace, *cloudprovider.Interface, string) {
+func SetupTest(cloudConfig CloudConfig) (*corev1.Namespace, *cloudprovider.Interface, string) {
 	var (
-		ns          = &corev1.Namespace{}
-		cp          cloudprovider.Interface
-		clusterName = "test"
+		ns = &corev1.Namespace{}
+		cp cloudprovider.Interface
 	)
 
 	BeforeEach(func(ctx SpecContext) {
@@ -145,9 +144,6 @@ func SetupTest() (*corev1.Namespace, *cloudprovider.Interface, string) {
 		defer func() {
 			_ = cloudConfigFile.Close()
 		}()
-		cloudConfig := CloudConfig{
-			ClusterName: clusterName,
-		}
 		cloudConfigData, err := yaml.Marshal(&cloudConfig)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(os.WriteFile(cloudConfigFile.Name(), cloudConfigData, 0666)).To(Succeed())
@@ -164,5 +160,5 @@ func SetupTest() (*corev1.Namespace, *cloudprovider.Interface, string) {
 		cp.Initialize(clientBuilder, cloudProviderCtx.Done())
 	})
 
-	return ns, &cp, clusterName
+	return ns, &cp, cloudConfig.ClusterName
 }