Repository: rook/rook
Pull request #8796 (2021-09-22 17:52:39)

test: run multiple object stores at once in CI
Multiple CephObjectStores should be able to run at the same time. In CI,
test this by:
1. Create the primary object store under test (store A) (this was
   already done)
2. Verify that store A starts and becomes ready (already done but is
   moved here)
3. Create a second object store (store B) (new)
4. Verify that store B starts and becomes ready (new)
5. Delete store B, and ensure it is deleted (new)
6. , etc. Continue to verify primary store A operations as before (this
  was already done)

This is necessary to ensure that the Rook operator is using
`radosgw-admin` and the RGW Admin Ops API properly to support multiple
ObjectStores running at once.

Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>

<!-- Please take a look at our [Contributing](https://rook.io/docs/rook/master/development-flow.html)
documentation before submitting a Pull Request!
Thank you for contributing to Rook! -->

**Description of your changes:**

**Which issue is resolved by this Pull Request:**
Resolves #

**Checklist:**

- [ ] **Commit Message Formatting**: Commit titles and messages follow guidelines in the [developer guide](https://rook.io/docs/rook/master/development-flow.html#commit-structure).
- [ ] **Skip Tests for Docs**: Add the flag for skipping the build if this is only a documentation change. See [here](https://github.com/rook/rook/blob/master/INSTALL.md#skip-ci) for the flag.
- [ ] **Skip Unrelated Tests**: Add a flag to run tests for a specific storage provider. See [test options](https://github.com/rook/rook/blob/master/INSTALL.md#test-storage-provider).
- [ ] Reviewed the developer guide on [Submitting a Pull Request](https://rook.io/docs/rook/master/development-flow.html#submitting-a-pull-request)
- [ ] Documentation has been updated, if necessary.
- [ ] Unit tests have been added, if necessary.
- [ ] Integration tests have been added, if necessary.
- [ ] Pending release notes updated with breaking and/or notable changes, if necessary.
- [ ] Upgrade from previous release is tested and upgrade user guide is updated, if necessary.
- [ ] Code generation (`make codegen`) has been run to update object specifications, if necessary.



Patch:
diff --git a/Documentation/ceph-csi-drivers.md b/Documentation/ceph-csi-drivers.md
index 9c216d00e..ccc3ba244 100644
--- a/Documentation/ceph-csi-drivers.md
+++ b/Documentation/ceph-csi-drivers.md
@@ -19,10 +19,6 @@ For documentation on consuming the storage:
 * RBD: See the [Block Storage](ceph-block.md) topic
 * CephFS: See the [Shared Filesystem](ceph-filesystem.md) topic
 
-## Supported Versions
-The supported Ceph CSI version is 3.3.0 or greater with Rook. Refer to ceph csi [releases](https://github.com/ceph/ceph-csi/releases)
-for more information.
-
 ## Static Provisioning
 
 Both drivers also support the creation of static PV and static PVC from existing RBD image/CephFS volume. Refer to [static PVC](https://github.com/ceph/ceph-csi/blob/devel/docs/static-pvc.md) for more information.
diff --git a/Documentation/helm-operator.md b/Documentation/helm-operator.md
index 9d245be20..3c5ba0324 100644
--- a/Documentation/helm-operator.md
+++ b/Documentation/helm-operator.md
@@ -108,7 +108,6 @@ The following tables lists the configurable parameters of the rook-operator char
 | `csi.rbdFSGroupPolicy`              | Policy for modifying a volume's ownership or permissions when the RBD PVC is being mounted                                  | ReadWriteOnceWithFSType                                   |
 | `csi.cephFSFSGroupPolicy`           | Policy for modifying a volume's ownership or permissions when the CephFS PVC is being mounted                               | `None`                                                    |
 | `csi.logLevel`                      | Set logging level for csi containers. Supported values from 0 to 5. 0 for general useful logs, 5 for trace level verbosity. | `0`                                                       |
-| `csi.provisionerReplicas`           | Set replicas for csi provisioner deployment.                                                                                | `2`                                                       |
 | `csi.enableGrpcMetrics`             | Enable Ceph CSI GRPC Metrics.                                                                                               | `false`                                                   |
 | `csi.enableCSIHostNetwork`          | Enable Host Networking for Ceph CSI nodeplugins.                                                                            | `false`                                                   |
 | `csi.provisionerTolerations`        | Array of tolerations in YAML format which will be added to CSI provisioner deployment.                                      | <none>                                                    |
diff --git a/cluster/charts/rook-ceph/templates/deployment.yaml b/cluster/charts/rook-ceph/templates/deployment.yaml
index 083265e3f..7efb599eb 100644
--- a/cluster/charts/rook-ceph/templates/deployment.yaml
+++ b/cluster/charts/rook-ceph/templates/deployment.yaml
@@ -275,10 +275,6 @@ spec:
         - name: CSI_LOG_LEVEL
           value: {{ .Values.csi.logLevel | quote }}
 {{- end }}
-{{- if .Values.csi.provisionerReplicas }}
-        - name: CSI_PROVISIONER_REPLICAS
-          value: {{ .Values.csi.provisionerReplicas | quote }}
-{{- end }}
 {{- if .Values.csi.csiRBDProvisionerResource }}
         - name: CSI_RBD_PROVISIONER_RESOURCE
           value: {{ .Values.csi.csiRBDProvisionerResource | quote }}
diff --git a/cluster/charts/rook-ceph/values.yaml b/cluster/charts/rook-ceph/values.yaml
index db2eed592..9d7962a43 100644
--- a/cluster/charts/rook-ceph/values.yaml
+++ b/cluster/charts/rook-ceph/values.yaml
@@ -87,9 +87,6 @@ csi:
   # sidecar with CSI provisioner pod, to enable set it to true.
   enableOMAPGenerator: false
 
-  # Set replicas for csi provisioner deployment.
-  provisionerReplicas: 2
-
   # Set logging level for csi containers.
   # Supported values from 0 to 5. 0 for general useful logs, 5 for trace level verbosity.
   #logLevel: 0
diff --git a/cluster/examples/kubernetes/ceph/operator-openshift.yaml b/cluster/examples/kubernetes/ceph/operator-openshift.yaml
index 5f433d8ab..c4ad20e4d 100644
--- a/cluster/examples/kubernetes/ceph/operator-openshift.yaml
+++ b/cluster/examples/kubernetes/ceph/operator-openshift.yaml
@@ -120,9 +120,6 @@ data:
   # Supported values from 0 to 5. 0 for general useful logs, 5 for trace level verbosity.
   # CSI_LOG_LEVEL: "0"
 
-  # Set replicas for csi provisioner deployment.
-  CSI_PROVISIONER_REPLICAS: "2"
-
   # OMAP generator generates the omap mapping between the PV name and the RBD image
   # which helps CSI to identify the rbd images for CSI operations.
   # CSI_ENABLE_OMAP_GENERATOR need to be enabled when we are using rbd mirroring feature.
diff --git a/cluster/examples/kubernetes/ceph/operator.yaml b/cluster/examples/kubernetes/ceph/operator.yaml
index b4d7d9794..0a3d70594 100644
--- a/cluster/examples/kubernetes/ceph/operator.yaml
+++ b/cluster/examples/kubernetes/ceph/operator.yaml
@@ -41,9 +41,6 @@ data:
   # Supported values from 0 to 5. 0 for general useful logs, 5 for trace level verbosity.
   # CSI_LOG_LEVEL: "0"
 
-  # Set replicas for csi provisioner deployment.
-  CSI_PROVISIONER_REPLICAS: "2"
-
   # OMAP generator will generate the omap mapping between the PV name and the RBD image.
   # CSI_ENABLE_OMAP_GENERATOR need to be enabled when we are using rbd mirroring feature.
   # By default OMAP generator sidecar is deployed with CSI provisioner pod, to disable
diff --git a/pkg/operator/ceph/csi/spec.go b/pkg/operator/ceph/csi/spec.go
index 7f8782349..05979b846 100644
--- a/pkg/operator/ceph/csi/spec.go
+++ b/pkg/operator/ceph/csi/spec.go
@@ -61,7 +61,7 @@ type Param struct {
 	CephFSLivenessMetricsPort      uint16
 	RBDGRPCMetricsPort             uint16
 	RBDLivenessMetricsPort         uint16
-	ProvisionerReplicas            int32
+	ProvisionerReplicas            uint8
 	CSICephFSPodLabels             map[string]string
 	CSIRBDPodLabels                map[string]string
 }
@@ -169,9 +169,6 @@ const (
 	// default log level for csi containers
 	defaultLogLevel uint8 = 0
 
-	// default provisioner replicas
-	defaultProvisionerReplicas int32 = 2
-
 	// update strategy
 	rollingUpdate = "RollingUpdate"
 	onDelete      = "OnDelete"
@@ -284,6 +281,13 @@ func (r *ReconcileCSI) startDrivers(ver *version.Info, ownerInfo *k8sutil.OwnerI
 	// critical pods in cluster but less priority than plugin pods
 	tp.ProvisionerPriorityClassName = k8sutil.GetValue(r.opConfig.Parameters, "CSI_PROVISIONER_PRIORITY_CLASSNAME", "")
 
+	// OMAP generator will be enabled by default
+	// If AllowUnsupported is set to false and if CSI version is less than
+	// <3.2.0 disable OMAP generator sidecar
+	if !v.SupportsOMAPController() {
+		tp.EnableOMAPGenerator = false
+	}
+
 	if strings.EqualFold(k8sutil.GetValue(r.opConfig.Parameters, "CSI_ENABLE_OMAP_GENERATOR", "false"), "true") {
 		tp.EnableOMAPGenerator = true
 	}
@@ -341,22 +345,14 @@ func (r *ReconcileCSI) startDrivers(ver *version.Info, ownerInfo *k8sutil.OwnerI
 		}
 	}
 
-	tp.ProvisionerReplicas = defaultProvisionerReplicas
+	tp.ProvisionerReplicas = 2
 	nodes, err := r.context.Clientset.CoreV1().Nodes().List(r.opManagerContext, metav1.ListOptions{})
 	if err == nil {
 		if len(nodes.Items) == 1 {
 			tp.ProvisionerReplicas = 1
-		} else {
-			replicas := k8sutil.GetValue(r.opConfig.Parameters, "CSI_PROVISIONER_REPLICAS", "2")
-			r, err := strconv.ParseInt(replicas, 10, 32)
-			if err != nil {
-				logger.Errorf("failed to parse CSI_PROVISIONER_REPLICAS. Defaulting to %d. %v", defaultProvisionerReplicas, err)
-			} else {
-				tp.ProvisionerReplicas = int32(r)
-			}
 		}
 	} else {
-		logger.Errorf("failed to get nodes. Defaulting the number of replicas of provisioner pods to %d. %v", tp.ProvisionerReplicas, err)
+		logger.Errorf("failed to get nodes. Defaulting the number of replicas of provisioner pods to 2. %v", err)
 	}
 
 	if EnableRBD {
diff --git a/pkg/operator/ceph/csi/template/cephfs/csi-cephfsplugin-provisioner-dep.yaml b/pkg/operator/ceph/csi/template/cephfs/csi-cephfsplugin-provisioner-dep.yaml
index c8d7c2a37..91a2521cd 100644
--- a/pkg/operator/ceph/csi/template/cephfs/csi-cephfsplugin-provisioner-dep.yaml
+++ b/pkg/operator/ceph/csi/template/cephfs/csi-cephfsplugin-provisioner-dep.yaml
@@ -34,6 +34,11 @@ spec:
             - name: ADDRESS
               value: /csi/csi-provisioner.sock
           imagePullPolicy: "IfNotPresent"
+          securityContext:
+            privileged: true
+            capabilities:
+              add: ["SYS_ADMIN"]
+            allowPrivilegeEscalation: true
           volumeMounts:
             - name: socket-dir
               mountPath: /csi
@@ -50,6 +55,11 @@ spec:
             - name: ADDRESS
               value: unix:///csi/csi-provisioner.sock
           imagePullPolicy: "IfNotPresent"
+          securityContext:
+            privileged: true
+            capabilities:
+              add: ["SYS_ADMIN"]
+            allowPrivilegeEscalation: true
           volumeMounts:
             - name: socket-dir
               mountPath: /csi
@@ -67,6 +77,11 @@ spec:
             - name: ADDRESS
               value: unix:///csi/csi-provisioner.sock
           imagePullPolicy: "IfNotPresent"
+          securityContext:
+            privileged: true
+            capabilities:
+              add: ["SYS_ADMIN"]
+            allowPrivilegeEscalation: true
           volumeMounts:
             - name: socket-dir
               mountPath: /csi
@@ -83,6 +98,11 @@ spec:
             - name: ADDRESS
               value: unix:///csi/csi-provisioner.sock
           imagePullPolicy: "IfNotPresent"
+          securityContext:
+            privileged: true
+            capabilities:
+              add: ["SYS_ADMIN"]
+            allowPrivilegeEscalation: true
           volumeMounts:
             - name: socket-dir
               mountPath: /csi
@@ -116,6 +136,11 @@ spec:
             - name: CSI_ENDPOINT
               value: unix:///csi/csi-provisioner.sock
           imagePullPolicy: "IfNotPresent"
+          securityContext:
+            privileged: true
+            capabilities:
+              add: ["SYS_ADMIN"]
+            allowPrivilegeEscalation: true
           volumeMounts:
             - name: socket-dir
               mountPath: /csi
@@ -150,6 +175,11 @@ spec:
             - name: socket-dir
               mountPath: /csi
           imagePullPolicy: "IfNotPresent"
+          securityContext:
+            privileged: true
+            capabilities:
+              add: ["SYS_ADMIN"]
+            allowPrivilegeEscalation: true
       volumes:
         - name: socket-dir
           emptyDir: {
diff --git a/pkg/operator/ceph/csi/version.go b/pkg/operator/ceph/csi/version.go
index bcc8166b3..685a488f2 100644
--- a/pkg/operator/ceph/csi/version.go
+++ b/pkg/operator/ceph/csi/version.go
@@ -25,16 +25,22 @@ import (
 )
 
 var (
-	//minimum supported version is 3.3.0
-	minimum = CephCSIVersion{3, 3, 0}
+	//minimum supported version is 3.0.0
+	minimum = CephCSIVersion{3, 0, 0}
 	//supportedCSIVersions are versions that rook supports
+	releasev310          = CephCSIVersion{3, 1, 0}
+	releasev320          = CephCSIVersion{3, 2, 0}
 	releasev330          = CephCSIVersion{3, 3, 0}
 	releasev340          = CephCSIVersion{3, 4, 0}
 	supportedCSIVersions = []CephCSIVersion{
 		minimum,
+		releasev310,
+		releasev320,
 		releasev330,
 		releasev340,
 	}
+	// omap generator is supported in v3.2.0+
+	omapSupportedVersions = releasev320
 	// for parsing the output of `cephcsi`
 	versionCSIPattern = regexp.MustCompile(`v(\d+)\.(\d+)\.(\d+)`)
 )
@@ -51,6 +57,33 @@ func (v *CephCSIVersion) String() string {
 		v.Major, v.Minor, v.Bugfix)
 }
 
+// SupportsOMAPController checks if the detected version supports OMAP generator
+func (v *CephCSIVersion) SupportsOMAPController() bool {
+
+	// if AllowUnsupported is set also a csi-image greater than the supported ones are allowed
+	if AllowUnsupported {
+		return true
+	}
+
+	if !v.isAtLeast(&minimum) {
+		return false
+	}
+
+	if v.Major > omapSupportedVersions.Major {
+		return true
+	}
+	if v.Major == omapSupportedVersions.Major {
+		if v.Minor > omapSupportedVersions.Minor {
+			return true
+		}
+		if v.Minor == omapSupportedVersions.Minor {
+			return v.Bugfix >= omapSupportedVersions.Bugfix
+		}
+	}
+
+	return false
+}
+
 // Supported checks if the detected version is part of the known supported CSI versions
 func (v *CephCSIVersion) Supported() bool {
 	if !v.isAtLeast(&minimum) {
diff --git a/pkg/operator/ceph/object/status.go b/pkg/operator/ceph/object/status.go
index ddf4c5ad4..a369a981b 100644
--- a/pkg/operator/ceph/object/status.go
+++ b/pkg/operator/ceph/object/status.go
@@ -89,12 +89,12 @@ func updateStatusBucket(client client.Client, name types.NamespacedName, status
 		}
 		objectStore.Status.BucketStatus = toCustomResourceStatus(objectStore.Status.BucketStatus, details, status)
 
-		// do not transition to other statuses once deletion begins
 		if objectStore.Status.Phase != cephv1.ConditionDeleting {
+			// do not transition to to other statuses once deletion begins
+			logger.Debugf("object store %q status not updated to %q because it is deleting", name.String(), status)
 			objectStore.Status.Phase = status
 		}
 
-		// but we still need to update the health checker status
 		if err := reporting.UpdateStatus(client, objectStore); err != nil {
 			return errors.Wrapf(err, "failed to set object store %q status to %v", name.String(), status)
 		}


Test patch:
diff --git a/pkg/operator/ceph/csi/version_test.go b/pkg/operator/ceph/csi/version_test.go
index c69ae0c4c..f09dacf1a 100644
--- a/pkg/operator/ceph/csi/version_test.go
+++ b/pkg/operator/ceph/csi/version_test.go
@@ -23,7 +23,11 @@ import (
 )
 
 var (
-	testMinVersion         = CephCSIVersion{3, 3, 0}
+	testMinVersion         = CephCSIVersion{2, 0, 0}
+	testReleaseV210        = CephCSIVersion{2, 1, 0}
+	testReleaseV300        = CephCSIVersion{3, 0, 0}
+	testReleaseV320        = CephCSIVersion{3, 2, 0}
+	testReleaseV321        = CephCSIVersion{3, 2, 1}
 	testReleaseV330        = CephCSIVersion{3, 3, 0}
 	testReleaseV340        = CephCSIVersion{3, 4, 0}
 	testVersionUnsupported = CephCSIVersion{4, 0, 0}
@@ -39,8 +43,44 @@ func TestIsAtLeast(t *testing.T) {
 	ret = testMinVersion.isAtLeast(&testMinVersion)
 	assert.Equal(t, true, ret)
 
+	// Test version which is greater (minor)
+	version = CephCSIVersion{2, 1, 0}
+	ret = testMinVersion.isAtLeast(&version)
+	assert.Equal(t, false, ret)
+
+	// Test version which is greater (bugfix)
+	version = CephCSIVersion{2, 2, 0}
+	ret = testMinVersion.isAtLeast(&version)
+	assert.Equal(t, false, ret)
+
+	// Test for v2.1.0
+	// Test version which is greater (bugfix)
+	version = CephCSIVersion{2, 0, 1}
+	ret = testReleaseV210.isAtLeast(&version)
+	assert.Equal(t, true, ret)
+
 	// Test version which is equal
-	ret = testReleaseV330.isAtLeast(&testReleaseV330)
+	ret = testReleaseV210.isAtLeast(&testReleaseV210)
+	assert.Equal(t, true, ret)
+
+	// Test version which is greater (minor)
+	version = CephCSIVersion{2, 1, 1}
+	ret = testReleaseV210.isAtLeast(&version)
+	assert.Equal(t, false, ret)
+
+	// Test version which is greater (bugfix)
+	version = CephCSIVersion{2, 2, 0}
+	ret = testReleaseV210.isAtLeast(&version)
+	assert.Equal(t, false, ret)
+
+	// Test for 3.0.0
+	// Test version which is equal
+	ret = testReleaseV300.isAtLeast(&testReleaseV300)
+	assert.Equal(t, true, ret)
+
+	// Test for 3.3.0
+	// Test version which is lesser
+	ret = testReleaseV330.isAtLeast(&testReleaseV300)
 	assert.Equal(t, true, ret)
 
 	// Test for 3.4.0
@@ -48,23 +88,50 @@ func TestIsAtLeast(t *testing.T) {
 	ret = testReleaseV340.isAtLeast(&testReleaseV330)
 	assert.Equal(t, true, ret)
 
+	// Test version which is greater (minor)
+	version = CephCSIVersion{3, 1, 1}
+	ret = testReleaseV300.isAtLeast(&version)
+	assert.Equal(t, false, ret)
+
+	// Test version which is greater (bugfix)
+	version = CephCSIVersion{3, 2, 0}
+	ret = testReleaseV300.isAtLeast(&version)
+	assert.Equal(t, false, ret)
 }
 
 func TestSupported(t *testing.T) {
 	AllowUnsupported = false
 	ret := testMinVersion.Supported()
-	assert.Equal(t, true, ret)
+	assert.Equal(t, false, ret)
 
 	ret = testVersionUnsupported.Supported()
 	assert.Equal(t, false, ret)
 
-	ret = testReleaseV330.Supported()
-	assert.Equal(t, true, ret)
-
 	ret = testReleaseV340.Supported()
 	assert.Equal(t, true, ret)
 }
 
+func TestSupportOMAPController(t *testing.T) {
+	AllowUnsupported = true
+	ret := testMinVersion.SupportsOMAPController()
+	assert.True(t, ret)
+
+	AllowUnsupported = false
+	ret = testMinVersion.SupportsOMAPController()
+	assert.False(t, ret)
+
+	ret = testReleaseV300.SupportsOMAPController()
+	assert.False(t, ret)
+
+	ret = testReleaseV320.SupportsOMAPController()
+	assert.True(t, ret)
+
+	ret = testReleaseV321.SupportsOMAPController()
+	assert.True(t, ret)
+
+	ret = testReleaseV330.SupportsOMAPController()
+	assert.True(t, ret)
+}
 func Test_extractCephCSIVersion(t *testing.T) {
 	expectedVersion := CephCSIVersion{3, 0, 0}
 	csiString := []byte(`Cephcsi Version: v3.0.0
diff --git a/tests/integration/ceph_object_test.go b/tests/integration/ceph_object_test.go
index f59eb9853..c78773de8 100644
--- a/tests/integration/ceph_object_test.go
+++ b/tests/integration/ceph_object_test.go
@@ -123,7 +123,12 @@ func runObjectE2ETest(helper *clients.TestClient, k8sh *utils.K8sHelper, s suite
 	}
 
 	logger.Infof("Running on Rook Cluster %s", namespace)
-	createCephObjectStore(s, helper, k8sh, namespace, storeName, 3, tlsEnable)
+	createCephObjectStore(s.T(), helper, k8sh, namespace, storeName, 3, tlsEnable)
+
+	// test that a second object store can be created (and deleted) while the first exists
+	testSecondObjectStore(s.T(), helper, k8sh, namespace, storeName+"-second", tlsEnable)
+
+	// now test operation of the first object store
 	testObjectStoreOperations(s, helper, k8sh, namespace, storeName)
 }
 
@@ -188,53 +193,41 @@ func checkCephObjectUser(
 	}
 }
 
-func createCephObjectStore(s suite.Suite, helper *clients.TestClient, k8sh *utils.K8sHelper, namespace, storeName string, replicaSize int, tlsEnable bool) {
+// create a CephObjectStore and wait for it to report ready status
+func createCephObjectStore(t *testing.T, helper *clients.TestClient, k8sh *utils.K8sHelper, namespace, storeName string, replicaSize int, tlsEnable bool) {
 	logger.Infof("Create Object Store %q with replica count %d", storeName, replicaSize)
 	rgwServiceName := "rook-ceph-rgw-" + storeName
 	if tlsEnable {
-		generateRgwTlsCertSecret(s, helper, k8sh, namespace, storeName, rgwServiceName)
+		t.Run("generate TLS certs", func(t *testing.T) {
+			generateRgwTlsCertSecret(t, helper, k8sh, namespace, storeName, rgwServiceName)
+		})
 	}
-	t := s.T()
 	t.Run("create CephObjectStore", func(t *testing.T) {
 		err := helper.ObjectClient.Create(namespace, storeName, 3, tlsEnable)
-		assert.Nil(s.T(), err)
+		assert.Nil(t, err)
+	})
 
+	t.Run("wait for RGWs to be running", func(t *testing.T) {
 		// check that ObjectStore is created
 		logger.Infof("Check that RGW pods are Running")
 		for i := 0; i < 24 && k8sh.CheckPodCountAndState("rook-ceph-rgw", namespace, 1, "Running") == false; i++ {
 			logger.Infof("(%d) RGW pod check sleeping for 5 seconds ...", i)
 			time.Sleep(5 * time.Second)
 		}
-		assert.True(s.T(), k8sh.CheckPodCountAndState("rook-ceph-rgw", namespace, 1, "Running"))
+		assert.True(t, k8sh.CheckPodCountAndState("rook-ceph-rgw", namespace, 1, "Running"))
 		logger.Info("RGW pods are running")
 		logger.Infof("Object store %q created successfully", storeName)
 	})
-}
 
-func testObjectStoreOperations(s suite.Suite, helper *clients.TestClient, k8sh *utils.K8sHelper, namespace, storeName string) {
 	ctx := context.TODO()
-	clusterInfo := client.AdminClusterInfo(namespace)
-	t := s.T()
-	t.Run(fmt.Sprintf("create CephObjectStoreUser %q", storeName), func(t *testing.T) {
-		createCephObjectUser(s, helper, k8sh, namespace, storeName, userid, true, true)
-		i := 0
-		for i = 0; i < 4; i++ {
-			if helper.ObjectUserClient.UserSecretExists(namespace, storeName, userid) {
-				break
-			}
-			logger.Info("waiting 5 more seconds for user secret to exist")
-			time.Sleep(5 * time.Second)
-		}
-		assert.NotEqual(t, 4, i)
-	})
 
 	// Check object store status
-	t.Run(fmt.Sprintf("verify ceph object store %q status", storeName), func(t *testing.T) {
+	t.Run("verify object store status", func(t *testing.T) {
 		retryCount := 30
 		i := 0
 		for i = 0; i < retryCount; i++ {
 			objectStore, err := k8sh.RookClientset.CephV1().CephObjectStores(namespace).Get(ctx, storeName, metav1.GetOptions{})
-			assert.Nil(s.T(), err)
+			assert.Nil(t, err)
 			if objectStore.Status == nil || objectStore.Status.BucketStatus == nil {
 				logger.Infof("(%d) object status check sleeping for 5 seconds ...%+v", i, objectStore.Status)
 				time.Sleep(5 * time.Second)
@@ -246,16 +239,47 @@ func testObjectStoreOperations(s suite.Suite, helper *clients.TestClient, k8sh *
 				time.Sleep(5 * time.Second)
 				continue
 			}
-			assert.Equal(s.T(), cephv1.ConditionConnected, objectStore.Status.BucketStatus.Health)
+			assert.Equal(t, cephv1.ConditionConnected, objectStore.Status.BucketStatus.Health)
 			// Info field has the endpoint in it
-			assert.NotEmpty(s.T(), objectStore.Status.Info)
-			assert.NotEmpty(s.T(), objectStore.Status.Info["endpoint"])
+			assert.NotEmpty(t, objectStore.Status.Info)
+			assert.NotEmpty(t, objectStore.Status.Info["endpoint"])
 			break
 		}
 		if i == retryCount {
 			t.Fatal("bucket status check failed. status is not connected")
 		}
 	})
+}
+
+// test creating a second object store while an object store already exists, once the second store
+// is found to be ready, it will be deleted. "storeName" should be unique.
+func testSecondObjectStore(t *testing.T, helper *clients.TestClient, k8sh *utils.K8sHelper, namespace, storeName string, tlsEnable bool) {
+	t.Run("run a second object store", func(t *testing.T) {
+		createCephObjectStore(t, helper, k8sh, namespace, storeName, 1, tlsEnable)
+
+		t.Run("delete the second object store", func(t *testing.T) {
+			deleteObjectStore(t, k8sh, namespace, storeName)
+			assertObjectStoreDeletion(t, k8sh, namespace, storeName)
+		})
+	})
+}
+
+func testObjectStoreOperations(s suite.Suite, helper *clients.TestClient, k8sh *utils.K8sHelper, namespace, storeName string) {
+	ctx := context.TODO()
+	clusterInfo := client.AdminClusterInfo(namespace)
+	t := s.T()
+	t.Run(fmt.Sprintf("create CephObjectStoreUser %q", storeName), func(t *testing.T) {
+		createCephObjectUser(s, helper, k8sh, namespace, storeName, userid, true, true)
+		i := 0
+		for i = 0; i < 4; i++ {
+			if helper.ObjectUserClient.UserSecretExists(namespace, storeName, userid) {
+				break
+			}
+			logger.Info("waiting 5 more seconds for user secret to exist")
+			time.Sleep(5 * time.Second)
+		}
+		assert.NotEqual(t, 4, i)
+	})
 
 	context := k8sh.MakeContext()
 	objectStore, err := k8sh.RookClientset.CephV1().CephObjectStores(namespace).Get(ctx, storeName, metav1.GetOptions{})
@@ -360,11 +384,7 @@ func testObjectStoreOperations(s suite.Suite, helper *clients.TestClient, k8sh *
 	})
 
 	t.Run("delete CephObjectStore should be blocked by OBC bucket and CephObjectStoreUser", func(t *testing.T) {
-		err := k8sh.DeleteResourceAndWait(false, "-n", namespace, "CephObjectStore", storeName)
-		assert.NoError(t, err)
-		// wait initially for the controller to detect deletion. Almost always enough, but not
-		// waiting will almost always fail the first check in the loop
-		time.Sleep(2 * time.Second)
+		deleteObjectStore(t, k8sh, namespace, storeName)
 
 		store := &cephv1.CephObjectStore{}
 		i := 0
@@ -449,58 +469,76 @@ func testObjectStoreOperations(s suite.Suite, helper *clients.TestClient, k8sh *
 		// wait initially since it will almost never detect on the first try without this.
 		time.Sleep(3 * time.Second)
 
-		store := &cephv1.CephObjectStore{}
-		i := 0
-		retry := 10
-		for i = 0; i < retry; i++ {
-			storeStr, err := k8sh.GetResource("-n", namespace, "CephObjectStore", storeName, "-o", "json")
-			assert.NoError(t, err)
-			logger.Infof("store: \n%s", storeStr)
+		assertObjectStoreDeletion(t, k8sh, namespace, storeName)
+	})
 
-			err = json.Unmarshal([]byte(storeStr), &store)
-			assert.NoError(t, err)
+	// TODO : Add case for brownfield/cleanup s3 client}
+}
 
-			cond := cephv1.FindStatusCondition(store.Status.Conditions, cephv1.ConditionDeletionIsBlocked)
-			if cond.Status == v1.ConditionFalse && cond.Reason == cephv1.ObjectHasNoDependentsReason {
-				// Let's give some time to the object to be updated
-				time.Sleep(5 * time.Second)
-				break
-			}
-			logger.Info("waiting 3 more seconds for CephObjectStore to be unblocked by dependents")
-			time.Sleep(3 * time.Second)
-		}
-		assert.NotEqual(t, retry, i)
+func deleteObjectStore(t *testing.T, k8sh *utils.K8sHelper, namespace, storeName string) {
+	err := k8sh.DeleteResourceAndWait(false, "-n", namespace, "CephObjectStore", storeName)
+	assert.NoError(t, err)
+	// wait initially for the controller to detect deletion. Almost always enough, but not
+	// waiting immediately after this will almost always fail the first check in the loop
+	time.Sleep(4 * time.Second)
+}
 
-		assert.Equal(t, cephv1.ConditionDeleting, store.Status.Phase) // phase == "Deleting"
-		// verify deletion is NOT blocked b/c object has dependents
-		cond := cephv1.FindStatusCondition(store.Status.Conditions, cephv1.ConditionDeletionIsBlocked)
-		assert.Equal(t, v1.ConditionFalse, cond.Status)
-		assert.Equal(t, cephv1.ObjectHasNoDependentsReason, cond.Reason)
+func assertObjectStoreDeletion(t *testing.T, k8sh *utils.K8sHelper, namespace, storeName string) {
+	store := &cephv1.CephObjectStore{}
+	i := 0
+	retry := 10
+	sleepTime := 3 * time.Second
+	for i = 0; i < retry; i++ {
+		storeStr, err := k8sh.GetResource("-n", namespace, "CephObjectStore", storeName, "-o", "json")
+		assert.NoError(t, err)
+		logger.Infof("store: \n%s", storeStr)
 
-		err := k8sh.WaitUntilResourceIsDeleted("CephObjectStore", namespace, storeName)
+		err = json.Unmarshal([]byte(storeStr), &store)
 		assert.NoError(t, err)
-	})
 
-	// TODO : Add case for brownfield/cleanup s3 client}
+		cond := cephv1.FindStatusCondition(store.Status.Conditions, cephv1.ConditionDeletionIsBlocked)
+		if cond == nil {
+			logger.Infof("waiting for CephObjectStore %q to have a deletion condition", storeName)
+			time.Sleep(sleepTime)
+			continue
+		}
+		if cond.Status == v1.ConditionFalse && cond.Reason == cephv1.ObjectHasNoDependentsReason {
+			// no longer blocked by dependents
+			time.Sleep(5 * time.Second) // Let's give some time to the object to be updated
+			break
+		}
+		logger.Infof("waiting 3 more seconds for CephObjectStore %q to be unblocked by dependents", storeName)
+		time.Sleep(sleepTime)
+	}
+	assert.NotEqual(t, retry, i)
+
+	assert.Equal(t, cephv1.ConditionDeleting, store.Status.Phase) // phase == "Deleting"
+	// verify deletion is NOT blocked b/c object has dependents
+	cond := cephv1.FindStatusCondition(store.Status.Conditions, cephv1.ConditionDeletionIsBlocked)
+	assert.Equal(t, v1.ConditionFalse, cond.Status)
+	assert.Equal(t, cephv1.ObjectHasNoDependentsReason, cond.Reason)
+
+	err := k8sh.WaitUntilResourceIsDeleted("CephObjectStore", namespace, storeName)
+	assert.NoError(t, err)
 }
 
-func generateRgwTlsCertSecret(s suite.Suite, helper *clients.TestClient, k8sh *utils.K8sHelper, namespace, storeName, rgwServiceName string) {
+func generateRgwTlsCertSecret(t *testing.T, helper *clients.TestClient, k8sh *utils.K8sHelper, namespace, storeName, rgwServiceName string) {
 	ctx := context.TODO()
 	root, err := utils.FindRookRoot()
-	require.NoError(s.T(), err, "failed to get rook root")
+	require.NoError(t, err, "failed to get rook root")
 	tlscertdir, err := ioutil.TempDir(root, "tlscertdir")
-	require.NoError(s.T(), err, "failed to create directory for TLS certs")
+	require.NoError(t, err, "failed to create directory for TLS certs")
 	defer os.RemoveAll(tlscertdir)
 	cmdArgs := utils.CommandArgs{Command: filepath.Join(root, "tests/scripts/generate-tls-config.sh"),
 		CmdArgs: []string{tlscertdir, rgwServiceName, namespace}}
 	cmdOut := utils.ExecuteCommand(cmdArgs)
-	require.NoError(s.T(), cmdOut.Err)
+	require.NoError(t, cmdOut.Err)
 	tlsKeyIn, err := ioutil.ReadFile(filepath.Join(tlscertdir, rgwServiceName+".key"))
-	require.NoError(s.T(), err)
+	require.NoError(t, err)
 	tlsCertIn, err := ioutil.ReadFile(filepath.Join(tlscertdir, rgwServiceName+".crt"))
-	require.NoError(s.T(), err)
+	require.NoError(t, err)
 	tlsCaCertIn, err := ioutil.ReadFile(filepath.Join(tlscertdir, rgwServiceName+".ca"))
-	require.NoError(s.T(), err)
+	require.NoError(t, err)
 	secretCertOut := fmt.Sprintf("%s%s%s", tlsKeyIn, tlsCertIn, tlsCaCertIn)
 	tlsK8sSecret := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -512,5 +550,5 @@ func generateRgwTlsCertSecret(s suite.Suite, helper *clients.TestClient, k8sh *u
 		},
 	}
 	_, err = k8sh.Clientset.CoreV1().Secrets(namespace).Create(ctx, tlsK8sSecret, metav1.CreateOptions{})
-	require.Nil(s.T(), err)
+	require.Nil(t, err)
 }
diff --git a/tests/scripts/github-action-helper.sh b/tests/scripts/github-action-helper.sh
index 9e94d2059..83a2e5a60 100755
--- a/tests/scripts/github-action-helper.sh
+++ b/tests/scripts/github-action-helper.sh
@@ -253,12 +253,7 @@ function write_object_to_cluster1_read_from_cluster2() {
   s3cmd -v -d --config=s3cfg --access_key=${ACCESS_KEY} --secret_key=${SECRET_KEY} --host=${CLUSTER_1_IP_ADDR} mb s3://bkt
   s3cmd -v -d --config=s3cfg --access_key=${ACCESS_KEY} --secret_key=${SECRET_KEY} --host=${CLUSTER_1_IP_ADDR} put ./1M.dat s3://bkt
   CLUSTER_2_IP_ADDR=$(kubectl -n rook-ceph-secondary get svc rook-ceph-rgw-zone-b-multisite-store -o jsonpath="{.spec.clusterIP}")
-  timeout 60 bash <<EOF
-until s3cmd -v -d --config=s3cfg --access_key=${ACCESS_KEY} --secret_key=${SECRET_KEY} --host=${CLUSTER_2_IP_ADDR} get s3://bkt/1M.dat 1M-get.dat --force; do
-  echo "waiting for object to be replicated"
-  sleep 5
-done
-EOF
+  s3cmd -v -d --config=s3cfg --access_key=${ACCESS_KEY} --secret_key=${SECRET_KEY} --host=${CLUSTER_2_IP_ADDR} get s3://bkt/1M.dat 1M-get.dat --force
   diff 1M.dat 1M-get.dat
 }