Set default CephDeviceClass in the status only if it's not set yet

The sc.Status.DefaultCephDeviceClass helps us to set the deviceClasses
on the pool on the CRs. Till now we are trying to determine
the field in each reconcile loop. But with the introduction of
multiple deviceClasses support in day-2, we can't always determine
it as the logic for the determination is that it chooses the first
non replica-1 deviceClass from the list of deviceClasses on the
cephCluster CR status. If we determine it in each reconcile loop,
we are running the risk of going back & forth between the multiple
non replica-1 deviceClasses.

Although the current ceph logic returns the list in a consistent order
so the problem shouldn't appear today, but any change in this logic
might introduce future bugs. as we have the assurance that any
additional deviceClass will be added in day-2 only so we can safely set
the default deviceClass in the status only if it's not set yet & not
touch it there after.

Signed-off-by: Malay Kumar Parida <[email protected]>

controllers/storagecluster/cephcluster.go

@@ -264,11 +264,6 @@ func (obj *ocsCephCluster) ensureCreated(r *StorageClusterReconciler, sc *ocsv1.
 		return reconcile.Result{}, nil
 	}
 
-	// Set the default CephDeviceClass in the StorageCluster status(not needed for external clusters)
-	if !sc.Spec.ExternalStorage.Enable && found.Status.CephStorage != nil {
-		sc.Status.DefaultCephDeviceClass = determineDefaultCephDeviceClass(found.Status.CephStorage.DeviceClasses, sc.Spec.ManagedResources.CephNonResilientPools.Enable, sc.Status.FailureDomainValues)
-	}
-
 	// Record actual Ceph container image version before attempting update
 	sc.Status.Images.Ceph.ActualImage = found.Spec.CephVersion.Image
 
@@ -349,6 +344,11 @@ func (obj *ocsCephCluster) ensureCreated(r *StorageClusterReconciler, sc *ocsv1.
 		}
 	}
 
+	// Set the default CephDeviceClass in the StorageCluster status if it's not set yet (not needed for external clusters)
+	if !sc.Spec.ExternalStorage.Enable && sc.Status.DefaultCephDeviceClass == "" && found.Status.CephStorage != nil {
+		sc.Status.DefaultCephDeviceClass = determineDefaultCephDeviceClass(found.Status.CephStorage.DeviceClasses, sc.Spec.ManagedResources.CephNonResilientPools.Enable, sc.Status.FailureDomainValues)
+	}
+
 	// Update the currentMonCount field in StoragCluster status from the cephCluster CR
 	if !sc.Spec.ExternalStorage.Enable {
 		sc.Status.CurrentMonCount = cephCluster.Spec.Mon.Count
@@ -1335,6 +1335,6 @@ func determineDefaultCephDeviceClass(foundDeviceClasses []rookCephv1.DeviceClass
 			}
 		}
 	}
-	// By default return "ssd"
-	return "ssd"
+	// if no device classes are found in status return empty string
+	return ""
 }

controllers/storagecluster/cephcluster_test.go

@@ -1535,7 +1535,7 @@ func TestDetermineDefaultCephDeviceClass(t *testing.T) {
 			foundDeviceClasses:    []rookCephv1.DeviceClasses{},
 			isReplica1:            false,
 			replica1DeviceClasses: []string{},
-			expectedDeviceClass:   "ssd",
+			expectedDeviceClass:   "",
 		},
 		{
 			label: "Case 2: Replica 1 not enabled & 1 DeviceClass is in status",
@@ -1567,7 +1567,7 @@ func TestDetermineDefaultCephDeviceClass(t *testing.T) {
 			foundDeviceClasses:    []rookCephv1.DeviceClasses{},
 			isReplica1:            true,
 			replica1DeviceClasses: []string{"zone1", "zone2", "zone3"},
-			expectedDeviceClass:   "ssd",
+			expectedDeviceClass:   "",
 		},
 		{
 			label: "Case 5:  Replica 1 enabled. Total less than n DeviceClass are in status, with only one non replica-1 DeviceClass",
@@ -1608,7 +1608,7 @@ func TestDetermineDefaultCephDeviceClass(t *testing.T) {
 			},
 			isReplica1:            true,
 			replica1DeviceClasses: []string{"zone1", "zone2", "zone3"},
-			expectedDeviceClass:   "ssd",
+			expectedDeviceClass:   "",
 		},
 		{
 			label: "Case 8: Replica 1 enabled & n+1 total DeviceClass are in status(n replica-1 DeviceClass, 1 non-replica-1 DeviceClass)",