From 65a96b7df55609857dde2da79d8ad68bde14dfa9 Mon Sep 17 00:00:00 2001 From: Dale Smith Date: Thu, 8 Feb 2024 17:00:58 +1300 Subject: [PATCH] Add OpenStackServerGroup CRD and Controller Implements new CRD for OpenstackServerGroup in v1alpha8 to allow managed Server Groups with standard policies, and adds ServerGroupRef to OpenstackMachine that references the new CRD and uses it for VM creation. Closes: https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/1256 --- PROJECT | 9 +- api/v1alpha5/zz_generated.conversion.go | 1 + api/v1alpha6/zz_generated.conversion.go | 1 + api/v1alpha7/zz_generated.conversion.go | 1 + api/v1alpha8/openstackmachine_types.go | 4 + api/v1alpha8/openstackservergroup_types.go | 76 +++++ api/v1alpha8/types.go | 8 +- api/v1alpha8/zz_generated.deepcopy.go | 114 +++++++ ...re.cluster.x-k8s.io_openstackclusters.yaml | 14 +- ...er.x-k8s.io_openstackclustertemplates.yaml | 12 + ...re.cluster.x-k8s.io_openstackmachines.yaml | 14 +- ...er.x-k8s.io_openstackmachinetemplates.yaml | 11 + ...luster.x-k8s.io_openstackservergroups.yaml | 90 +++++ config/crd/kustomization.yaml | 2 + .../cainjection_in_openstackservergroups.yaml | 8 + .../webhook_in_openstackservergroups.yaml | 17 + .../openstackservergroup_editor_role.yaml | 24 ++ .../openstackservergroup_viewer_role.yaml | 20 ++ config/rbac/role.yaml | 20 ++ controllers/openstackmachine_controller.go | 37 +- .../openstackmachine_controller_test.go | 77 +++++ .../openstackservergroup_controller.go | 214 ++++++++++++ .../openstackservergroup_controller_test.go | 322 ++++++++++++++++++ main.go | 10 + pkg/clients/compute.go | 31 ++ pkg/clients/mock/compute.go | 29 ++ .../services/compute/referenced_resources.go | 4 +- pkg/cloud/services/compute/servergroup.go | 20 +- pkg/scope/mock.go | 7 + pkg/scope/provider.go | 23 ++ pkg/scope/scope.go | 3 +- 31 files changed, 1209 insertions(+), 14 deletions(-) create mode 100644 api/v1alpha8/openstackservergroup_types.go create mode 100644 config/crd/bases/infrastructure.cluster.x-k8s.io_openstackservergroups.yaml create mode 100644 config/crd/patches/cainjection_in_openstackservergroups.yaml create mode 100644 config/crd/patches/webhook_in_openstackservergroups.yaml create mode 100644 config/rbac/openstackservergroup_editor_role.yaml create mode 100644 config/rbac/openstackservergroup_viewer_role.yaml create mode 100644 controllers/openstackservergroup_controller.go create mode 100644 controllers/openstackservergroup_controller_test.go diff --git a/PROJECT b/PROJECT index 6514dde9b3..7b5f56bc40 100644 --- a/PROJECT +++ b/PROJECT @@ -42,18 +42,21 @@ resources: kind: OpenStackClusterTemplate version: v1alpha7 - group: infrastructure - version: v1alpha8 kind: OpenStackCluster -- group: infrastructure version: v1alpha8 - kind: OpenStackMachine - group: infrastructure + kind: OpenStackMachine version: v1alpha8 +- group: infrastructure kind: OpenStackMachineTemplate + version: v1alpha8 - group: infrastructure kind: OpenStackClusterTemplate version: v1alpha8 - group: infrastructure kind: OpenStackFloatingIPPool version: v1alpha1 +- group: infrastructure + kind: OpenstackServerGroup + version: v1alpha8 version: "2" diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index 8ec8034142..320eda889a 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -1154,6 +1154,7 @@ func autoConvert_v1alpha8_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec( out.RootVolume = (*RootVolume)(unsafe.Pointer(in.RootVolume)) // 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 out.IdentityRef = (*OpenStackIdentityReference)(unsafe.Pointer(in.IdentityRef)) return nil } diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index 2bbcd48d10..cf859c47c5 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -1177,6 +1177,7 @@ func autoConvert_v1alpha8_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec( out.RootVolume = (*RootVolume)(unsafe.Pointer(in.RootVolume)) // 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 out.IdentityRef = (*OpenStackIdentityReference)(unsafe.Pointer(in.IdentityRef)) return nil } diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go index c29aaf334f..b4f34646dc 100644 --- a/api/v1alpha7/zz_generated.conversion.go +++ b/api/v1alpha7/zz_generated.conversion.go @@ -1298,6 +1298,7 @@ func autoConvert_v1alpha8_OpenStackMachineSpec_To_v1alpha7_OpenStackMachineSpec( out.RootVolume = (*RootVolume)(unsafe.Pointer(in.RootVolume)) out.AdditionalBlockDevices = *(*[]AdditionalBlockDevice)(unsafe.Pointer(&in.AdditionalBlockDevices)) // WARNING: in.ServerGroup requires manual conversion: does not exist in peer-type + // WARNING: in.ServerGroupRef requires manual conversion: does not exist in peer-type out.IdentityRef = (*OpenStackIdentityReference)(unsafe.Pointer(in.IdentityRef)) return nil } diff --git a/api/v1alpha8/openstackmachine_types.go b/api/v1alpha8/openstackmachine_types.go index bc98e02a98..c8647f8e5c 100644 --- a/api/v1alpha8/openstackmachine_types.go +++ b/api/v1alpha8/openstackmachine_types.go @@ -88,6 +88,10 @@ type OpenStackMachineSpec struct { // +optional ServerGroup *ServerGroupFilter `json:"serverGroup,omitempty"` + // The server group ref to assign the machine to. + // +optional + ServerGroupRef *ServerGroupRef `json:"serverGroupRef,omitempty"` + // IdentityRef is a reference to a identity to be used when reconciling this cluster // +optional IdentityRef *OpenStackIdentityReference `json:"identityRef,omitempty"` diff --git a/api/v1alpha8/openstackservergroup_types.go b/api/v1alpha8/openstackservergroup_types.go new file mode 100644 index 0000000000..1b9b82400a --- /dev/null +++ b/api/v1alpha8/openstackservergroup_types.go @@ -0,0 +1,76 @@ +/* +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 v1alpha8 + +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 { + // The name of the cloud to use from the clouds secret + // +optional + CloudName string `json:"cloudName"` + + // 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"` +} + +func init() { + objectTypes = append(objectTypes, &OpenStackServerGroup{}, &OpenStackServerGroupList{}) +} diff --git a/api/v1alpha8/types.go b/api/v1alpha8/types.go index 8c206d803d..2f681334a0 100644 --- a/api/v1alpha8/types.go +++ b/api/v1alpha8/types.go @@ -240,6 +240,12 @@ type ServerGroupFilter struct { Name string `json:"name,omitempty"` } +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 @@ -489,7 +495,7 @@ type APIServerLoadBalancer struct { // ReferencedMachineResources contains resolved references to resources required by the machine. type ReferencedMachineResources 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 or ServerGroupRef. // +optional ServerGroupID string `json:"serverGroupID,omitempty"` diff --git a/api/v1alpha8/zz_generated.deepcopy.go b/api/v1alpha8/zz_generated.deepcopy.go index 2b52b02cd2..b402b554c1 100644 --- a/api/v1alpha8/zz_generated.deepcopy.go +++ b/api/v1alpha8/zz_generated.deepcopy.go @@ -745,6 +745,11 @@ func (in *OpenStackMachineSpec) DeepCopyInto(out *OpenStackMachineSpec) { *out = new(ServerGroupFilter) **out = **in } + 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) @@ -895,6 +900,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 @@ -1190,6 +1289,21 @@ func (in *ServerGroupFilter) DeepCopy() *ServerGroupFilter { 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 aa96ad27ea..485e5f7010 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5268,6 +5268,17 @@ spec: name: 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. @@ -5713,7 +5724,8 @@ spec: type: string 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 + or ServerGroupRef. type: string type: object sshKeyName: 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 cb48bc42f4..3d54e5ede5 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2701,6 +2701,18 @@ spec: name: 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 67da66b7ea..b4c5c0a3f1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -2064,6 +2064,17 @@ spec: name: 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. @@ -2214,7 +2225,8 @@ spec: type: string 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 + or ServerGroupRef. type: string type: object type: object 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 c96a204003..143c6c8c4c 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -1743,6 +1743,17 @@ spec: name: 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..5b759810dd --- /dev/null +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackservergroups.yaml @@ -0,0 +1,90 @@ +--- +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: v1alpha8 + 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: + cloudName: + description: The name of the cloud to use from the clouds secret + type: string + identityRef: + description: IdentityRef is a reference to a identity to be used when + reconciling this resource + properties: + kind: + description: |- + Kind of the identity. Must be supported by the infrastructure + provider and may be either cluster or namespace-scoped. + minLength: 1 + type: string + name: + description: |- + Name of the infrastructure identity to be used. + Must be either a cluster-scoped resource, or namespaced-scoped + resource the same namespace as the resource(s) being provisioned. + type: string + required: + - kind + - 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 c153237cc4..466e8355dd 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 6a40632602..d49936468c 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/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index 71972fdeff..7a9e3fc5f8 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -147,12 +147,18 @@ func (r *OpenStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req return reconcile.Result{}, err } - // Resolve and store referenced resources + // Resolve and store referenced OpenStack resources err = compute.ResolveReferencedMachineResources(scope, &openStackMachine.Spec, &openStackMachine.Status.ReferencedResources) if err != nil { return reconcile.Result{}, err } + // Resolve referenced resources CAPO resources, using the K8s client + err = resolveReferencedClientResources(ctx, r.Client, openStackMachine) + if err != nil { + return reconcile.Result{}, err + } + // Handle deleted machines if !openStackMachine.DeletionTimestamp.IsZero() { return r.reconcileDelete(scope, cluster, infraCluster, machine, openStackMachine) @@ -162,6 +168,33 @@ func (r *OpenStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req return r.reconcileNormal(ctx, scope, cluster, infraCluster, machine, openStackMachine) } +func resolveReferencedClientResources(ctx context.Context, ctrlClient client.Client, openStackMachine *infrav1.OpenStackMachine) error { + var spec *infrav1.OpenStackMachineSpec = &openStackMachine.Spec + var resources *infrav1.ReferencedMachineResources = &openStackMachine.Status.ReferencedResources + var namespace string = openStackMachine.ObjectMeta.Namespace + + // Resolve ServerGroupRef if it's not resolved already + if spec.ServerGroupRef != nil && resources.ServerGroupID == "" { + servergroup := &infrav1.OpenStackServerGroup{} + + // Get OpenStackServerGroup resource from K8s + err := ctrlClient.Get(ctx, client.ObjectKey{ + Namespace: namespace, + Name: spec.ServerGroupRef.Name}, servergroup) + if err != nil { + return err + } + + // Requeue Reconcile, unless it's in Ready state. + if !servergroup.Status.Ready { + return errors.New("referenced OpenStackServerGroup is not ready yet") + } + + resources.ServerGroupID = servergroup.Status.ID + } + return nil +} + func patchMachine(ctx context.Context, patchHelper *patch.Helper, openStackMachine *infrav1.OpenStackMachine, machine *clusterv1.Machine, options ...patch.Option) error { // Always update the readyCondition by summarizing the state of other conditions. applicableConditions := []clusterv1.ConditionType{ @@ -396,7 +429,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, shelved) 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 32db7b92bb..1c7cc58d8e 100644 --- a/controllers/openstackmachine_controller_test.go +++ b/controllers/openstackmachine_controller_test.go @@ -260,3 +260,80 @@ func Test_machineToInstanceSpec(t *testing.T) { }) } } + +/* +func Test_resolveReferencedClientResources(t *testing.T) { + RegisterTestingT(t) + + tests := []struct { + name string + ctx *context.Context + mockCtrl *gomock.Controller + openStackMachine func() *infrav1.OpenStackMachine + wantInstanceSpec func() *compute.InstanceSpec + }{ + { + name: "Defaults", + ctx: context.TODO(), + mockCtrl: gomock.NewController(GinkgoT()), + openStackMachine: getDefaultOpenStackMachine, + wantInstanceSpec: getDefaultInstanceSpec, + }, + { + name: "Control plane security group", + openStackCluster: func() *infrav1.OpenStackCluster { + c := getDefaultOpenStackCluster() + c.Spec.ManagedSecurityGroups = true + return c + }, + machine: func() *clusterv1.Machine { + m := getDefaultMachine() + m.Labels = map[string]string{ + clusterv1.MachineControlPlaneLabel: "true", + } + return m + }, + openStackMachine: getDefaultOpenStackMachine, + wantInstanceSpec: func() *compute.InstanceSpec { + i := getDefaultInstanceSpec() + i.SecurityGroups = []infrav1.SecurityGroupFilter{{ID: controlPlaneSecurityGroupUUID}} + return i + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + //ctx context.Context, ctrlClient client.Client, openStackMachine *infrav1.OpenStackMachine + got := resolveReferencedClientResources(tt.openStackCluster(), tt.machine(), tt.openStackMachine(), "user-data") + Expect(got).To(Equal(tt.wantInstanceSpec())) + }) + } +} +*/ + +/* + +These are tests for resolveReferencedClientResources + { + testName: "ServerGroupRef should be used", + serverGroupRef: &infrav1.ServerGroupRef{Name: "openstack-server-group-name"}, + expectComputeMock: func(m *mock.MockComputeClientMockRecorder) {}, + expectImageMock: func(m *mock.MockImageClientMockRecorder) {}, + expectClientMock: func() + want: &infrav1.ReferencedMachineResources{ImageID: imageID1, ServerGroupID: serverGroupID2}, + wantErr: false, + }, + { + testName: "ServerGroupRef not ready", + serverGroupRef: + want: &infrav1.ReferencedMachineResources{ImageID: imageID1, ServerGroupID: serverGroupID2}, + wantErr: false, + }, + { + testName: "ServerGroupID has higher priority than ServerGroupRef" + serverGroupFilter: , + serverGroupRef: + want: &infrav1.ReferencedMachineResources{ImageID: imageID1, ServerGroupID: serverGroupID1}, + wantErr: false, + }, +*/ diff --git a/controllers/openstackservergroup_controller.go b/controllers/openstackservergroup_controller.go new file mode 100644 index 0000000000..dd249f7d3c --- /dev/null +++ b/controllers/openstackservergroup_controller.go @@ -0,0 +1,214 @@ +/* +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/openstack/compute/v2/extensions/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/v1alpha8" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" +) + +// 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 OpenStackMachine 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}) + } + }() + + scope, err := r.ScopeFactory.NewClientScopeFromServerGroup(ctx, r.Client, openStackServerGroup, r.CaCertificates, 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(ctx, 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.Scope, openStackServerGroup *infrav1.OpenStackServerGroup) (result ctrl.Result, reterr error) { + scope.Logger().Info("Reconciling ServerGroup delete") + + serverGroupName := openStackServerGroup.Name + + computeService, err := compute.NewService(scope) + if err != nil { + return ctrl.Result{}, err + } + + // Get the servergroup by name, even if our K8s resource already has the ID field set. + // TODO(dalees): If this returns a 404 do we try to delete with existing UUID? Do we just assume success? + serverGroup, err := computeService.GetServerGroupByName(serverGroupName, true) + // Retry if the failure was anything other than Not Found. + if err != nil { + return ctrl.Result{}, err + } + + if serverGroup != nil { + err = computeService.DeleteServerGroup(serverGroup.ID) + if err != nil { + return ctrl.Result{}, err + } + } + + controllerutil.RemoveFinalizer(openStackServerGroup, infrav1.ServerGroupFinalizer) + scope.Logger().Info("Reconciled ServerGroup delete successfully") + return ctrl.Result{}, nil +} + +func (r *OpenStackServerGroupReconciler) reconcileNormal(ctx context.Context, scope scope.Scope, openStackServerGroup *infrav1.OpenStackServerGroup) (result ctrl.Result, reterr error) { + + // 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 ServerGroup") + + 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 ServerGroup 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 ServerGroup instance, we need to create it.", "name", serverGroupName, "policy", openStackServerGroup.Spec.Policy) + + serverGroup, err = computeService.CreateServerGroup(serverGroupName, openStackServerGroup.Spec.Policy) + 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..75e4b98480 --- /dev/null +++ b/controllers/openstackservergroup_controller_test.go @@ -0,0 +1,322 @@ +/* +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/go-logr/logr" + "github.com/golang/mock/gomock" + "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/servergroups" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + 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/v1alpha8" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" +) + +var ( + reconcilerServerGroup *OpenStackServerGroupReconciler + //ctx context.Context + testServerGroup *infrav1.OpenStackServerGroup + //testNamespace string + //mockCtrl *gomock.Controller + //mockScopeFactory *scope.MockScopeFactory +) + +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, "", logr.Discard()) + 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 even if its uuid is not stored in status", 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 delete an existing servergroup even if its uuid is not stored in status", 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()) + + // Define and record the existing resource the reconcile will see. + servergroups := []servergroups.ServerGroup{ + { + Name: "delete-existing-servergroup-no-uuid", + ID: "existing-servergroup-uuid", + Policies: []string{"anti-affinity"}, + }, + } + computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT() + computeClientRecorder.ListServerGroups().Return(servergroups, nil) + computeClientRecorder.DeleteServerGroup("existing-servergroup-uuid").Return(nil) + + // Reconcile our resource, and make sure it finds the expected resource, then deletes it. + scope, err := mockScopeFactory.NewClientScopeFromServerGroup(ctx, k8sClient, testServerGroup, nil, logr.Discard()) + Expect(err).To(BeNil()) + 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: "", + 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) + + // Reconcile our resource, and make sure it finds the expected resource, then deletes it. + scope, err := mockScopeFactory.NewClientScopeFromServerGroup(ctx, k8sClient, testServerGroup, nil, logr.Discard()) + Expect(err).To(BeNil()) + 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) + expected_error := gophercloud.ErrDefault500{} + computeClientRecorder.CreateServerGroup("requeue-on-openstack-error", "anti-affinity").Return(nil, expected_error) + + // Reconcile our resource, and make sure it adopted the expected resource. + scope, err := mockScopeFactory.NewClientScopeFromServerGroup(ctx, k8sClient, testServerGroup, nil, logr.Discard()) + Expect(err).To(BeNil()) + result, err := reconcilerServerGroup.reconcileNormal(ctx, 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(expected_error)) + 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/main.go b/main.go index f9dfdd5d03..71ed69641b 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 64ae059fc7..333b3d9d70 100644 --- a/pkg/clients/compute.go +++ b/pkg/clients/compute.go @@ -63,6 +63,8 @@ type ComputeClient interface { DeleteAttachedInterface(serverID, portID string) error ListServerGroups() ([]servergroups.ServerGroup, error) + CreateServerGroup(name string, policy string) (*servergroups.ServerGroup, error) + DeleteServerGroup(id string) error } type computeClient struct{ client *gophercloud.ServiceClient } @@ -162,6 +164,27 @@ func (c computeClient) ListServerGroups() ([]servergroups.ServerGroup, error) { return servergroups.ExtractServerGroups(allPages) } +func (c computeClient) CreateServerGroup(name string, policy string) (*servergroups.ServerGroup, 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(c.client, opts).ExtractInto(&servergroup) + if mc.ObserveRequest(err) != nil { + return nil, err + } + return &servergroup, err +} + +func (c computeClient) DeleteServerGroup(id string) error { + mc := metrics.NewMetricPrometheusContext("server_group", "delete") + err := servergroups.Delete(c.client, id).ExtractErr() + return mc.ObserveRequestIgnoreNotFoundorConflict(err) +} + type computeErrorClient struct{ error } // NewComputeErrorClient returns a ComputeClient in which every method returns the given error. @@ -204,3 +227,11 @@ func (e computeErrorClient) DeleteAttachedInterface(_, _ string) error { func (e computeErrorClient) ListServerGroups() ([]servergroups.ServerGroup, error) { return nil, e.error } + +func (e computeErrorClient) CreateServerGroup(_ string, _ string) (*servergroups.ServerGroup, error) { + return nil, 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 77e7b7a8b2..a6411bb2aa 100644 --- a/pkg/clients/mock/compute.go +++ b/pkg/clients/mock/compute.go @@ -70,6 +70,21 @@ func (mr *MockComputeClientMockRecorder) CreateServer(arg0 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateServer", reflect.TypeOf((*MockComputeClient)(nil).CreateServer), arg0) } +// CreateServerGroup mocks base method. +func (m *MockComputeClient) CreateServerGroup(arg0, arg1 string) (*servergroups.ServerGroup, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateServerGroup", arg0, arg1) + ret0, _ := ret[0].(*servergroups.ServerGroup) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateServerGroup indicates an expected call of CreateServerGroup. +func (mr *MockComputeClientMockRecorder) CreateServerGroup(arg0, arg1 interface{}) *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() @@ -98,6 +113,20 @@ func (mr *MockComputeClientMockRecorder) DeleteServer(arg0 interface{}) *gomock. 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 interface{}) *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/referenced_resources.go b/pkg/cloud/services/compute/referenced_resources.go index b9350f8d06..7aa4bd005b 100644 --- a/pkg/cloud/services/compute/referenced_resources.go +++ b/pkg/cloud/services/compute/referenced_resources.go @@ -22,8 +22,8 @@ import ( ) // 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. +// the resources referenced in the OpenStackMachineSpec by querying the OpenStack APIs and K8s resources. +// It'll return an error if resources cannot be found or their filters are ambiguous. func ResolveReferencedMachineResources(scope scope.Scope, spec *infrav1.OpenStackMachineSpec, resources *infrav1.ReferencedMachineResources) error { compute, err := NewService(scope) if err != nil { diff --git a/pkg/cloud/services/compute/servergroup.go b/pkg/cloud/services/compute/servergroup.go index b084a99634..6b1a4a3628 100644 --- a/pkg/cloud/services/compute/servergroup.go +++ b/pkg/cloud/services/compute/servergroup.go @@ -27,6 +27,8 @@ import ( // GetServerGroupID looks up a server group using the passed filter and returns // its ID. It'll return an error when server group is not found or there are multiple. func (s *Service) GetServerGroupID(serverGroupFilter *infrav1.ServerGroupFilter) (string, error) { + // NOTE(dalees): This is an early exit if ID is set, and never hits the Nova API with the passed ID. This is not what the function description says it does. + // but it's probably okay; creating a server using the ID will surface the error. if serverGroupFilter.ID != "" { return serverGroupFilter.ID, nil } @@ -37,7 +39,7 @@ func (s *Service) GetServerGroupID(serverGroupFilter *infrav1.ServerGroupFilter) } // otherwise fallback to looking up by name, which is slower - serverGroup, err := s.getServerGroupByName(serverGroupFilter.Name) + serverGroup, err := s.GetServerGroupByName(serverGroupFilter.Name, false) if err != nil { return "", err } @@ -45,7 +47,8 @@ func (s *Service) GetServerGroupID(serverGroupFilter *infrav1.ServerGroupFilter) 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 @@ -61,11 +64,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) (*servergroups.ServerGroup, error) { + return s.getComputeClient().CreateServerGroup(serverGroupName, policy) +} + +func (s *Service) DeleteServerGroup(id string) error { + return s.getComputeClient().DeleteServerGroup(id) +} diff --git a/pkg/scope/mock.go b/pkg/scope/mock.go index 7dd2364b92..e042feef72 100644 --- a/pkg/scope/mock.go +++ b/pkg/scope/mock.go @@ -88,6 +88,13 @@ func (f *MockScopeFactory) NewClientScopeFromFloatingIPPool(_ context.Context, _ return f, nil } +func (f *MockScopeFactory) NewClientScopeFromServerGroup(_ context.Context, _ client.Client, _ *infrav1.OpenStackServerGroup, _ []byte, _ logr.Logger) (Scope, error) { + if f.clientScopeCreateError != nil { + return nil, f.clientScopeCreateError + } + return f, nil +} + func (f *MockScopeFactory) NewComputeClient() (clients.ComputeClient, error) { return f.ComputeClient, nil } diff --git a/pkg/scope/provider.go b/pkg/scope/provider.go index 707b08b5b9..9cc26523e7 100644 --- a/pkg/scope/provider.go +++ b/pkg/scope/provider.go @@ -128,6 +128,29 @@ func (f *providerScopeFactory) NewClientScopeFromFloatingIPPool(ctx context.Cont return NewCachedProviderScope(f.clientCache, cloud, caCert, logger) } +func (f *providerScopeFactory) NewClientScopeFromServerGroup(ctx context.Context, ctrlClient client.Client, openStackServerGroup *infrav1.OpenStackServerGroup, defaultCACert []byte, logger logr.Logger) (Scope, error) { + var cloud clientconfig.Cloud + var caCert []byte + + if openStackServerGroup.Spec.IdentityRef != nil { + var err error + cloud, caCert, err = getCloudFromSecret(ctx, ctrlClient, openStackServerGroup.Namespace, openStackServerGroup.Spec.IdentityRef.Name, openStackServerGroup.Spec.CloudName) + if err != nil { + return nil, err + } + } + + if caCert == nil { + caCert = defaultCACert + } + + if f.clientCache == nil { + return NewProviderScope(cloud, caCert, logger) + } + + return NewCachedProviderScope(f.clientCache, cloud, caCert, logger) +} + func getScopeCacheKey(cloud clientconfig.Cloud) (string, error) { key, err := hash.ComputeSpewHash(cloud) if err != nil { diff --git a/pkg/scope/scope.go b/pkg/scope/scope.go index 5c8f24b968..caceb442a3 100644 --- a/pkg/scope/scope.go +++ b/pkg/scope/scope.go @@ -40,11 +40,12 @@ func NewFactory(maxCacheSize int) Factory { } } -// Factory instantiates a new Scope using credentials from either a cluster or a machine. +// Factory instantiates a new Scope using credentials from the provided resources. type Factory interface { NewClientScopeFromMachine(ctx context.Context, ctrlClient client.Client, openStackMachine *infrav1.OpenStackMachine, openStackCluster *infrav1.OpenStackCluster, defaultCACert []byte, logger logr.Logger) (Scope, error) NewClientScopeFromCluster(ctx context.Context, ctrlClient client.Client, openStackCluster *infrav1.OpenStackCluster, defaultCACert []byte, logger logr.Logger) (Scope, error) NewClientScopeFromFloatingIPPool(ctx context.Context, ctrlClient client.Client, openStackCluster *v1alpha1.OpenStackFloatingIPPool, defaultCACert []byte, logger logr.Logger) (Scope, error) + NewClientScopeFromServerGroup(ctx context.Context, ctrlClient client.Client, openStackServerGroup *infrav1.OpenStackServerGroup, defaultCACert []byte, logger logr.Logger) (Scope, error) } // Scope contains arguments common to most operations.