metal-stack · robertvolkmann · Jan 27, 2025 · Jan 27, 2025
diff --git a/api/v1alpha1/metalstackcluster_types.go b/api/v1alpha1/metalstackcluster_types.go
@@ -31,7 +31,6 @@ const (
 
 	ClusterControlPlaneEndpointDefaultPort = 443
 
-	ClusterNodeNetworkEnsured    clusterv1.ConditionType = "ClusterNodeNetworkEnsured"
 	ClusterControlPlaneIPEnsured clusterv1.ConditionType = "ClusterControlPlaneIPEnsured"
 )
 
@@ -52,9 +51,7 @@ type MetalStackClusterSpec struct {
 	ProjectID string `json:"projectID"`
 
 	// NodeNetworkID is the network ID in metal-stack in which the worker nodes and the firewall of the cluster are placed.
-	// If not provided this will automatically be acquired during reconcile.
-	// +optional
-	NodeNetworkID *string `json:"nodeNetworkID,omitempty"`
+	NodeNetworkID string `json:"nodeNetworkID"`
 
 	// ControlPlaneIP is the ip address in metal-stack on which the control plane will be exposed.
 	// If this ip and the control plane endpoint are not provided, an ephemeral ip will automatically be acquired during reconcile.

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_metalstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_metalstackclusters.yaml
@@ -92,9 +92,8 @@ spec:
                   Static ip addresses will not be deleted.
                 type: string
               nodeNetworkID:
-                description: |-
-                  NodeNetworkID is the network ID in metal-stack in which the worker nodes and the firewall of the cluster are placed.
-                  If not provided this will automatically be acquired during reconcile.
+                description: NodeNetworkID is the network ID in metal-stack in which
+                  the worker nodes and the firewall of the cluster are placed.
                 type: string
               partition:
                 description: Partition is the data center partition in which the resources
@@ -105,6 +104,7 @@ spec:
                   in which the associated metal-stack resources are created.
                 type: string
             required:
+            - nodeNetworkID
             - partition
             - projectID
             type: object

diff --git a/internal/controller/metalstackcluster_controller.go b/internal/controller/metalstackcluster_controller.go
@@ -140,16 +140,6 @@ func (r *MetalStackClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func (r *clusterReconciler) reconcile() error {
-	nodeNetworkID, err := r.ensureNodeNetwork()
-	if err != nil {
-		conditions.MarkFalse(r.infraCluster, v1alpha1.ClusterNodeNetworkEnsured, "InternalError", clusterv1.ConditionSeverityError, "%s", err.Error())
-		return fmt.Errorf("unable to ensure node network: %w", err)
-	}
-	conditions.MarkTrue(r.infraCluster, v1alpha1.ClusterNodeNetworkEnsured)
-	r.infraCluster.Spec.NodeNetworkID = &nodeNetworkID
-
-	r.log.Info("reconciled node network", "network-id", nodeNetworkID)
-
 	if r.infraCluster.Spec.ControlPlaneEndpoint.Host == "" {
 		ip, err := r.ensureControlPlaneIP()
 		if err != nil {
@@ -189,51 +179,12 @@ func (r *clusterReconciler) delete() error {
 	}
 	r.infraCluster.Spec.ControlPlaneIP = nil
 
-	err = r.deleteNodeNetwork()
-	if err != nil {
-		return fmt.Errorf("unable to delete node network: %w", err)
-	}
-	r.infraCluster.Spec.NodeNetworkID = nil
-
 	r.log.Info("deletion finished, removing finalizer")
 	controllerutil.RemoveFinalizer(r.infraCluster, v1alpha1.ClusterFinalizer)
 
 	return err
 }
 
-func (r *clusterReconciler) ensureNodeNetwork() (string, error) {
-	if r.infraCluster.Spec.NodeNetworkID != nil {
-		return *r.infraCluster.Spec.NodeNetworkID, nil
-	}
-
-	resp, err := r.metalClient.Network().AllocateNetwork(network.NewAllocateNetworkParams().WithBody(&models.V1NetworkAllocateRequest{
-		Projectid:   r.infraCluster.Spec.ProjectID,
-		Partitionid: r.infraCluster.Spec.Partition,
-		Name:        r.infraCluster.GetName(),
-		Description: fmt.Sprintf("%s/%s", r.infraCluster.GetNamespace(), r.infraCluster.GetName()),
-		Labels:      map[string]string{tag.ClusterID: string(r.infraCluster.GetUID())},
-	}).WithContext(r.ctx), nil)
-	if err != nil {
-		return "", fmt.Errorf("error creating node network: %w", err)
-	}
-
-	return *resp.Payload.ID, nil
-}
-
-func (r *clusterReconciler) deleteNodeNetwork() error {
-	if r.infraCluster.Spec.NodeNetworkID == nil {
-		return nil
-	}
-
-	_, err := r.metalClient.Network().FreeNetwork(network.NewFreeNetworkParams().WithID(*r.infraCluster.Spec.NodeNetworkID).WithContext(r.ctx), nil)
-	if err != nil {
-		return err
-	}
-	r.log.Info("deleted node network")
-
-	return nil
-}
-
 func (r *clusterReconciler) ensureControlPlaneIP() (string, error) {
 	if r.infraCluster.Spec.ControlPlaneIP != nil {
 		return *r.infraCluster.Spec.ControlPlaneIP, nil

diff --git a/internal/controller/metalstackcluster_controller_test.go b/internal/controller/metalstackcluster_controller_test.go
@@ -107,7 +107,7 @@ var _ = Describe("MetalStackCluster Controller", func() {
 			resource.Spec = v1alpha1.MetalStackClusterSpec{
 				ControlPlaneEndpoint: v1alpha1.APIEndpoint{},
 				ProjectID:            "test-project",
-				NodeNetworkID:        nil,
+				NodeNetworkID:        "node-network-id",
 				ControlPlaneIP:       nil,
 				Partition:            "test-partition",
 			}
@@ -156,26 +156,6 @@ var _ = Describe("MetalStackCluster Controller", func() {
 					}, nil)
 				},
 				Network: func(m *mock.Mock) {
-					m.On("AllocateNetwork", testcommon.MatchIgnoreContext(testingT, metalnetwork.NewAllocateNetworkParams().WithBody(&models.V1NetworkAllocateRequest{
-						Name:        resource.Name,
-						Description: resource.Namespace + "/" + resource.Name,
-						Labels: map[string]string{
-							"cluster.metal-stack.io/id": string(resource.UID),
-						},
-						Partitionid: "test-partition",
-						Projectid:   "test-project",
-					})), nil).Return(&metalnetwork.AllocateNetworkCreated{
-						Payload: &models.V1NetworkResponse{
-							Labels: map[string]string{
-								"cluster.metal-stack.io/id": string(resource.UID),
-							},
-							Partitionid: "test-partition",
-							Projectid:   "test-project",
-							ID:          ptr.To("test-network"),
-							Prefixes:    []string{"192.168.42.0/24"},
-						},
-					}, nil)
-
 					m.On("FindNetworks", testcommon.MatchIgnoreContext(testingT, metalnetwork.NewFindNetworksParams().WithBody(&models.V1NetworkFindRequest{
 						Labels: map[string]string{
 							"network.metal-stack.io/default": "",
@@ -215,10 +195,6 @@ var _ = Describe("MetalStackCluster Controller", func() {
 
 			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed())
 
-			Expect(resource.Status.Conditions).To(ContainElement(MatchFields(IgnoreExtras, Fields{
-				"Type":   Equal(v1alpha1.ClusterNodeNetworkEnsured),
-				"Status": Equal(corev1.ConditionTrue),
-			})))
 			Expect(resource.Status.Conditions).To(ContainElement(MatchFields(IgnoreExtras, Fields{
 				"Type":   Equal(v1alpha1.ClusterControlPlaneIPEnsured),
 				"Status": Equal(corev1.ConditionTrue),
@@ -244,7 +220,7 @@ var _ = Describe("MetalStackCluster Controller", func() {
 			resource.Spec = v1alpha1.MetalStackClusterSpec{
 				ControlPlaneEndpoint: v1alpha1.APIEndpoint{},
 				ProjectID:            "test-project",
-				NodeNetworkID:        &nodeNetworkID,
+				NodeNetworkID:        nodeNetworkID,
 				ControlPlaneIP:       &controlPlaneIP,
 				Partition:            "test-partition",
 			}
@@ -287,10 +263,6 @@ var _ = Describe("MetalStackCluster Controller", func() {
 
 					return resource.Status.Conditions
 				}, "20s").Should(ContainElements(
-					MatchFields(IgnoreExtras, Fields{
-						"Type":   Equal(v1alpha1.ClusterNodeNetworkEnsured),
-						"Status": Equal(corev1.ConditionTrue),
-					}),
 					MatchFields(IgnoreExtras, Fields{
 						"Type":   Equal(v1alpha1.ClusterControlPlaneIPEnsured),
 						"Status": Equal(corev1.ConditionTrue),
@@ -336,10 +308,6 @@ var _ = Describe("MetalStackCluster Controller", func() {
 
 					return resource.Status.Conditions
 				}, "20s").Should(ContainElements(
-					MatchFields(IgnoreExtras, Fields{
-						"Type":   Equal(v1alpha1.ClusterNodeNetworkEnsured),
-						"Status": Equal(corev1.ConditionTrue),
-					}),
 					MatchFields(IgnoreExtras, Fields{
 						"Type":   Equal(v1alpha1.ClusterControlPlaneIPEnsured),
 						"Status": Equal(corev1.ConditionTrue),

diff --git a/internal/controller/metalstackmachine_controller.go b/internal/controller/metalstackmachine_controller.go
@@ -172,11 +172,6 @@ func (r *MetalStackMachineReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func (r *machineReconciler) reconcile() (ctrl.Result, error) {
-	if r.infraCluster.Spec.NodeNetworkID == nil {
-		// this should not happen because before setting this id the cluster status should not become ready, but we check it anyway
-		return ctrl.Result{}, errors.New("waiting until node network id was set to infrastructure cluster status")
-	}
-
 	if r.infraCluster.Spec.ControlPlaneEndpoint.Host == "" {
 		return ctrl.Result{}, errors.New("waiting until control plane ip was set to infrastructure cluster spec")
 	}
@@ -276,7 +271,7 @@ func (r *machineReconciler) create() (*models.V1MachineResponse, error) {
 		nws = []*models.V1MachineAllocationNetwork{
 			{
 				Autoacquire: ptr.To(true),
-				Networkid:   r.infraCluster.Spec.NodeNetworkID,
+				Networkid:   &r.infraCluster.Spec.NodeNetworkID,
 			},
 		}
 	)