Skip to content

Commit 1bf5371

Browse files
committed
fixing a nil pointer deference bug when network.status is empty
Signed-off-by: Bharath Nallapeta <[email protected]>
1 parent c14cb8c commit 1bf5371

File tree

2 files changed

+99
-9
lines changed

2 files changed

+99
-9
lines changed

controllers/openstackmachine_controller.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,28 @@ func (r *OpenStackMachineReconciler) getOrCreateMachineServer(ctx context.Contex
583583
}
584584
return openStackCluster.Spec.IdentityRef
585585
}()
586-
machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network.ID)
586+
// If user has provided .spec.ports (non-empty), skip .Status.Network entirely
587+
// Otherwise, fall back to openStackCluster.Status.Network.ID
588+
// This supports HCP - Hosted Control Plane usage
589+
defaultNetworkID := ""
590+
if len(openStackMachine.Spec.Ports) == 0 {
591+
if openStackCluster.Status.Network == nil || openStackCluster.Status.Network.ID == "" {
592+
return nil, fmt.Errorf(
593+
"no user-defined ports and openStackCluster.Status.Network is nil/empty; cannot create server",
594+
)
595+
}
596+
defaultNetworkID = openStackCluster.Status.Network.ID
597+
}
598+
599+
defaultSecGroup := getManagedSecurityGroup(openStackCluster, machine)
600+
var secGroup *string
601+
// If the user explicitly provided security groups in OpenStackMachine, skip the cluster’s managed SG.
602+
if len(openStackMachine.Spec.SecurityGroups) > 0 {
603+
secGroup = nil
604+
} else {
605+
secGroup = defaultSecGroup
606+
}
607+
machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, secGroup, defaultNetworkID)
587608
machineServer = &infrav1alpha1.OpenStackServer{
588609
ObjectMeta: metav1.ObjectMeta{
589610
Labels: map[string]string{

controllers/openstackmachine_controller_test.go

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,16 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
9595
tags := []string{"tag1", "tag2"}
9696
userData := &corev1.LocalObjectReference{Name: "server-data-secret"}
9797
tests := []struct {
98-
name string
99-
spec *infrav1.OpenStackMachineSpec
100-
want *infrav1alpha1.OpenStackServerSpec
98+
name string
99+
passWorkerSG bool
100+
passNetID bool
101+
spec *infrav1.OpenStackMachineSpec
102+
want *infrav1alpha1.OpenStackServerSpec
101103
}{
102104
{
103-
name: "Test a minimum OpenStackMachineSpec to OpenStackServerSpec conversion",
105+
name: "Test a minimum OpenStackMachineSpec to OpenStackServerSpec conversion",
106+
passWorkerSG: true,
107+
passNetID: true,
104108
spec: &infrav1.OpenStackMachineSpec{
105109
Flavor: ptr.To(flavorName),
106110
Image: image,
@@ -117,7 +121,9 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
117121
},
118122
},
119123
{
120-
name: "Test a OpenStackMachineSpec to OpenStackServerSpec conversion with an additional security group",
124+
name: "Test a OpenStackMachineSpec to OpenStackServerSpec conversion with an additional security group",
125+
passWorkerSG: true,
126+
passNetID: true,
121127
spec: &infrav1.OpenStackMachineSpec{
122128
Flavor: ptr.To(flavorName),
123129
Image: image,
@@ -138,13 +144,76 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
138144
UserDataRef: userData,
139145
},
140146
},
147+
{
148+
name: "HPC scenario with user-provided ports ignoring cluster network",
149+
passWorkerSG: false,
150+
passNetID: false,
151+
spec: &infrav1.OpenStackMachineSpec{
152+
Flavor: ptr.To(flavorName),
153+
Image: image,
154+
SSHKeyName: sshKeyName,
155+
Ports: []infrav1.PortOpts{
156+
{
157+
Network: &infrav1.NetworkParam{
158+
ID: ptr.To(networkUUID),
159+
},
160+
},
161+
},
162+
SecurityGroups: []infrav1.SecurityGroupParam{
163+
{
164+
ID: ptr.To(extraSecurityGroupUUID),
165+
},
166+
},
167+
},
168+
want: &infrav1alpha1.OpenStackServerSpec{
169+
Flavor: ptr.To(flavorName),
170+
IdentityRef: identityRef,
171+
Image: image,
172+
SSHKeyName: sshKeyName,
173+
Ports: []infrav1.PortOpts{
174+
{
175+
Network: &infrav1.NetworkParam{
176+
ID: ptr.To(networkUUID),
177+
},
178+
SecurityGroups: []infrav1.SecurityGroupParam{
179+
{
180+
ID: ptr.To(extraSecurityGroupUUID),
181+
},
182+
},
183+
},
184+
},
185+
Tags: tags,
186+
UserDataRef: userData,
187+
},
188+
},
141189
}
142190
for i := range tests {
143191
tt := tests[i]
144192
t.Run(tt.name, func(t *testing.T) {
145-
spec := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, openStackCluster.Status.Network.ID)
146-
if !reflect.DeepEqual(spec, tt.want) {
147-
t.Errorf("openStackMachineSpecToOpenStackServerSpec() got = %+v, want %+v", spec, tt.want)
193+
// Conditionally pass the cluster’s worker SG
194+
var workerSGPtr *string
195+
if tt.passWorkerSG {
196+
workerSGPtr = &openStackCluster.Status.WorkerSecurityGroup.ID
197+
}
198+
199+
// Conditionally pass the cluster’s network ID
200+
var netID string
201+
if tt.passNetID && openStackCluster.Status.Network != nil {
202+
netID = openStackCluster.Status.Network.ID
203+
}
204+
205+
got := openStackMachineSpecToOpenStackServerSpec(
206+
tt.spec,
207+
identityRef,
208+
tags,
209+
"",
210+
userData,
211+
workerSGPtr,
212+
netID,
213+
)
214+
215+
if !reflect.DeepEqual(got, tt.want) {
216+
t.Errorf("openStackMachineSpecToOpenStackServerSpec() got = %+v, want %+v", got, tt.want)
148217
}
149218
})
150219
}

0 commit comments

Comments
 (0)