ClientProfile Controller: Add ClientProfile deletion and cleanup logic

Signed-off-by: nb-ohad <[email protected]>

internal/controller/clientprofile_controller.go

@@ -51,6 +51,7 @@ type ClientProfileReconcile struct {
 	log           logr.Logger
 	clientProfile csiv1a1.ClientProfile
 	cephConn      csiv1a1.CephConnection
+	cleanUp       bool
 }
 
 // csiClusterRrcordInfo represent the structure of a serialized csi record
@@ -74,6 +75,10 @@ type csiClusterInfoRecord struct {
 	} `json:"readAffinity,omitempty"`
 }
 
+const (
+	cleanupFinalizer = "csi.ceph.com/cleanup"
+)
+
 var configMapUpdateLock = sync.Mutex{}
 
 //+kubebuilder:rbac:groups=csi.ceph.io,resources=clientprofile,verbs=get;list;watch;create;update;patch;delete
@@ -128,23 +133,29 @@ func (r *ClientProfileReconcile) reconcile() error {
 		return err
 	}
 
-	// Ensure the ceph connection resource has an owner reference (not controller reference)
-	// for the current reconciled config resource
-	if !utils.IsOwnedBy(&r.cephConn, &r.clientProfile) {
-		if err := ctrlutil.SetOwnerReference(&r.clientProfile, &r.cephConn, r.Scheme); err != nil {
-			r.log.Error(err, "Failed adding an owner reference on CephConnection")
-			return err
-		}
-		if err := r.Update(r.ctx, &r.cephConn); err != nil {
-			r.log.Error(err, "Failed to update CephConnection")
+	// Ensure a finalizer on the ClientProfile to allow proper clean up
+	if ctrlutil.AddFinalizer(&r.clientProfile, cleanupFinalizer) {
+		if err := r.Update(r.ctx, &r.clientProfile); err != nil {
+			r.log.Error(err, "Failed to add a cleanup finalizer on ClientProfile")
 			return err
 		}
 	}
 
+	if err := r.reconcileCephConnection(); err != nil {
+		return err
+	}
 	if err := r.reconcileCephCsiClusterInfo(); err != nil {
 		return err
 	}
 
+	if r.cleanUp {
+		ctrlutil.RemoveFinalizer(&r.clientProfile, cleanupFinalizer)
+		if err := r.Update(r.ctx, &r.clientProfile); err != nil {
+			r.log.Error(err, "Failed to add a cleanup finalizer on config resource")
+			return err
+		}
+	}
+
 	return nil
 }
 
@@ -154,6 +165,7 @@ func (r *ClientProfileReconcile) loadAndValidate() error {
 		r.log.Error(err, "Failed loading configs.csi.ceph.io resource")
 		return err
 	}
+	r.cleanUp = r.clientProfile.DeletionTimestamp != nil
 
 	// Validate a pointer to a ceph cluster resource
 	if r.clientProfile.Spec.CephConnectionRef.Name == "" {
@@ -195,6 +207,28 @@ func (r *ClientProfileReconcile) loadAndValidate() error {
 	return nil
 }
 
+func (r *ClientProfileReconcile) reconcileCephConnection() error {
+	log := r.log.WithValues("cephConnectionName", r.cephConn.Name)
+	log.Info("Reconciling CephConnection")
+
+	if needsUpdate, err := utils.ToggleOwnerReference(
+		!r.cleanUp,
+		&r.clientProfile,
+		&r.cephConn,
+		r.Scheme,
+	); err != nil {
+		r.log.Error(err, "Failed to toggle owner reference on CephConnection")
+		return err
+	} else if needsUpdate {
+		if err := r.Update(r.ctx, &r.cephConn); err != nil {
+			r.log.Error(err, "Failed to update CephConnection")
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (r *ClientProfileReconcile) reconcileCephCsiClusterInfo() error {
 	csiConfigMap := corev1.ConfigMap{}
 	csiConfigMap.Name = utils.CsiConfigVolume.Name
@@ -203,10 +237,6 @@ func (r *ClientProfileReconcile) reconcileCephCsiClusterInfo() error {
 	log := r.log.WithValues("csiConfigMapName", csiConfigMap.Name)
 	log.Info("Reconciling Ceph CSI Cluster Info")
 
-	// Creating the desired record in advance to miimize the amount execution time
-	// the code run while serializing access to the configmap
-	record := composeCsiClusterInfoRecord(&r.clientProfile, &r.cephConn)
-
 	// Using a lock to serialized the updating of the config map.
 	// Although the code will run perfetcly fine without the lock, there will be a higher
 	// chance to fail on the create/update operation because another concurrent reconcile loop
@@ -217,8 +247,13 @@ func (r *ClientProfileReconcile) reconcileCephCsiClusterInfo() error {
 	defer configMapUpdateLock.Unlock()
 
 	_, err := ctrlutil.CreateOrUpdate(r.ctx, r.Client, &csiConfigMap, func() error {
-		if err := ctrlutil.SetOwnerReference(&r.clientProfile, &csiConfigMap, r.Scheme); err != nil {
-			log.Error(err, "Failed setting an owner reference on Ceph CSI config map")
+		if _, err := utils.ToggleOwnerReference(
+			!r.cleanUp,
+			&r.clientProfile,
+			&csiConfigMap,
+			r.Scheme,
+		); err != nil {
+			log.Error(err, "Failed togggling owner reference for Ceph CSI config map")
 			return err
 		}
 
@@ -238,23 +273,32 @@ func (r *ClientProfileReconcile) reconcileCephCsiClusterInfo() error {
 			return record.ClusterId == r.clientProfile.Name
 		})
 
-		// overwrite an existing entry or append a new one
-		if index > -1 {
-			clusterInfoList[index] = record
-		} else {
-			clusterInfoList = append(clusterInfoList, record)
+		if !r.cleanUp {
+			// Overwrite an existing entry or append a new one
+			record := composeCsiClusterInfoRecord(&r.clientProfile, &r.cephConn)
+			if index > -1 {
+				clusterInfoList[index] = record
+			} else {
+				clusterInfoList = append(clusterInfoList, record)
+			}
+		} else if index > -1 {
+			// An O(1) unordered in-place delete of a record
+			// Will not shrink the capacity of the slice
+			length := len(clusterInfoList)
+			clusterInfoList[index] = clusterInfoList[length-1]
+			clusterInfoList = clusterInfoList[:length-1]
 		}
 
 		// Serialize the list and store it back into the config map
-		if bytes, err := json.Marshal(clusterInfoList); err == nil {
+		if bytes, err := json.Marshal(clusterInfoList); err != nil {
+			log.Error(err, "Failed to serialize cluster info list")
+			return err
+		} else {
 			if csiConfigMap.Data == nil {
 				csiConfigMap.Data = map[string]string{}
 			}
 			csiConfigMap.Data[utils.CsiConfigMapConfigKey] = string(bytes)
 			return nil
-		} else {
-			log.Error(err, "Failed to serialize cluster info list")
-			return err
 		}
 	})
 

internal/utils/meta.go

@@ -18,6 +18,8 @@ package utils
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 // AddAnnotation adds an annotation to a resource metadata, returns true if added else false
@@ -44,3 +46,24 @@ func IsOwnedBy(obj, owner metav1.Object) bool {
 	}
 	return false
 }
+
+// ToggleOwnerReference adds or remove an owner reference for the given owner based on the first argument.
+// The function return true if the owner reference list had changed and false it it didn't
+func ToggleOwnerReference(on bool, obj, owner metav1.Object, scheme *runtime.Scheme) (bool, error) {
+	ownerRefExists := IsOwnedBy(obj, owner)
+
+	if on {
+		if !ownerRefExists {
+			if err := ctrlutil.SetOwnerReference(obj, owner, scheme); err != nil {
+				return false, err
+			}
+			return true, nil
+		}
+	} else if ownerRefExists {
+		if err := ctrlutil.RemoveOwnerReference(obj, owner, scheme); err != nil {
+			return false, err
+		}
+		return true, nil
+	}
+	return false, nil
+}