testing the waters for convergence

.dockerignore

@@ -0,0 +1,2 @@
+bin
+tools

api/v1/storagecluster_types.go

@@ -91,10 +91,6 @@ type StorageClusterSpec struct {
 	// the effective usable storage capacity.
 	OverprovisionControl []OverprovisionControlSpec `json:"overprovisionControl,omitempty"`
 
-	// AllowRemoteStorageConsumers Indicates that the OCS cluster should deploy the needed
-	// components to enable connections from remote consumers.
-	AllowRemoteStorageConsumers bool `json:"allowRemoteStorageConsumers,omitempty"`
-
 	// ProviderAPIServerServiceType Indicates the ServiceType for OCS Provider API Server Service.
 	// The supported values are NodePort or LoadBalancer. The default ServiceType is NodePort if the value is empty.
 	// This will only be used when AllowRemoteStorageConsumers is set to true

api/v1alpha1/storageconsumer_types.go

@@ -53,8 +53,6 @@ type CephResourcesSpec struct {
 	Name string `json:"name,omitempty"`
 	// Phase describes the phase of created ceph resource
 	Phase string `json:"status,omitempty"`
-	// CephClients holds the name of CephClients mapped to the created ceph resource
-	CephClients map[string]string `json:"cephClients,omitempty"`
 }
 
 // StorageConsumerStatus defines the observed state of StorageConsumer

config/crd/bases/ocs.openshift.io_storageclusters.yaml

@@ -59,11 +59,6 @@ spec:
           spec:
             description: StorageClusterSpec defines the desired state of StorageCluster
             properties:
-              allowRemoteStorageConsumers:
-                description: |-
-                  AllowRemoteStorageConsumers Indicates that the OCS cluster should deploy the needed
-                  components to enable connections from remote consumers.
-                type: boolean
               arbiter:
                 description: |-
                   ArbiterSpec specifies the storage cluster options related to arbiter.

config/crd/bases/ocs.openshift.io_storageconsumers.yaml

@@ -56,12 +56,6 @@ spec:
                   description: CephResourcesSpec hold details of created ceph resources
                     required for external storage
                   properties:
-                    cephClients:
-                      additionalProperties:
-                        type: string
-                      description: CephClients holds the name of CephClients mapped
-                        to the created ceph resource
-                      type: object
                     kind:
                       description: Kind describes the kind of created ceph resource
                       type: string

config/crd/bases/ocs.openshift.io_storagerequests.yaml

@@ -68,12 +68,6 @@ spec:
                   description: CephResourcesSpec hold details of created ceph resources
                     required for external storage
                   properties:
-                    cephClients:
-                      additionalProperties:
-                        type: string
-                      description: CephClients holds the name of CephClients mapped
-                        to the created ceph resource
-                      type: object
                     kind:
                       description: Kind describes the kind of created ceph resource
                       type: string

config/rbac/role.yaml

@@ -312,7 +312,6 @@ rules:
   - create
   - delete
   - get
-  - list
   - update
   - watch
 - apiGroups:

controllers/ocsinitialization/ocsinitialization_controller.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"reflect"
-	"slices"
 	"strconv"
 	"strings"
 
@@ -499,17 +498,13 @@ func (r *OCSInitializationReconciler) getCsiTolerations(csiTolerationKey string)
 // When any value in the configmap is updated, the rook-ceph-operator pod is restarted to pick up the new values.
 func (r *OCSInitializationReconciler) ensureOcsOperatorConfigExists(initialData *ocsv1.OCSInitialization) error {
 
-	allowConsumers := slices.ContainsFunc(r.clusters.GetInternalStorageClusters(), func(sc ocsv1.StorageCluster) bool {
-		return sc.Spec.AllowRemoteStorageConsumers
-	})
-
 	ocsOperatorConfigData := map[string]string{
 		util.ClusterNameKey:              util.GetClusterID(r.ctx, r.Client, &r.Log),
 		util.RookCurrentNamespaceOnlyKey: strconv.FormatBool(!(len(r.clusters.GetStorageClusters()) > 1)),
 		util.EnableTopologyKey:           r.getEnableTopologyKeyValue(),
 		util.TopologyDomainLabelsKey:     r.getTopologyDomainLabelsKeyValue(),
 		util.EnableNFSKey:                r.getEnableNFSKeyValue(),
-		util.DisableCSIDriverKey:         strconv.FormatBool(allowConsumers),
+		util.DisableCSIDriverKey:         strconv.FormatBool(true),
 	}
 
 	ocsOperatorConfig := &corev1.ConfigMap{

controllers/storagecluster/cephfilesystem.go

@@ -135,13 +135,9 @@ func (obj *ocsCephFilesystems) ensureCreated(r *StorageClusterReconciler, instan
 				return reconcile.Result{}, err
 			}
 		}
-		// create default csi subvolumegroup for the filesystem
-		// skip for the ocs provider mode
-		if !instance.Spec.AllowRemoteStorageConsumers {
-			err = r.createDefaultSubvolumeGroup(cephFilesystem.Name, cephFilesystem.Namespace, cephFilesystem.ObjectMeta.OwnerReferences)
-			if err != nil {
-				return reconcile.Result{}, err
-			}
+		err = r.createDefaultSubvolumeGroup(cephFilesystem.Name, cephFilesystem.Namespace, cephFilesystem.ObjectMeta.OwnerReferences)
+		if err != nil {
+			return reconcile.Result{}, err
 		}
 	}
 
@@ -240,15 +236,11 @@ func (obj *ocsCephFilesystems) ensureDeleted(r *StorageClusterReconciler, sc *oc
 			return reconcile.Result{}, fmt.Errorf("uninstall: Unable to retrieve CephFileSystem %v: %v", cephFilesystem.Name, err)
 		}
 
-		// delete csi subvolume group for particular filesystem
-		// skip for the ocs provider mode
-		if !sc.Spec.AllowRemoteStorageConsumers {
-			cephSVGName := generateNameForCephSubvolumeGroup(cephFilesystem.Name)
-			err = r.deleteDefaultSubvolumeGroup(cephFilesystem.Name, cephFilesystem.Namespace)
-			if err != nil {
-				r.Log.Error(err, "Uninstall: unable to delete subvolumegroup", "subvolumegroup", klog.KRef(cephFilesystem.Namespace, cephSVGName))
-				return reconcile.Result{}, err
-			}
+		cephSVGName := generateNameForCephSubvolumeGroup(cephFilesystem.Name)
+		err = r.deleteDefaultSubvolumeGroup(cephFilesystem.Name, cephFilesystem.Namespace)
+		if err != nil {
+			r.Log.Error(err, "Uninstall: unable to delete subvolumegroup", "subvolumegroup", klog.KRef(cephFilesystem.Namespace, cephSVGName))
+			return reconcile.Result{}, err
 		}
 		if cephFilesystem.GetDeletionTimestamp().IsZero() {
 			r.Log.Info("Uninstall: Deleting cephFilesystem.", "CephFileSystem", klog.KRef(foundCephFilesystem.Namespace, foundCephFilesystem.Name))

controllers/storagecluster/cephfilesystem_test.go

@@ -17,33 +17,18 @@ import (
 
 func TestCephFileSystem(t *testing.T) {
 	var cases = []struct {
-		label                  string
-		createRuntimeObjects   bool
-		remoteStorageConsumers bool
+		label                string
+		createRuntimeObjects bool
 	}{
 		{
-			label:                  "Not in provider mode",
-			createRuntimeObjects:   false,
-			remoteStorageConsumers: false,
-		},
-		{
-			label:                  "In provider mode",
-			createRuntimeObjects:   false,
-			remoteStorageConsumers: true,
+			label:                "Not in provider mode",
+			createRuntimeObjects: false,
 		},
 	}
-	spList := getMockStorageProfiles()
-
 	for _, c := range cases {
 		var objects []client.Object
 
-		providerModeSpec := &api.StorageClusterSpec{
-			AllowRemoteStorageConsumers:  c.remoteStorageConsumers,
-			ProviderAPIServerServiceType: "",
-		}
-
-		t, reconcilerOCSInit, cr, requestOCSInit, _ := initStorageClusterResourceCreateUpdateTestProviderMode(
-			t, objects, providerModeSpec, spList, c.remoteStorageConsumers)
+		t, reconcilerOCSInit, cr, requestOCSInit := initStorageClusterResourceCreateUpdateTestProviderMode(t, objects)
 		if c.createRuntimeObjects {
 			objects = createUpdateRuntimeObjects(t) //nolint:staticcheck //no need to use objects as they update in runtime
 		}

controllers/storagecluster/external_resources_test.go

@@ -571,6 +571,7 @@ func TestErasureCodedExternalResources(t *testing.T) {
 			},
 		},
 	}
+	cr.UID = "temp"
 	externalResource := []ExternalResource{
 		{
 			Name: "ceph-rbd",

controllers/storagecluster/generate.go

@@ -127,7 +127,11 @@ func generateNameForSnapshotClassSecret(instance *ocsv1.StorageCluster, snapshot
 			}
 		}
 	}
-	return fmt.Sprintf("rook-csi-%s-provisioner", snapshotType)
+	return util.GenerateCephClientSecretName(
+		string(snapshotType),
+		"provisioner",
+		string(instance.UID),
+	)
 }
 
 func generateNameForCephRbdMirror(initData *ocsv1.StorageCluster) string {

controllers/storagecluster/initialization_reconciler_test.go

@@ -149,13 +149,9 @@ func createUpdateRuntimeObjects(t *testing.T) []client.Object {
 	return updateRTObjects
 }
 
-func initStorageClusterResourceCreateUpdateTestProviderMode(t *testing.T, runtimeObjs []client.Object,
-	customSpec *api.StorageClusterSpec, storageProfiles *api.StorageProfileList, remoteConsumers bool) (*testing.T, StorageClusterReconciler,
-	*api.StorageCluster, reconcile.Request, []reconcile.Request) {
+func initStorageClusterResourceCreateUpdateTestProviderMode(t *testing.T, runtimeObjs []client.Object) (
+	*testing.T, StorageClusterReconciler, *api.StorageCluster, reconcile.Request) {
 	cr := createDefaultStorageCluster()
-	if customSpec != nil {
-		_ = mergo.Merge(&cr.Spec, customSpec)
-	}
 	requestOCSInit := reconcile.Request{
 		NamespacedName: types.NamespacedName{
 			Name:      "ocsinit",
@@ -176,47 +172,13 @@ func initStorageClusterResourceCreateUpdateTestProviderMode(t *testing.T, runtim
 		rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, tbd)
 	}
 
-	if remoteConsumers {
-
-		clientConfigMap := &v1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{Name: ocsClientConfigMapName},
-		}
-
-		rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, clientConfigMap)
-
-		util.AddAnnotation(cr, "ocs.openshift.io/deployment-mode", "provider")
-	}
-
-	// Unpacks StorageProfile list to runtime objects array
-	var storageProfileRuntimeObjects []runtime.Object
-	for _, sp := range storageProfiles.Items {
-		storageProfileRuntimeObjects = append(storageProfileRuntimeObjects, &sp)
-	}
-	rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, storageProfileRuntimeObjects...)
-
 	reconciler := createFakeInitializationStorageClusterReconciler(t, rtObjsToCreateReconciler...)
 
 	_ = reconciler.Client.Create(context.TODO(), cr)
 	for _, rtObj := range runtimeObjs {
 		_ = reconciler.Client.Create(context.TODO(), rtObj)
 	}
 
-	var requestsStorageProfiles []reconcile.Request
-	for _, sp := range storageProfiles.Items {
-		requestSP := reconcile.Request{
-			NamespacedName: types.NamespacedName{
-				Name:      sp.Name,
-				Namespace: cr.Namespace,
-			},
-		}
-		_ = reconciler.Client.Create(context.TODO(), &sp)
-		requestsStorageProfiles = append(requestsStorageProfiles, requestSP)
-		requests = append(requests, requestSP)
-
-	}
-
-	spList := &api.StorageProfileList{}
-	_ = reconciler.Client.List(context.TODO(), spList)
 	for _, request := range requests {
 		err := os.Setenv("OPERATOR_NAMESPACE", request.Namespace)
 		assert.NoError(t, err)
@@ -227,7 +189,7 @@ func initStorageClusterResourceCreateUpdateTestProviderMode(t *testing.T, runtim
 		assert.NoError(t, err)
 	}
 
-	return t, reconciler, cr, requestOCSInit, requestsStorageProfiles
+	return t, reconciler, cr, requestOCSInit
 }
 
 func initStorageClusterResourceCreateUpdateTest(t *testing.T, runtimeObjs []client.Object,
@@ -380,6 +342,10 @@ func createFakeInitializationStorageClusterReconciler(t *testing.T, obj ...runti
 		},
 	}
 
+	clientConfigMap := &v1.ConfigMap{}
+	clientConfigMap.Name = ocsClientConfigMapName
+	clientConfigMap.Namespace = sc.Namespace
+
 	statusSubresourceObjs := []client.Object{sc}
 	var runtimeObjects []runtime.Object
 
@@ -398,6 +364,7 @@ func createFakeInitializationStorageClusterReconciler(t *testing.T, obj ...runti
 		mockNodeList.DeepCopy(),
 		cbp,
 		cfs,
+		clientConfigMap,
 		cnfs,
 		cnfsbp,
 		cnfssvc,
@@ -410,7 +377,7 @@ func createFakeInitializationStorageClusterReconciler(t *testing.T, obj ...runti
 		ocsProviderServiceDeployment,
 		ocsProviderService,
 		createVirtualMachineCRD(),
-		createStorageClientCRD(),
+		CreateStorageClientCRD(),
 	)
 	client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(runtimeObjects...).WithStatusSubresource(statusSubresourceObjs...).Build()
 

controllers/storagecluster/noobaa_system_reconciler.go

@@ -139,9 +139,7 @@ func (r *StorageClusterReconciler) setNooBaaDesiredState(nb *nbv1.NooBaa, sc *oc
 		"app": "noobaa",
 	}
 
-	if sc.Spec.AllowRemoteStorageConsumers {
-		util.AddAnnotation(nb, "MulticloudObjectGatewayProviderMode", "true")
-	}
+	util.AddAnnotation(nb, "MulticloudObjectGatewayProviderMode", "true")
 
 	if !r.IsNoobaaStandalone {
 		storageClassName := generateNameForCephBlockPoolSC(sc)

controllers/storagecluster/provider_server.go

@@ -59,14 +59,6 @@ func (o *ocsProviderServer) ensureCreated(r *StorageClusterReconciler, instance
 
 func (o *ocsProviderServer) ensureDeleted(r *StorageClusterReconciler, instance *ocsv1.StorageCluster) (reconcile.Result, error) {
 
-	// We do not check instance.Spec.AllowRemoteStorageConsumers because provider can disable this functionality
-	// and we need to delete the resources even the flag is not enabled (uninstall case).
-
-	// This func is directly called by the ensureCreated if the flag is disabled and deletes the resource
-	// Which means we do not need to call ensureDeleted while reconciling unless we are uninstalling
-
-	// NOTE: Do not add the check
-
 	if err := r.verifyNoStorageConsumerExist(instance); err != nil {
 		return reconcile.Result{}, err
 	}