diff --git a/PROJECT b/PROJECT index 9db67d8145..9616ccb04d 100644 --- a/PROJECT +++ b/PROJECT @@ -44,4 +44,7 @@ resources: - group: infrastructure kind: OpenStackFloatingIPPool version: v1alpha1 +- group: infrastructure + kind: OpenstackServerGroup + version: v1beta1 version: "2" diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index 8f75cb6e82..e53a3fba22 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -1257,6 +1257,7 @@ func autoConvert_v1beta1_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(i } // WARNING: in.AdditionalBlockDevices requires manual conversion: does not exist in peer-type // WARNING: in.ServerGroup requires manual conversion: does not exist in peer-type + // WARNING: in.ServerGroupRef requires manual conversion: does not exist in peer-type if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef *out = new(OpenStackIdentityReference) diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go index 82777d6580..4d28692e61 100644 --- a/api/v1alpha7/zz_generated.conversion.go +++ b/api/v1alpha7/zz_generated.conversion.go @@ -1510,6 +1510,7 @@ func autoConvert_v1beta1_OpenStackMachineSpec_To_v1alpha7_OpenStackMachineSpec(i out.AdditionalBlockDevices = nil } // WARNING: in.ServerGroup requires manual conversion: does not exist in peer-type + // WARNING: in.ServerGroupRef requires manual conversion: does not exist in peer-type if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef *out = new(OpenStackIdentityReference) diff --git a/api/v1beta1/openstackmachine_types.go b/api/v1beta1/openstackmachine_types.go index a25e53ec9b..2449fd6ba6 100644 --- a/api/v1beta1/openstackmachine_types.go +++ b/api/v1beta1/openstackmachine_types.go @@ -87,6 +87,10 @@ type OpenStackMachineSpec struct { // +optional ServerGroup *ServerGroupParam `json:"serverGroup,omitempty"` + // The server group ref to assign the machine to. + // +optional + ServerGroupRef *ServerGroupRef `json:"serverGroupRef,omitempty"` + // IdentityRef is a reference to a secret holding OpenStack credentials // to be used when reconciling this machine. If not specified, the // credentials specified in the cluster will be used. diff --git a/api/v1beta1/openstackservergroup_types.go b/api/v1beta1/openstackservergroup_types.go new file mode 100644 index 0000000000..49a7b5c469 --- /dev/null +++ b/api/v1beta1/openstackservergroup_types.go @@ -0,0 +1,80 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + // ServerGroupFinalizer allows ReconcileOpenStackServerGroup to clean up OpenStack resources associated with OpenStackServerGroup before + // removing it from the apiserver. + ServerGroupFinalizer = "openstackservergroup.infrastructure.cluster.x-k8s.io" +) + +// OpenStackServerGroupSpec defines the desired state of OpenStackServerGroup. +type OpenStackServerGroupSpec struct { + // Policy is a string with some valid values; affinity, anti-affinity, soft-affinity, soft-anti-affinity. + Policy string `json:"policy"` + + // IdentityRef is a reference to a identity to be used when reconciling this resource + // +optional + IdentityRef *OpenStackIdentityReference `json:"identityRef,omitempty"` +} + +// OpenStackServerGroupStatus defines the observed state of OpenStackServerGroup. +type OpenStackServerGroupStatus struct { + // Ready is true when the resource is created. + // +optional + Ready bool `json:"ready"` + + // UUID of provisioned ServerGroup + ID string `json:"uuid"` +} + +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status + +// OpenStackServerGroup is the Schema for the openstackservergroups API. +type OpenStackServerGroup struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec OpenStackServerGroupSpec `json:"spec,omitempty"` + Status OpenStackServerGroupStatus `json:"status,omitempty"` +} + +//+kubebuilder:object:root=true + +// OpenStackServerGroupList contains a list of OpenStackServerGroup. +type OpenStackServerGroupList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []OpenStackServerGroup `json:"items"` +} + +// GetIdentifyRef returns the object's namespace and IdentityRef if it has an IdentityRef, or nulls if it does not. +func (r *OpenStackServerGroup) GetIdentityRef() (*string, *OpenStackIdentityReference) { + if r.Spec.IdentityRef != nil { + return &r.Namespace, r.Spec.IdentityRef + } + return nil, nil +} + +func init() { + objectTypes = append(objectTypes, &OpenStackServerGroup{}, &OpenStackServerGroupList{}) +} diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 49e654bfd3..bc3dde7520 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -613,6 +613,12 @@ func (f *ServerGroupFilter) IsZero() bool { return f.Name == nil } +type ServerGroupRef struct { + // Name of the OpenStackServerGroup resource to be used. + // Must be in the same namespace as the resource(s) being provisioned. + Name string `json:"name"` +} + // BlockDeviceType defines the type of block device to create. type BlockDeviceType string @@ -881,7 +887,7 @@ func (s *APIServerLoadBalancer) IsEnabled() bool { // 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. + // ServerGroupID is the ID of the server group the machine should be added to and is calculated based on ServerGroupFilter and ServerGroupRef. // +optional ServerGroupID string `json:"serverGroupID,omitempty"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 83b7a42e2d..b69f45daa0 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1014,6 +1014,11 @@ func (in *OpenStackMachineSpec) DeepCopyInto(out *OpenStackMachineSpec) { *out = new(ServerGroupParam) (*in).DeepCopyInto(*out) } + if in.ServerGroupRef != nil { + in, out := &in.ServerGroupRef, &out.ServerGroupRef + *out = new(ServerGroupRef) + **out = **in + } if in.IdentityRef != nil { in, out := &in.IdentityRef, &out.IdentityRef *out = new(OpenStackIdentityReference) @@ -1183,6 +1188,100 @@ func (in *OpenStackMachineTemplateSpec) DeepCopy() *OpenStackMachineTemplateSpec return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OpenStackServerGroup) DeepCopyInto(out *OpenStackServerGroup) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenStackServerGroup. +func (in *OpenStackServerGroup) DeepCopy() *OpenStackServerGroup { + if in == nil { + return nil + } + out := new(OpenStackServerGroup) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *OpenStackServerGroup) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OpenStackServerGroupList) DeepCopyInto(out *OpenStackServerGroupList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]OpenStackServerGroup, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenStackServerGroupList. +func (in *OpenStackServerGroupList) DeepCopy() *OpenStackServerGroupList { + if in == nil { + return nil + } + out := new(OpenStackServerGroupList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *OpenStackServerGroupList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OpenStackServerGroupSpec) DeepCopyInto(out *OpenStackServerGroupSpec) { + *out = *in + if in.IdentityRef != nil { + in, out := &in.IdentityRef, &out.IdentityRef + *out = new(OpenStackIdentityReference) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenStackServerGroupSpec. +func (in *OpenStackServerGroupSpec) DeepCopy() *OpenStackServerGroupSpec { + if in == nil { + return nil + } + out := new(OpenStackServerGroupSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OpenStackServerGroupStatus) DeepCopyInto(out *OpenStackServerGroupStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenStackServerGroupStatus. +func (in *OpenStackServerGroupStatus) DeepCopy() *OpenStackServerGroupStatus { + if in == nil { + return nil + } + out := new(OpenStackServerGroupStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PortOpts) DeepCopyInto(out *PortOpts) { *out = *in @@ -1638,6 +1737,21 @@ func (in *ServerGroupParam) DeepCopy() *ServerGroupParam { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ServerGroupRef) DeepCopyInto(out *ServerGroupRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServerGroupRef. +func (in *ServerGroupRef) DeepCopy() *ServerGroupRef { + if in == nil { + return nil + } + out := new(ServerGroupRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServerMetadata) DeepCopyInto(out *ServerMetadata) { *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 be5b113572..f0c69dbd3e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -4083,6 +4083,17 @@ spec: format: uuid type: string type: object + serverGroupRef: + description: The server group ref to assign the machine to. + properties: + name: + description: |- + Name of the OpenStackServerGroup resource to be used. + Must be in the same namespace as the resource(s) being provisioned. + type: string + required: + - name + type: object serverMetadata: description: Metadata mapping. Allows you to create a map of key value pairs to add to the server instance. @@ -5077,7 +5088,8 @@ spec: type: array serverGroupID: description: ServerGroupID is the ID of the server group the - machine should be added to and is calculated based on ServerGroupFilter. + machine should be added to and is calculated based on ServerGroupFilter + and ServerGroupRef. type: string type: object resources: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index eb75f7731e..dcb2f6d61b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2581,6 +2581,18 @@ spec: format: uuid type: string type: object + serverGroupRef: + description: The server group ref to assign the machine + to. + properties: + name: + description: |- + Name of the OpenStackServerGroup resource to be used. + Must be in the same namespace as the resource(s) being provisioned. + type: string + required: + - name + type: object serverMetadata: description: Metadata mapping. Allows you to create a map of key value pairs to add to the server instance. 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 7bfc99e50a..9739b58503 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1899,6 +1899,17 @@ spec: format: uuid type: string type: object + serverGroupRef: + description: The server group ref to assign the machine to. + properties: + name: + description: |- + Name of the OpenStackServerGroup resource to be used. + Must be in the same namespace as the resource(s) being provisioned. + type: string + required: + - name + type: object serverMetadata: description: Metadata mapping. Allows you to create a map of key value pairs to add to the server instance. @@ -2225,7 +2236,8 @@ spec: type: array serverGroupID: description: ServerGroupID is the ID of the server group the machine - should be added to and is calculated based on ServerGroupFilter. + should be added to and is calculated based on ServerGroupFilter + and ServerGroupRef. type: string type: object resources: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml index 611a03b6c7..c0ec967b24 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -1685,6 +1685,17 @@ spec: format: uuid type: string type: object + serverGroupRef: + description: The server group ref to assign the machine to. + properties: + name: + description: |- + Name of the OpenStackServerGroup resource to be used. + Must be in the same namespace as the resource(s) being provisioned. + type: string + required: + - name + type: object serverMetadata: description: Metadata mapping. Allows you to create a map of key value pairs to add to the server instance. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackservergroups.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackservergroups.yaml new file mode 100644 index 0000000000..d6233481b7 --- /dev/null +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackservergroups.yaml @@ -0,0 +1,85 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: openstackservergroups.infrastructure.cluster.x-k8s.io +spec: + group: infrastructure.cluster.x-k8s.io + names: + kind: OpenStackServerGroup + listKind: OpenStackServerGroupList + plural: openstackservergroups + singular: openstackservergroup + scope: Namespaced + versions: + - name: v1beta1 + schema: + openAPIV3Schema: + description: OpenStackServerGroup is the Schema for the openstackservergroups + API. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: OpenStackServerGroupSpec defines the desired state of OpenStackServerGroup. + properties: + identityRef: + description: IdentityRef is a reference to a identity to be used when + reconciling this resource + properties: + cloudName: + description: CloudName specifies the name of the entry in the + clouds.yaml file to use. + type: string + name: + description: |- + Name is the name of a secret in the same namespace as the resource being provisioned. + The secret must contain a key named `clouds.yaml` which contains an OpenStack clouds.yaml file. + The secret may optionally contain a key named `cacert` containing a PEM-encoded CA certificate. + type: string + required: + - cloudName + - name + type: object + policy: + description: Policy is a string with some valid values; affinity, + anti-affinity, soft-affinity, soft-anti-affinity. + type: string + required: + - policy + type: object + status: + description: OpenStackServerGroupStatus defines the observed state of + OpenStackServerGroup. + properties: + ready: + description: Ready is true when the resource is created. + type: boolean + uuid: + description: UUID of provisioned ServerGroup + type: string + required: + - uuid + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index a4c9e050cf..25c2a21cb8 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -10,6 +10,7 @@ resources: - bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml - bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml - bases/infrastructure.cluster.x-k8s.io_openstackfloatingippools.yaml +- bases/infrastructure.cluster.x-k8s.io_openstackservergroups.yaml # +kubebuilder:scaffold:crdkustomizeresource patches: @@ -20,6 +21,7 @@ patches: - path: patches/webhook_in_openstackmachinetemplates.yaml - path: patches/webhook_in_openstackclustertemplates.yaml #- patches/webhook_in_openstackfloatingippools.yaml +#- patches/webhook_in_openstackservergroups.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch # the following config is for teaching kustomize how to do kustomization for CRDs. diff --git a/config/crd/patches/cainjection_in_openstackservergroups.yaml b/config/crd/patches/cainjection_in_openstackservergroups.yaml new file mode 100644 index 0000000000..cdf5cd763c --- /dev/null +++ b/config/crd/patches/cainjection_in_openstackservergroups.yaml @@ -0,0 +1,8 @@ +# The following patch adds a directive for certmanager to inject CA into the CRD +# CRD conversion requires k8s 1.13 or later. +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) + name: openstackservergroups.infrastructure.cluster.x-k8s.io diff --git a/config/crd/patches/webhook_in_openstackservergroups.yaml b/config/crd/patches/webhook_in_openstackservergroups.yaml new file mode 100644 index 0000000000..83eaa4e206 --- /dev/null +++ b/config/crd/patches/webhook_in_openstackservergroups.yaml @@ -0,0 +1,17 @@ +# The following patch enables conversion webhook for CRD +# CRD conversion requires k8s 1.13 or later. +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: openstackservergroups.infrastructure.cluster.x-k8s.io +spec: + conversion: + strategy: Webhook + webhookClientConfig: + # this is "\n" used as a placeholder, otherwise it will be rejected by the apiserver for being blank, + # but we're going to set it later using the cert-manager (or potentially a patch if not using cert-manager) + caBundle: Cg== + service: + namespace: system + name: webhook-service + path: /convert diff --git a/config/rbac/openstackservergroup_editor_role.yaml b/config/rbac/openstackservergroup_editor_role.yaml new file mode 100644 index 0000000000..9309a024dc --- /dev/null +++ b/config/rbac/openstackservergroup_editor_role.yaml @@ -0,0 +1,24 @@ +# permissions for end users to edit openstackservergroups. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: openstackservergroup-editor-role +rules: +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - openstackservergroups + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - openstackservergroups/status + verbs: + - get diff --git a/config/rbac/openstackservergroup_viewer_role.yaml b/config/rbac/openstackservergroup_viewer_role.yaml new file mode 100644 index 0000000000..3a120eae83 --- /dev/null +++ b/config/rbac/openstackservergroup_viewer_role.yaml @@ -0,0 +1,20 @@ +# permissions for end users to view openstackservergroups. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: openstackservergroup-viewer-role +rules: +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - openstackservergroups + verbs: + - get + - list + - watch +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - openstackservergroups/status + verbs: + - get diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 4790f87801..9bf4912704 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -113,6 +113,26 @@ rules: - get - patch - update +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - openstackservergroups + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - openstackservergroups/status + verbs: + - get + - patch + - update - apiGroups: - ipam.cluster.x-k8s.io resources: diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index 6d2881f5c2..08010e876c 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -124,14 +124,14 @@ func (r *OpenStackClusterReconciler) Reconcile(ctx context.Context, req ctrl.Req // Handle deleted clusters if !openStackCluster.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, scope, cluster, openStackCluster) + return r.reconcileDelete(ctx, r.Client, scope, cluster, openStackCluster) } // Handle non-deleted clusters - return reconcileNormal(scope, cluster, openStackCluster) + return reconcileNormal(ctx, r.Client, scope, cluster, openStackCluster) } -func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) { +func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, ctrlClient client.Client, scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) { scope.Logger().Info("Reconciling Cluster delete") // Wait for machines to be deleted before removing the finalizer as they @@ -154,7 +154,7 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope // We attempt to delete it even if no status was written, just in case if openStackCluster.Status.Network != nil { // Attempt to resolve bastion resources before delete. We don't need to worry about starting if the resources have changed on update. - if _, err := resolveBastionResources(scope, clusterResourceName, openStackCluster); err != nil { + if _, err := resolveBastionResources(ctx, ctrlClient, scope, clusterResourceName, openStackCluster); err != nil { return reconcile.Result{}, err } @@ -218,43 +218,64 @@ func contains(arr []string, target string) bool { return false } -func resolveBastionResources(scope *scope.WithLogger, clusterResourceName string, openStackCluster *infrav1.OpenStackCluster) (bool, error) { +func resolveBastionResources(ctx context.Context, ctrlClient client.Client, scope *scope.WithLogger, clusterResourceName string, openStackCluster *infrav1.OpenStackCluster) (bool, error) { // Resolve and store resources for the bastion - if openStackCluster.Spec.Bastion.IsEnabled() { - if openStackCluster.Status.Bastion == nil { - openStackCluster.Status.Bastion = &infrav1.BastionStatus{} - } - 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, resolved, - clusterResourceName, bastionName(clusterResourceName), - openStackCluster, getBastionSecurityGroupID(openStackCluster)) + if !openStackCluster.Spec.Bastion.IsEnabled() { + return false, nil + } + if openStackCluster.Status.Bastion == nil { + openStackCluster.Status.Bastion = &infrav1.BastionStatus{} + } + bastionSpec := openStackCluster.Spec.Bastion.Spec + if bastionSpec == 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 + } + // Resolve resources from OpenStack + changed, err := compute.ResolveMachineSpec(scope, + bastionSpec, resolved, + clusterResourceName, bastionName(clusterResourceName), + openStackCluster, getBastionSecurityGroupID(openStackCluster)) + if err != nil { + return false, err + } + + // Resolve the OpenStackServerGroup Reference using K8sClient (lower priority than spec.ServerGroup) + if resolved.ServerGroupID == "" && bastionSpec.ServerGroupRef != nil && bastionSpec.ServerGroupRef.Name != "" { + servergroup := &infrav1.OpenStackServerGroup{} + + // Get OpenStackServerGroup resource from K8s + err := ctrlClient.Get(ctx, client.ObjectKey{ + Namespace: openStackCluster.Namespace, + Name: bastionSpec.ServerGroupRef.Name, + }, servergroup) if err != nil { - return false, err - } - if changed { - // 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 + return changed, err } - err = compute.AdoptMachineResources(scope, resolved, resources) - if err != nil { - return false, err + // Store the resolved UUID, once it's ready and set. + if servergroup.Status.Ready && servergroup.Status.ID != "" { + resolved.ServerGroupID = servergroup.Status.ID + changed = true } } - return false, nil + + if changed { + // 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, resolved, resources) + return false, err } func deleteBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error { @@ -345,7 +366,7 @@ func deleteBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStac return nil } -func reconcileNormal(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) { //nolint:unparam +func reconcileNormal(ctx context.Context, ctrlClient client.Client, scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) { //nolint:unparam scope.Logger().Info("Reconciling Cluster") // If the OpenStackCluster doesn't have our finalizer, add it. @@ -364,7 +385,7 @@ func reconcileNormal(scope *scope.WithLogger, cluster *clusterv1.Cluster, openSt return reconcile.Result{}, err } - result, err := reconcileBastion(scope, cluster, openStackCluster) + result, err := reconcileBastion(ctx, ctrlClient, scope, cluster, openStackCluster) if err != nil { return reconcile.Result{}, err } @@ -399,11 +420,11 @@ func reconcileNormal(scope *scope.WithLogger, cluster *clusterv1.Cluster, openSt return reconcile.Result{}, nil } -func reconcileBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (*ctrl.Result, error) { +func reconcileBastion(ctx context.Context, ctrlClient client.Client, scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (*ctrl.Result, error) { scope.Logger().V(4).Info("Reconciling Bastion") clusterResourceName := names.ClusterResourceName(cluster) - changed, err := resolveBastionResources(scope, clusterResourceName, openStackCluster) + changed, err := resolveBastionResources(ctx, ctrlClient, scope, clusterResourceName, openStackCluster) if err != nil { return nil, err } diff --git a/controllers/openstackcluster_controller_test.go b/controllers/openstackcluster_controller_test.go index 79c21e15aa..98101e1cda 100644 --- a/controllers/openstackcluster_controller_test.go +++ b/controllers/openstackcluster_controller_test.go @@ -286,7 +286,7 @@ var _ = Describe("OpenStackCluster controller", func() { networkClientRecorder.ListFloatingIP(floatingips.ListOpts{PortID: "portID1"}).Return(make([]floatingips.FloatingIP, 1), nil) - res, err := reconcileBastion(scope, capiCluster, testCluster) + res, err := reconcileBastion(ctx, k8sClient, scope, capiCluster, testCluster) expectedStatus := &infrav1.BastionStatus{ ID: "adopted-bastion-uuid", State: "ACTIVE", @@ -369,7 +369,7 @@ var _ = Describe("OpenStackCluster controller", func() { networkClientRecorder.ListFloatingIP(floatingips.ListOpts{PortID: "portID1"}).Return([]floatingips.FloatingIP{{FloatingIP: "1.2.3.4"}}, nil) - res, err := reconcileBastion(scope, capiCluster, testCluster) + res, err := reconcileBastion(ctx, k8sClient, scope, capiCluster, testCluster) Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{ ID: "adopted-fip-bastion-uuid", FloatingIP: "1.2.3.4", @@ -447,7 +447,7 @@ var _ = Describe("OpenStackCluster controller", func() { computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT() computeClientRecorder.GetServer("requeue-bastion-uuid").Return(&server, nil) - res, err := reconcileBastion(scope, capiCluster, testCluster) + res, err := reconcileBastion(ctx, k8sClient, scope, capiCluster, testCluster) Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{ ID: "requeue-bastion-uuid", State: "BUILD", diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index 0ea39a0992..473be04f6a 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -155,24 +155,49 @@ func (r *OpenStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req // Handle deleted machines if !openStackMachine.DeletionTimestamp.IsZero() { - return r.reconcileDelete(scope, clusterResourceName, infraCluster, machine, openStackMachine) + return r.reconcileDelete(ctx, scope, clusterResourceName, infraCluster, machine, openStackMachine) } // Handle non-deleted clusters return r.reconcileNormal(ctx, scope, clusterResourceName, infraCluster, machine, openStackMachine) } -func resolveMachineResources(scope *scope.WithLogger, clusterResourceName string, openStackCluster *infrav1.OpenStackCluster, openStackMachine *infrav1.OpenStackMachine, machine *clusterv1.Machine) (bool, error) { +func resolveMachineResources(ctx context.Context, ctrlClient client.Client, 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 - return compute.ResolveMachineSpec(scope, + + // Resolve resources from OpenStack + changed, err := compute.ResolveMachineSpec(scope, &openStackMachine.Spec, resolved, clusterResourceName, openStackMachine.Name, openStackCluster, getManagedSecurityGroup(openStackCluster, machine)) + if err != nil { + return changed, err + } + + // Resolve the OpenStackServerGroup Reference using K8sClient (lower priority than spec.ServerGroup) + if resolved.ServerGroupID == "" && openStackMachine.Spec.ServerGroupRef != nil && openStackMachine.Spec.ServerGroupRef.Name != "" { + servergroup := &infrav1.OpenStackServerGroup{} + + // Get OpenStackServerGroup resource from K8s + err := ctrlClient.Get(ctx, client.ObjectKey{ + Namespace: openStackCluster.Namespace, + Name: openStackMachine.Spec.ServerGroupRef.Name, + }, servergroup) + if err != nil { + return changed, err + } + + // Store the resolved UUID, once it's ready and set. + if servergroup.Status.Ready && servergroup.Status.ID != "" { + resolved.ServerGroupID = servergroup.Status.ID + changed = true + } + } + return changed, err } func adoptMachineResources(scope *scope.WithLogger, openStackMachine *infrav1.OpenStackMachine) error { @@ -238,7 +263,7 @@ func (r *OpenStackMachineReconciler) SetupWithManager(ctx context.Context, mgr c Complete(r) } -func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, clusterResourceName string, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine) (ctrl.Result, error) { //nolint:unparam +func (r *OpenStackMachineReconciler) reconcileDelete(ctx context.Context, scope *scope.WithLogger, clusterResourceName string, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine) (ctrl.Result, error) { //nolint:unparam scope.Logger().Info("Reconciling Machine delete") computeService, err := compute.NewService(scope) @@ -276,7 +301,7 @@ func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, cl // This code can and should be deleted in a future release when we are // sure that all machines have been reconciled at least by a v0.10 or // later controller. - if _, err := resolveMachineResources(scope, clusterResourceName, openStackCluster, openStackMachine, machine); err != nil { + if _, err := resolveMachineResources(ctx, r.Client, scope, clusterResourceName, openStackCluster, openStackMachine, machine); err != nil { // Return the error, but allow the resource to be removed anyway. controllerutil.RemoveFinalizer(openStackMachine, infrav1.MachineFinalizer) return ctrl.Result{}, err @@ -552,7 +577,7 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope return ctrl.Result{}, nil } - changed, err := resolveMachineResources(scope, clusterResourceName, openStackCluster, openStackMachine, machine) + changed, err := resolveMachineResources(ctx, r.Client, scope, clusterResourceName, openStackCluster, openStackMachine, machine) if err != nil { return ctrl.Result{}, err } @@ -667,7 +692,7 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope scope.Logger().Info("Waiting for instance to become ACTIVE", "id", instanceStatus.ID(), "status", instanceStatus.State()) return ctrl.Result{RequeueAfter: waitForBuildingInstanceToReconcile}, nil default: - // The other state is normal (for example, migrating, shutoff) but we don't want to proceed until it's ACTIVE + // The other state is normal (for example; migrating, shutoff) but we don't want to proceed until it's ACTIVE // due to potential conflict or unexpected actions scope.Logger().Info("Waiting for instance to become ACTIVE", "id", instanceStatus.ID(), "status", instanceStatus.State()) conditions.MarkUnknown(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceNotReadyReason, "Instance state is not handled: %s", instanceStatus.State()) diff --git a/controllers/openstackmachine_controller_test.go b/controllers/openstackmachine_controller_test.go index 5212ca4944..0a0d5db26c 100644 --- a/controllers/openstackmachine_controller_test.go +++ b/controllers/openstackmachine_controller_test.go @@ -553,7 +553,7 @@ func Test_reconcileDelete(t *testing.T) { osMachine.Name = openStackMachineName osMachine.Finalizers = []string{infrav1.MachineFinalizer} - _, err := reconciler.reconcileDelete(scopeWithLogger, openStackMachineName, &openStackCluster, &machine, &tt.osMachine) + _, err := reconciler.reconcileDelete(ctx, scopeWithLogger, openStackMachineName, &openStackCluster, &machine, &tt.osMachine) if tt.wantErr { g.Expect(err).To(HaveOccurred()) diff --git a/controllers/openstackservergroup_controller.go b/controllers/openstackservergroup_controller.go new file mode 100644 index 0000000000..52f8239cc7 --- /dev/null +++ b/controllers/openstackservergroup_controller.go @@ -0,0 +1,215 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "reflect" + + "github.com/go-logr/logr" + "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servergroups" + apierrors "k8s.io/apimachinery/pkg/api/errors" + kerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/cluster-api/util/annotations" + "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/cluster-api/util/predicates" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" + capoerrors "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/errors" +) + +// OpenStackServerGroupReconciler reconciles a OpenstackServerGroup object. +type OpenStackServerGroupReconciler struct { + client.Client + Recorder record.EventRecorder + WatchFilterValue string + ScopeFactory scope.Factory + CaCertificates []byte // PEM encoded ca certificates. +} + +//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackservergroups,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackservergroups/status,verbs=get;update;patch + +func (r *OpenStackServerGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, reterr error) { + log := ctrl.LoggerFrom(ctx) + + // Fetch the OpenStackServerGroup instance + openStackServerGroup := &infrav1.OpenStackServerGroup{} + err := r.Client.Get(ctx, req.NamespacedName, openStackServerGroup) + if err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } + return ctrl.Result{}, err + } + + log = log.WithValues("openStackServerGroup", openStackServerGroup.Name) + + log.Info("OpenStackServerGroup is about to reconcile") + + if annotations.HasPaused(openStackServerGroup) { + log.Info("OpenStackServerGroup is marked as paused. Won't reconcile") + return ctrl.Result{}, nil + } + + // Initialize the patch helper + patchHelper, err := patch.NewHelper(openStackServerGroup, r.Client) + if err != nil { + return ctrl.Result{}, err + } + + // Always patch the openStackServerGroup when exiting this function so we can persist any changes. + defer func() { + if err := patchServerGroup(ctx, patchHelper, openStackServerGroup); err != nil { + result = ctrl.Result{} + reterr = kerrors.NewAggregate([]error{reterr, err}) + } + }() + + clientScope, err := r.ScopeFactory.NewClientScopeFromObject(ctx, r.Client, r.CaCertificates, log, openStackServerGroup) + scope := scope.NewWithLogger(clientScope, log) + if err != nil { + return ctrl.Result{}, err + } + + // Handle deleted servergroups + if !openStackServerGroup.DeletionTimestamp.IsZero() { + return r.reconcileDelete(scope, openStackServerGroup) + } + + // Handle non-deleted servergroups + return r.reconcileNormal(scope, openStackServerGroup) +} + +func patchServerGroup(ctx context.Context, patchHelper *patch.Helper, openStackServerGroup *infrav1.OpenStackServerGroup, options ...patch.Option) error { + return patchHelper.Patch(ctx, openStackServerGroup, options...) +} + +func (r *OpenStackServerGroupReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For( + &infrav1.OpenStackServerGroup{}, + builder.WithPredicates( + predicate.Funcs{ + // Avoid reconciling if the event triggering the reconciliation is related to incremental status updates + UpdateFunc: func(e event.UpdateEvent) bool { + oldServerGroup := e.ObjectOld.(*infrav1.OpenStackServerGroup).DeepCopy() + newServerGroup := e.ObjectNew.(*infrav1.OpenStackServerGroup).DeepCopy() + oldServerGroup.Status = infrav1.OpenStackServerGroupStatus{} + newServerGroup.Status = infrav1.OpenStackServerGroupStatus{} + oldServerGroup.ObjectMeta.ResourceVersion = "" + newServerGroup.ObjectMeta.ResourceVersion = "" + return !reflect.DeepEqual(oldServerGroup, newServerGroup) + }, + }, + ), + ). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue)). + Complete(r) +} + +func (r *OpenStackServerGroupReconciler) reconcileDelete(scope *scope.WithLogger, openStackServerGroup *infrav1.OpenStackServerGroup) (result ctrl.Result, reterr error) { //nolint:unparam + scope.Logger().Info("Reconciling OpenStackServerGroup delete") + + computeService, err := compute.NewService(scope) + if err != nil { + return ctrl.Result{}, err + } + + serverGroupID := openStackServerGroup.Status.ID + if serverGroupID != "" { + err = computeService.DeleteServerGroup(serverGroupID) + if err != nil { + if capoerrors.IsNotFound(err) { + scope.Logger().Info("Server Group did not exist in OpenStack. Treating deletion as success") + } else { + return ctrl.Result{}, err + } + } + } + + controllerutil.RemoveFinalizer(openStackServerGroup, infrav1.ServerGroupFinalizer) + scope.Logger().Info("Reconciled OpenStackServerGroup delete successfully") + return ctrl.Result{}, nil +} + +func (r *OpenStackServerGroupReconciler) reconcileNormal(scope *scope.WithLogger, openStackServerGroup *infrav1.OpenStackServerGroup) (result ctrl.Result, reterr error) { //nolint:unparam + // If the OpenStackServerGroup doesn't have our finalizer, add it. + if controllerutil.AddFinalizer(openStackServerGroup, infrav1.ServerGroupFinalizer) { + scope.Logger().Info("Reconciling ServerGroup: Adding finalizer") + // Register the finalizer immediately to avoid orphaning OpenStack resources on delete + // NOTE(dalees): This return without Requeue relies on patchServerGroup to persist the change, and the watch triggers an immediate reconcile. + return ctrl.Result{}, nil + } + + scope.Logger().Info("Reconciling OpenStackServerGroup") + + computeService, err := compute.NewService(scope) + if err != nil { + return ctrl.Result{}, err + } + + serverGroupStatus, err := r.getOrCreate(scope.Logger(), openStackServerGroup, computeService) + if err != nil || serverGroupStatus == nil { + return ctrl.Result{}, err + } + + // Update the resource with any new information. + openStackServerGroup.Status.Ready = true + openStackServerGroup.Status.ID = serverGroupStatus.ID + + scope.Logger().Info("Reconciled OpenStackServerGroup successfully") + return ctrl.Result{}, nil +} + +func (r *OpenStackServerGroupReconciler) getOrCreate(logger logr.Logger, openStackServerGroup *infrav1.OpenStackServerGroup, computeService *compute.Service) (*servergroups.ServerGroup, error) { + serverGroupName := openStackServerGroup.Name + + serverGroup, err := computeService.GetServerGroupByName(serverGroupName, false) + if err != nil && serverGroup != nil { + // More than one server group was found with the same name. + // We should not create another, nor should we use the first found. + return nil, err + } + if err == nil { + return serverGroup, nil + } + + logger.Info("Unable to get OpenStackServerGroup instance, we need to create it.", "name", serverGroupName, "policy", openStackServerGroup.Spec.Policy) + + err = computeService.CreateServerGroup(serverGroupName, openStackServerGroup.Spec.Policy) + if err != nil { + return nil, err + } + + // Perform another GET after creation; Gophercloud doesn't give us the created ServerGroup ID on create + serverGroup, err = computeService.GetServerGroupByName(serverGroupName, false) + if err != nil { + return nil, err + } + + return serverGroup, err +} diff --git a/controllers/openstackservergroup_controller_test.go b/controllers/openstackservergroup_controller_test.go new file mode 100644 index 0000000000..6e87b4e6ff --- /dev/null +++ b/controllers/openstackservergroup_controller_test.go @@ -0,0 +1,362 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "fmt" + + "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servergroups" + . "github.com/onsi/ginkgo/v2" //nolint:revive + . "github.com/onsi/gomega" //nolint:revive + "go.uber.org/mock/gomock" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/test/framework" + "sigs.k8s.io/cluster-api/util/annotations" + "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" +) + +var ( + reconcilerServerGroup *OpenStackServerGroupReconciler + testServerGroup *infrav1.OpenStackServerGroup +) + +var _ = Describe("OpenStackServerGroup controller", func() { + testServerGroupName := "test-servergroup" + testNum := 0 + + BeforeEach(func() { + ctx = context.TODO() + testNum++ + testNamespace = fmt.Sprintf("testservergroup-%d", testNum) + + // Create a standard ServerGroup definition for all tests + testServerGroup = &infrav1.OpenStackServerGroup{ + TypeMeta: metav1.TypeMeta{ + APIVersion: infrav1.GroupVersion.Group + "/" + infrav1.GroupVersion.Version, + Kind: "OpenStackServerGroup", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: testServerGroupName, + Namespace: testNamespace, + }, + Spec: infrav1.OpenStackServerGroupSpec{}, + Status: infrav1.OpenStackServerGroupStatus{}, + } + // Set finalizers, so the first reconcile doesn't need to by default. + testServerGroup.SetFinalizers([]string{infrav1.ServerGroupFinalizer}) + + input := framework.CreateNamespaceInput{ + Creator: k8sClient, + Name: testNamespace, + } + framework.CreateNamespace(ctx, input) + + mockCtrl = gomock.NewController(GinkgoT()) + mockScopeFactory = scope.NewMockScopeFactory(mockCtrl, "") + reconcilerServerGroup = func() *OpenStackServerGroupReconciler { + return &OpenStackServerGroupReconciler{ + Client: k8sClient, + ScopeFactory: mockScopeFactory, + } + }() + }) + + AfterEach(func() { + orphan := metav1.DeletePropagationOrphan + deleteOptions := client.DeleteOptions{ + PropagationPolicy: &orphan, + } + + // Delete OpenstackServerGroup + patchHelper, err := patch.NewHelper(testServerGroup, k8sClient) + Expect(err).To(BeNil()) + err = patchHelper.Patch(ctx, testServerGroup) + Expect(err).To(BeNil()) + err = k8sClient.Delete(ctx, testServerGroup, &deleteOptions) + Expect(err).To(BeNil()) + input := framework.DeleteNamespaceInput{ + Deleter: k8sClient, + Name: testNamespace, + } + framework.DeleteNamespace(ctx, input) + }) + + It("should do nothing when servergroup resource is paused", func() { + testServerGroup.SetName("paused") + annotations.AddAnnotations(testServerGroup, map[string]string{clusterv1.PausedAnnotation: "true"}) + testServerGroup.SetFinalizers([]string{}) + + err := k8sClient.Create(ctx, testServerGroup) + Expect(err).To(BeNil()) + req := createRequestFromServerGroup(testServerGroup) + + result, err := reconcilerServerGroup.Reconcile(ctx, req) + Expect(err).To(BeNil()) + Expect(result).To(Equal(reconcile.Result{})) + + // Ensure Finalizer was not set by paused reconcile + err = k8sClient.Get(ctx, req.NamespacedName, testServerGroup) + Expect(err).To(BeNil()) + Expect(testServerGroup.GetFinalizers()).To(BeNil()) + }) + + It("should do nothing when unable to get OS client", func() { + testServerGroup.SetName("no-openstack-client") + + err := k8sClient.Create(ctx, testServerGroup) + Expect(err).To(BeNil()) + req := createRequestFromServerGroup(testServerGroup) + + clientCreateErr := fmt.Errorf("Test failure") + mockScopeFactory.SetClientScopeCreateError(clientCreateErr) + + result, err := reconcilerServerGroup.Reconcile(ctx, req) + Expect(err).To(MatchError(clientCreateErr)) + Expect(result).To(Equal(reconcile.Result{})) + }) + + It("should add a finalizer on the first reconcile", func() { + testServerGroup.SetName("finalizer-add") + testServerGroup.SetFinalizers([]string{}) + + err := k8sClient.Create(ctx, testServerGroup) + Expect(err).To(BeNil()) + + // Reconcile our resource and make sure the finalizer was set + req := createRequestFromServerGroup(testServerGroup) + result, err := reconcilerServerGroup.Reconcile(ctx, req) + Expect(err).To(BeNil()) + Expect(result).To(Equal(reconcile.Result{})) + + // Retrieve the server group from K8s client + err = k8sClient.Get(ctx, req.NamespacedName, testServerGroup) + Expect(err).To(BeNil()) + + Expect(testServerGroup.GetFinalizers()).To(Equal([]string{infrav1.ServerGroupFinalizer})) + }) + + It("should adopt an existing servergroup by name if status has no uuid set", func() { + testServerGroup.SetName("adopt-existing-servergroup") + + // Set up servergroup spec, and status with no uuid + testServerGroup.Spec = infrav1.OpenStackServerGroupSpec{ + Policy: "anti-affinity", + } + err := k8sClient.Create(ctx, testServerGroup) + Expect(err).To(BeNil()) + testServerGroup.Status = infrav1.OpenStackServerGroupStatus{ + ID: "", + Ready: false, + } + // Write the test resource to k8s client + err = k8sClient.Status().Update(ctx, testServerGroup) + Expect(err).To(BeNil()) + + // Define and record the existing resource the reconcile will see. + servergroups := []servergroups.ServerGroup{ + { + Name: "adopt-existing-servergroup", + ID: "adopted-servergroup-uuid", + Policies: []string{"anti-affinity"}, + }, + } + computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT() + computeClientRecorder.ListServerGroups().Return(servergroups, nil) + + // Reconcile our resource, and make sure it adopted the expected resource. + req := createRequestFromServerGroup(testServerGroup) + result, err := reconcilerServerGroup.Reconcile(ctx, req) + Expect(err).To(BeNil()) + Expect(result).To(Equal(reconcile.Result{})) + + // Retrieve the server group from K8s client + err = k8sClient.Get(ctx, req.NamespacedName, testServerGroup) + Expect(err).To(BeNil()) + + Expect(testServerGroup.Status.ID).To(Equal("adopted-servergroup-uuid")) + Expect(testServerGroup.Status.Ready).To(BeTrue()) + }) + + It("should requeue and not adopt if multiple servergroups exist with the same name", func() { + testServerGroup.SetName("multiple-named-servergroup") + + // Set up servergroup spec, and status with no uuid + testServerGroup.Spec = infrav1.OpenStackServerGroupSpec{ + Policy: "anti-affinity", + } + err := k8sClient.Create(ctx, testServerGroup) + Expect(err).To(BeNil()) + testServerGroup.Status = infrav1.OpenStackServerGroupStatus{ + ID: "", + Ready: false, + } + // Write the test resource to k8s client + err = k8sClient.Status().Update(ctx, testServerGroup) + Expect(err).To(BeNil()) + + // Define and record the existing resources the reconcile will see. + servergroups := []servergroups.ServerGroup{ + { + Name: "multiple-named-servergroup", + ID: "multiple-named-servergroup-uuid1", + Policies: []string{"anti-affinity"}, + }, + { + Name: "multiple-named-servergroup", + ID: "multiple-named-servergroup-uuid2", + Policies: []string{"soft-anti-affinity"}, + }, + } + computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT() + computeClientRecorder.ListServerGroups().Return(servergroups, nil) + + // Reconcile our resource, it should return an error + req := createRequestFromServerGroup(testServerGroup) + result, err := reconcilerServerGroup.Reconcile(ctx, req) + Expect(err).NotTo(BeNil()) + Expect(result).To(Equal(reconcile.Result{})) + + // Retrieve the server group from K8s client + err = k8sClient.Get(ctx, req.NamespacedName, testServerGroup) + Expect(err).To(BeNil()) + + // No ServerGroup should have been adopted + Expect(testServerGroup.Status.ID).To(Equal("")) + Expect(testServerGroup.Status.Ready).To(BeFalse()) + }) + + It("should succeed reconcile delete even with no servergroup adopted", func() { + testServerGroup.SetName("delete-existing-servergroup-no-uuid") + + // Set up servergroup spec, and status with no uuid. + testServerGroup.Spec = infrav1.OpenStackServerGroupSpec{ + Policy: "anti-affinity", + } + err := k8sClient.Create(ctx, testServerGroup) + Expect(err).To(BeNil()) + testServerGroup.Status = infrav1.OpenStackServerGroupStatus{ + ID: "", + Ready: false, + } + // Write the test resource to k8s client + err = k8sClient.Status().Update(ctx, testServerGroup) + Expect(err).To(BeNil()) + + // Reconcile our resource, and make sure it finds the expected resource, then deletes it. + log := GinkgoLogr + clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testServerGroup) + Expect(err).To(BeNil()) + scope := scope.NewWithLogger(clientScope, log) + + result, err := reconcilerServerGroup.reconcileDelete(scope, testServerGroup) + Expect(err).To(BeNil()) + Expect(result).To(Equal(reconcile.Result{})) + + // Finalizer should now be removed. + Expect(testServerGroup.GetFinalizers()).To(Equal([]string{})) + }) + + It("should succeed reconcile delete even if the servergroup does not exist", func() { + testServerGroup.SetName("delete-servergroup-not-exist") + + // Set up servergroup spec, and status with no uuid. + testServerGroup.Spec = infrav1.OpenStackServerGroupSpec{ + Policy: "anti-affinity", + } + err := k8sClient.Create(ctx, testServerGroup) + Expect(err).To(BeNil()) + testServerGroup.Status = infrav1.OpenStackServerGroupStatus{ + ID: "1234", + Ready: true, + } + // Write the test resource to k8s client + err = k8sClient.Status().Update(ctx, testServerGroup) + Expect(err).To(BeNil()) + + // Define and record the function calls the reconcile will see. + computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT() + computeClientRecorder.DeleteServerGroup("1234").Return(gophercloud.ErrResourceNotFound{}) + + // Reconcile our resource, and make sure it finds the expected resource, then deletes it. + log := GinkgoLogr + clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testServerGroup) + Expect(err).To(BeNil()) + scope := scope.NewWithLogger(clientScope, log) + + result, err := reconcilerServerGroup.reconcileDelete(scope, testServerGroup) + Expect(err).To(BeNil()) + Expect(result).To(Equal(reconcile.Result{})) + + // Finalizer should now be removed. + Expect(testServerGroup.GetFinalizers()).To(Equal([]string{})) + }) + + It("should requeue if the service returns temporary errors", func() { + testServerGroup.SetName("requeue-on-openstack-error") + + // Set up servergroup spec + testServerGroup.Spec = infrav1.OpenStackServerGroupSpec{ + Policy: "anti-affinity", + } + err := k8sClient.Create(ctx, testServerGroup) + Expect(err).To(BeNil()) + testServerGroup.Status = infrav1.OpenStackServerGroupStatus{ + ID: "", + Ready: false, + } + // Write the test resource to k8s client + err = k8sClient.Status().Update(ctx, testServerGroup) + Expect(err).To(BeNil()) + + // Define and record the existing resource the reconcile will see. + servergroups := []servergroups.ServerGroup{} + computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT() + computeClientRecorder.ListServerGroups().Return(servergroups, nil) + expectedError := gophercloud.ErrUnexpectedResponseCode{Actual: 500} + computeClientRecorder.CreateServerGroup("requeue-on-openstack-error", "anti-affinity").Return(expectedError) + + // Reconcile our resource, and make sure it adopted the expected resource. + log := GinkgoLogr + clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testServerGroup) + Expect(err).To(BeNil()) + scope := scope.NewWithLogger(clientScope, log) + + result, err := reconcilerServerGroup.reconcileNormal(scope, testServerGroup) + // Expect error to surface with empty result. + // Due to the error, the reconcile should be re-tried with default backoff by ControllerRuntime + Expect(err).To(Equal(expectedError)) + Expect(result).To(Equal(reconcile.Result{})) + }) +}) + +func createRequestFromServerGroup(openStackServerGroup *infrav1.OpenStackServerGroup) reconcile.Request { + return reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: openStackServerGroup.GetName(), + Namespace: openStackServerGroup.GetNamespace(), + }, + } +} diff --git a/docs/book/src/api/v1beta1/api.md b/docs/book/src/api/v1beta1/api.md index 0c16435c93..a7807bb2d9 100644 --- a/docs/book/src/api/v1beta1/api.md +++ b/docs/book/src/api/v1beta1/api.md @@ -716,6 +716,20 @@ ServerGroupParam
serverGroupRef
The server group ref to assign the machine to.
+identityRef
(Appears on: OpenStackClusterSpec, -OpenStackMachineSpec) +OpenStackMachineSpec, +OpenStackServerGroupSpec)
OpenStackIdentityReference is a reference to an infrastructure @@ -3343,6 +3358,20 @@ ServerGroupParam
serverGroupRef
The server group ref to assign the machine to.
+identityRef
serverGroupRef
The server group ref to assign the machine to.
+identityRef
+
OpenStackServerGroup is the Schema for the openstackservergroups API.
+ +Field | +Description | +||||
---|---|---|---|---|---|
+metadata + +Kubernetes meta/v1.ObjectMeta + + |
+
+Refer to the Kubernetes API documentation for the fields of the
+metadata field.
+ |
+||||
+spec + + +OpenStackServerGroupSpec + + + |
+
+ + +
|
+||||
+status + + +OpenStackServerGroupStatus + + + |
++ | +
+(Appears on: +OpenStackServerGroup) +
++
OpenStackServerGroupSpec defines the desired state of OpenStackServerGroup.
+ +Field | +Description | +
---|---|
+policy + +string + + |
+
+ Policy is a string with some valid values; affinity, anti-affinity, soft-affinity, soft-anti-affinity. + |
+
+identityRef + + +OpenStackIdentityReference + + + |
+
+(Optional)
+ IdentityRef is a reference to a identity to be used when reconciling this resource + |
+
+(Appears on: +OpenStackServerGroup) +
++
OpenStackServerGroupStatus defines the observed state of OpenStackServerGroup.
+ +Field | +Description | +
---|---|
+ready + +bool + + |
+
+(Optional)
+ Ready is true when the resource is created. + |
+
+uuid + +string + + |
+
+ UUID of provisioned ServerGroup + |
+
@@ -4009,7 +4218,7 @@ string
ServerGroupID is the ID of the server group the machine should be added to and is calculated based on ServerGroupFilter.
+ServerGroupID is the ID of the server group the machine should be added to and is calculated based on ServerGroupFilter and ServerGroupRef.
+(Appears on: +OpenStackMachineSpec) +
++
+Field | +Description | +
---|---|
+name + +string + + |
+
+ Name of the OpenStackServerGroup resource to be used. +Must be in the same namespace as the resource(s) being provisioned. + |
+
diff --git a/main.go b/main.go index 76329967b3..2f21e74046 100644 --- a/main.go +++ b/main.go @@ -350,6 +350,16 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager, caCerts []byte, sco setupLog.Error(err, "unable to create controller", "controller", "FloatingIPPool") os.Exit(1) } + if err := (&controllers.OpenStackServerGroupReconciler{ + Client: mgr.GetClient(), + Recorder: mgr.GetEventRecorderFor("servergroup-controller"), + WatchFilterValue: watchFilterValue, + ScopeFactory: scopeFactory, + CaCertificates: caCerts, + }).SetupWithManager(ctx, mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "OpenStackServerGroup") + os.Exit(1) + } } func setupWebhooks(mgr ctrl.Manager) { diff --git a/pkg/clients/compute.go b/pkg/clients/compute.go index 269597698b..4299a7ddd2 100644 --- a/pkg/clients/compute.go +++ b/pkg/clients/compute.go @@ -58,6 +58,8 @@ type ComputeClient interface { DeleteAttachedInterface(serverID, portID string) error ListServerGroups() ([]servergroups.ServerGroup, error) + CreateServerGroup(name string, policy string) error + DeleteServerGroup(id string) error } type computeClient struct{ client *gophercloud.ServiceClient } @@ -156,6 +158,27 @@ func (c computeClient) ListServerGroups() ([]servergroups.ServerGroup, error) { return servergroups.ExtractServerGroups(allPages) } +func (c computeClient) CreateServerGroup(name string, policy string) error { + var servergroup servergroups.ServerGroup + mc := metrics.NewMetricPrometheusContext("server_group", "create") + // Use microversion <2.64 policies format. + opts := servergroups.CreateOpts{ + Name: name, + Policies: []string{policy}, + } + err := servergroups.Create(context.TODO(), c.client, opts).ExtractInto(&servergroup) + if mc.ObserveRequest(err) != nil { + return err + } + return err +} + +func (c computeClient) DeleteServerGroup(id string) error { + mc := metrics.NewMetricPrometheusContext("server_group", "delete") + err := servergroups.Delete(context.TODO(), c.client, id).ExtractErr() + return mc.ObserveRequestIgnoreNotFoundorConflict(err) +} + type computeErrorClient struct{ error } // NewComputeErrorClient returns a ComputeClient in which every method returns the given error. @@ -198,3 +221,11 @@ func (e computeErrorClient) DeleteAttachedInterface(_, _ string) error { func (e computeErrorClient) ListServerGroups() ([]servergroups.ServerGroup, error) { return nil, e.error } + +func (e computeErrorClient) CreateServerGroup(_ string, _ string) error { + return e.error +} + +func (e computeErrorClient) DeleteServerGroup(_ string) error { + return e.error +} diff --git a/pkg/clients/mock/compute.go b/pkg/clients/mock/compute.go index 7fc6ef5f00..c1b97fec8f 100644 --- a/pkg/clients/mock/compute.go +++ b/pkg/clients/mock/compute.go @@ -74,6 +74,20 @@ func (mr *MockComputeClientMockRecorder) CreateServer(arg0, arg1 any) *gomock.Ca return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateServer", reflect.TypeOf((*MockComputeClient)(nil).CreateServer), arg0, arg1) } +// CreateServerGroup mocks base method. +func (m *MockComputeClient) CreateServerGroup(arg0, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateServerGroup", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateServerGroup indicates an expected call of CreateServerGroup. +func (mr *MockComputeClientMockRecorder) CreateServerGroup(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateServerGroup", reflect.TypeOf((*MockComputeClient)(nil).CreateServerGroup), arg0, arg1) +} + // DeleteAttachedInterface mocks base method. func (m *MockComputeClient) DeleteAttachedInterface(arg0, arg1 string) error { m.ctrl.T.Helper() @@ -102,6 +116,20 @@ func (mr *MockComputeClientMockRecorder) DeleteServer(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteServer", reflect.TypeOf((*MockComputeClient)(nil).DeleteServer), arg0) } +// DeleteServerGroup mocks base method. +func (m *MockComputeClient) DeleteServerGroup(arg0 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteServerGroup", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteServerGroup indicates an expected call of DeleteServerGroup. +func (mr *MockComputeClientMockRecorder) DeleteServerGroup(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteServerGroup", reflect.TypeOf((*MockComputeClient)(nil).DeleteServerGroup), arg0) +} + // GetFlavorFromName mocks base method. func (m *MockComputeClient) GetFlavorFromName(arg0 string) (*flavors.Flavor, error) { m.ctrl.T.Helper() diff --git a/pkg/cloud/services/compute/servergroup.go b/pkg/cloud/services/compute/servergroup.go index c208362ab9..4b643aad3e 100644 --- a/pkg/cloud/services/compute/servergroup.go +++ b/pkg/cloud/services/compute/servergroup.go @@ -38,7 +38,7 @@ func (s *Service) GetServerGroupID(serverGroupParam *infrav1.ServerGroupParam) ( } // otherwise fallback to looking up by name, which is slower - serverGroup, err := s.getServerGroupByName(*serverGroupParam.Filter.Name) + serverGroup, err := s.GetServerGroupByName(*serverGroupParam.Filter.Name, false) if err != nil { return "", err } @@ -46,7 +46,8 @@ func (s *Service) GetServerGroupID(serverGroupParam *infrav1.ServerGroupParam) ( return serverGroup.ID, nil } -func (s *Service) getServerGroupByName(serverGroupName string) (*servergroups.ServerGroup, error) { +// TODO(dalees): This second parameter is messy. It would be better to differentiate 404, 'too many' and 'actual failure' in the caller. +func (s *Service) GetServerGroupByName(serverGroupName string, ignoreNotFound bool) (*servergroups.ServerGroup, error) { allServerGroups, err := s.getComputeClient().ListServerGroups() if err != nil { return nil, err @@ -62,11 +63,22 @@ func (s *Service) getServerGroupByName(serverGroupName string) (*servergroups.Se switch len(serverGroups) { case 0: + if ignoreNotFound { + return nil, nil + } return nil, fmt.Errorf("no server group with name %s could be found", serverGroupName) case 1: return &serverGroups[0], nil default: // this will never happen due to duplicate IDs, only duplicate names, so our error message is worded accordingly - return nil, fmt.Errorf("too many server groups with name %s were found", serverGroupName) + return &serverGroups[0], fmt.Errorf("too many server groups with name %s were found", serverGroupName) } } + +func (s *Service) CreateServerGroup(serverGroupName string, policy string) error { + return s.getComputeClient().CreateServerGroup(serverGroupName, policy) +} + +func (s *Service) DeleteServerGroup(id string) error { + return s.getComputeClient().DeleteServerGroup(id) +}