Merge pull request #2789 from sp98/BZ-2311043

Bug 2311043: Revert "Migrate osds to bluestore"

controllers/storagecluster/cephcluster.go

@@ -72,6 +72,8 @@ const (
 	networkProvider           = "multus"
 	publicNetworkSelectorKey  = "public"
 	clusterNetworkSelectorKey = "cluster"
+	// DisasterRecoveryTargetAnnotation signifies that the cluster is intended to be used for Disaster Recovery
+	DisasterRecoveryTargetAnnotation = "ocs.openshift.io/clusterIsDisasterRecoveryTarget"
 )
 
 const (
@@ -272,8 +274,9 @@ func (obj *ocsCephCluster) ensureCreated(r *StorageClusterReconciler, sc *ocsv1.
 	// Record actual Ceph container image version before attempting update
 	sc.Status.Images.Ceph.ActualImage = found.Spec.CephVersion.Image
 
-	// Update OSD store to `bluestore`
-	cephCluster.Spec.Storage.Store = updateOSDStore(found.Spec.Storage.Store)
+	// Allow migration of OSD to bluestore-rdr if RDR optimization annotation is added on an existing cluster.
+	// Prevent changing the bluestore-rdr settings if they are already applied in the existing ceph cluster.
+	cephCluster.Spec.Storage.Store = determineOSDStore(sc, cephCluster.Spec.Storage.Store, found.Spec.Storage.Store)
 
 	// Add it to the list of RelatedObjects if found
 	objectRef, err := reference.GetReference(r.Scheme, found)
@@ -421,6 +424,11 @@ func newCephCluster(sc *ocsv1.StorageCluster, cephImage string, kmsConfigMap *co
 		MaxLogSize:  &maxLogSize,
 	}
 
+	osdStore := getOSDStoreConfig(sc)
+	if osdStore.Type != "" {
+		reqLogger.Info("osd store settings", osdStore)
+	}
+
 	cephCluster := &rookCephv1.CephCluster{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      generateNameForCephCluster(sc),
@@ -453,6 +461,7 @@ func newCephCluster(sc *ocsv1.StorageCluster, cephImage string, kmsConfigMap *co
 				AllowDeviceClassUpdate:       sc.Spec.ManagedResources.CephCluster.AllowDeviceClassUpdate,
 				AllowOsdCrushWeightUpdate:    true,
 				StorageClassDeviceSets:       newStorageClassDeviceSets(sc),
+				Store:                        osdStore,
 				FlappingRestartIntervalHours: 24,
 				FullRatio:                    sc.Spec.ManagedResources.CephCluster.FullRatio,
 				NearFullRatio:                sc.Spec.ManagedResources.CephCluster.NearFullRatio,
@@ -1296,14 +1305,45 @@ func getIPFamilyConfig(c client.Client) (rookCephv1.IPFamilyType, bool, error) {
 	return rookCephv1.IPv4, false, nil
 }
 
-func updateOSDStore(existingOSDStore rookCephv1.OSDStore) rookCephv1.OSDStore {
+func getOSDStoreConfig(sc *ocsv1.StorageCluster) rookCephv1.OSDStore {
+	osdStore := rookCephv1.OSDStore{}
+	if !sc.Spec.ExternalStorage.Enable && optimizeDisasterRecovery(sc) {
+		osdStore.Type = string(rookCephv1.StoreTypeBlueStoreRDR)
+	}
+
+	return osdStore
+}
+
+// optimizeDisasterRecovery returns true if any RDR optimizations are required
+func optimizeDisasterRecovery(sc *ocsv1.StorageCluster) bool {
+	if annotation, found := sc.GetAnnotations()[DisasterRecoveryTargetAnnotation]; found {
+		if annotation == "true" {
+			return true
+		}
+	}
+
+	return false
+}
+
+func determineOSDStore(sc *ocsv1.StorageCluster, newOSDStore, existingOSDStore rookCephv1.OSDStore) rookCephv1.OSDStore {
 	if existingOSDStore.Type == string(rookCephv1.StoreTypeBlueStoreRDR) {
+		return existingOSDStore
+	} else if !sc.Spec.ExternalStorage.Enable && (isBluestore(existingOSDStore) && optimizeDisasterRecovery(sc)) {
 		return rookCephv1.OSDStore{
-			Type:        string(rookCephv1.StoreTypeBlueStore),
+			Type:        string(rookCephv1.StoreTypeBlueStoreRDR),
 			UpdateStore: "yes-really-update-store",
 		}
 	}
-	return existingOSDStore
+
+	return newOSDStore
+}
+
+func isBluestore(store rookCephv1.OSDStore) bool {
+	if store.Type == string(rookCephv1.StoreTypeBlueStore) || store.Type == "" {
+		return true
+	}
+
+	return false
 }
 
 func getOsdCount(sc *ocsv1.StorageCluster) int {

controllers/storagecluster/cephcluster_test.go

@@ -1479,27 +1479,82 @@ func TestGetIPFamilyConfig(t *testing.T) {
 	}
 }
 
-func TestEnsureRDRMigration(t *testing.T) {
-	testSkipPrometheusRules = true
+func TestCephClusterStoreType(t *testing.T) {
+	sc := &ocsv1.StorageCluster{}
+
+	t.Run("ensure no bluestore optimization", func(t *testing.T) {
+		actual := newCephCluster(sc, "", nil, log)
+		assert.Equal(t, "", actual.Spec.Storage.Store.Type)
+	})
+
+	t.Run("ensure bluestore optimization based on annotation for internal clusters", func(t *testing.T) {
+		annotations := map[string]string{
+			DisasterRecoveryTargetAnnotation: "true",
+		}
+		sc.Annotations = annotations
+		actual := newCephCluster(sc, "", nil, log)
+		assert.Equal(t, "bluestore-rdr", actual.Spec.Storage.Store.Type)
+	})
+
+	t.Run("ensure no bluestore optimization for external clusters", func(t *testing.T) {
+		sc.Spec.ExternalStorage.Enable = true
+		actual := newCephCluster(sc, "", nil, log)
+		assert.Equal(t, "", actual.Spec.Storage.Store.Type)
+	})
+}
+
+func TestEnsureRDROptmizations(t *testing.T) {
 	sc := &ocsv1.StorageCluster{}
 	mockStorageCluster.DeepCopyInto(sc)
 	sc.Status.Images.Ceph = &ocsv1.ComponentImageStatus{}
+	sc.Annotations[DisasterRecoveryTargetAnnotation] = "true"
 	reconciler := createFakeStorageClusterReconciler(t, networkConfig)
 
-	expected := newCephCluster(mockStorageCluster.DeepCopy(), "", nil, log)
+	// Ensure bluestore-rdr store type if RDR optimization annotation is added
+	var obj ocsCephCluster
+	_, err := obj.ensureCreated(&reconciler, sc)
+	assert.NilError(t, err)
+	actual := &rookCephv1.CephCluster{}
+	err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: generateNameForCephClusterFromString(sc.Name), Namespace: sc.Namespace}, actual)
+	assert.NilError(t, err)
+	assert.Equal(t, string(rookCephv1.StoreTypeBlueStoreRDR), actual.Spec.Storage.Store.Type)
 
-	expected.Spec.Storage.Store.Type = string(rookCephv1.StoreTypeBlueStoreRDR)
-	err := reconciler.Client.Create(context.TODO(), expected)
+	// Ensure bluestoreRDR store is not overridden if required annotations are removed later on
+	testSkipPrometheusRules = true
+	delete(sc.Annotations, DisasterRecoveryTargetAnnotation)
+	_, err = obj.ensureCreated(&reconciler, sc)
 	assert.NilError(t, err)
+	actual = &rookCephv1.CephCluster{}
+	err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: generateNameForCephClusterFromString(sc.Name), Namespace: sc.Namespace}, actual)
+	assert.NilError(t, err)
+	assert.Equal(t, string(rookCephv1.StoreTypeBlueStoreRDR), actual.Spec.Storage.Store.Type)
+}
+
+func TestEnsureRDRMigration(t *testing.T) {
+	sc := &ocsv1.StorageCluster{}
+	mockStorageCluster.DeepCopyInto(sc)
+	sc.Status.Images.Ceph = &ocsv1.ComponentImageStatus{}
+	reconciler := createFakeStorageClusterReconciler(t, networkConfig)
 
-	// Ensure bluestore-rdr store type is reset to bluestore
+	// Ensure bluestore store type if RDR optimization annotation is not added
 	var obj ocsCephCluster
-	_, err = obj.ensureCreated(&reconciler, sc)
+	_, err := obj.ensureCreated(&reconciler, sc)
 	assert.NilError(t, err)
 	actual := &rookCephv1.CephCluster{}
 	err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: generateNameForCephClusterFromString(sc.Name), Namespace: sc.Namespace}, actual)
 	assert.NilError(t, err)
-	assert.Equal(t, string(rookCephv1.StoreTypeBlueStore), actual.Spec.Storage.Store.Type)
+	assert.Equal(t, "", actual.Spec.Storage.Store.Type)
+	assert.Equal(t, "", actual.Spec.Storage.Store.UpdateStore)
+
+	// Ensure bluestoreRDR migration is set if RDR optimization annotation is added later on
+	testSkipPrometheusRules = true
+	sc.Annotations[DisasterRecoveryTargetAnnotation] = "true"
+	_, err = obj.ensureCreated(&reconciler, sc)
+	assert.NilError(t, err)
+	actual = &rookCephv1.CephCluster{}
+	err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: generateNameForCephClusterFromString(sc.Name), Namespace: sc.Namespace}, actual)
+	assert.NilError(t, err)
+	assert.Equal(t, string(rookCephv1.StoreTypeBlueStoreRDR), actual.Spec.Storage.Store.Type)
 	assert.Equal(t, "yes-really-update-store", actual.Spec.Storage.Store.UpdateStore)
 }
 

