Skip to content

Commit ec77c18

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#1967 from shiftstack/machineresourcespointers
⚠️ Resolved and Resources become optional pointer types
2 parents 59689c9 + 2758a6d commit ec77c18

16 files changed

+207
-105
lines changed

‎api/v1alpha5/conversion_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func TestConvertFrom(t *testing.T) {
8787
Spec: OpenStackMachineSpec{},
8888
ObjectMeta: metav1.ObjectMeta{
8989
Annotations: map[string]string{
90-
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"flavor\":\"\",\"image\":{}},\"status\":{\"ready\":false,\"resolved\":{},\"resources\":{}}}",
90+
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"flavor\":\"\",\"image\":{}},\"status\":{\"ready\":false}}",
9191
},
9292
},
9393
},

‎api/v1alpha6/openstackcluster_conversion.go‎

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -367,12 +367,7 @@ func restorev1beta1ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst *
367367
dst.WorkerSecurityGroup = previous.WorkerSecurityGroup
368368
dst.BastionSecurityGroup = previous.BastionSecurityGroup
369369

370-
if previous.Bastion != nil {
371-
dst.Bastion.Resolved = previous.Bastion.Resolved
372-
}
373-
if previous.Bastion != nil && previous.Bastion.Resources.Ports != nil {
374-
dst.Bastion.Resources.Ports = previous.Bastion.Resources.Ports
375-
}
370+
restorev1beta1BastionStatus(previous.Bastion, dst.Bastion)
376371
}
377372

378373
func Convert_v1beta1_OpenStackClusterStatus_To_v1alpha6_OpenStackClusterStatus(in *infrav1.OpenStackClusterStatus, out *OpenStackClusterStatus, s apiconversion.Scope) error {
@@ -412,36 +407,15 @@ func Convert_v1alpha6_OpenStackClusterStatus_To_v1beta1_OpenStackClusterStatus(i
412407
/* Bastion */
413408

414409
func restorev1beta1Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) {
415-
if *previous != nil {
416-
if *dst != nil && (*previous).Spec != nil && (*dst).Spec != nil {
417-
restorev1beta1MachineSpec((*previous).Spec, (*dst).Spec)
418-
}
419-
420-
optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
421-
optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
410+
if previous == nil || dst == nil || *previous == nil || *dst == nil {
411+
return
412+
}
413+
if *dst != nil && (*previous).Spec != nil && (*dst).Spec != nil {
414+
restorev1beta1MachineSpec((*previous).Spec, (*dst).Spec)
422415
}
423-
}
424-
425-
func Convert_v1alpha6_Instance_To_v1beta1_BastionStatus(in *Instance, out *infrav1.BastionStatus, _ apiconversion.Scope) error {
426-
// BastionStatus is the same as Spec with unused fields removed
427-
out.ID = in.ID
428-
out.Name = in.Name
429-
out.SSHKeyName = in.SSHKeyName
430-
out.State = infrav1.InstanceState(in.State)
431-
out.IP = in.IP
432-
out.FloatingIP = in.FloatingIP
433-
return nil
434-
}
435416

436-
func Convert_v1beta1_BastionStatus_To_v1alpha6_Instance(in *infrav1.BastionStatus, out *Instance, _ apiconversion.Scope) error {
437-
// BastionStatus is the same as Spec with unused fields removed
438-
out.ID = in.ID
439-
out.Name = in.Name
440-
out.SSHKeyName = in.SSHKeyName
441-
out.State = InstanceState(in.State)
442-
out.IP = in.IP
443-
out.FloatingIP = in.FloatingIP
444-
return nil
417+
optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP)
418+
optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone)
445419
}
446420

