Skip to content

Commit 6d42cbb

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2639 from nonowheels63/bug-2617-main
🐛 correction of issue #2617 selected fixed subnet not applied
2 parents a722c05 + e78608c commit 6d42cbb

File tree

3 files changed

+139
-32
lines changed

3 files changed

+139
-32
lines changed

‎controllers/openstackcluster_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) (*
594594
if bastion.AvailabilityZone != nil {
595595
az = *bastion.AvailabilityZone
596596
}
597-
openStackServerSpec, err := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster.Status.Network)
597+
openStackServerSpec, err := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster)
598598
if err != nil {
599599
return nil, err
600600
}

‎controllers/openstackmachine_controller.go‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -480,13 +480,15 @@ func (r *OpenStackMachineReconciler) getMachineServer(ctx context.Context, openS
480480

481481
// openStackMachineSpecToOpenStackServerSpec converts an OpenStackMachineSpec to an OpenStackServerSpec.
482482
// It returns the OpenStackServerSpec object and an error if there is any.
483-
func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, clusterNetwork *infrav1.NetworkStatusWithSubnets) (*infrav1alpha1.OpenStackServerSpec, error) {
483+
func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, openStackCluster *infrav1.OpenStackCluster) (*infrav1alpha1.OpenStackServerSpec, error) {
484484
// Determine default network ID if the cluster status exposes one.
485485
var defaultNetworkID string
486-
if clusterNetwork != nil {
487-
defaultNetworkID = clusterNetwork.ID
486+
if openStackCluster != nil {
487+
clusterNetwork := openStackCluster.Status.Network
488+
if clusterNetwork != nil {
489+
defaultNetworkID = clusterNetwork.ID
490+
}
488491
}
489-
490492
// If no cluster network is available AND the machine spec did not define any ports with a network, we cannot choose a network.
491493
if defaultNetworkID == "" && len(openStackMachineSpec.Ports) == 0 {
492494
return nil, capoerrors.Terminal(infrav1.InvalidMachineSpecReason, "no network configured: cluster network is missing and machine spec does not define ports with a network")
@@ -532,22 +534,35 @@ func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.Ope
532534
if len(openStackMachineSpec.Ports) == 0 {
533535
serverPorts = make([]infrav1.PortOpts, 1)
534536
}
537+
538+
var clusterSubnets []infrav1.FixedIP
539+
if len(openStackCluster.Spec.Subnets) > 0 {
540+
clusterSubnets = make([]infrav1.FixedIP, len(openStackCluster.Spec.Subnets))
541+
for idx, sn := range openStackCluster.Spec.Subnets {
542+
clusterSubnets[idx] = infrav1.FixedIP{
543+
Subnet: &sn,
544+
}
545+
}
546+
}
547+
535548
for i := range serverPorts {
549+
serverPort := &serverPorts[i]
536550
// Only inject the default network when we actually have an ID.
537-
if serverPorts[i].Network == nil && defaultNetworkID != "" {
538-
serverPorts[i].Network = &infrav1.NetworkParam{
539-
ID: &defaultNetworkID,
551+
if serverPort.Network == nil && defaultNetworkID != "" {
552+
serverPort.Network = &infrav1.NetworkParam{
553+
ID: &openStackCluster.Status.Network.ID,
540554
}
555+
serverPort.FixedIPs = clusterSubnets
541556
}
542-
if len(serverPorts[i].SecurityGroups) == 0 && defaultSecGroup != nil {
543-
serverPorts[i].SecurityGroups = []infrav1.SecurityGroupParam{
557+
if len(serverPort.SecurityGroups) == 0 && defaultSecGroup != nil {
558+
serverPort.SecurityGroups = []infrav1.SecurityGroupParam{
544559
{
545560
ID: defaultSecGroup,
546561
},
547562
}
548563
}
549564
if len(openStackMachineSpec.SecurityGroups) > 0 {
550-
serverPorts[i].SecurityGroups = append(serverPorts[i].SecurityGroups, openStackMachineSpec.SecurityGroups...)
565+
serverPort.SecurityGroups = append(serverPort.SecurityGroups, openStackMachineSpec.SecurityGroups...)
551566
}
552567
}
553568
openStackServerSpec.Ports = serverPorts
@@ -601,7 +616,7 @@ func (r *OpenStackMachineReconciler) getOrCreateMachineServer(ctx context.Contex
601616
}
602617
return openStackCluster.Spec.IdentityRef
603618
}()
604-
machineServerSpec, err := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network)
619+
machineServerSpec, err := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster)
605620
if err != nil {
606621
return nil, err
607622
}

‎controllers/openstackmachine_controller_test.go‎

Lines changed: 112 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,61 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
6464
},
6565
},
6666
}
67+
openStackClusterWithSubnet := &infrav1.OpenStackCluster{
68+
Spec: infrav1.OpenStackClusterSpec{
69+
ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{},
70+
Subnets: []infrav1.SubnetParam{
71+
{
72+
ID: ptr.To(subnetUUID),
73+
},
74+
},
75+
},
76+
Status: infrav1.OpenStackClusterStatus{
77+
WorkerSecurityGroup: &infrav1.SecurityGroupStatus{
78+
ID: workerSecurityGroupUUID,
79+
},
80+
Network: &infrav1.NetworkStatusWithSubnets{
81+
NetworkStatus: infrav1.NetworkStatus{
82+
ID: networkUUID,
83+
},
84+
},
85+
},
86+
}
87+
openStackClusterNoNetwork := &infrav1.OpenStackCluster{
88+
Spec: infrav1.OpenStackClusterSpec{
89+
ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{},
90+
Subnets: []infrav1.SubnetParam{
91+
{
92+
ID: ptr.To(subnetUUID),
93+
},
94+
},
95+
},
96+
Status: infrav1.OpenStackClusterStatus{
97+
WorkerSecurityGroup: &infrav1.SecurityGroupStatus{
98+
ID: workerSecurityGroupUUID,
99+
},
100+
},
101+
}
102+
openStackClusterNetworkWithoutID := &infrav1.OpenStackCluster{
103+
Spec: infrav1.OpenStackClusterSpec{
104+
ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{},
105+
Subnets: []infrav1.SubnetParam{
106+
{
107+
ID: ptr.To(subnetUUID),
108+
},
109+
},
110+
},
111+
Status: infrav1.OpenStackClusterStatus{
112+
WorkerSecurityGroup: &infrav1.SecurityGroupStatus{
113+
ID: workerSecurityGroupUUID,
114+
},
115+
Network: &infrav1.NetworkStatusWithSubnets{
116+
NetworkStatus: infrav1.NetworkStatus{
117+
ID: "",
118+
},
119+
},
120+
},
121+
}
67122
portOpts := []infrav1.PortOpts{
68123
{
69124
Network: &infrav1.NetworkParam{
@@ -91,24 +146,43 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
91146
},
92147
},
93148
}
149+
portOptsWithAdditionalSubnet := []infrav1.PortOpts{
150+
{
151+
Network: &infrav1.NetworkParam{
152+
ID: ptr.To(openStackCluster.Status.Network.ID),
153+
},
154+
SecurityGroups: []infrav1.SecurityGroupParam{
155+
{
156+
ID: ptr.To(openStackCluster.Status.WorkerSecurityGroup.ID),
157+
},
158+
},
159+
FixedIPs: []infrav1.FixedIP{
160+
{
161+
Subnet: &infrav1.SubnetParam{
162+
ID: ptr.To(subnetUUID),
163+
},
164+
},
165+
},
166+
},
167+
}
94168
image := infrav1.ImageParam{Filter: &infrav1.ImageFilter{Name: ptr.To("my-image")}}
95169
tags := []string{"tag1", "tag2"}
96170
userData := &corev1.LocalObjectReference{Name: "server-data-secret"}
97171
tests := []struct {
98-
name string
99-
spec *infrav1.OpenStackMachineSpec
100-
clusterNetwork *infrav1.NetworkStatusWithSubnets
101-
want *infrav1alpha1.OpenStackServerSpec
102-
wantErr bool
172+
name string
173+
cluster *infrav1.OpenStackCluster
174+
spec *infrav1.OpenStackMachineSpec
175+
want *infrav1alpha1.OpenStackServerSpec
176+
wantErr bool
103177
}{
104178
{
105-
name: "Test a minimum OpenStackMachineSpec to OpenStackServerSpec conversion",
179+
name: "Test a minimum OpenStackMachineSpec to OpenStackServerSpec conversion",
180+
cluster: openStackCluster,
106181
spec: &infrav1.OpenStackMachineSpec{
107182
Flavor: ptr.To(flavorName),
108183
Image: image,
109184
SSHKeyName: sshKeyName,
110185
},
111-
clusterNetwork: openStackCluster.Status.Network,
112186
want: &infrav1alpha1.OpenStackServerSpec{
113187
Flavor: ptr.To(flavorName),
114188
IdentityRef: identityRef,
@@ -120,7 +194,8 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
120194
},
121195
},
122196
{
123-
name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with an additional security group",
197+
name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with an additional security group",
198+
cluster: openStackCluster,
124199
spec: &infrav1.OpenStackMachineSpec{
125200
Flavor: ptr.To(flavorName),
126201
Image: image,
@@ -131,7 +206,6 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
131206
},
132207
},
133208
},
134-
clusterNetwork: openStackCluster.Status.Network,
135209
want: &infrav1alpha1.OpenStackServerSpec{
136210
Flavor: ptr.To(flavorName),
137211
IdentityRef: identityRef,
@@ -143,14 +217,32 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
143217
},
144218
},
145219
{
146-
name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with flavor and flavorID specified",
220+
name: "Test a OpenStackMachineSpec to OpenStackServerSpec conversion with a specified subnet",
221+
cluster: openStackClusterWithSubnet,
222+
spec: &infrav1.OpenStackMachineSpec{
223+
Flavor: ptr.To(flavorName),
224+
Image: image,
225+
SSHKeyName: sshKeyName,
226+
},
227+
want: &infrav1alpha1.OpenStackServerSpec{
228+
Flavor: ptr.To(flavorName),
229+
IdentityRef: identityRef,
230+
Image: image,
231+
SSHKeyName: sshKeyName,
232+
Ports: portOptsWithAdditionalSubnet,
233+
Tags: tags,
234+
UserDataRef: userData,
235+
},
236+
},
237+
{
238+
name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with flavor and flavorID specified",
239+
cluster: openStackCluster,
147240
spec: &infrav1.OpenStackMachineSpec{
148241
Flavor: ptr.To(flavorName),
149242
FlavorID: ptr.To(flavorUUID),
150243
Image: image,
151244
SSHKeyName: sshKeyName,
152245
},
153-
clusterNetwork: openStackCluster.Status.Network,
154246
want: &infrav1alpha1.OpenStackServerSpec{
155247
Flavor: ptr.To(flavorName),
156248
FlavorID: ptr.To(flavorUUID),
@@ -163,13 +255,13 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
163255
},
164256
},
165257
{
166-
name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with flavorID specified but not flavor",
258+
name: "Test an OpenStackMachineSpec to OpenStackServerSpec conversion with flavorID specified but not flavor",
259+
cluster: openStackCluster,
167260
spec: &infrav1.OpenStackMachineSpec{
168261
FlavorID: ptr.To(flavorUUID),
169262
Image: image,
170263
SSHKeyName: sshKeyName,
171264
},
172-
clusterNetwork: openStackCluster.Status.Network,
173265
want: &infrav1alpha1.OpenStackServerSpec{
174266
FlavorID: ptr.To(flavorUUID),
175267
IdentityRef: identityRef,
@@ -188,7 +280,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
188280
}},
189281
SecurityGroups: []infrav1.SecurityGroupParam{{ID: ptr.To(extraSecurityGroupUUID)}},
190282
},
191-
clusterNetwork: nil,
283+
cluster: openStackClusterNoNetwork,
192284
want: &infrav1alpha1.OpenStackServerSpec{
193285
IdentityRef: identityRef,
194286
Ports: []infrav1.PortOpts{{
@@ -209,7 +301,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
209301
Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
210302
}},
211303
},
212-
clusterNetwork: nil,
304+
cluster: openStackClusterNoNetwork,
213305
want: &infrav1alpha1.OpenStackServerSpec{
214306
IdentityRef: identityRef,
215307
Ports: []infrav1.PortOpts{{
@@ -228,9 +320,9 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
228320
SSHKeyName: sshKeyName,
229321
// No ports defined
230322
},
231-
clusterNetwork: nil,
232-
want: nil,
233-
wantErr: true,
323+
cluster: openStackClusterNoNetwork,
324+
want: nil,
325+
wantErr: true,
234326
},
235327
{
236328
name: "Empty cluster network ID, machine defines explicit ports",
@@ -241,7 +333,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
241333
Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
242334
}},
243335
},
244-
clusterNetwork: &infrav1.NetworkStatusWithSubnets{NetworkStatus: infrav1.NetworkStatus{ID: ""}},
336+
cluster: openStackClusterNetworkWithoutID,
245337
want: &infrav1alpha1.OpenStackServerSpec{
246338
Flavor: ptr.To(flavorName),
247339
IdentityRef: identityRef,
@@ -258,7 +350,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
258350
for i := range tests {
259351
tt := tests[i]
260352
t.Run(tt.name, func(t *testing.T) {
261-
spec, err := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, tt.clusterNetwork)
353+
spec, err := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, tt.cluster)
262354
if (err != nil) != tt.wantErr {
263355
t.Errorf("openStackMachineSpecToOpenStackServerSpec() error = %v, wantErr %v", err, tt.wantErr)
264356
return

0 commit comments

Comments
 (0)