🐛 correction of issue #2617 selected fixed subnet not applied

Author: ablandamou

controllers/openstackcluster_controller.go

@@ -571,7 +571,7 @@ func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) *i
 	if bastion.AvailabilityZone != nil {
 		az = *bastion.AvailabilityZone
 	}
-	openStackServerSpec := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster.Status.Network.ID)
+	openStackServerSpec := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster)
 
 	return openStackServerSpec
 }

controllers/openstackmachine_controller.go

@@ -475,7 +475,7 @@ func (r *OpenStackMachineReconciler) getMachineServer(ctx context.Context, openS
 
 // openStackMachineSpecToOpenStackServerSpec converts an OpenStackMachineSpec to an OpenStackServerSpec.
 // It returns the OpenStackServerSpec object and an error if there is any.
-func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, defaultNetworkID string) *infrav1alpha1.OpenStackServerSpec {
+func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, openStackCluster *infrav1.OpenStackCluster) *infrav1alpha1.OpenStackServerSpec {
 	openStackServerSpec := &infrav1alpha1.OpenStackServerSpec{
 		AdditionalBlockDevices:            openStackMachineSpec.AdditionalBlockDevices,
 		ConfigDrive:                       openStackMachineSpec.ConfigDrive,
@@ -516,21 +516,39 @@ func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.Ope
 	if len(openStackMachineSpec.Ports) == 0 {
 		serverPorts = make([]infrav1.PortOpts, 1)
 	}
+
+	var fixedIPs []infrav1.FixedIP
+	if len(openStackCluster.Spec.Subnets) > 0 {
+		fixedIPs = make([]infrav1.FixedIP, len(openStackCluster.Spec.Subnets))
+		for idx, sn := range openStackCluster.Spec.Subnets {
+			fixedIPs[idx] = infrav1.FixedIP{
+				Subnet: &sn,
+			}
+		}
+	}
+
 	for i := range serverPorts {
-		if serverPorts[i].Network == nil {
-			serverPorts[i].Network = &infrav1.NetworkParam{
-				ID: &defaultNetworkID,
+		serverPort := &serverPorts[i]
+		if serverPort.Network == nil {
+			if openStackCluster.Status.Network.ID != "" {
+				serverPort.Network = &infrav1.NetworkParam{
+					ID: &openStackCluster.Status.Network.ID,
+				}
+				if len(fixedIPs) > 0 {
+					serverPort.FixedIPs = fixedIPs
+				}
+
 			}
 		}
-		if len(serverPorts[i].SecurityGroups) == 0 && defaultSecGroup != nil {
-			serverPorts[i].SecurityGroups = []infrav1.SecurityGroupParam{
+		if len(serverPort.SecurityGroups) == 0 && defaultSecGroup != nil {
+			serverPort.SecurityGroups = []infrav1.SecurityGroupParam{
 				{
 					ID: defaultSecGroup,
 				},
 			}
 		}
 		if len(openStackMachineSpec.SecurityGroups) > 0 {
-			serverPorts[i].SecurityGroups = append(serverPorts[i].SecurityGroups, openStackMachineSpec.SecurityGroups...)
+			serverPort.SecurityGroups = append(serverPort.SecurityGroups, openStackMachineSpec.SecurityGroups...)
 		}
 	}
 	openStackServerSpec.Ports = serverPorts
@@ -584,7 +602,7 @@ func (r *OpenStackMachineReconciler) getOrCreateMachineServer(ctx context.Contex
 			}
 			return openStackCluster.Spec.IdentityRef
 		}()
-		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network.ID)
+		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster)
 		machineServer = &infrav1alpha1.OpenStackServer{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{

controllers/openstackmachine_controller_test.go

@@ -64,6 +64,26 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 			},
 		},
 	}
+	openStackClusterWithSubnet := &infrav1.OpenStackCluster{
+		Spec: infrav1.OpenStackClusterSpec{
+			ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{},
+			Subnets: []infrav1.SubnetParam{
+				{
+					ID: ptr.To(subnetUUID),
+				},
+			},
+		},
+		Status: infrav1.OpenStackClusterStatus{
+			WorkerSecurityGroup: &infrav1.SecurityGroupStatus{
+				ID: workerSecurityGroupUUID,
+			},
+			Network: &infrav1.NetworkStatusWithSubnets{
+				NetworkStatus: infrav1.NetworkStatus{
+					ID: networkUUID,
+				},
+			},
+		},
+	}
 	portOpts := []infrav1.PortOpts{
 		{
 			Network: &infrav1.NetworkParam{
@@ -91,16 +111,37 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 			},
 		},
 	}
+	portOptsWithAdditionalSubnet := []infrav1.PortOpts{
+		{
+			Network: &infrav1.NetworkParam{
+				ID: ptr.To(openStackCluster.Status.Network.ID),
+			},
+			SecurityGroups: []infrav1.SecurityGroupParam{
+				{
+					ID: ptr.To(openStackCluster.Status.WorkerSecurityGroup.ID),
+				},
+			},
+			FixedIPs: []infrav1.FixedIP{
+				{
+					Subnet: &infrav1.SubnetParam{
+						ID: ptr.To(subnetUUID),
+					},
+				},
+			},
+		},
+	}
 	image := infrav1.ImageParam{Filter: &infrav1.ImageFilter{Name: ptr.To("my-image")}}
 	tags := []string{"tag1", "tag2"}
 	userData := &corev1.LocalObjectReference{Name: "server-data-secret"}
 	tests := []struct {
-		name string
-		spec *infrav1.OpenStackMachineSpec
-		want *infrav1alpha1.OpenStackServerSpec
+		name    string
+		cluster *infrav1.OpenStackCluster
+		spec    *infrav1.OpenStackMachineSpec
+		want    *infrav1alpha1.OpenStackServerSpec
 	}{
 		{
-			name: "Test a minimum OpenStackMachineSpec to OpenStackServerSpec conversion",
+			name:    "Test a minimum OpenStackMachineSpec to OpenStackServerSpec conversion",
+			cluster: openStackCluster,
 			spec: &infrav1.OpenStackMachineSpec{
 				Flavor:     ptr.To(flavorName),
 				Image:      image,
@@ -117,7 +158,8 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 			},
 		},
 		{
-			name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with an additional security group",
+			name:    "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with an additional security group",
+			cluster: openStackCluster,
 			spec: &infrav1.OpenStackMachineSpec{
 				Flavor:     ptr.To(flavorName),
 				Image:      image,
@@ -139,7 +181,26 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 			},
 		},
 		{
-			name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with flavor and flavorID specified",
+			name:    "Test a OpenStackMachineSpec to OpenStackServerSpec conversion with a specified subnet",
+			cluster: openStackClusterWithSubnet,
+			spec: &infrav1.OpenStackMachineSpec{
+				Flavor:     ptr.To(flavorName),
+				Image:      image,
+				SSHKeyName: sshKeyName,
+			},
+			want: &infrav1alpha1.OpenStackServerSpec{
+				Flavor:      ptr.To(flavorName),
+				IdentityRef: identityRef,
+				Image:       image,
+				SSHKeyName:  sshKeyName,
+				Ports:       portOptsWithAdditionalSubnet,
+				Tags:        tags,
+				UserDataRef: userData,
+			},
+		},
+		{
+			name:    "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with flavor and flavorID specified",
+			cluster: openStackCluster,
 			spec: &infrav1.OpenStackMachineSpec{
 				Flavor:     ptr.To(flavorName),
 				FlavorID:   ptr.To(flavorUUID),
@@ -158,7 +219,8 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 			},
 		},
 		{
-			name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with flavorID specified but not flavor",
+			name:    "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with flavorID specified but not flavor",
+			cluster: openStackCluster,
 			spec: &infrav1.OpenStackMachineSpec{
 				FlavorID:   ptr.To(flavorUUID),
 				Image:      image,
@@ -178,7 +240,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 	for i := range tests {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
-			spec := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, openStackCluster.Status.Network.ID)
+			spec := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, tt.cluster)
 			if !reflect.DeepEqual(spec, tt.want) {
 				t.Errorf("openStackMachineSpecToOpenStackServerSpec() got = %+v, want %+v", spec, tt.want)
 			}