csi: ensure correct csi config map owner during creation

In the primary CSI reconcile, ensure the CSI config map
(`rook-ceph-csi-config`) has the correct owner info.

This corrects any pre-existing config maps that might have incorrect
owner info, which has observed to include references to CephClusters.
The config map should only have a single reference, and it should refer
to the operator deployment.

If any existing Rook clusters have a CSI config map which has a
CephCluster as an owner, this change will ensure that the config map is
not deleted when the CephCluster is deleted. This is especially
important for any environments with multiple CephClusters installed.

Signed-off-by: Blaine Gardner <blaine.gardner@ibm.com>

pkg/operator/ceph/csi/cluster_config.go

@@ -254,12 +254,50 @@ func CreateCsiConfigMap(ctx context.Context, namespace string, clientset kuberne
 		if !k8serrors.IsAlreadyExists(err) {
 			return errors.Wrapf(err, "failed to create initial csi config map %q (in %q)", configMap.Name, namespace)
 		}
+		// CM already exists; update owner refs to it if needed
+		// this corrects issues where the csi config map was sometimes created with CephCluster
+		// owner ref, which would result in the cm being deleted if that cluster was deleted
+		if err := updateCsiConfigMapOwnerRefs(ctx, namespace, clientset, ownerInfo); err != nil {
+			return errors.Wrapf(err, "failed to ensure csi config map %q (in %q) owner references", configMap.Name, namespace)
+		}
 	}
 
 	logger.Infof("successfully created csi config map %q", configMap.Name)
 	return nil
 }
 
+// check the owner references on the csi config map, and fix incorrect references if needed
+func updateCsiConfigMapOwnerRefs(ctx context.Context, namespace string, clientset kubernetes.Interface, expectedOwnerInfo *k8sutil.OwnerInfo) error {
+	cm, err := clientset.CoreV1().ConfigMaps(namespace).Get(ctx, ConfigName, metav1.GetOptions{})
+	if err != nil {
+		return errors.Wrapf(err, "failed to fetch csi config map %q (in %q) which already exists", ConfigName, namespace)
+	}
+
+	existingOwners := cm.GetOwnerReferences()
+	var currentOwner *metav1.OwnerReference = nil
+	if len(existingOwners) == 1 {
+		currentOwner = &existingOwners[0] // currentOwner is nil unless there is exactly one owner on the cm
+	}
+	// if there is exactly one owner, and it is correct --> no fix needed
+	if currentOwner != nil && (currentOwner.UID == expectedOwnerInfo.GetUID()) {
+		logger.Debugf("csi config map %q (in %q) has the expected owner; owner id: %q", ConfigName, namespace, currentOwner.UID)
+		return nil
+	}
+
+	// must fix owner refs
+	logger.Infof("updating csi configmap %q (in %q) owner info", ConfigName, namespace)
+	cm.OwnerReferences = []metav1.OwnerReference{}
+	if err := expectedOwnerInfo.SetControllerReference(cm); err != nil {
+		return errors.Wrapf(err, "failed to set updated owner reference on csi config map %q (in %q)", ConfigName, namespace)
+	}
+	_, err = clientset.CoreV1().ConfigMaps(namespace).Update(ctx, cm, metav1.UpdateOptions{})
+	if err != nil {
+		return errors.Wrapf(err, "failed to update csi config map %q (in %q) to update its owner reference", ConfigName, namespace)
+	}
+
+	return nil
+}
+
 // SaveClusterConfig updates the config map used to provide ceph-csi with
 // basic cluster configuration. The clusterNamespace and clusterInfo are
 // used to determine what "cluster" in the config map will be updated and

pkg/operator/ceph/csi/cluster_config_test.go

@@ -17,17 +17,22 @@ limitations under the License.
 package csi
 
 import (
+	"context"
 	"encoding/json"
 	"reflect"
 	"strings"
 	"testing"
 
+	cephcsi "github.com/ceph/ceph-csi/api/deploy/kubernetes"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	cephclient "github.com/rook/rook/pkg/daemon/ceph/client"
 	"github.com/rook/rook/pkg/operator/ceph/cluster/osd/topology"
+	"github.com/rook/rook/pkg/operator/k8sutil"
+	"github.com/rook/rook/pkg/operator/test"
 	"github.com/stretchr/testify/assert"
-
-	cephcsi "github.com/ceph/ceph-csi/api/deploy/kubernetes"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 )
 
 func unmarshal(s string) ([]CSIClusterConfigEntry, error) {
@@ -892,3 +897,125 @@ func TestUpdateNetNamespaceFilePath(t *testing.T) {
 		}
 	})
 }
