diff --git a/api/v1alpha5/conversion_test.go b/api/v1alpha5/conversion_test.go index 82afcd82e8..7fbf79bda6 100644 --- a/api/v1alpha5/conversion_test.go +++ b/api/v1alpha5/conversion_test.go @@ -87,7 +87,7 @@ func TestConvertFrom(t *testing.T) { Spec: OpenStackMachineSpec{}, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"flavor\":\"\",\"image\":{}},\"status\":{\"ready\":false,\"resolved\":{},\"resources\":{}}}", + "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"flavor\":\"\",\"image\":{}},\"status\":{\"ready\":false}}", }, }, }, diff --git a/api/v1alpha6/openstackcluster_conversion.go b/api/v1alpha6/openstackcluster_conversion.go index 95131f6c3f..92fe7df622 100644 --- a/api/v1alpha6/openstackcluster_conversion.go +++ b/api/v1alpha6/openstackcluster_conversion.go @@ -367,12 +367,7 @@ func restorev1beta1ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst * dst.WorkerSecurityGroup = previous.WorkerSecurityGroup dst.BastionSecurityGroup = previous.BastionSecurityGroup - if previous.Bastion != nil { - dst.Bastion.Resolved = previous.Bastion.Resolved - } - if previous.Bastion != nil && previous.Bastion.Resources.Ports != nil { - dst.Bastion.Resources.Ports = previous.Bastion.Resources.Ports - } + restorev1beta1BastionStatus(previous.Bastion, dst.Bastion) } 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 /* Bastion */ func restorev1beta1Bastion(previous **infrav1.Bastion, dst **infrav1.Bastion) { - if *previous != nil { - if *dst != nil && (*previous).Spec != nil && (*dst).Spec != nil { - restorev1beta1MachineSpec((*previous).Spec, (*dst).Spec) - } - - optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP) - optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone) + if previous == nil || dst == nil || *previous == nil || *dst == nil { + return + } + if *dst != nil && (*previous).Spec != nil && (*dst).Spec != nil { + restorev1beta1MachineSpec((*previous).Spec, (*dst).Spec) } -} - -func Convert_v1alpha6_Instance_To_v1beta1_BastionStatus(in *Instance, out *infrav1.BastionStatus, _ apiconversion.Scope) error { - // BastionStatus is the same as Spec with unused fields removed - out.ID = in.ID - out.Name = in.Name - out.SSHKeyName = in.SSHKeyName - out.State = infrav1.InstanceState(in.State) - out.IP = in.IP - out.FloatingIP = in.FloatingIP - return nil -} -func Convert_v1beta1_BastionStatus_To_v1alpha6_Instance(in *infrav1.BastionStatus, out *Instance, _ apiconversion.Scope) error { - // BastionStatus is the same as Spec with unused fields removed - out.ID = in.ID - out.Name = in.Name - out.SSHKeyName = in.SSHKeyName - out.State = InstanceState(in.State) - out.IP = in.IP - out.FloatingIP = in.FloatingIP - return nil + optional.RestoreString(&(*previous).FloatingIP, &(*dst).FloatingIP) + optional.RestoreString(&(*previous).AvailabilityZone, &(*dst).AvailabilityZone) } 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 return optional.Convert_optional_String_To_string(&in.FloatingIP, &out.Instance.FloatingIP, s) } + +/* Bastion status */ + +func restorev1beta1BastionStatus(previous *infrav1.BastionStatus, dst *infrav1.BastionStatus) { + if previous == nil || dst == nil { + return + } + + // Resolved and resources have no equivalents + dst.Resolved = previous.Resolved + dst.Resources = previous.Resources +} + +func Convert_v1alpha6_Instance_To_v1beta1_BastionStatus(in *Instance, out *infrav1.BastionStatus, _ apiconversion.Scope) error { + // BastionStatus is the same as Spec with unused fields removed + out.ID = in.ID + out.Name = in.Name + out.SSHKeyName = in.SSHKeyName + out.State = infrav1.InstanceState(in.State) + out.IP = in.IP + out.FloatingIP = in.FloatingIP + return nil +} + +func Convert_v1beta1_BastionStatus_To_v1alpha6_Instance(in *infrav1.BastionStatus, out *Instance, _ apiconversion.Scope) error { + // BastionStatus is the same as Spec with unused fields removed + out.ID = in.ID + out.Name = in.Name + out.SSHKeyName = in.SSHKeyName + out.State = InstanceState(in.State) + out.IP = in.IP + out.FloatingIP = in.FloatingIP + return nil +} diff --git a/api/v1alpha6/openstackmachine_conversion.go b/api/v1alpha6/openstackmachine_conversion.go index d0f01cac1b..ad88c7afc0 100644 --- a/api/v1alpha6/openstackmachine_conversion.go +++ b/api/v1alpha6/openstackmachine_conversion.go @@ -90,13 +90,13 @@ var v1beta1OpenStackMachineRestorer = conversion.RestorerFor[*infrav1.OpenStackM restorev1beta1MachineSpec, ), "depresources": conversion.UnconditionalFieldRestorer( - func(c *infrav1.OpenStackMachine) *infrav1.MachineResources { + func(c *infrav1.OpenStackMachine) **infrav1.MachineResources { return &c.Status.Resources }, ), // No equivalent in v1alpha6 "refresources": conversion.UnconditionalFieldRestorer( - func(c *infrav1.OpenStackMachine) *infrav1.ResolvedMachineSpec { + func(c *infrav1.OpenStackMachine) **infrav1.ResolvedMachineSpec { return &c.Status.Resolved }, ), diff --git a/api/v1alpha7/openstackcluster_conversion.go b/api/v1alpha7/openstackcluster_conversion.go index 6cbd9a5118..3defae462a 100644 --- a/api/v1alpha7/openstackcluster_conversion.go +++ b/api/v1alpha7/openstackcluster_conversion.go @@ -348,14 +348,11 @@ func restorev1alpha7ClusterStatus(previous *OpenStackClusterStatus, dst *OpenSta } func restorev1beta1ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst *infrav1.OpenStackClusterStatus) { - // ReferencedResources have no equivalent in v1alpha7 - if previous.Bastion != nil { - dst.Bastion.Resolved = previous.Bastion.Resolved + if previous == nil || dst == nil { + return } - if previous.Bastion != nil && previous.Bastion.Resources.Ports != nil { - dst.Bastion.Resources.Ports = previous.Bastion.Resources.Ports - } + restorev1beta1BastionStatus(previous.Bastion, dst.Bastion) } 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 return optional.Convert_optional_String_To_string(&in.FloatingIP, &out.Instance.FloatingIP, s) } +/* Bastion status */ + +func restorev1beta1BastionStatus(previous *infrav1.BastionStatus, dst *infrav1.BastionStatus) { + if previous == nil || dst == nil { + return + } + + // Resolved and resources have no equivalents + dst.Resolved = previous.Resolved + dst.Resources = previous.Resources +} + func Convert_v1beta1_BastionStatus_To_v1alpha7_BastionStatus(in *infrav1.BastionStatus, out *BastionStatus, s apiconversion.Scope) error { // ReferencedResources have no equivalent in v1alpha7 return autoConvert_v1beta1_BastionStatus_To_v1alpha7_BastionStatus(in, out, s) diff --git a/api/v1alpha7/openstackmachine_conversion.go b/api/v1alpha7/openstackmachine_conversion.go index 08ef6f94ce..fa0655288c 100644 --- a/api/v1alpha7/openstackmachine_conversion.go +++ b/api/v1alpha7/openstackmachine_conversion.go @@ -77,13 +77,13 @@ var v1beta1OpenStackMachineRestorer = conversion.RestorerFor[*infrav1.OpenStackM restorev1beta1MachineSpec, ), "depresources": conversion.UnconditionalFieldRestorer( - func(c *infrav1.OpenStackMachine) *infrav1.MachineResources { + func(c *infrav1.OpenStackMachine) **infrav1.MachineResources { return &c.Status.Resources }, ), // No equivalent in v1alpha7 "refresources": conversion.UnconditionalFieldRestorer( - func(c *infrav1.OpenStackMachine) *infrav1.ResolvedMachineSpec { + func(c *infrav1.OpenStackMachine) **infrav1.ResolvedMachineSpec { return &c.Status.Resolved }, ), diff --git a/api/v1beta1/openstackmachine_types.go b/api/v1beta1/openstackmachine_types.go index 95e5edfe88..f222f71761 100644 --- a/api/v1beta1/openstackmachine_types.go +++ b/api/v1beta1/openstackmachine_types.go @@ -128,10 +128,12 @@ type OpenStackMachineStatus struct { // Resolved contains parts of the machine spec with all external // references fully resolved. - Resolved ResolvedMachineSpec `json:"resolved,omitempty"` + // +optional + Resolved *ResolvedMachineSpec `json:"resolved,omitempty"` // Resources contains references to OpenStack resources created for the machine. - Resources MachineResources `json:"resources,omitempty"` + // +optional + Resources *MachineResources `json:"resources,omitempty"` FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"` diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index a8a9288393..d2d90d8f6e 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -358,14 +358,21 @@ type AddressPair struct { } type BastionStatus struct { - ID string `json:"id,omitempty"` - Name string `json:"name,omitempty"` - SSHKeyName string `json:"sshKeyName,omitempty"` - State InstanceState `json:"state,omitempty"` - IP string `json:"ip,omitempty"` - FloatingIP string `json:"floatingIP,omitempty"` - Resolved ResolvedMachineSpec `json:"resolved,omitempty"` - Resources MachineResources `json:"resources,omitempty"` + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` + SSHKeyName string `json:"sshKeyName,omitempty"` + State InstanceState `json:"state,omitempty"` + IP string `json:"ip,omitempty"` + FloatingIP string `json:"floatingIP,omitempty"` + + // Resolved contains parts of the bastion's machine spec with all + // external references fully resolved. + // +optional + Resolved *ResolvedMachineSpec `json:"resolved,omitempty"` + + // Resources contains references to OpenStack resources created for the bastion. + // +optional + Resources *MachineResources `json:"resources,omitempty"` } type RootVolume struct { diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 892e950daf..b5d80bafff 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -146,8 +146,16 @@ func (in *Bastion) DeepCopy() *Bastion { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BastionStatus) DeepCopyInto(out *BastionStatus) { *out = *in - in.Resolved.DeepCopyInto(&out.Resolved) - in.Resources.DeepCopyInto(&out.Resources) + if in.Resolved != nil { + in, out := &in.Resolved, &out.Resolved + *out = new(ResolvedMachineSpec) + (*in).DeepCopyInto(*out) + } + if in.Resources != nil { + in, out := &in.Resources, &out.Resources + *out = new(MachineResources) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BastionStatus. @@ -964,8 +972,16 @@ func (in *OpenStackMachineStatus) DeepCopyInto(out *OpenStackMachineStatus) { *out = new(InstanceState) **out = **in } - in.Resolved.DeepCopyInto(&out.Resolved) - in.Resources.DeepCopyInto(&out.Resources) + if in.Resolved != nil { + in, out := &in.Resolved, &out.Resolved + *out = new(ResolvedMachineSpec) + (*in).DeepCopyInto(*out) + } + if in.Resources != nil { + in, out := &in.Resources, &out.Resources + *out = new(MachineResources) + (*in).DeepCopyInto(*out) + } if in.FailureReason != nil { in, out := &in.FailureReason, &out.FailureReason *out = new(errors.MachineStatusError) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index abdbf1c5da..4aa7ca14b0 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -6251,8 +6251,9 @@ spec: name: type: string resolved: - description: ResolvedMachineSpec contains resolved references - to resources required by the machine. + description: |- + Resolved contains parts of the bastion's machine spec with all + external references fully resolved. properties: imageID: description: ImageID is the ID of the image to use for the @@ -6435,6 +6436,8 @@ spec: type: string type: object resources: + description: Resources contains references to OpenStack resources + created for the bastion. properties: ports: description: Ports is the status of the ports created for diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index f1ab14b09c..1af03280f6 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -227,8 +227,13 @@ func resolveBastionResources(scope *scope.WithLogger, clusterResourceName string if openStackCluster.Spec.Bastion.Spec == nil { return false, fmt.Errorf("bastion spec is nil when bastion is enabled, this shouldn't happen") } + resolved := openStackCluster.Status.Bastion.Resolved + if resolved == nil { + resolved = &infrav1.ResolvedMachineSpec{} + openStackCluster.Status.Bastion.Resolved = resolved + } changed, err := compute.ResolveMachineSpec(scope, - openStackCluster.Spec.Bastion.Spec, &openStackCluster.Status.Bastion.Resolved, + openStackCluster.Spec.Bastion.Spec, resolved, clusterResourceName, bastionName(clusterResourceName), openStackCluster, getBastionSecurityGroupID(openStackCluster)) if err != nil { @@ -238,10 +243,13 @@ func resolveBastionResources(scope *scope.WithLogger, clusterResourceName string // If the resolved machine spec changed we need to restart the reconcile to avoid inconsistencies between reconciles. return true, nil } + resources := openStackCluster.Status.Bastion.Resources + if resources == nil { + resources = &infrav1.MachineResources{} + openStackCluster.Status.Bastion.Resources = resources + } - err = compute.AdoptMachineResources(scope, - &openStackCluster.Status.Bastion.Resolved, - &openStackCluster.Status.Bastion.Resources) + err = compute.AdoptMachineResources(scope, resolved, resources) if err != nil { return false, err } @@ -268,8 +276,10 @@ func deleteBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStac } } + bastionStatus := openStackCluster.Status.Bastion + var instanceStatus *compute.InstanceStatus - if openStackCluster.Status.Bastion != nil && openStackCluster.Status.Bastion.ID != "" { + if bastionStatus != nil && bastionStatus.ID != "" { instanceStatus, err = computeService.GetInstanceStatus(openStackCluster.Status.Bastion.ID) if err != nil { return err @@ -308,18 +318,18 @@ func deleteBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStac } } - if openStackCluster.Status.Bastion != nil && len(openStackCluster.Status.Bastion.Resources.Ports) > 0 { + if bastionStatus != nil && bastionStatus.Resources != nil { trunkSupported, err := networkingService.IsTrunkExtSupported() if err != nil { return err } - for _, port := range openStackCluster.Status.Bastion.Resources.Ports { + for _, port := range bastionStatus.Resources.Ports { if err := networkingService.DeleteInstanceTrunkAndPort(openStackCluster, port, trunkSupported); err != nil { handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete port: %w", err)) return fmt.Errorf("failed to delete port: %w", err) } } - openStackCluster.Status.Bastion.Resources.Ports = nil + bastionStatus.Resources.Ports = nil } scope.Logger().Info("Deleted Bastion") @@ -542,7 +552,10 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, cluster * // v1beta1 API validations prevent this from happening in normal circumstances. bastion.Spec = &infrav1.OpenStackMachineSpec{} } - resolved := &openStackCluster.Status.Bastion.Resolved + resolved := openStackCluster.Status.Bastion.Resolved + if resolved == nil { + return nil, errors.New("bastion resolved is nil") + } machineSpec := bastion.Spec instanceSpec := &compute.InstanceSpec{ @@ -579,7 +592,10 @@ func getBastionSecurityGroupID(openStackCluster *infrav1.OpenStackCluster) *stri func getOrCreateBastionPorts(openStackCluster *infrav1.OpenStackCluster, networkingService *networking.Service) error { desiredPorts := openStackCluster.Status.Bastion.Resolved.Ports - resources := &openStackCluster.Status.Bastion.Resources + resources := openStackCluster.Status.Bastion.Resources + if resources == nil { + return errors.New("bastion resources are nil") + } if len(desiredPorts) == len(resources.Ports) { return nil diff --git a/controllers/openstackcluster_controller_test.go b/controllers/openstackcluster_controller_test.go index b738cafe47..29cfd40a21 100644 --- a/controllers/openstackcluster_controller_test.go +++ b/controllers/openstackcluster_controller_test.go @@ -237,7 +237,7 @@ var _ = Describe("OpenStackCluster controller", func() { Expect(err).To(BeNil()) testCluster.Status = infrav1.OpenStackClusterStatus{ Bastion: &infrav1.BastionStatus{ - Resolved: infrav1.ResolvedMachineSpec{ + Resolved: &infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -245,7 +245,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, }, }, - Resources: infrav1.MachineResources{ + Resources: &infrav1.MachineResources{ Ports: []infrav1.PortStatus{ { ID: "portID1", @@ -286,7 +286,7 @@ var _ = Describe("OpenStackCluster controller", func() { expectedStatus := &infrav1.BastionStatus{ ID: "adopted-bastion-uuid", State: "ACTIVE", - Resolved: infrav1.ResolvedMachineSpec{ + Resolved: &infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -294,7 +294,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, }, }, - Resources: infrav1.MachineResources{ + Resources: &infrav1.MachineResources{ Ports: []infrav1.PortStatus{ { ID: "portID1", @@ -327,7 +327,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, Bastion: &infrav1.BastionStatus{ ID: "adopted-fip-bastion-uuid", - Resolved: infrav1.ResolvedMachineSpec{ + Resolved: &infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -335,7 +335,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, }, }, - Resources: infrav1.MachineResources{ + Resources: &infrav1.MachineResources{ Ports: []infrav1.PortStatus{ { ID: "portID1", @@ -369,7 +369,7 @@ var _ = Describe("OpenStackCluster controller", func() { ID: "adopted-fip-bastion-uuid", FloatingIP: "1.2.3.4", State: "ACTIVE", - Resolved: infrav1.ResolvedMachineSpec{ + Resolved: &infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -377,7 +377,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, }, }, - Resources: infrav1.MachineResources{ + Resources: &infrav1.MachineResources{ Ports: []infrav1.PortStatus{ { ID: "portID1", @@ -409,7 +409,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, Bastion: &infrav1.BastionStatus{ ID: "requeue-bastion-uuid", - Resolved: infrav1.ResolvedMachineSpec{ + Resolved: &infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -417,7 +417,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, }, }, - Resources: infrav1.MachineResources{ + Resources: &infrav1.MachineResources{ Ports: []infrav1.PortStatus{ { ID: "portID1", @@ -445,7 +445,7 @@ var _ = Describe("OpenStackCluster controller", func() { Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{ ID: "requeue-bastion-uuid", State: "BUILD", - Resolved: infrav1.ResolvedMachineSpec{ + Resolved: &infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -453,7 +453,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, }, }, - Resources: infrav1.MachineResources{ + Resources: &infrav1.MachineResources{ Ports: []infrav1.PortStatus{ { ID: "portID1", @@ -475,7 +475,7 @@ var _ = Describe("OpenStackCluster controller", func() { Expect(err).To(BeNil()) testCluster.Status = infrav1.OpenStackClusterStatus{ Bastion: &infrav1.BastionStatus{ - Resolved: infrav1.ResolvedMachineSpec{ + Resolved: &infrav1.ResolvedMachineSpec{ ImageID: "imageID", }, }, @@ -528,7 +528,7 @@ var _ = Describe("OpenStackCluster controller", func() { } testCluster.Status = infrav1.OpenStackClusterStatus{ Bastion: &infrav1.BastionStatus{ - Resources: infrav1.MachineResources{ + Resources: &infrav1.MachineResources{ Ports: []infrav1.PortStatus{ { ID: "port-id", @@ -613,7 +613,7 @@ var _ = Describe("OpenStackCluster controller", func() { } testCluster.Status = infrav1.OpenStackClusterStatus{ Bastion: &infrav1.BastionStatus{ - Resources: infrav1.MachineResources{ + Resources: &infrav1.MachineResources{ Ports: []infrav1.PortStatus{ { ID: "port-id", diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index 246cf2bfc3..969ec5b61a 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -92,6 +92,7 @@ func (r *OpenStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req } log = log.WithValues("openStackMachine", openStackMachine.Name) + log.V(4).Info("Reconciling OpenStackMachine") // Fetch the Machine. machine, err := util.GetOwnerMachine(ctx, r.Client, openStackMachine.ObjectMeta) @@ -162,9 +163,14 @@ func (r *OpenStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req } func resolveMachineResources(scope *scope.WithLogger, clusterResourceName string, openStackCluster *infrav1.OpenStackCluster, openStackMachine *infrav1.OpenStackMachine, machine *clusterv1.Machine) (bool, error) { + resolved := openStackMachine.Status.Resolved + if resolved == nil { + resolved = &infrav1.ResolvedMachineSpec{} + openStackMachine.Status.Resolved = resolved + } // Resolve and store resources changed, err := compute.ResolveMachineSpec(scope, - &openStackMachine.Spec, &openStackMachine.Status.Resolved, + &openStackMachine.Spec, resolved, clusterResourceName, openStackMachine.Name, openStackCluster, getManagedSecurityGroup(openStackCluster, machine)) if err != nil { @@ -175,8 +181,14 @@ func resolveMachineResources(scope *scope.WithLogger, clusterResourceName string return true, nil } + resources := openStackMachine.Status.Resources + if resources == nil { + resources = &infrav1.MachineResources{} + openStackMachine.Status.Resources = resources + } + // Adopt any existing resources - return false, compute.AdoptMachineResources(scope, &openStackMachine.Status.Resolved, &openStackMachine.Status.Resources) + return false, compute.AdoptMachineResources(scope, resolved, resources) } func patchMachine(ctx context.Context, patchHelper *patch.Helper, openStackMachine *infrav1.OpenStackMachine, machine *clusterv1.Machine, options ...patch.Option) error { @@ -296,7 +308,10 @@ func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, cl } } - instanceSpec := machineToInstanceSpec(openStackCluster, machine, openStackMachine, "") + instanceSpec, err := machineToInstanceSpec(openStackCluster, machine, openStackMachine, "") + if err != nil { + return ctrl.Result{}, err + } if err := computeService.DeleteInstance(openStackMachine, instanceStatus, instanceSpec); err != nil { conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceDeleteFailedReason, clusterv1.ConditionSeverityError, "Deleting instance failed: %v", err) @@ -308,10 +323,12 @@ func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, cl return ctrl.Result{}, err } - portsStatus := openStackMachine.Status.Resources.Ports - for _, port := range portsStatus { - if err := networkingService.DeleteInstanceTrunkAndPort(openStackMachine, port, trunkSupported); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to delete port %q: %w", port.ID, err) + if openStackMachine.Status.Resources != nil { + portsStatus := openStackMachine.Status.Resources.Ports + for _, port := range portsStatus { + if err := networkingService.DeleteInstanceTrunkAndPort(openStackMachine, port, trunkSupported); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to delete port %q: %w", port.ID, err) + } } } @@ -485,6 +502,7 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope } if changed, err := resolveMachineResources(scope, clusterResourceName, openStackCluster, openStackMachine, machine); changed || err != nil { + scope.Logger().V(6).Info("Machine resources updated, requeuing") return ctrl.Result{}, err } @@ -658,8 +676,15 @@ func (r *OpenStackMachineReconciler) reconcileAPIServerLoadBalancer(scope *scope } func getOrCreateMachinePorts(openStackMachine *infrav1.OpenStackMachine, networkingService *networking.Service) error { - desiredPorts := openStackMachine.Status.Resolved.Ports - resources := &openStackMachine.Status.Resources + resolved := openStackMachine.Status.Resolved + if resolved == nil { + return errors.New("machine resolved is nil") + } + resources := openStackMachine.Status.Resources + if resources == nil { + return errors.New("machine resources is nil") + } + desiredPorts := resolved.Ports if len(desiredPorts) == len(resources.Ports) { return nil @@ -696,7 +721,11 @@ func (r *OpenStackMachineReconciler) getOrCreateInstance(logger logr.Logger, ope return nil, nil } } - instanceSpec := machineToInstanceSpec(openStackCluster, machine, openStackMachine, userData) + instanceSpec, err := machineToInstanceSpec(openStackCluster, machine, openStackMachine, userData) + if err != nil { + return nil, err + } + logger.Info("Machine does not exist, creating Machine", "name", openStackMachine.Name) instanceStatus, err = computeService.CreateInstance(openStackMachine, instanceSpec, portIDs) if err != nil { @@ -707,7 +736,12 @@ func (r *OpenStackMachineReconciler) getOrCreateInstance(logger logr.Logger, ope return instanceStatus, nil } -func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, userData string) *compute.InstanceSpec { +func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, userData string) (*compute.InstanceSpec, error) { + resolved := openStackMachine.Status.Resolved + if resolved == nil { + return nil, errors.New("machine resolved is nil") + } + serverMetadata := make(map[string]string, len(openStackMachine.Spec.ServerMetadata)) for i := range openStackMachine.Spec.ServerMetadata { key := openStackMachine.Spec.ServerMetadata[i].Key @@ -717,7 +751,7 @@ func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine * instanceSpec := compute.InstanceSpec{ Name: openStackMachine.Name, - ImageID: openStackMachine.Status.Resolved.ImageID, + ImageID: resolved.ImageID, Flavor: openStackMachine.Spec.Flavor, SSHKeyName: openStackMachine.Spec.SSHKeyName, UserData: userData, @@ -725,7 +759,7 @@ func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine * ConfigDrive: openStackMachine.Spec.ConfigDrive != nil && *openStackMachine.Spec.ConfigDrive, RootVolume: openStackMachine.Spec.RootVolume, AdditionalBlockDevices: openStackMachine.Spec.AdditionalBlockDevices, - ServerGroupID: openStackMachine.Status.Resolved.ServerGroupID, + ServerGroupID: resolved.ServerGroupID, Trunk: openStackMachine.Spec.Trunk, } @@ -736,7 +770,7 @@ func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine * instanceSpec.Tags = compute.InstanceTags(&openStackMachine.Spec, openStackCluster) - return &instanceSpec + return &instanceSpec, nil } // getManagedSecurityGroup returns the ID of the security group managed by the diff --git a/controllers/openstackmachine_controller_test.go b/controllers/openstackmachine_controller_test.go index bc2b4897f3..c941f11d04 100644 --- a/controllers/openstackmachine_controller_test.go +++ b/controllers/openstackmachine_controller_test.go @@ -95,7 +95,7 @@ func getDefaultOpenStackMachine() *infrav1.OpenStackMachine { ServerGroup: &infrav1.ServerGroupFilter{ID: serverGroupUUID}, }, Status: infrav1.OpenStackMachineStatus{ - Resolved: infrav1.ResolvedMachineSpec{ + Resolved: &infrav1.ResolvedMachineSpec{ ImageID: imageUUID, ServerGroupID: serverGroupUUID, }, @@ -160,7 +160,7 @@ func Test_machineToInstanceSpec(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - got := machineToInstanceSpec(tt.openStackCluster(), tt.machine(), tt.openStackMachine(), "user-data") + got, _ := machineToInstanceSpec(tt.openStackCluster(), tt.machine(), tt.openStackMachine(), "user-data") wanted := tt.wantInstanceSpec() g.Expect(got).To(Equal(wanted), cmp.Diff(got, wanted)) diff --git a/docs/book/src/api/v1beta1/api.md b/docs/book/src/api/v1beta1/api.md index fc84797f7a..c60863aaf2 100644 --- a/docs/book/src/api/v1beta1/api.md +++ b/docs/book/src/api/v1beta1/api.md @@ -1220,6 +1220,9 @@ ResolvedMachineSpec
Resolved contains parts of the bastion’s machine spec with all +external references fully resolved.
Resources contains references to OpenStack resources created for the bastion.
Resolved contains parts of the machine spec with all external references fully resolved.
Resources contains references to OpenStack resources created for the machine.