diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md index b4c4069c8..fff37255e 100644 --- a/Documentation/CRDs/specification.md +++ b/Documentation/CRDs/specification.md @@ -5140,19 +5140,6 @@ CephxStatus -rbdMirrorPeer
- - -CephxStatus - - - - -

RBDMirrorPeer represents the cephx key rotation status of the rbd-mirror-peer user

- - - - csi
@@ -5166,6 +5153,19 @@ CephxStatusWithKeyCount +rbdMirrorPeer
+ + +CephxStatus + + + + +

RBDMirrorPeer represents the cephx key rotation status of the rbd-mirror-peer user

+ + + + crashCollector
diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index 4ec5324d7..8d35753e1 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/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 diff --git a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go index bc2053f9d..47b8a8c56 100644 --- a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go +++ b/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) diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index f015e6298..47097ae9b 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/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 } diff --git a/pkg/operator/ceph/cluster/cluster_test.go b/pkg/operator/ceph/cluster/cluster_test.go index ec1b61e19..2d779643e 100644 --- a/pkg/operator/ceph/cluster/cluster_test.go +++ b/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 + }) +} diff --git a/pkg/operator/ceph/cluster/controller.go b/pkg/operator/ceph/cluster/controller.go index e2ca699b4..1428528dd 100644 --- a/pkg/operator/ceph/cluster/controller.go +++ b/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 diff --git a/pkg/operator/ceph/cluster/mgr/mgr.go b/pkg/operator/ceph/cluster/mgr/mgr.go index 5209b3956..0ecdf20b7 100644 --- a/pkg/operator/ceph/cluster/mgr/mgr.go +++ b/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") diff --git a/pkg/operator/ceph/cluster/mgr/mgr_test.go b/pkg/operator/ceph/cluster/mgr/mgr_test.go index 6dfb0f695..77b957a85 100644 --- a/pkg/operator/ceph/cluster/mgr/mgr_test.go +++ b/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, }, }, } diff --git a/pkg/operator/ceph/cluster/mon/health_test.go b/pkg/operator/ceph/cluster/mon/health_test.go index 80392f37f..06e67317b 100644 --- a/pkg/operator/ceph/cluster/mon/health_test.go +++ b/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{}, }, }, } diff --git a/pkg/operator/ceph/cluster/mon/mon.go b/pkg/operator/ceph/cluster/mon/mon.go index 3d1186764..b4cfaa4e4 100644 --- a/pkg/operator/ceph/cluster/mon/mon.go +++ b/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) diff --git a/pkg/operator/ceph/cluster/mon/mon_test.go b/pkg/operator/ceph/cluster/mon/mon_test.go index 97648bcea..50714edea 100644 --- a/pkg/operator/ceph/cluster/mon/mon_test.go +++ b/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, }, }, } diff --git a/pkg/operator/ceph/cluster/nodedaemon/keyring.go b/pkg/operator/ceph/cluster/nodedaemon/keyring.go index 1a61f67f2..5a35464ed 100644 --- a/pkg/operator/ceph/cluster/nodedaemon/keyring.go +++ b/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") } diff --git a/pkg/operator/ceph/cluster/nodedaemon/keyring_test.go b/pkg/operator/ceph/cluster/nodedaemon/keyring_test.go index 31c06d60a..484d7e778 100644 --- a/pkg/operator/ceph/cluster/nodedaemon/keyring_test.go +++ b/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, }, }, } diff --git a/pkg/operator/ceph/cluster/osd/osd.go b/pkg/operator/ceph/cluster/osd/osd.go index 5a9ff7f1d..cb9d104b2 100644 --- a/pkg/operator/ceph/cluster/osd/osd.go +++ b/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 { diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index 291be518b..8518b0302 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/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) }) } diff --git a/pkg/operator/ceph/controller/mirror_peer.go b/pkg/operator/ceph/controller/mirror_peer.go index cb8ff9c24..27cf1fe1f 100644 --- a/pkg/operator/ceph/controller/mirror_peer.go +++ b/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") diff --git a/pkg/operator/ceph/pool/controller.go b/pkg/operator/ceph/pool/controller.go index bb4fb84ec..5ea77b70d 100644 --- a/pkg/operator/ceph/pool/controller.go +++ b/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) } diff --git a/pkg/operator/ceph/pool/controller_test.go b/pkg/operator/ceph/pool/controller_test.go index 048da2f56..396c129c8 100644 --- a/pkg/operator/ceph/pool/controller_test.go +++ b/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) }) }