From 6265521ca7b53d84b27ad8fec7537876018464e0 Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Tue, 6 Jun 2023 14:08:40 +0200 Subject: [PATCH 1/3] Clean up ManilaShare instances When an entry in the ManilaShares spec is deleted, the corresponding instance must also be deleted. Note that removing the instance backend currently does not clean up the service entry in the manila DB. That will be handled in a follow-up patch. Signed-off-by: Francesco Pantano --- controllers/manila_controller.go | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/controllers/manila_controller.go b/controllers/manila_controller.go index bd1c5f90..8af250a1 100644 --- a/controllers/manila_controller.go +++ b/controllers/manila_controller.go @@ -815,6 +815,11 @@ func (r *ManilaReconciler) reconcileNormal(ctx context.Context, instance *manila instance.Status.Conditions.MarkTrue(condition.CronJobReadyCondition, condition.CronJobReadyMessage) // create CronJob - end + err = r.shareCleanup(ctx, instance) + if err != nil { + return ctrl.Result{}, err + } + r.Log.Info(fmt.Sprintf("Reconciled Service '%s' successfully", instance.Name)) // update the overall status condition if service is ready if instance.IsReady() { @@ -1243,3 +1248,37 @@ func (r *ManilaReconciler) checkManilaShareGeneration( } return true, nil } + +// shareCleanup - Delete manila share instances that no longer appears in the spec. +func (r *ManilaReconciler) shareCleanup( + ctx context.Context, + instance *manilav1beta1.Manila, +) error { + // Generate a list of share CRs + shares := &manilav1beta1.ManilaShareList{} + listOpts := []client.ListOption{ + client.InNamespace(instance.Namespace), + } + if err := r.Client.List(ctx, shares, listOpts...); err != nil { + r.Log.Error(err, "Unable to retrieve Manila Share CRs %v") + return nil + } + for _, share := range shares.Items { + // Skip shares CRs that we don't own + if manila.GetOwningManilaName(&share) != instance.Name { + continue + } + + // Delete the manilaShare if it's no longer in the spec + _, exists := instance.Spec.ManilaShares[share.ShareName()] + if !exists && share.DeletionTimestamp.IsZero() { + err := r.Client.Delete(ctx, &share) + if err != nil && !k8s_errors.IsNotFound(err) { + err = fmt.Errorf("Error cleaning up %s: %w", share.Name, err) + return err + } + delete(instance.Status.ManilaSharesReadyCounts, share.ShareName()) + } + } + return nil +} From 3aee7c955dc4d891f3c24b84628d1cc7039a6c2f Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Wed, 18 Sep 2024 14:04:48 +0200 Subject: [PATCH 2/3] Add kuttl steps in manila-multibackend to validate share cleanup This patch introduces an additional kuttl step to validate the share cleanup code that has been introduced. In particular, after scaling down share0, the current code patches the top-level manila CR and removes the entry, resulting in a cleanup of share0, that is not part of the deployment anymore. Signed-off-by: Francesco Pantano --- .../tests/manila-multibackend/04-assert.yaml | 233 ++++++++++++++++++ .../manila-multibackend/04-remove-share.yaml | 5 + ...nup-manila.yaml => 05-cleanup-manila.yaml} | 0 .../{04-errors.yaml => 05-errors.yaml} | 0 4 files changed, 238 insertions(+) create mode 100644 test/kuttl/tests/manila-multibackend/04-assert.yaml create mode 100644 test/kuttl/tests/manila-multibackend/04-remove-share.yaml rename test/kuttl/tests/manila-multibackend/{04-cleanup-manila.yaml => 05-cleanup-manila.yaml} (100%) rename test/kuttl/tests/manila-multibackend/{04-errors.yaml => 05-errors.yaml} (100%) diff --git a/test/kuttl/tests/manila-multibackend/04-assert.yaml b/test/kuttl/tests/manila-multibackend/04-assert.yaml new file mode 100644 index 00000000..c0d5a708 --- /dev/null +++ b/test/kuttl/tests/manila-multibackend/04-assert.yaml @@ -0,0 +1,233 @@ +# Check for: +# +# - 1 manilaAPI +# - 2 manilaScheduler +# - 3 manilaShares +# - 4 extraMounts + +apiVersion: manila.openstack.org/v1beta1 +kind: Manila +metadata: + name: manila +spec: + customServiceConfig: | + [DEFAULT] + debug = true + databaseInstance: openstack + databaseAccount: manila + manilaAPI: + customServiceConfig: | + [DEFAULT] + enabled_share_protocols=nfs,cephfs,cifs + replicas: 1 + resources: {} + manilaScheduler: + customServiceConfig: '# add your customization here' + replicas: 1 + resources: {} + manilaShares: + share1: + replicas: 1 + customServiceConfig: | + [DEFAULT] + enabled_share_backends=cephfsnfs + [cephfsnfs] + driver_handles_share_servers=False + share_backend_name=cephfs + share_driver=manila.share.drivers.cephfs.driver.CephFSDriver + cephfs_auth_id=openstack + cephfs_cluster_name=ceph + cephfs_nfs_cluster_id=cephfs + cephfs_protocol_helper_type=NFS + passwordSelectors: + service: ManilaPassword + preserveJobs: false + rabbitMqClusterName: rabbitmq + secret: osp-secret + serviceUser: manila +status: + conditions: + - message: Setup complete + reason: Ready + status: "True" + type: Ready + - message: CronJob completed + reason: Ready + status: "True" + type: CronJobReady + - message: DB create completed + reason: Ready + status: "True" + type: DBReady + - message: DBsync completed + reason: Ready + status: "True" + type: DBSyncReady + - message: Input data complete + reason: Ready + status: "True" + type: InputReady + - message: Setup complete + reason: Ready + status: "True" + type: ManilaAPIReady + - message: Setup complete + reason: Ready + status: "True" + type: ManilaSchedulerReady + - message: Deployment completed + reason: Ready + status: "True" + type: ManilaShareReady + - message: MariaDBAccount creation complete + reason: Ready + status: "True" + type: MariaDBAccountReady + - message: " Memcached instance has been provisioned" + reason: Ready + status: "True" + type: MemcachedReady + - message: NetworkAttachments completed + reason: Ready + status: "True" + type: NetworkAttachmentsReady + - message: RabbitMqTransportURL successfully created + reason: Ready + status: "True" + type: RabbitMqTransportURLReady + - message: RoleBinding created + reason: Ready + status: "True" + type: RoleBindingReady + - message: Role created + reason: Ready + status: "True" + type: RoleReady + - message: ServiceAccount created + reason: Ready + status: "True" + type: ServiceAccountReady + - message: Service config create completed + reason: Ready + status: "True" + type: ServiceConfigReady + databaseHostname: openstack.manila-kuttl-tests.svc + manilaAPIReadyCount: 1 + manilaSchedulerReadyCount: 1 + manilaSharesReadyCounts: + share1: 1 + transportURLSecret: rabbitmq-transport-url-manila-manila-transport +--- +apiVersion: batch/v1 +kind: CronJob +metadata: + name: manila-db-purge +spec: + jobTemplate: + metadata: + labels: + service: manila + spec: + completions: 1 + parallelism: 1 + template: + spec: + containers: + - args: + - -c + - /usr/bin/manila-manage --config-dir /etc/manila/manila.conf.d + db purge 30 + command: + - /bin/bash + name: manila-db-purge + volumeMounts: + - mountPath: /etc/manila/manila.conf.d + name: db-purge-config-data + readOnly: true + serviceAccount: manila-manila + serviceAccountName: manila-manila + volumes: + - name: db-purge-config-data + secret: + defaultMode: 420 + items: + - key: 00-config.conf + path: 00-config.conf + secretName: manila-config-data + schedule: 1 0 * * * + suspend: false +--- +apiVersion: manila.openstack.org/v1beta1 +kind: ManilaShare +metadata: + name: manila-share-share1 +spec: + databaseAccount: manila + passwordSelectors: + service: ManilaPassword + replicas: 1 + resources: {} + secret: osp-secret + serviceAccount: manila-manila + serviceUser: manila + transportURLSecret: rabbitmq-transport-url-manila-manila-transport +status: + conditions: + - message: Setup complete + reason: Ready + status: "True" + type: Ready + - message: Deployment completed + reason: Ready + status: "True" + type: DeploymentReady + - message: Input data complete + reason: Ready + status: "True" + type: InputReady + - message: NetworkAttachments completed + reason: Ready + status: "True" + type: NetworkAttachmentsReady + - message: Service config create completed + reason: Ready + status: "True" + type: ServiceConfigReady + - message: Input data complete + reason: Ready + status: "True" + type: TLSInputReady + readyCount: 1 +--- +# when using image digests the containerImage URLs are SHA's so we verify them with a script +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +commands: + - script: | + tupleTemplate='{{ range (index .spec.template.spec.containers 1).env }}{{ .name }}{{ "#" }}{{ .value}}{{"\n"}}{{ end }}' + imageTuples=$(oc get -n openstack-operators deployment manila-operator-controller-manager -o go-template="$tupleTemplate") + # format of imageTuple is: RELATED_IMAGE_MANILA_# separated by newlines + for ITEM in $(echo $imageTuples); do + # it is an image + if echo $ITEM | grep 'RELATED_IMAGE' &> /dev/null; then + NAME=$(echo $ITEM | sed -e 's|^RELATED_IMAGE_MANILA_\([^_]*\)_.*|\1|') + IMG_FROM_ENV=$(echo $ITEM | sed -e 's|^.*#\(.*\)|\1|') + template='{{.spec.containerImage}}' + case $NAME in + API) + SERVICE_IMAGE=$(oc get -n $NAMESPACE manilaapi manila-api -o go-template="$template") + ;; + SHARE) + SERVICE_IMAGE=$(oc get -n $NAMESPACE manilashares manila-share-share1 -o go-template="$template") + ;; + SCHEDULER) + SERVICE_IMAGE=$(oc get -n $NAMESPACE manilascheduler manila-scheduler -o go-template="$template") + ;; + esac + if [ "$SERVICE_IMAGE" != "$IMG_FROM_ENV" ]; then + echo "$NAME image does not equal $VALUE" + exit 1 + fi + fi + done + exit 0 diff --git a/test/kuttl/tests/manila-multibackend/04-remove-share.yaml b/test/kuttl/tests/manila-multibackend/04-remove-share.yaml new file mode 100644 index 00000000..0132c746 --- /dev/null +++ b/test/kuttl/tests/manila-multibackend/04-remove-share.yaml @@ -0,0 +1,5 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: | + oc patch manila -n $NAMESPACE manila --type='json' -p='[{"op": "remove", "path": "/spec/manilaShares/share0"}]' diff --git a/test/kuttl/tests/manila-multibackend/04-cleanup-manila.yaml b/test/kuttl/tests/manila-multibackend/05-cleanup-manila.yaml similarity index 100% rename from test/kuttl/tests/manila-multibackend/04-cleanup-manila.yaml rename to test/kuttl/tests/manila-multibackend/05-cleanup-manila.yaml diff --git a/test/kuttl/tests/manila-multibackend/04-errors.yaml b/test/kuttl/tests/manila-multibackend/05-errors.yaml similarity index 100% rename from test/kuttl/tests/manila-multibackend/04-errors.yaml rename to test/kuttl/tests/manila-multibackend/05-errors.yaml From 9685a171ab15df992faaa9a8d63669861c719356 Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Wed, 18 Sep 2024 14:11:34 +0200 Subject: [PATCH 3/3] Remove databaseHostname from CR kuttl status This field, that depends on the namespace where kuttl is executed, is not useful in the effort of troubleshooting and making these tests easily reproducible, hence removing from the assert. Signed-off-by: Francesco Pantano --- test/kuttl/tests/manila-multibackend/01-assert.yaml | 2 -- test/kuttl/tests/manila-multibackend/03-assert.yaml | 1 - test/kuttl/tests/manila-multibackend/04-assert.yaml | 1 - 3 files changed, 4 deletions(-) diff --git a/test/kuttl/tests/manila-multibackend/01-assert.yaml b/test/kuttl/tests/manila-multibackend/01-assert.yaml index f2c551de..eaad594e 100644 --- a/test/kuttl/tests/manila-multibackend/01-assert.yaml +++ b/test/kuttl/tests/manila-multibackend/01-assert.yaml @@ -123,7 +123,6 @@ status: reason: Ready status: "True" type: ServiceConfigReady - databaseHostname: openstack.manila-kuttl-tests.svc manilaAPIReadyCount: 1 manilaSchedulerReadyCount: 1 manilaSharesReadyCounts: @@ -175,7 +174,6 @@ kind: ManilaShare metadata: name: manila-share-share0 spec: - databaseHostname: openstack.manila-kuttl-tests.svc databaseAccount: manila passwordSelectors: service: ManilaPassword diff --git a/test/kuttl/tests/manila-multibackend/03-assert.yaml b/test/kuttl/tests/manila-multibackend/03-assert.yaml index 2574d079..f880b3ea 100644 --- a/test/kuttl/tests/manila-multibackend/03-assert.yaml +++ b/test/kuttl/tests/manila-multibackend/03-assert.yaml @@ -126,7 +126,6 @@ status: reason: Ready status: "True" type: ServiceConfigReady - databaseHostname: openstack.manila-kuttl-tests.svc manilaAPIReadyCount: 1 manilaSchedulerReadyCount: 1 manilaSharesReadyCounts: diff --git a/test/kuttl/tests/manila-multibackend/04-assert.yaml b/test/kuttl/tests/manila-multibackend/04-assert.yaml index c0d5a708..bf8084e5 100644 --- a/test/kuttl/tests/manila-multibackend/04-assert.yaml +++ b/test/kuttl/tests/manila-multibackend/04-assert.yaml @@ -111,7 +111,6 @@ status: reason: Ready status: "True" type: ServiceConfigReady - databaseHostname: openstack.manila-kuttl-tests.svc manilaAPIReadyCount: 1 manilaSchedulerReadyCount: 1 manilaSharesReadyCounts: