core: finalize CephCluster CephX status handling

Cleanup up overall usage of CephX statuses in CephCluster.

Convert all CephX statuses to non-pointer types. There is no specific
need for pointer types, and keeping these as non-pointers means there is
not a risk of nil pointer exceptions in all the various Rook controllers
for upgraded clusters.

Move initialization of CephCluster CephX status items to
`preMonStartupActions()`. This helps resolve 2 issues:

1. 'Uninitialized' state was being set for external CephClusters where
   CephX statuses are not relevant.
2. 'Uninitialized' state was being set for upgraded clusters whose key
   status cannot be determined.

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

Documentation/CRDs/specification.md

@@ -5140,19 +5140,6 @@ CephxStatus
 </tr>
 <tr>
 <td>
-<code>rbdMirrorPeer</code><br/>
-<em>
-<a href="#ceph.rook.io/v1.CephxStatus">
-CephxStatus
-</a>
-</em>
-</td>
-<td>
-<p>RBDMirrorPeer represents the cephx key rotation status of the <code>rbd-mirror-peer</code> user</p>
-</td>
-</tr>
-<tr>
-<td>
 <code>csi</code><br/>
 <em>
 <a href="#ceph.rook.io/v1.CephxStatusWithKeyCount">
@@ -5166,6 +5153,19 @@ CephxStatusWithKeyCount
 </tr>
 <tr>
 <td>
+<code>rbdMirrorPeer</code><br/>
+<em>
+<a href="#ceph.rook.io/v1.CephxStatus">
+CephxStatus
+</a>
+</em>
+</td>
+<td>
+<p>RBDMirrorPeer represents the cephx key rotation status of the <code>rbd-mirror-peer</code> user</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>crashCollector</code><br/>
 <em>
 <a href="#ceph.rook.io/v1.CephxStatus">

pkg/apis/ceph.rook.io/v1/types.go

@@ -516,14 +516,14 @@ type CephExporterSpec struct {
 
 // ClusterStatus represents the status of a Ceph cluster
 type ClusterStatus struct {
-	State       ClusterState        `json:"state,omitempty"`
-	Phase       ConditionType       `json:"phase,omitempty"`
-	Message     string              `json:"message,omitempty"`
-	Conditions  []Condition         `json:"conditions,omitempty"`
-	CephStatus  *CephStatus         `json:"ceph,omitempty"`
-	Cephx       *ClusterCephxStatus `json:"cephx,omitempty"`
-	CephStorage *CephStorage        `json:"storage,omitempty"`
-	CephVersion *ClusterVersion     `json:"version,omitempty"`
+	State       ClusterState       `json:"state,omitempty"`
+	Phase       ConditionType      `json:"phase,omitempty"`
+	Message     string             `json:"message,omitempty"`
+	Conditions  []Condition        `json:"conditions,omitempty"`
+	CephStatus  *CephStatus        `json:"ceph,omitempty"`
+	Cephx       ClusterCephxStatus `json:"cephx,omitempty"`
+	CephStorage *CephStorage       `json:"storage,omitempty"`
+	CephVersion *ClusterVersion    `json:"version,omitempty"`
 	// ObservedGeneration is the latest generation observed by the controller.
 	// +optional
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
@@ -749,19 +749,19 @@ type LocalCephxStatus struct {
 // ClusterCephxStatus defines the cephx key rotation status of various daemons on the cephCluster resource
 type ClusterCephxStatus struct {
 	// Mon represents the CephX key status of the Monitor daemons
-	Mon *CephxStatus `json:"mon,omitempty"`
+	Mon CephxStatus `json:"mon,omitempty"`
 	// Mgr represents the cephx key rotation status of the ceph manager daemon
-	Mgr *CephxStatus `json:"mgr,omitempty"`
+	Mgr CephxStatus `json:"mgr,omitempty"`
 	// OSD shows the CephX key status of of OSDs
-	OSD *CephxStatus `json:"osd,omitempty"`
-	// RBDMirrorPeer represents the cephx key rotation status of the `rbd-mirror-peer` user
-	RBDMirrorPeer *CephxStatus `json:"rbdMirrorPeer,omitempty"`
+	OSD CephxStatus `json:"osd,omitempty"`
 	// CSI shows the CephX key status for Ceph-CSI components.
-	CSI *CephxStatusWithKeyCount `json:"csi,omitempty"`
+	CSI CephxStatusWithKeyCount `json:"csi,omitempty"`
+	// RBDMirrorPeer represents the cephx key rotation status of the `rbd-mirror-peer` user
+	RBDMirrorPeer CephxStatus `json:"rbdMirrorPeer,omitempty"`
 	// Crash Collector represents the cephx key rotation status of the crash collector daemon
-	CrashCollector *CephxStatus `json:"crashCollector,omitempty"`
+	CrashCollector CephxStatus `json:"crashCollector,omitempty"`
 	// Ceph Exporter represents the cephx key rotation status of the ceph exporter daemon
-	CephExporter *CephxStatus `json:"cephExporter,omitempty"`
+	CephExporter CephxStatus `json:"cephExporter,omitempty"`
 }
 
 // MonSpec represents the specification of the monitor

pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go

@@ -2077,41 +2077,13 @@ func (in *ClusterCephxConfig) DeepCopy() *ClusterCephxConfig {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ClusterCephxStatus) DeepCopyInto(out *ClusterCephxStatus) {
 	*out = *in
-	if in.Mon != nil {
-		in, out := &in.Mon, &out.Mon
-		*out = new(CephxStatus)
-		**out = **in
-	}
-	if in.Mgr != nil {
-		in, out := &in.Mgr, &out.Mgr
-		*out = new(CephxStatus)
-		**out = **in
-	}
-	if in.OSD != nil {
-		in, out := &in.OSD, &out.OSD
-		*out = new(CephxStatus)
-		**out = **in
-	}
-	if in.RBDMirrorPeer != nil {
-		in, out := &in.RBDMirrorPeer, &out.RBDMirrorPeer
-		*out = new(CephxStatus)
-		**out = **in
-	}
-	if in.CSI != nil {
-		in, out := &in.CSI, &out.CSI
-		*out = new(CephxStatusWithKeyCount)
-		**out = **in
-	}
-	if in.CrashCollector != nil {
-		in, out := &in.CrashCollector, &out.CrashCollector
-		*out = new(CephxStatus)
-		**out = **in
-	}
-	if in.CephExporter != nil {
-		in, out := &in.CephExporter, &out.CephExporter
-		*out = new(CephxStatus)
-		**out = **in
-	}
+	out.Mon = in.Mon
+	out.Mgr = in.Mgr
+	out.OSD = in.OSD
+	out.CSI = in.CSI
+	out.RBDMirrorPeer = in.RBDMirrorPeer
+	out.CrashCollector = in.CrashCollector
+	out.CephExporter = in.CephExporter
 	return
 }
 
@@ -2279,11 +2251,7 @@ func (in *ClusterStatus) DeepCopyInto(out *ClusterStatus) {
 		*out = new(CephStatus)
 		(*in).DeepCopyInto(*out)
 	}
-	if in.Cephx != nil {
-		in, out := &in.Cephx, &out.Cephx
-		*out = new(ClusterCephxStatus)
-		(*in).DeepCopyInto(*out)
-	}
+	out.Cephx = in.Cephx
 	if in.CephStorage != nil {
 		in, out := &in.CephStorage, &out.CephStorage
 		*out = new(CephStorage)

pkg/operator/ceph/cluster/cluster.go

@@ -50,6 +50,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/util/retry"
 )
 
 const (
@@ -453,6 +454,11 @@ func (c *cluster) replaceDefaultCrushMap(newRoot string) (err error) {
 
 // preMonStartupActions is a collection of actions to run before the monitors are reconciled.
 func (c *cluster) preMonStartupActions(cephVersion cephver.CephVersion) error {
+	err := initClusterCephxStatus(c)
+	if err != nil {
+		return errors.Wrap(err, "failed to initialized cluster cephx status")
+	}
+
 	return nil
 }
 
@@ -815,24 +821,52 @@ func (c *cluster) fetchSecretValue(selector v1.SecretKeySelector) (string, error
 	return string(val), nil
 }
 
-// initClusterCephxStatus set `Uninitialized` state for the cephXstatus for new clusters.
-func initClusterCephxStatus(c *clusterd.Context, cluster *cephv1.CephCluster) error {
-	uninitializedStatus := keyring.UninitializedCephxStatus()
-	cluster.Status.Cephx = &cephv1.ClusterCephxStatus{
-		OSD:           &uninitializedStatus,
-		RBDMirrorPeer: &uninitializedStatus,
-		Mgr:           &uninitializedStatus,
-		Mon:           &uninitializedStatus,
-		CSI: &cephv1.CephxStatusWithKeyCount{
-			CephxStatus: uninitializedStatus,
-		},
-		CrashCollector: &uninitializedStatus,
-		CephExporter:   &uninitializedStatus,
+// initClusterCephxStatus set `Uninitialized` cephx status for new clusters.
+// this should not be run for external mode clusters
+func initClusterCephxStatus(c *cluster) error {
+	initErr := c.ClusterInfo.IsInitialized()
+	if initErr == nil {
+		logger.Debugf("not setting uninitialized cephx status on already initialized CephCluster in namespace %q", c.Namespace)
+		return nil
+	}
+	if c.ClusterInfo.Context.Err() != nil {
+		// most IsInitialized() errors mean the cluster is new, but if clusterInfo.Context is
+		// nil, it is a 'real' error to return
+		return c.ClusterInfo.Context.Err()
 	}
 
-	if err := reporting.UpdateStatus(c.Client, cluster); err != nil {
-		return errors.Wrapf(err, "failed to update cluster cephx status")
-	}
+	err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		clusterObj := &cephv1.CephCluster{}
+		err := c.context.Client.Get(c.ClusterInfo.Context, c.namespacedName, clusterObj)
+		if err != nil {
+			return errors.Wrapf(err, "failed to get cluster in order to initialize its cephx status")
+		}
 
-	return nil
+		emptyStatus := cephv1.CephxStatus{}
+		// mon cephx status is one of the first set after mons are successfully running, so we only need to check it
+		if clusterObj.Status.Cephx.Mon != emptyStatus {
+			return nil // do not initialize multiple times
+		}
+
+		uninitializedStatus := keyring.UninitializedCephxStatus()
+		logger.Infof("initializing cephx status for CephCluster in namespace %q", c.Namespace)
+		clusterObj.Status.Cephx = cephv1.ClusterCephxStatus{
+			Mon: uninitializedStatus,
+			Mgr: uninitializedStatus,
+			// OSD statuses are determined entirely within OSD reconcile - don't set uninitialized here
+			CSI: cephv1.CephxStatusWithKeyCount{
+				CephxStatus: uninitializedStatus,
+			},
+			RBDMirrorPeer:  uninitializedStatus,
+			CrashCollector: uninitializedStatus,
+			CephExporter:   uninitializedStatus,
+		}
+
+		if err := reporting.UpdateStatus(c.context.Client, clusterObj); err != nil {
+			return errors.Wrapf(err, "failed to initialize cluster cephx status")
+		}
+
+		return nil
+	})
+	return err
 }

pkg/operator/ceph/cluster/cluster_test.go

@@ -27,11 +27,13 @@ import (
 
 	"github.com/pkg/errors"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
+	"github.com/rook/rook/pkg/client/clientset/versioned/scheme"
 	"github.com/rook/rook/pkg/clusterd"
 	cephclient "github.com/rook/rook/pkg/daemon/ceph/client"
 	"github.com/rook/rook/pkg/operator/ceph/cluster/mon"
 	"github.com/rook/rook/pkg/operator/ceph/cluster/nodedaemon"
 	"github.com/rook/rook/pkg/operator/ceph/cluster/telemetry"
+	"github.com/rook/rook/pkg/operator/ceph/config/keyring"
 	"github.com/rook/rook/pkg/operator/ceph/csi"
 	cephver "github.com/rook/rook/pkg/operator/ceph/version"
 	testop "github.com/rook/rook/pkg/operator/test"
@@ -41,6 +43,8 @@ import (
 	"gopkg.in/ini.v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 func TestPreClusterStartValidation(t *testing.T) {
@@ -594,3 +598,173 @@ func TestFetchCephConfigFromSecrets(t *testing.T) {
 		})
 	}
 }
+
+func Test_initClusterCephxStatus(t *testing.T) {
+	ns := "ns"
+	clusterName := "testcluster"
+
+	newCluster := func(cephxStatus cephv1.CephxStatus, clusterInfo *cephclient.ClusterInfo) *cluster {
+		cephCluster := &cephv1.CephCluster{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      clusterName,
+				Namespace: ns,
+			},
+			Status: cephv1.ClusterStatus{
+				Cephx: cephv1.ClusterCephxStatus{
+					Mon: cephxStatus,
+					// don't set other statuses for new clusters so we can tell if obj was updated
+				},
+			},
+		}
+		s := scheme.Scheme
+		s.AddKnownTypes(cephv1.SchemeGroupVersion, &cephv1.CephClusterList{})
+		cl := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(cephCluster).Build()
+
+		return &cluster{
+			ClusterInfo:    clusterInfo,
+			Namespace:      ns,
+			namespacedName: types.NamespacedName{Namespace: ns, Name: clusterName},
+			context: &clusterd.Context{
+				Client: cl,
+			},
+		}
+	}
+
+	readyClusterInfo := func() *cephclient.ClusterInfo {
+		return &cephclient.ClusterInfo{
+			FSID:          "1234567890",
+			MonitorSecret: "AAAAAAAAAAAAA==",
+			CephCred: cephclient.CephCred{
+				Username: "client.admin",
+				Secret:   "BBBBBBBBBBBBB==",
+			},
+			Context: context.TODO(),
+		}
+	}
+
+	notreadyClusterInfo := func() *cephclient.ClusterInfo {
+		return &cephclient.ClusterInfo{
+			Context: context.TODO(),
+		}
+	}
+
+	t.Run("cluster ready, empty status", func(t *testing.T) {
+		c := newCluster(cephv1.CephxStatus{}, readyClusterInfo())
+
+		err := initClusterCephxStatus(c)
+		assert.NoError(t, err)
+
+		cluster := cephv1.CephCluster{}
+		err = c.context.Client.Get(context.TODO(), c.namespacedName, &cluster)
+		assert.NoError(t, err)
+		assert.Equal(t, cephv1.ClusterCephxStatus{}, cluster.Status.Cephx)
+	})
+
+	t.Run("cluster ready, status already uninit", func(t *testing.T) {
+		c := newCluster(keyring.UninitializedCephxStatus(), readyClusterInfo())
+
+		err := initClusterCephxStatus(c)
+		assert.NoError(t, err)
+
+		cluster := cephv1.CephCluster{}
+		err = c.context.Client.Get(context.TODO(), c.namespacedName, &cluster)
+		assert.NoError(t, err)
+		assert.Equal(t, keyring.UninitializedCephxStatus(), cluster.Status.Cephx.Mon)
+		assert.Equal(t, cephv1.CephxStatus{}, cluster.Status.Cephx.Mgr) // no status means no update
+	})
+
+	t.Run("cluster ready, status nonzero", func(t *testing.T) {
+		monStatus := cephv1.CephxStatus{KeyGeneration: 3, KeyCephVersion: "19.2.3-0"}
+		c := newCluster(monStatus, readyClusterInfo())
+
+		err := initClusterCephxStatus(c)
+		assert.NoError(t, err)
+
+		cluster := cephv1.CephCluster{}
+		err = c.context.Client.Get(context.TODO(), c.namespacedName, &cluster)
+		assert.NoError(t, err)
+		assert.Equal(t, monStatus, cluster.Status.Cephx.Mon)
+		assert.Equal(t, cephv1.CephxStatus{}, cluster.Status.Cephx.Mgr) // no status means no update
+	})
+
+	t.Run("cluster not ready, empty status", func(t *testing.T) {
+		c := newCluster(cephv1.CephxStatus{}, notreadyClusterInfo())
+
+		err := initClusterCephxStatus(c)
+		assert.NoError(t, err)
+
+		cluster := cephv1.CephCluster{}
+		err = c.context.Client.Get(context.TODO(), c.namespacedName, &cluster)
+		assert.NoError(t, err)
+		assert.Equal(t, keyring.UninitializedCephxStatus(), cluster.Status.Cephx.Mon)
+		assert.Equal(t, keyring.UninitializedCephxStatus(), cluster.Status.Cephx.Mgr)
+		assert.Equal(t, keyring.UninitializedCephxStatus(), cluster.Status.Cephx.CSI.CephxStatus)
+		assert.Equal(t, keyring.UninitializedCephxStatus(), cluster.Status.Cephx.RBDMirrorPeer)
+		assert.Equal(t, keyring.UninitializedCephxStatus(), cluster.Status.Cephx.CrashCollector)
+		assert.Equal(t, keyring.UninitializedCephxStatus(), cluster.Status.Cephx.CephExporter)
+	})
+
+	t.Run("cluster not ready, status already uninit", func(t *testing.T) {
+		c := newCluster(keyring.UninitializedCephxStatus(), notreadyClusterInfo())
+
+		err := initClusterCephxStatus(c)
+		assert.NoError(t, err)
+
+		cluster := cephv1.CephCluster{}
+		err = c.context.Client.Get(context.TODO(), c.namespacedName, &cluster)
+		assert.NoError(t, err)
+		assert.Equal(t, keyring.UninitializedCephxStatus(), cluster.Status.Cephx.Mon)
+		assert.Equal(t, cephv1.CephxStatus{}, cluster.Status.Cephx.Mgr) // no status means no update
+	})
+
+	t.Run("cluster not ready, status nonzero", func(t *testing.T) {
+		monStatus := cephv1.CephxStatus{KeyGeneration: 3, KeyCephVersion: "19.2.3-0"}
+		c := newCluster(monStatus, notreadyClusterInfo())
+
+		err := initClusterCephxStatus(c)
+		assert.NoError(t, err)
+
+		cluster := cephv1.CephCluster{}
+		err = c.context.Client.Get(context.TODO(), c.namespacedName, &cluster)
+		assert.NoError(t, err)
+		assert.Equal(t, monStatus, cluster.Status.Cephx.Mon)
+		assert.Equal(t, cephv1.CephxStatus{}, cluster.Status.Cephx.Mgr) // no status means no update
+	})
+
+	t.Run("cluster ctx canceled, status empty", func(t *testing.T) {
+		notReady := notreadyClusterInfo()
+		newCtx, cancelFunc := context.WithCancel(notReady.Context)
+		notReady.Context = newCtx
+		cancelFunc()
+
+		c := newCluster(cephv1.CephxStatus{}, notReady) // would normally need initialized
+
+		err := initClusterCephxStatus(c)
+		assert.Error(t, err)
+
+		cluster := cephv1.CephCluster{}
+		err = c.context.Client.Get(context.TODO(), c.namespacedName, &cluster)
+		assert.NoError(t, err)
+		assert.Equal(t, cephv1.CephxStatus{}, cluster.Status.Cephx.Mon)
+		assert.Equal(t, cephv1.CephxStatus{}, cluster.Status.Cephx.Mgr) // no status means no update
+	})
+
+	t.Run("cluster ctx canceled, status nonzero", func(t *testing.T) {
+		notReady := notreadyClusterInfo()
+		newCtx, cancelFunc := context.WithCancel(notReady.Context)
+		notReady.Context = newCtx
+		cancelFunc()
+
+		monStatus := cephv1.CephxStatus{KeyGeneration: 3, KeyCephVersion: "19.2.3-0"}
+		c := newCluster(monStatus, notReady)
+
+		err := initClusterCephxStatus(c)
+		assert.Error(t, err)
+
+		cluster := cephv1.CephCluster{}
+		err = c.context.Client.Get(context.TODO(), c.namespacedName, &cluster)
+		assert.NoError(t, err)
+		assert.Equal(t, monStatus, cluster.Status.Cephx.Mon)
+		assert.Equal(t, cephv1.CephxStatus{}, cluster.Status.Cephx.Mgr) // no status means no update
+	})
+}

pkg/operator/ceph/cluster/controller.go

@@ -385,13 +385,6 @@ func (c *ClusterController) reconcileCephCluster(clusterObj *cephv1.CephCluster,
 		return nil
 	}
 
-	if clusterObj.Status.Cephx == nil {
-		err := initClusterCephxStatus(c.context, clusterObj)
-		if err != nil {
-			return errors.Wrap(err, "failed to initialized cluster cephx status")
-		}
-	}
-
 	cluster, ok := c.clusterMap[clusterObj.Namespace]
 	if !ok {
 		// It's a new cluster so let's populate the struct

pkg/operator/ceph/cluster/mgr/mgr.go

@@ -622,7 +622,7 @@ func shouldRotateMgrKeys(c *clusterd.Context, clusterInfo *cephclient.ClusterInf
 	// TODO: for rotation WithCephVersionUpdate fix this to have the right runningCephVersion and desiredCephVersion
 	runningCephVersion := clusterInfo.CephVersion
 
-	shouldRotateKeys, err := keyring.ShouldRotateCephxKeys(clusterObj.Spec.Security.CephX.Daemon, runningCephVersion, desiredCephVersion, *clusterObj.Status.Cephx.Mgr)
+	shouldRotateKeys, err := keyring.ShouldRotateCephxKeys(clusterObj.Spec.Security.CephX.Daemon, runningCephVersion, desiredCephVersion, clusterObj.Status.Cephx.Mgr)
 	if err != nil {
 		return false, errors.Wrap(err, "failed to check if mgr daemon keys should be rotated or not")
 	}
@@ -636,8 +636,8 @@ func updateMgrCephxStatus(c *clusterd.Context, clusterInfo *cephclient.ClusterIn
 		if err := c.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cluster); err != nil {
 			return errors.Wrapf(err, "failed to get cluster %v to update the conditions.", clusterInfo.NamespacedName())
 		}
-		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.Daemon, clusterInfo.CephVersion, *cluster.Status.Cephx.Mgr)
-		cluster.Status.Cephx.Mgr = &updatedStatus
+		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.Daemon, clusterInfo.CephVersion, cluster.Status.Cephx.Mgr)
+		cluster.Status.Cephx.Mgr = updatedStatus
 		logger.Debugf("updating mgr daemon cephx status to %+v", cluster.Status.Cephx.Mgr)
 		if err := reporting.UpdateStatus(c.Client, cluster); err != nil {
 			return errors.Wrap(err, "failed to update cluster cephx status for mgr daemon")

pkg/operator/ceph/cluster/mgr/mgr_test.go

@@ -92,8 +92,8 @@ func createNewCluster(t *testing.T) *Cluster {
 			},
 		},
 		Status: cephv1.ClusterStatus{
-			Cephx: &cephv1.ClusterCephxStatus{
-				Mgr: &uninitializedStatus,
+			Cephx: cephv1.ClusterCephxStatus{
+				Mgr: uninitializedStatus,
 			},
 		},
 	}

pkg/operator/ceph/cluster/mon/health_test.go

@@ -313,8 +313,8 @@ func TestEvictMonOnSameNode(t *testing.T) {
 			},
 		},
 		Status: cephv1.ClusterStatus{
-			Cephx: &cephv1.ClusterCephxStatus{
-				RBDMirrorPeer: &cephv1.CephxStatus{},
+			Cephx: cephv1.ClusterCephxStatus{
+				RBDMirrorPeer: cephv1.CephxStatus{},
 			},
 		},
 	}
@@ -865,8 +865,8 @@ func TestExternalMons_notInSpec_InQuorum(t *testing.T) {
 			},
 		},
 		Status: cephv1.ClusterStatus{
-			Cephx: &cephv1.ClusterCephxStatus{
-				RBDMirrorPeer: &cephv1.CephxStatus{},
+			Cephx: cephv1.ClusterCephxStatus{
+				RBDMirrorPeer: cephv1.CephxStatus{},
 			},
 		},
 	}
@@ -1319,8 +1319,8 @@ func getCephCluster(name, namespace string) *cephv1.CephCluster {
 			},
 		},
 		Status: cephv1.ClusterStatus{
-			Cephx: &cephv1.ClusterCephxStatus{
-				RBDMirrorPeer: &cephv1.CephxStatus{},
+			Cephx: cephv1.ClusterCephxStatus{
+				RBDMirrorPeer: cephv1.CephxStatus{},
 			},
 		},
 	}

pkg/operator/ceph/cluster/mon/mon.go

@@ -1807,7 +1807,7 @@ func (c *Cluster) RotateMonCephxKeys(clusterObj *cephv1.CephCluster) (bool, erro
 	// TODO: for rotation WithCephVersionUpdate fix this to have the right runningCephVersion and desiredCephVersion
 	runningCephVersion := c.ClusterInfo.CephVersion
 
-	shouldRotateMonKeys, err := keyring.ShouldRotateCephxKeys(clusterObj.Spec.Security.CephX.Daemon, runningCephVersion, desiredCephVersion, *clusterObj.Status.Cephx.Mon)
+	shouldRotateMonKeys, err := keyring.ShouldRotateCephxKeys(clusterObj.Spec.Security.CephX.Daemon, runningCephVersion, desiredCephVersion, clusterObj.Status.Cephx.Mon)
 	if err != nil {
 		return false, errors.Wrapf(err, "failed to check if mon daemon keys should be rotated in the namespace %q", c.ClusterInfo.Namespace)
 	}
@@ -1849,8 +1849,8 @@ func (c *Cluster) UpdateMonCephxStatus(didRotate bool) error {
 		if err := c.context.Client.Get(c.ClusterInfo.Context, c.ClusterInfo.NamespacedName(), cluster); err != nil {
 			return errors.Wrapf(err, "failed to get cluster %v to update the mon cephx status.", c.ClusterInfo.NamespacedName())
 		}
-		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.Daemon, c.ClusterInfo.CephVersion, *cluster.Status.Cephx.Mon)
-		cluster.Status.Cephx.Mon = &updatedStatus
+		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.Daemon, c.ClusterInfo.CephVersion, cluster.Status.Cephx.Mon)
+		cluster.Status.Cephx.Mon = updatedStatus
 		logger.Debugf("updating mon daemon cephx status to %+v", cluster.Status.Cephx.Mgr)
 		if err := reporting.UpdateStatus(c.context.Client, cluster); err != nil {
 			return errors.Wrapf(err, "failed to update cluster cephx status for mon daemon in the namespace %q", c.ClusterInfo.Namespace)

pkg/operator/ceph/cluster/mon/mon_test.go

@@ -1191,8 +1191,8 @@ func TestRotateMonCephxKeys(t *testing.T) {
 			},
 		},
 		Status: cephv1.ClusterStatus{
-			Cephx: &cephv1.ClusterCephxStatus{
-				Mon: &uninitializedStatus,
+			Cephx: cephv1.ClusterCephxStatus{
+				Mon: uninitializedStatus,
 			},
 		},
 	}

pkg/operator/ceph/cluster/nodedaemon/keyring.go

@@ -89,7 +89,7 @@ func createCrashCollectorKeyring(s *keyring.SecretStore, context *clusterd.Conte
 	shouldRotateCephxKeys, err := keyring.ShouldRotateCephxKeys(
 		clusterObj.Spec.Security.CephX.Daemon,
 		clusterInfo.CephVersion, clusterInfo.CephVersion,
-		*clusterObj.Status.Cephx.CrashCollector,
+		clusterObj.Status.Cephx.CrashCollector,
 	)
 	if err != nil {
 		return "", errors.Wrapf(err, "failed to check if cephx keys should be rotated for crash collector %q", crashCollectorKeyringUsername)
@@ -119,8 +119,8 @@ func updateCrashCollectorCephxStatus(context *clusterd.Context, clusterInfo *cli
 		if err := context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cluster); err != nil {
 			return errors.Wrapf(err, "failed to get cluster %v to update the conditions.", clusterInfo.NamespacedName())
 		}
-		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.Daemon, clusterInfo.CephVersion, *cluster.Status.Cephx.CrashCollector)
-		cluster.Status.Cephx.CrashCollector = &updatedStatus
+		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.Daemon, clusterInfo.CephVersion, cluster.Status.Cephx.CrashCollector)
+		cluster.Status.Cephx.CrashCollector = updatedStatus
 		if err := reporting.UpdateStatus(context.Client, cluster); err != nil {
 			return errors.Wrap(err, "failed to update cluster cephx status for crash collector daemon")
 		}
@@ -205,7 +205,7 @@ func createExporterKeyring(s *keyring.SecretStore, context *clusterd.Context, cl
 	shouldRotateCephxKeys, err := keyring.ShouldRotateCephxKeys(
 		clusterObj.Spec.Security.CephX.Daemon,
 		clusterInfo.CephVersion, clusterInfo.CephVersion,
-		*clusterObj.Status.Cephx.CephExporter,
+		clusterObj.Status.Cephx.CephExporter,
 	)
 	if err != nil {
 		return "", errors.Wrapf(err, "failed to check if cephx keys should be rotated for ceph exporter %q", exporterKeyringUsername)
@@ -234,8 +234,8 @@ func updateCephExporterCephxStatus(context *clusterd.Context, clusterInfo *clien
 		if err := context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cluster); err != nil {
 			return errors.Wrapf(err, "failed to get cluster %v to update the conditions.", clusterInfo.NamespacedName())
 		}
-		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.Daemon, clusterInfo.CephVersion, *cluster.Status.Cephx.CephExporter)
-		cluster.Status.Cephx.CephExporter = &updatedStatus
+		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.Daemon, clusterInfo.CephVersion, cluster.Status.Cephx.CephExporter)
+		cluster.Status.Cephx.CephExporter = updatedStatus
 		if err := reporting.UpdateStatus(context.Client, cluster); err != nil {
 			return errors.Wrap(err, "failed to update cluster cephx status for ceph exporter daemon")
 		}

pkg/operator/ceph/cluster/nodedaemon/keyring_test.go

@@ -85,8 +85,8 @@ func TestCreateCrashCollectorKeyring(t *testing.T) {
 			CephStatus: &cephv1.CephStatus{
 				Health: "",
 			},
-			Cephx: &cephv1.ClusterCephxStatus{
-				CrashCollector: &status,
+			Cephx: cephv1.ClusterCephxStatus{
+				CrashCollector: status,
 			},
 		},
 	}
@@ -177,8 +177,8 @@ func TestCreateCephExporterKeyring(t *testing.T) {
 			CephStatus: &cephv1.CephStatus{
 				Health: "",
 			},
-			Cephx: &cephv1.ClusterCephxStatus{
-				CephExporter: &status,
+			Cephx: cephv1.ClusterCephxStatus{
+				CephExporter: status,
 			},
 		},
 	}

pkg/operator/ceph/cluster/osd/osd.go

@@ -1073,10 +1073,7 @@ func (c *Cluster) updateCephOsdStorageStatus() error {
 		cephCluster.Status.CephStorage = &cephClusterStorage
 
 		if cephx != nil {
-			if cephCluster.Status.Cephx == nil {
-				cephCluster.Status.Cephx = &cephv1.ClusterCephxStatus{}
-			}
-			cephCluster.Status.Cephx.OSD = cephx
+			cephCluster.Status.Cephx.OSD = *cephx
 		}
 
 		if err := reporting.UpdateStatus(c.context.Client, &cephCluster); err != nil {

pkg/operator/ceph/cluster/osd/osd_test.go

@@ -952,7 +952,7 @@ func Test_updateCephOsdStorageStatus_cephx(t *testing.T) {
 		assert.NoError(t, err)
 		err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster)
 		assert.NoError(t, err)
-		assert.Nil(t, cephCluster.Status.Cephx)
+		assert.Equal(t, cephv1.ClusterCephxStatus{}, cephCluster.Status.Cephx)
 	})
 
 	t.Run("unset cephx status, no cephcluster status", func(t *testing.T) {
@@ -1046,7 +1046,7 @@ func Test_updateCephOsdStorageStatus_cephx(t *testing.T) {
 		err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster)
 		assert.NoError(t, err)
 		assert.NotNil(t, cephCluster.Status.Cephx)
-		assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "v20"}, *cephCluster.Status.Cephx.OSD)
+		assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "v20"}, cephCluster.Status.Cephx.OSD)
 	})
 
 	t.Run("gen 1 and unset cephx status", func(t *testing.T) {
@@ -1076,7 +1076,7 @@ func Test_updateCephOsdStorageStatus_cephx(t *testing.T) {
 		err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster)
 		assert.NoError(t, err)
 		assert.NotNil(t, cephCluster.Status.Cephx)
-		assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 0, KeyCephVersion: ""}, *cephCluster.Status.Cephx.OSD)
+		assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 0, KeyCephVersion: ""}, cephCluster.Status.Cephx.OSD)
 	})
 
 	t.Run("gen 1 and gen 2 cephx status", func(t *testing.T) {
@@ -1108,7 +1108,7 @@ func Test_updateCephOsdStorageStatus_cephx(t *testing.T) {
 		err = context.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cephCluster)
 		assert.NoError(t, err)
 		assert.NotNil(t, cephCluster.Status.Cephx)
-		assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "v20"}, *cephCluster.Status.Cephx.OSD)
+		assert.Equal(t, cephv1.CephxStatus{KeyGeneration: 1, KeyCephVersion: "v20"}, cephCluster.Status.Cephx.OSD)
 	})
 }
 

pkg/operator/ceph/controller/mirror_peer.go

@@ -249,7 +249,7 @@ func shouldRotateMirrorPeerKeys(c *clusterd.Context, clusterInfo *cephclient.Clu
 	desiredCephVersion := clusterInfo.CephVersion
 	runningCephVersion := clusterInfo.CephVersion
 
-	shouldRotateKeys, err := keyring.ShouldRotateCephxKeys(cephObj.Spec.Security.CephX.RBDMirrorPeer, runningCephVersion, desiredCephVersion, *cephObj.Status.Cephx.RBDMirrorPeer)
+	shouldRotateKeys, err := keyring.ShouldRotateCephxKeys(cephObj.Spec.Security.CephX.RBDMirrorPeer, runningCephVersion, desiredCephVersion, cephObj.Status.Cephx.RBDMirrorPeer)
 	if err != nil {
 		return false, errors.Wrap(err, "failed to check if mirror peer keys should be rotated or not")
 	}
@@ -264,8 +264,8 @@ func updateCephClusterCephxRbdMirrorStatus(c *clusterd.Context, clusterInfo *cep
 		if err := c.Client.Get(clusterInfo.Context, clusterInfo.NamespacedName(), cluster); err != nil {
 			return errors.Wrapf(err, "failed to get cluster %v to update the conditions.", clusterInfo.NamespacedName())
 		}
-		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.RBDMirrorPeer, clusterInfo.CephVersion, *cluster.Status.Cephx.RBDMirrorPeer)
-		cluster.Status.Cephx.RBDMirrorPeer = &updatedStatus
+		updatedStatus := keyring.UpdatedCephxStatus(didRotate, cluster.Spec.Security.CephX.RBDMirrorPeer, clusterInfo.CephVersion, cluster.Status.Cephx.RBDMirrorPeer)
+		cluster.Status.Cephx.RBDMirrorPeer = updatedStatus
 		logger.Debugf("updating rbd-mirror cephx status to %+v", cluster.Status.Cephx.RBDMirrorPeer)
 		if err := reporting.UpdateStatus(c.Client, cluster); err != nil {
 			return errors.Wrap(err, "failed to update cluster cephx status for rbd-mirror")

pkg/operator/ceph/pool/controller.go

@@ -364,7 +364,7 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile
 		}
 
 		// update rbdMirror cephXStatus immediately after bootstrapping the peer token
-		statusErr = r.updateStatus(request.NamespacedName, cephv1.ConditionProgressing, observedGeneration, cephCluster.Status.Cephx.RBDMirrorPeer)
+		statusErr = r.updateStatus(request.NamespacedName, cephv1.ConditionProgressing, observedGeneration, &cephCluster.Status.Cephx.RBDMirrorPeer)
 		if statusErr != nil {
 			return opcontroller.ImmediateRetryResult, *cephBlockPool, errors.Wrapf(statusErr, "failed to update %q status to %q", request.NamespacedName, cephv1.ConditionProgressing)
 		}

pkg/operator/ceph/pool/controller_test.go

@@ -317,7 +317,7 @@ func TestCephBlockPoolController(t *testing.T) {
 				CephStatus: &cephv1.CephStatus{
 					Health: "",
 				},
-				Cephx: &cephv1.ClusterCephxStatus{},
+				Cephx: cephv1.ClusterCephxStatus{},
 			},
 		}
 
@@ -1026,7 +1026,7 @@ func TestMirrorPeerKeyRotationStatus(t *testing.T) {
 			CephStatus: &cephv1.CephStatus{
 				Health: "HEALTH_OK",
 			},
-			Cephx: &cephv1.ClusterCephxStatus{},
+			Cephx: cephv1.ClusterCephxStatus{},
 		},
 	}
 
@@ -1188,7 +1188,7 @@ func TestMirrorPeerKeyRotationStatus(t *testing.T) {
 	})
 
 	t.Run("PeerToken cephx status should be updated based on the cephCluster cephx status", func(t *testing.T) {
-		RDBMirrorPeerStatus := &cephv1.CephxStatus{
+		RDBMirrorPeerStatus := cephv1.CephxStatus{
 			KeyGeneration:  2,
 			KeyCephVersion: "20.2.0-0",
 		}
@@ -1201,6 +1201,6 @@ func TestMirrorPeerKeyRotationStatus(t *testing.T) {
 		pool := &cephv1.CephBlockPool{}
 		err = r.client.Get(context.TODO(), req.NamespacedName, pool)
 		assert.NoError(t, err)
-		assert.Equal(t, *RDBMirrorPeerStatus, pool.Status.Cephx.PeerToken)
+		assert.Equal(t, RDBMirrorPeerStatus, pool.Status.Cephx.PeerToken)
 	})
 }