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.