+
+func Test_updateCsiConfigMapOwnerRefs(t *testing.T) {
+	ctx := context.TODO()
+	ns := "test-ns"
+	ownerController := true
+	blockOwnerDel := true
+	opDeployRef := metav1.OwnerReference{
+		APIVersion:         "v1",
+		Kind:               "Deployment",
+		Name:               "rook-ceph-operator",
+		UID:                "e55604f2-710c-4353-9a3e-9d23ea2d6eb9", // random uuid
+		Controller:         &ownerController,
+		BlockOwnerDeletion: &blockOwnerDel,
+	}
+	expectedOwnerInfo := k8sutil.NewOwnerInfoWithOwnerRef(&opDeployRef, ns)
+
+	minimalCsiConfigMap := func() *corev1.ConfigMap {
+		return &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:            ConfigName,
+				Namespace:       ns,
+				OwnerReferences: []metav1.OwnerReference{},
+			},
+		}
+	}
+
+	t.Run("no configmap", func(t *testing.T) {
+		clientset := test.New(t, 1)
+		err := updateCsiConfigMapOwnerRefs(ctx, ns, clientset, expectedOwnerInfo)
+		assert.ErrorContains(t, err, "failed to fetch csi config map")
+		assert.ErrorContains(t, err, "which already exists")
+	})
+
+	assertOwner := func(t *testing.T, clientset kubernetes.Interface) {
+		t.Helper()
+
+		cm, err := clientset.CoreV1().ConfigMaps(ns).Get(ctx, ConfigName, metav1.GetOptions{})
+		assert.NoError(t, err)
+		assert.Len(t, cm.GetOwnerReferences(), 1)
+		cmOwner := cm.GetOwnerReferences()[0]
+		assert.Equal(t, "v1", cmOwner.APIVersion)
+		assert.Equal(t, "Deployment", cmOwner.Kind)
+		assert.Equal(t, "rook-ceph-operator", cmOwner.Name)
+		assert.Equal(t, "e55604f2-710c-4353-9a3e-9d23ea2d6eb9", string(cmOwner.UID))
+		assert.True(t, *cmOwner.Controller)
+		assert.True(t, *cmOwner.BlockOwnerDeletion)
+	}
+
+	t.Run("no existing owner ref", func(t *testing.T) {
+		clientset := test.New(t, 1)
+		cm := minimalCsiConfigMap()
+		_, err := clientset.CoreV1().ConfigMaps(ns).Create(ctx, cm, metav1.CreateOptions{})
+		assert.NoError(t, err)
+
+		err = updateCsiConfigMapOwnerRefs(ctx, ns, clientset, expectedOwnerInfo)
+		assert.NoError(t, err)
+		assertOwner(t, clientset)
+	})
+
+	t.Run("correct existing owner ref", func(t *testing.T) {
+		clientset := test.New(t, 1)
+		cm := minimalCsiConfigMap()
+		cm.OwnerReferences = []metav1.OwnerReference{
+			*opDeployRef.DeepCopy(), // correct ref
+		}
+		_, err := clientset.CoreV1().ConfigMaps(ns).Create(ctx, cm, metav1.CreateOptions{})
+		assert.NoError(t, err)
+
+		err = updateCsiConfigMapOwnerRefs(ctx, ns, clientset, expectedOwnerInfo)
+		assert.NoError(t, err)
+		assertOwner(t, clientset)
+	})
+
+	t.Run("single incorrect existing owner ref", func(t *testing.T) {
+		clientset := test.New(t, 1)
+		cm := minimalCsiConfigMap()
+
+		ownerController := true
+		blockOwnerDel := true
+		cm.OwnerReferences = []metav1.OwnerReference{
+			{
+				APIVersion:         "ceph.rook.io/v1",
+				Kind:               "CephCluster",
+				Name:               "my-cluster",
+				UID:                "a77777a7-777a-7777-7a7a-7a77aa7a7aa7", // random uuid
+				Controller:         &ownerController,
+				BlockOwnerDeletion: &blockOwnerDel,
+			},
+		}
+		_, err := clientset.CoreV1().ConfigMaps(ns).Create(ctx, cm, metav1.CreateOptions{})
+		assert.NoError(t, err)
+
+		err = updateCsiConfigMapOwnerRefs(ctx, ns, clientset, expectedOwnerInfo)
+		assert.NoError(t, err)
+		assertOwner(t, clientset)
+	})
+
+	t.Run("multiple existing owner refs, one correct", func(t *testing.T) {
+		clientset := test.New(t, 1)
+		cm := minimalCsiConfigMap()
+
+		ownerController := true
+		blockOwnerDel := true
+		cm.OwnerReferences = []metav1.OwnerReference{
+			*opDeployRef.DeepCopy(), // correct ref
+			{
+				APIVersion:         "ceph.rook.io/v1",
+				Kind:               "CephCluster",
+				Name:               "my-cluster",
+				UID:                "a77777a7-777a-7777-7a7a-7a77aa7a7aa7", // random uuid
+				Controller:         &ownerController,
+				BlockOwnerDeletion: &blockOwnerDel,
+			},
+		}
+		_, err := clientset.CoreV1().ConfigMaps(ns).Create(ctx, cm, metav1.CreateOptions{})
+		assert.NoError(t, err)
+
+		err = updateCsiConfigMapOwnerRefs(ctx, ns, clientset, expectedOwnerInfo)
+		assert.NoError(t, err)
+		assertOwner(t, clientset)
+	})
+}

pkg/operator/k8sutil/resources.go

@@ -144,7 +144,13 @@ func (info *OwnerInfo) SetControllerReference(object metav1.Object) error {
 
 // GetUID gets the UID of the owner
 func (info *OwnerInfo) GetUID() types.UID {
-	return info.owner.GetUID()
+	if info.owner != nil {
+		return info.owner.GetUID()
+	}
+	if info.ownerRef != nil {
+		return info.ownerRef.UID
+	}
+	return types.UID("")
 }
 
 func MergeResourceRequirements(first, second v1.ResourceRequirements) v1.ResourceRequirements {