docs/design/disaster-recovery-optimization.md

@@ -0,0 +1,33 @@
+# FEATURE: Regional Disaster Recovery Optimization
+
+## Summary
+
+Implement use of `bluestore-rdr` as the OSD backend store for customers using ODF with Regional Disaster Recovery (RDR).
+
+## Motivation
+
+- Ceph uses BlueStore as a special-purpose storage back end designed specifically for managing data on disk for Ceph OSD workloads. Ceph team is working on improving this backend storage that will help in significantly improving the performance, especially in disaster recovery scenarios. This new OSD backend is known as bluestore-rdr. In the future, other backends will also need to be supported, such as seastore.
+
+## Goals
+
+- Customers should be able to use the optimized OSD store when creating a new ODF cluster
+
+## Non Goals/Deferred Goals
+
+- Customers should be able to update their existing ODF clusters to use the optimized OSD store
+
+## Proposal
+
+- Enable the optimized OSD backend store for new RDR clusters.
+
+
+### Greenfield
+- If `ocs.openshift.io/clusterIsDisasterRecoveryTarget: true` annotation is available in the `StorageCluster` then configure OSD store type in the `cephCluster` as:
+
+```YAML
+spec:
+  storage:
+    store:
+        type: bluestore-rdr
+```
+- Customers should not be able to remove the optimizations once they are applied. If the annotation is removed `storageCluster` some how, operator should ensure that OSD store optimization settings are not overridden in the `cephCluster` CR