From 61c031d2d5e225083504caeacec6b0f3cad7fd6d Mon Sep 17 00:00:00 2001 From: Blaine Gardner Date: Tue, 16 Apr 2024 13:09:39 -0600 Subject: [PATCH] operator: load cluster owner info in LoadClusterInfo The CreateOrLoadClusterInfo (and therefore LoadClusterInfo) methods were not loading the ClusterInfo.OwnerInfo. This should help future CRD controllers get the full ClusterInfo struct without having missing information. Current controllers that need ClusterInfo fill the field themselves. During testing, I observed one corner case where an upgraded cluster was missing the `rook-ceph-csi-config` configmap. The cluster had a CephFilesystemSubVolumeGroup resource created, and the reconcile for that resource was attempting to create the missing CSI configmap and failing with a nil pointer exception due to the missing OwnerInfo field in ClusterInfo. This cluster condition hasn't been reproduced in healthy environments, and it is unknown how the CSI configmap came to be missing. However, the case did expose the missing loaded info as a potential for causing nil pointer exceptions during corner cases when code is otherwise correct. Signed-off-by: Blaine Gardner (cherry picked from commit bd9447ea4c52ae05f96aa46c278d3b3cd3f829c2) --- pkg/operator/ceph/controller/cluster_info.go | 3 ++ .../ceph/controller/cluster_info_test.go | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/pkg/operator/ceph/controller/cluster_info.go b/pkg/operator/ceph/controller/cluster_info.go index eb85146ad..e9f3d8eb9 100644 --- a/pkg/operator/ceph/controller/cluster_info.go +++ b/pkg/operator/ceph/controller/cluster_info.go @@ -141,6 +141,9 @@ func CreateOrLoadClusterInfo(clusterdContext *clusterd.Context, context context. } else { return nil, maxMonID, monMapping, errors.New("failed to find either the cluster admin key or the username") } + if len(secrets.OwnerReferences) > 0 { + clusterInfo.OwnerInfo = k8sutil.NewOwnerInfoWithOwnerRef(&secrets.GetOwnerReferences()[0], namespace) + } logger.Debugf("found existing monitor secrets for cluster %s", clusterInfo.Namespace) } diff --git a/pkg/operator/ceph/controller/cluster_info_test.go b/pkg/operator/ceph/controller/cluster_info_test.go index 7b7695553..cdfa02913 100644 --- a/pkg/operator/ceph/controller/cluster_info_test.go +++ b/pkg/operator/ceph/controller/cluster_info_test.go @@ -29,6 +29,7 @@ import ( exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -74,6 +75,36 @@ func TestCreateClusterSecrets(t *testing.T) { assert.NoError(t, err) assert.Equal(t, adminSecret, string(secret.Data["ceph-secret"])) + // ensure secret owner info can be loaded and is useful + // this is what the owner info looks like in a live cluster + ownerController := true + blockOwnerDel := true + secret.OwnerReferences[0] = metav1.OwnerReference{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephCluster", + Name: "my-cluster", + UID: "e55604f2-710c-4353-9a3e-9d23ea2d6eb9", // random uuid + Controller: &ownerController, + BlockOwnerDeletion: &blockOwnerDel, + } + _, err = clientset.CoreV1().Secrets(namespace).Update(ctx, secret, metav1.UpdateOptions{}) + assert.NoError(t, err) + info, _, _, err = CreateOrLoadClusterInfo(context, ctx, namespace, ownerInfo, cephClusterSpec) + assert.NoError(t, err) + // use the SetOwnerReference() method to ensure that the loaded OwnerInfo is usable and correct + cm := corev1.ConfigMap{} + cm.Name = "bob" + cm.Namespace = namespace + err = info.OwnerInfo.SetOwnerReference(&cm) + assert.NoError(t, err) + cmOwner := cm.OwnerReferences[0] + assert.Equal(t, "ceph.rook.io/v1", cmOwner.APIVersion) + assert.Equal(t, "CephCluster", cmOwner.Kind) + assert.Equal(t, "my-cluster", cmOwner.Name) + assert.Equal(t, "e55604f2-710c-4353-9a3e-9d23ea2d6eb9", string(cmOwner.UID)) + assert.True(t, *cmOwner.Controller) + assert.True(t, *cmOwner.BlockOwnerDeletion) + // For backward compatibility check that the admin secret can be loaded as previously specified // Update the secret as if created in an old cluster delete(secret.Data, CephUserSecretKey)