From 5d11950182257ee750db3c84534ba4621b100544 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Thu, 21 Mar 2024 13:02:10 +0000 Subject: [PATCH] Rename ReferencedResources to Resolved This struct has already moved beyond external references, and seems be evolving towards a fully-resolved machine spec. For want of a better name we call it 'resolved', which is more accurate. --- api/v1alpha5/conversion.go | 4 +- api/v1alpha5/conversion_test.go | 2 +- api/v1alpha5/zz_generated.conversion.go | 2 +- api/v1alpha6/openstackcluster_conversion.go | 2 +- api/v1alpha6/openstackmachine_conversion.go | 4 +- api/v1alpha6/zz_generated.conversion.go | 2 +- api/v1alpha7/openstackcluster_conversion.go | 2 +- api/v1alpha7/openstackmachine_conversion.go | 4 +- api/v1alpha7/zz_generated.conversion.go | 4 +- api/v1beta1/openstackmachine_types.go | 5 +- api/v1beta1/types.go | 20 ++--- api/v1beta1/zz_generated.deepcopy.go | 48 +++++------ ...re.cluster.x-k8s.io_openstackclusters.yaml | 4 +- ...re.cluster.x-k8s.io_openstackmachines.yaml | 7 +- controllers/openstackcluster_controller.go | 16 ++-- .../openstackcluster_controller_test.go | 14 ++-- controllers/openstackmachine_controller.go | 16 ++-- .../openstackmachine_controller_test.go | 2 +- docs/book/src/api/v1beta1/api.md | 81 ++++++++++--------- .../services/compute/machine_resources.go | 2 +- .../services/compute/referenced_resources.go | 28 +++---- .../compute/referenced_resources_test.go | 20 ++--- 22 files changed, 146 insertions(+), 143 deletions(-) diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go index d39cf87314..5c2516b891 100644 --- a/api/v1alpha5/conversion.go +++ b/api/v1alpha5/conversion.go @@ -455,7 +455,7 @@ func Convert_v1alpha5_Instance_To_v1beta1_BastionStatus(in *Instance, out *infra out.State = infrav1.InstanceState(in.State) out.IP = in.IP out.FloatingIP = in.FloatingIP - out.ReferencedResources.ServerGroupID = in.ServerGroupID + out.Resolved.ServerGroupID = in.ServerGroupID return nil } @@ -467,7 +467,7 @@ func Convert_v1beta1_BastionStatus_To_v1alpha5_Instance(in *infrav1.BastionStatu out.State = InstanceState(in.State) out.IP = in.IP out.FloatingIP = in.FloatingIP - out.ServerGroupID = in.ReferencedResources.ServerGroupID + out.ServerGroupID = in.Resolved.ServerGroupID return nil } diff --git a/api/v1alpha5/conversion_test.go b/api/v1alpha5/conversion_test.go index b7dbbbac2b..82afcd82e8 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,"referencedResources":{},"resources":{}}}`, + "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"flavor\":\"\",\"image\":{}},\"status\":{\"ready\":false,\"resolved\":{},\"resources\":{}}}", }, }, }, diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index c7cac5b353..f5d208397e 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -1212,7 +1212,7 @@ func autoConvert_v1beta1_OpenStackMachineStatus_To_v1alpha5_OpenStackMachineStat out.Ready = in.Ready out.Addresses = *(*[]corev1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.InstanceState = (*InstanceState)(unsafe.Pointer(in.InstanceState)) - // WARNING: in.ReferencedResources requires manual conversion: does not exist in peer-type + // WARNING: in.Resolved requires manual conversion: does not exist in peer-type // WARNING: in.Resources requires manual conversion: does not exist in peer-type out.FailureReason = (*errors.MachineStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) diff --git a/api/v1alpha6/openstackcluster_conversion.go b/api/v1alpha6/openstackcluster_conversion.go index 8d0a87efea..95131f6c3f 100644 --- a/api/v1alpha6/openstackcluster_conversion.go +++ b/api/v1alpha6/openstackcluster_conversion.go @@ -368,7 +368,7 @@ func restorev1beta1ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst * dst.BastionSecurityGroup = previous.BastionSecurityGroup if previous.Bastion != nil { - dst.Bastion.ReferencedResources = previous.Bastion.ReferencedResources + dst.Bastion.Resolved = previous.Bastion.Resolved } if previous.Bastion != nil && previous.Bastion.Resources.Ports != nil { dst.Bastion.Resources.Ports = previous.Bastion.Resources.Ports diff --git a/api/v1alpha6/openstackmachine_conversion.go b/api/v1alpha6/openstackmachine_conversion.go index 7f92ec0b65..d0f01cac1b 100644 --- a/api/v1alpha6/openstackmachine_conversion.go +++ b/api/v1alpha6/openstackmachine_conversion.go @@ -96,8 +96,8 @@ var v1beta1OpenStackMachineRestorer = conversion.RestorerFor[*infrav1.OpenStackM ), // No equivalent in v1alpha6 "refresources": conversion.UnconditionalFieldRestorer( - func(c *infrav1.OpenStackMachine) *infrav1.ReferencedMachineResources { - return &c.Status.ReferencedResources + func(c *infrav1.OpenStackMachine) *infrav1.ResolvedMachineSpec { + return &c.Status.Resolved }, ), } diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index e9bdedeb0f..b35e716ad4 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -1247,7 +1247,7 @@ func autoConvert_v1beta1_OpenStackMachineStatus_To_v1alpha6_OpenStackMachineStat out.Ready = in.Ready out.Addresses = *(*[]corev1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.InstanceState = (*InstanceState)(unsafe.Pointer(in.InstanceState)) - // WARNING: in.ReferencedResources requires manual conversion: does not exist in peer-type + // WARNING: in.Resolved requires manual conversion: does not exist in peer-type // WARNING: in.Resources requires manual conversion: does not exist in peer-type out.FailureReason = (*errors.MachineStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) diff --git a/api/v1alpha7/openstackcluster_conversion.go b/api/v1alpha7/openstackcluster_conversion.go index 641148b654..6cbd9a5118 100644 --- a/api/v1alpha7/openstackcluster_conversion.go +++ b/api/v1alpha7/openstackcluster_conversion.go @@ -350,7 +350,7 @@ 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.ReferencedResources = previous.Bastion.ReferencedResources + dst.Bastion.Resolved = previous.Bastion.Resolved } if previous.Bastion != nil && previous.Bastion.Resources.Ports != nil { diff --git a/api/v1alpha7/openstackmachine_conversion.go b/api/v1alpha7/openstackmachine_conversion.go index 9ee3b01c8c..08ef6f94ce 100644 --- a/api/v1alpha7/openstackmachine_conversion.go +++ b/api/v1alpha7/openstackmachine_conversion.go @@ -83,8 +83,8 @@ var v1beta1OpenStackMachineRestorer = conversion.RestorerFor[*infrav1.OpenStackM ), // No equivalent in v1alpha7 "refresources": conversion.UnconditionalFieldRestorer( - func(c *infrav1.OpenStackMachine) *infrav1.ReferencedMachineResources { - return &c.Status.ReferencedResources + func(c *infrav1.OpenStackMachine) *infrav1.ResolvedMachineSpec { + return &c.Status.Resolved }, ), } diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go index f7b10f6064..5646dbe6f8 100644 --- a/api/v1alpha7/zz_generated.conversion.go +++ b/api/v1alpha7/zz_generated.conversion.go @@ -573,7 +573,7 @@ func autoConvert_v1beta1_BastionStatus_To_v1alpha7_BastionStatus(in *v1beta1.Bas out.State = InstanceState(in.State) out.IP = in.IP out.FloatingIP = in.FloatingIP - // WARNING: in.ReferencedResources requires manual conversion: does not exist in peer-type + // WARNING: in.Resolved requires manual conversion: does not exist in peer-type // WARNING: in.Resources requires manual conversion: does not exist in peer-type return nil } @@ -1447,7 +1447,7 @@ func autoConvert_v1beta1_OpenStackMachineStatus_To_v1alpha7_OpenStackMachineStat out.Ready = in.Ready out.Addresses = *(*[]corev1.NodeAddress)(unsafe.Pointer(&in.Addresses)) out.InstanceState = (*InstanceState)(unsafe.Pointer(in.InstanceState)) - // WARNING: in.ReferencedResources requires manual conversion: does not exist in peer-type + // WARNING: in.Resolved requires manual conversion: does not exist in peer-type // WARNING: in.Resources requires manual conversion: does not exist in peer-type out.FailureReason = (*errors.MachineStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) diff --git a/api/v1beta1/openstackmachine_types.go b/api/v1beta1/openstackmachine_types.go index 43d4f05574..95e5edfe88 100644 --- a/api/v1beta1/openstackmachine_types.go +++ b/api/v1beta1/openstackmachine_types.go @@ -126,8 +126,9 @@ type OpenStackMachineStatus struct { // +optional InstanceState *InstanceState `json:"instanceState,omitempty"` - // ReferencedResources contains resolved references to resources that the machine depends on. - ReferencedResources ReferencedMachineResources `json:"referencedResources,omitempty"` + // Resolved contains parts of the machine spec with all external + // references fully resolved. + Resolved ResolvedMachineSpec `json:"resolved,omitempty"` // Resources contains references to OpenStack resources created for the machine. Resources MachineResources `json:"resources,omitempty"` diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 01316d955f..a8a9288393 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -358,14 +358,14 @@ 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"` - ReferencedResources ReferencedMachineResources `json:"referencedResources,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 ResolvedMachineSpec `json:"resolved,omitempty"` + Resources MachineResources `json:"resources,omitempty"` } type RootVolume struct { @@ -658,8 +658,8 @@ func (s *APIServerLoadBalancer) IsEnabled() bool { return s != nil && (s.Enabled == nil || *s.Enabled) } -// ReferencedMachineResources contains resolved references to resources required by the machine. -type ReferencedMachineResources struct { +// ResolvedMachineSpec contains resolved references to resources required by the machine. +type ResolvedMachineSpec struct { // ServerGroupID is the ID of the server group the machine should be added to and is calculated based on ServerGroupFilter. // +optional ServerGroupID string `json:"serverGroupID,omitempty"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index d9e22e65c4..892e950daf 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -146,7 +146,7 @@ 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.ReferencedResources.DeepCopyInto(&out.ReferencedResources) + in.Resolved.DeepCopyInto(&out.Resolved) in.Resources.DeepCopyInto(&out.Resources) } @@ -964,7 +964,7 @@ func (in *OpenStackMachineStatus) DeepCopyInto(out *OpenStackMachineStatus) { *out = new(InstanceState) **out = **in } - in.ReferencedResources.DeepCopyInto(&out.ReferencedResources) + in.Resolved.DeepCopyInto(&out.Resolved) in.Resources.DeepCopyInto(&out.Resources) if in.FailureReason != nil { in, out := &in.FailureReason, &out.FailureReason @@ -1155,28 +1155,6 @@ func (in *PortStatus) DeepCopy() *PortStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ReferencedMachineResources) DeepCopyInto(out *ReferencedMachineResources) { - *out = *in - if in.Ports != nil { - in, out := &in.Ports, &out.Ports - *out = make([]ResolvedPortSpec, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ReferencedMachineResources. -func (in *ReferencedMachineResources) DeepCopy() *ReferencedMachineResources { - if in == nil { - return nil - } - out := new(ReferencedMachineResources) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResolvedFixedIP) DeepCopyInto(out *ResolvedFixedIP) { *out = *in @@ -1202,6 +1180,28 @@ func (in *ResolvedFixedIP) DeepCopy() *ResolvedFixedIP { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ResolvedMachineSpec) DeepCopyInto(out *ResolvedMachineSpec) { + *out = *in + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = make([]ResolvedPortSpec, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ResolvedMachineSpec. +func (in *ResolvedMachineSpec) DeepCopy() *ResolvedMachineSpec { + if in == nil { + return nil + } + out := new(ResolvedMachineSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResolvedPortSpec) DeepCopyInto(out *ResolvedPortSpec) { *out = *in 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 fda2df3277..abdbf1c5da 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -6250,8 +6250,8 @@ spec: type: string name: type: string - referencedResources: - description: ReferencedMachineResources contains resolved references + resolved: + description: ResolvedMachineSpec contains resolved references to resources required by the machine. properties: imageID: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml index 8cea0408b3..3d866edc37 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -2453,9 +2453,10 @@ spec: ready: description: Ready is true when the provider resource is ready. type: boolean - referencedResources: - description: ReferencedResources contains resolved references to resources - that the machine depends on. + resolved: + description: |- + Resolved contains parts of the machine spec with all external + references fully resolved. properties: imageID: description: ImageID is the ID of the image to use for the machine diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index dbc2527042..f1ab14b09c 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -227,20 +227,20 @@ 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") } - changed, err := compute.ResolveReferencedMachineResources(scope, - openStackCluster.Spec.Bastion.Spec, &openStackCluster.Status.Bastion.ReferencedResources, + changed, err := compute.ResolveMachineSpec(scope, + openStackCluster.Spec.Bastion.Spec, &openStackCluster.Status.Bastion.Resolved, clusterResourceName, bastionName(clusterResourceName), openStackCluster, getBastionSecurityGroupID(openStackCluster)) if err != nil { return false, err } if changed { - // If the referenced resources have changed, we need to update the OpenStackCluster status now. + // If the resolved machine spec changed we need to restart the reconcile to avoid inconsistencies between reconciles. return true, nil } err = compute.AdoptMachineResources(scope, - &openStackCluster.Status.Bastion.ReferencedResources, + &openStackCluster.Status.Bastion.Resolved, &openStackCluster.Status.Bastion.Resources) if err != nil { return false, err @@ -542,16 +542,16 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, cluster * // v1beta1 API validations prevent this from happening in normal circumstances. bastion.Spec = &infrav1.OpenStackMachineSpec{} } - referencedResources := &openStackCluster.Status.Bastion.ReferencedResources + resolved := &openStackCluster.Status.Bastion.Resolved machineSpec := bastion.Spec instanceSpec := &compute.InstanceSpec{ Name: bastionName(cluster.Name), Flavor: machineSpec.Flavor, SSHKeyName: machineSpec.SSHKeyName, - ImageID: referencedResources.ImageID, + ImageID: resolved.ImageID, RootVolume: machineSpec.RootVolume, - ServerGroupID: referencedResources.ServerGroupID, + ServerGroupID: resolved.ServerGroupID, Tags: compute.InstanceTags(machineSpec, openStackCluster), } if bastion.AvailabilityZone != nil { @@ -578,7 +578,7 @@ func getBastionSecurityGroupID(openStackCluster *infrav1.OpenStackCluster) *stri } func getOrCreateBastionPorts(openStackCluster *infrav1.OpenStackCluster, networkingService *networking.Service) error { - desiredPorts := openStackCluster.Status.Bastion.ReferencedResources.Ports + desiredPorts := openStackCluster.Status.Bastion.Resolved.Ports resources := &openStackCluster.Status.Bastion.Resources if len(desiredPorts) == len(resources.Ports) { diff --git a/controllers/openstackcluster_controller_test.go b/controllers/openstackcluster_controller_test.go index a7aa20892a..b738cafe47 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{ - ReferencedResources: infrav1.ReferencedMachineResources{ + Resolved: infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -286,7 +286,7 @@ var _ = Describe("OpenStackCluster controller", func() { expectedStatus := &infrav1.BastionStatus{ ID: "adopted-bastion-uuid", State: "ACTIVE", - ReferencedResources: infrav1.ReferencedMachineResources{ + Resolved: infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -327,7 +327,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, Bastion: &infrav1.BastionStatus{ ID: "adopted-fip-bastion-uuid", - ReferencedResources: infrav1.ReferencedMachineResources{ + Resolved: infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -369,7 +369,7 @@ var _ = Describe("OpenStackCluster controller", func() { ID: "adopted-fip-bastion-uuid", FloatingIP: "1.2.3.4", State: "ACTIVE", - ReferencedResources: infrav1.ReferencedMachineResources{ + Resolved: infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -409,7 +409,7 @@ var _ = Describe("OpenStackCluster controller", func() { }, Bastion: &infrav1.BastionStatus{ ID: "requeue-bastion-uuid", - ReferencedResources: infrav1.ReferencedMachineResources{ + Resolved: infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -445,7 +445,7 @@ var _ = Describe("OpenStackCluster controller", func() { Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{ ID: "requeue-bastion-uuid", State: "BUILD", - ReferencedResources: infrav1.ReferencedMachineResources{ + Resolved: infrav1.ResolvedMachineSpec{ ImageID: "imageID", Ports: []infrav1.ResolvedPortSpec{ { @@ -475,7 +475,7 @@ var _ = Describe("OpenStackCluster controller", func() { Expect(err).To(BeNil()) testCluster.Status = infrav1.OpenStackClusterStatus{ Bastion: &infrav1.BastionStatus{ - ReferencedResources: infrav1.ReferencedMachineResources{ + Resolved: infrav1.ResolvedMachineSpec{ ImageID: "imageID", }, }, diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index a537d389f0..246cf2bfc3 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -162,21 +162,21 @@ 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) { - // Resolve and store referenced resources - changed, err := compute.ResolveReferencedMachineResources(scope, - &openStackMachine.Spec, &openStackMachine.Status.ReferencedResources, + // Resolve and store resources + changed, err := compute.ResolveMachineSpec(scope, + &openStackMachine.Spec, &openStackMachine.Status.Resolved, clusterResourceName, openStackMachine.Name, openStackCluster, getManagedSecurityGroup(openStackCluster, machine)) if err != nil { return false, err } if changed { - // If the referenced resources have changed, we need to update the OpenStackMachine status now. + // If the resolved machine spec changed we need to start the reconcile again to prevent inconsistency between reconciles. return true, nil } // Adopt any existing resources - return false, compute.AdoptMachineResources(scope, &openStackMachine.Status.ReferencedResources, &openStackMachine.Status.Resources) + return false, compute.AdoptMachineResources(scope, &openStackMachine.Status.Resolved, &openStackMachine.Status.Resources) } func patchMachine(ctx context.Context, patchHelper *patch.Helper, openStackMachine *infrav1.OpenStackMachine, machine *clusterv1.Machine, options ...patch.Option) error { @@ -658,7 +658,7 @@ func (r *OpenStackMachineReconciler) reconcileAPIServerLoadBalancer(scope *scope } func getOrCreateMachinePorts(openStackMachine *infrav1.OpenStackMachine, networkingService *networking.Service) error { - desiredPorts := openStackMachine.Status.ReferencedResources.Ports + desiredPorts := openStackMachine.Status.Resolved.Ports resources := &openStackMachine.Status.Resources if len(desiredPorts) == len(resources.Ports) { @@ -717,7 +717,7 @@ func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine * instanceSpec := compute.InstanceSpec{ Name: openStackMachine.Name, - ImageID: openStackMachine.Status.ReferencedResources.ImageID, + ImageID: openStackMachine.Status.Resolved.ImageID, Flavor: openStackMachine.Spec.Flavor, SSHKeyName: openStackMachine.Spec.SSHKeyName, UserData: userData, @@ -725,7 +725,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.ReferencedResources.ServerGroupID, + ServerGroupID: openStackMachine.Status.Resolved.ServerGroupID, Trunk: openStackMachine.Spec.Trunk, } diff --git a/controllers/openstackmachine_controller_test.go b/controllers/openstackmachine_controller_test.go index 1351f3e2f6..bc2b4897f3 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{ - ReferencedResources: infrav1.ReferencedMachineResources{ + Resolved: infrav1.ResolvedMachineSpec{ ImageID: imageUUID, ServerGroupID: serverGroupUUID, }, diff --git a/docs/book/src/api/v1beta1/api.md b/docs/book/src/api/v1beta1/api.md index 8909adc3c7..fc84797f7a 100644 --- a/docs/book/src/api/v1beta1/api.md +++ b/docs/book/src/api/v1beta1/api.md @@ -1212,10 +1212,10 @@ string -referencedResources
+resolved
- -ReferencedMachineResources + +ResolvedMachineSpec @@ -3286,15 +3286,16 @@ InstanceState -referencedResources
+resolved
- -ReferencedMachineResources + +ResolvedMachineSpec -

ReferencedResources contains resolved references to resources that the machine depends on.

+

Resolved contains parts of the machine spec with all external +references fully resolved.

@@ -3789,15 +3790,14 @@ string -

ReferencedMachineResources +

ResolvedFixedIP

(Appears on: -BastionStatus, -OpenStackMachineStatus) +ResolvedPortSpec)

-

ReferencedMachineResources contains resolved references to resources required by the machine.

+

ResolvedFixedIP is a FixedIP with the Subnet resolved to an ID.

@@ -3809,52 +3809,42 @@ string - - - -
-serverGroupID
+subnet
string 		(Optional) -

ServerGroupID is the ID of the server group the machine should be added to and is calculated based on ServerGroupFilter.

+

SubnetID is the id of a subnet to create the fixed IP of a port in.
-imageID
+ipAddress
string 		(Optional) -

ImageID is the ID of the image to use for the machine and is calculated based on ImageFilter.

-
-ports
- - -[]ResolvedPortSpec - - -		 -(Optional) -

Ports is the fully resolved list of ports to create for the machine.

+

IPAddress is a specific IP address to assign to the port. If SubnetID +is also specified, IPAddress must be a valid IP address in the +subnet. If Subnet is not specified, IPAddress must be a valid IP +address in any subnet of the port’s network.
-

ResolvedFixedIP +

ResolvedMachineSpec

(Appears on: -ResolvedPortSpec) +BastionStatus, +OpenStackMachineStatus)

-

ResolvedFixedIP is a FixedIP with the Subnet resolved to an ID.

+

ResolvedMachineSpec contains resolved references to resources required by the machine.

@@ -3866,29 +3856,40 @@ string + + + + @@ -3897,7 +3898,7 @@ address in any subnet of the port’s network.

(Appears on: -ReferencedMachineResources) +ResolvedMachineSpec)

ResolvedPortSpec is a PortOpts with all contained references fully resolved.

diff --git a/pkg/cloud/services/compute/machine_resources.go b/pkg/cloud/services/compute/machine_resources.go index 7061fed468..ced3faff76 100644 --- a/pkg/cloud/services/compute/machine_resources.go +++ b/pkg/cloud/services/compute/machine_resources.go @@ -22,7 +22,7 @@ import ( "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) -func AdoptMachineResources(scope *scope.WithLogger, referencedResources *infrav1.ReferencedMachineResources, resources *infrav1.MachineResources) error { +func AdoptMachineResources(scope *scope.WithLogger, referencedResources *infrav1.ResolvedMachineSpec, resources *infrav1.MachineResources) error { networkingService, err := networking.NewService(scope) if err != nil { return err diff --git a/pkg/cloud/services/compute/referenced_resources.go b/pkg/cloud/services/compute/referenced_resources.go index 72cd91047e..c571dfe365 100644 --- a/pkg/cloud/services/compute/referenced_resources.go +++ b/pkg/cloud/services/compute/referenced_resources.go @@ -25,13 +25,13 @@ import ( "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) -// ResolveReferencedMachineResources is responsible for populating ReferencedMachineResources with IDs of -// the resources referenced in the OpenStackMachineSpec by querying the OpenStack APIs. It'll return error -// if resources cannot be found or their filters are ambiguous. -// Note that we only set the fields in ReferencedMachineResources that are not set yet. This is ok because: +// ResolveMachineSpec is responsible for populating a ResolvedMachineSpec from +// an OpenStackMachineSpec and any external dependencies. The result contains no +// external dependencies, and does not require any complex logic on creation. +// Note that we only set the fields in ResolvedMachineSpec that are not set yet. This is ok because: // - OpenStackMachine is immutable, so we can't change the spec after the machine is created. // - the bastion is mutable, but we delete the bastion when the spec changes, so the bastion status will be empty. -func ResolveReferencedMachineResources(scope *scope.WithLogger, spec *infrav1.OpenStackMachineSpec, resources *infrav1.ReferencedMachineResources, clusterResourceName, baseName string, openStackCluster *infrav1.OpenStackCluster, managedSecurityGroup *string) (changed bool, err error) { +func ResolveMachineSpec(scope *scope.WithLogger, spec *infrav1.OpenStackMachineSpec, resolved *infrav1.ResolvedMachineSpec, clusterResourceName, baseName string, openStackCluster *infrav1.OpenStackCluster, managedSecurityGroup *string) (changed bool, err error) { changed = false computeService, err := NewService(scope) @@ -44,23 +44,23 @@ func ResolveReferencedMachineResources(scope *scope.WithLogger, spec *infrav1.Op return changed, err } - // ServerGroup is optional, so we only need to resolve it if it's set in the spec and not in ReferencedMachineResources yet. - if spec.ServerGroup != nil && resources.ServerGroupID == "" { + // ServerGroup is optional, so we only need to resolve it if it's set in the spec + if spec.ServerGroup != nil && resolved.ServerGroupID == "" { serverGroupID, err := computeService.GetServerGroupID(spec.ServerGroup) if err != nil { return changed, err } - resources.ServerGroupID = serverGroupID + resolved.ServerGroupID = serverGroupID changed = true } - // Image is required, so we need to resolve it if it's not set in ReferencedMachineResources yet. - if resources.ImageID == "" { + // Image is required, so we need to resolve it if it's not set + if resolved.ImageID == "" { imageID, err := computeService.GetImageID(spec.Image) if err != nil { return changed, err } - resources.ImageID = imageID + resolved.ImageID = imageID changed = true } @@ -68,17 +68,17 @@ func ResolveReferencedMachineResources(scope *scope.WithLogger, spec *infrav1.Op // call this from places where we know it should have been set, but the // cluster status is externally-provided data so we check it anyway. if openStackCluster.Status.Network == nil { - return changed, fmt.Errorf("called ResolveReferencedMachineResources with nil OpenStackCluster.Status.Network") + return changed, fmt.Errorf("called ResolveMachineSpec with nil OpenStackCluster.Status.Network") } // Network resources are required in order to get ports options. - if len(resources.Ports) == 0 { + if len(resolved.Ports) == 0 { defaultNetwork := openStackCluster.Status.Network portsOpts, err := networkingService.ConstructPorts(spec, clusterResourceName, baseName, defaultNetwork, managedSecurityGroup, InstanceTags(spec, openStackCluster)) if err != nil { return changed, err } - resources.Ports = portsOpts + resolved.Ports = portsOpts changed = true } diff --git a/pkg/cloud/services/compute/referenced_resources_test.go b/pkg/cloud/services/compute/referenced_resources_test.go index 47d2a6adb9..1f13ce5427 100644 --- a/pkg/cloud/services/compute/referenced_resources_test.go +++ b/pkg/cloud/services/compute/referenced_resources_test.go @@ -60,8 +60,8 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { expectComputeMock func(m *mock.MockComputeClientMockRecorder) expectImageMock func(m *mock.MockImageClientMockRecorder) expectNetworkMock func(m *mock.MockNetworkClientMockRecorder) - before *infrav1.ReferencedMachineResources - want *infrav1.ReferencedMachineResources + before *infrav1.ResolvedMachineSpec + want *infrav1.ResolvedMachineSpec wantErr bool }{ { @@ -70,7 +70,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { ServerGroup: &infrav1.ServerGroupFilter{ID: serverGroupID1}, Image: infrav1.ImageFilter{ID: pointer.String(imageID1)}, }, - want: &infrav1.ReferencedMachineResources{ + want: &infrav1.ResolvedMachineSpec{ ImageID: imageID1, ServerGroupID: serverGroupID1, Ports: defaultPorts, @@ -81,7 +81,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { spec: infrav1.OpenStackMachineSpec{ Image: infrav1.ImageFilter{ID: pointer.String(imageID1)}, }, - want: &infrav1.ReferencedMachineResources{ + want: &infrav1.ResolvedMachineSpec{ ImageID: imageID1, Ports: defaultPorts, }, @@ -92,7 +92,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { Image: infrav1.ImageFilter{ID: pointer.String(imageID1)}, ServerGroup: &infrav1.ServerGroupFilter{}, }, - want: &infrav1.ReferencedMachineResources{ + want: &infrav1.ResolvedMachineSpec{ ImageID: imageID1, Ports: defaultPorts, }, @@ -108,7 +108,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { []servergroups.ServerGroup{}, nil) }, - want: &infrav1.ReferencedMachineResources{}, + want: &infrav1.ResolvedMachineSpec{}, wantErr: true, }, { @@ -119,7 +119,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { expectImageMock: func(m *mock.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return([]images.Image{}, nil) }, - want: &infrav1.ReferencedMachineResources{}, + want: &infrav1.ResolvedMachineSpec{}, wantErr: true, }, { @@ -134,7 +134,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { }, }, }, - want: &infrav1.ReferencedMachineResources{ + want: &infrav1.ResolvedMachineSpec{ ImageID: imageID1, Ports: []infrav1.ResolvedPortSpec{ { @@ -181,13 +181,13 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { resources := tt.before if resources == nil { - resources = &infrav1.ReferencedMachineResources{} + resources = &infrav1.ResolvedMachineSpec{} } clusterResourceName := "test-cluster" baseName := "test-instance" scope := scope.NewWithLogger(mockScopeFactory, log) - _, err := ResolveReferencedMachineResources(scope, &tt.spec, resources, clusterResourceName, baseName, openStackCluster, tt.managedSecurityGroup) + _, err := ResolveMachineSpec(scope, &tt.spec, resources, clusterResourceName, baseName, openStackCluster, tt.managedSecurityGroup) if tt.wantErr { g.Expect(err).Error() return
-subnet
+serverGroupID
string 		(Optional) -

SubnetID is the id of a subnet to create the fixed IP of a port in.

+

ServerGroupID is the ID of the server group the machine should be added to and is calculated based on ServerGroupFilter.
-ipAddress
+imageID
string 		(Optional) -

IPAddress is a specific IP address to assign to the port. If SubnetID -is also specified, IPAddress must be a valid IP address in the -subnet. If Subnet is not specified, IPAddress must be a valid IP -address in any subnet of the port’s network.

+

ImageID is the ID of the image to use for the machine and is calculated based on ImageFilter.

+
+ports
+ + +[]ResolvedPortSpec + + +		 +(Optional) +

Ports is the fully resolved list of ports to create for the machine.