447421
func Convert_v1alpha6_Bastion_To_v1beta1_Bastion(in *Bastion, out *infrav1.Bastion, s apiconversion.Scope) error {
@@ -496,3 +470,37 @@ func Convert_v1beta1_Bastion_To_v1alpha6_Bastion(in *infrav1.Bastion, out *Basti
496470

497471
return optional.Convert_optional_String_To_string(&in.FloatingIP, &out.Instance.FloatingIP, s)
498472
}
473+
474+
/* Bastion status */
475+
476+
func restorev1beta1BastionStatus(previous *infrav1.BastionStatus, dst *infrav1.BastionStatus) {
477+
if previous == nil || dst == nil {
478+
return
479+
}
480+
481+
// Resolved and resources have no equivalents
482+
dst.Resolved = previous.Resolved
483+
dst.Resources = previous.Resources
484+
}
485+
486+
func Convert_v1alpha6_Instance_To_v1beta1_BastionStatus(in *Instance, out *infrav1.BastionStatus, _ apiconversion.Scope) error {
487+
// BastionStatus is the same as Spec with unused fields removed
488+
out.ID = in.ID
489+
out.Name = in.Name
490+
out.SSHKeyName = in.SSHKeyName
491+
out.State = infrav1.InstanceState(in.State)
492+
out.IP = in.IP
493+
out.FloatingIP = in.FloatingIP
494+
return nil
495+
}
496+
497+
func Convert_v1beta1_BastionStatus_To_v1alpha6_Instance(in *infrav1.BastionStatus, out *Instance, _ apiconversion.Scope) error {
498+
// BastionStatus is the same as Spec with unused fields removed
499+
out.ID = in.ID
500+
out.Name = in.Name
501+
out.SSHKeyName = in.SSHKeyName
502+
out.State = InstanceState(in.State)
503+
out.IP = in.IP
504+
out.FloatingIP = in.FloatingIP
505+
return nil
506+
}

‎api/v1alpha6/openstackmachine_conversion.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,13 @@ var v1beta1OpenStackMachineRestorer = conversion.RestorerFor[*infrav1.OpenStackM
9090
restorev1beta1MachineSpec,
9191
),
9292
"depresources": conversion.UnconditionalFieldRestorer(
93-
func(c *infrav1.OpenStackMachine) *infrav1.MachineResources {
93+
func(c *infrav1.OpenStackMachine) **infrav1.MachineResources {
9494
return &c.Status.Resources
9595
},
9696
),
9797
// No equivalent in v1alpha6
9898
"refresources": conversion.UnconditionalFieldRestorer(
99-
func(c *infrav1.OpenStackMachine) *infrav1.ResolvedMachineSpec {
99+
func(c *infrav1.OpenStackMachine) **infrav1.ResolvedMachineSpec {
100100
return &c.Status.Resolved
101101
},
102102
),

‎api/v1alpha7/openstackcluster_conversion.go‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -348,14 +348,11 @@ func restorev1alpha7ClusterStatus(previous *OpenStackClusterStatus, dst *OpenSta
348348
}
349349

350350
func restorev1beta1ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst *infrav1.OpenStackClusterStatus) {
351-
// ReferencedResources have no equivalent in v1alpha7
352-
if previous.Bastion != nil {
353-
dst.Bastion.Resolved = previous.Bastion.Resolved
351+
if previous == nil || dst == nil {
352+
return
354353
}
355354

356-
if previous.Bastion != nil && previous.Bastion.Resources.Ports != nil {
357-
dst.Bastion.Resources.Ports = previous.Bastion.Resources.Ports
358-
}
355+
restorev1beta1BastionStatus(previous.Bastion, dst.Bastion)
359356
}
360357

361358
func Convert_v1beta1_OpenStackClusterStatus_To_v1alpha7_OpenStackClusterStatus(in *infrav1.OpenStackClusterStatus, out *OpenStackClusterStatus, s apiconversion.Scope) error {
@@ -434,6 +431,18 @@ func Convert_v1beta1_Bastion_To_v1alpha7_Bastion(in *infrav1.Bastion, out *Basti
434431
return optional.Convert_optional_String_To_string(&in.FloatingIP, &out.Instance.FloatingIP, s)
435432
}
436433

434+
/* Bastion status */
435+
436+
func restorev1beta1BastionStatus(previous *infrav1.BastionStatus, dst *infrav1.BastionStatus) {
437+
if previous == nil || dst == nil {
438+
return
439+
}
440+
441+
// Resolved and resources have no equivalents
442+
dst.Resolved = previous.Resolved
443+
dst.Resources = previous.Resources
444+
}
445+
437446
func Convert_v1beta1_BastionStatus_To_v1alpha7_BastionStatus(in *infrav1.BastionStatus, out *BastionStatus, s apiconversion.Scope) error {
438447
// ReferencedResources have no equivalent in v1alpha7
439448
return autoConvert_v1beta1_BastionStatus_To_v1alpha7_BastionStatus(in, out, s)

‎api/v1alpha7/openstackmachine_conversion.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,13 @@ var v1beta1OpenStackMachineRestorer = conversion.RestorerFor[*infrav1.OpenStackM
7777
restorev1beta1MachineSpec,
7878
),
7979
"depresources": conversion.UnconditionalFieldRestorer(
80-
func(c *infrav1.OpenStackMachine) *infrav1.MachineResources {
80+
func(c *infrav1.OpenStackMachine) **infrav1.MachineResources {
8181
return &c.Status.Resources
8282
},
8383
),
8484
// No equivalent in v1alpha7
8585
"refresources": conversion.UnconditionalFieldRestorer(
86-
func(c *infrav1.OpenStackMachine) *infrav1.ResolvedMachineSpec {
86+
func(c *infrav1.OpenStackMachine) **infrav1.ResolvedMachineSpec {
8787
return &c.Status.Resolved
8888
},
8989
),

‎api/v1beta1/openstackmachine_types.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,12 @@ type OpenStackMachineStatus struct {
128128

129129
// Resolved contains parts of the machine spec with all external
130130
// references fully resolved.
131-
Resolved ResolvedMachineSpec `json:"resolved,omitempty"`
131+
// +optional
132+
Resolved *ResolvedMachineSpec `json:"resolved,omitempty"`
132133

133134
// Resources contains references to OpenStack resources created for the machine.
134-
Resources MachineResources `json:"resources,omitempty"`
135+
// +optional
136+
Resources *MachineResources `json:"resources,omitempty"`
135137

136138
FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"`
137139

‎api/v1beta1/types.go‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -358,14 +358,21 @@ type AddressPair struct {
358358
}
359359

360360
type BastionStatus struct {
361-
ID string `json:"id,omitempty"`
362-
Name string `json:"name,omitempty"`
363-
SSHKeyName string `json:"sshKeyName,omitempty"`
364-
State InstanceState `json:"state,omitempty"`
365-
IP string `json:"ip,omitempty"`
366-
FloatingIP string `json:"floatingIP,omitempty"`
367-
Resolved ResolvedMachineSpec `json:"resolved,omitempty"`
368-
Resources MachineResources `json:"resources,omitempty"`
361+
ID string `json:"id,omitempty"`
362+
Name string `json:"name,omitempty"`
363+
SSHKeyName string `json:"sshKeyName,omitempty"`
364+
State InstanceState `json:"state,omitempty"`
365+
IP string `json:"ip,omitempty"`
366+
FloatingIP string `json:"floatingIP,omitempty"`
367+
368+
// Resolved contains parts of the bastion's machine spec with all
369+
// external references fully resolved.
370+
// +optional
371+
Resolved *ResolvedMachineSpec `json:"resolved,omitempty"`
372+
373+
// Resources contains references to OpenStack resources created for the bastion.
374+
// +optional
375+
Resources *MachineResources `json:"resources,omitempty"`
369376
}
370377

371378
type RootVolume struct {

‎api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 20 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml‎

Lines changed: 5 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎controllers/openstackcluster_controller.go‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,13 @@ func resolveBastionResources(scope *scope.WithLogger, clusterResourceName string
227227
if openStackCluster.Spec.Bastion.Spec == nil {
228228
return false, fmt.Errorf("bastion spec is nil when bastion is enabled, this shouldn't happen")
229229
}
230+
resolved := openStackCluster.Status.Bastion.Resolved
231+
if resolved == nil {
232+
resolved = &infrav1.ResolvedMachineSpec{}
233+
openStackCluster.Status.Bastion.Resolved = resolved
234+
}
230235
changed, err := compute.ResolveMachineSpec(scope,
231-
openStackCluster.Spec.Bastion.Spec, &openStackCluster.Status.Bastion.Resolved,
236+
openStackCluster.Spec.Bastion.Spec, resolved,
232237
clusterResourceName, bastionName(clusterResourceName),
233238
openStackCluster, getBastionSecurityGroupID(openStackCluster))
234239
if err != nil {
@@ -238,10 +243,13 @@ func resolveBastionResources(scope *scope.WithLogger, clusterResourceName string
238243
// If the resolved machine spec changed we need to restart the reconcile to avoid inconsistencies between reconciles.
239244
return true, nil
240245
}
246+
resources := openStackCluster.Status.Bastion.Resources
247+
if resources == nil {
248+
resources = &infrav1.MachineResources{}
249+
openStackCluster.Status.Bastion.Resources = resources
250+
}
241251

242-
err = compute.AdoptMachineResources(scope,
243-
&openStackCluster.Status.Bastion.Resolved,
244-
&openStackCluster.Status.Bastion.Resources)
252+
err = compute.AdoptMachineResources(scope, resolved, resources)
245253
if err != nil {
246254
return false, err
247255
}
@@ -268,8 +276,10 @@ func deleteBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStac
268276
}
269277
}
270278

279+
bastionStatus := openStackCluster.Status.Bastion
280+
271281
var instanceStatus *compute.InstanceStatus
272-
if openStackCluster.Status.Bastion != nil && openStackCluster.Status.Bastion.ID != "" {
282+
if bastionStatus != nil && bastionStatus.ID != "" {
273283
instanceStatus, err = computeService.GetInstanceStatus(openStackCluster.Status.Bastion.ID)
274284
if err != nil {
275285
return err
@@ -308,18 +318,18 @@ func deleteBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStac
308318
}
309319
}
310320

311-
if openStackCluster.Status.Bastion != nil && len(openStackCluster.Status.Bastion.Resources.Ports) > 0 {
321+
if bastionStatus != nil && bastionStatus.Resources != nil {
312322
trunkSupported, err := networkingService.IsTrunkExtSupported()
313323
if err != nil {
314324
return err
315325
}
316-
for _, port := range openStackCluster.Status.Bastion.Resources.Ports {
326+
for _, port := range bastionStatus.Resources.Ports {
317327
if err := networkingService.DeleteInstanceTrunkAndPort(openStackCluster, port, trunkSupported); err != nil {
318328
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete port: %w", err))
319329
return fmt.Errorf("failed to delete port: %w", err)
320330
}
321331
}
322-
openStackCluster.Status.Bastion.Resources.Ports = nil
332+
bastionStatus.Resources.Ports = nil
323333
}
324334

325335
scope.Logger().Info("Deleted Bastion")
@@ -542,7 +552,10 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, cluster *
542552
// v1beta1 API validations prevent this from happening in normal circumstances.
543553
bastion.Spec = &infrav1.OpenStackMachineSpec{}
544554
}
545-
resolved := &openStackCluster.Status.Bastion.Resolved
555+
resolved := openStackCluster.Status.Bastion.Resolved
556+
if resolved == nil {
557+
return nil, errors.New("bastion resolved is nil")
558+
}
546559

547560
machineSpec := bastion.Spec
548561
instanceSpec := &compute.InstanceSpec{
@@ -579,7 +592,10 @@ func getBastionSecurityGroupID(openStackCluster *infrav1.OpenStackCluster) *stri
579592

580593
func getOrCreateBastionPorts(openStackCluster *infrav1.OpenStackCluster, networkingService *networking.Service) error {
581594
desiredPorts := openStackCluster.Status.Bastion.Resolved.Ports
582-
resources := &openStackCluster.Status.Bastion.Resources
595+
resources := openStackCluster.Status.Bastion.Resources
596+
if resources == nil {
597+
return errors.New("bastion resources are nil")
598+
}
583599

584600
if len(desiredPorts) == len(resources.Ports) {
585601
return nil

0 commit comments

Comments
 (0)