Merge pull request #738 from red-hat-storage/sync_us--master

Syncing latest changes from upstream master for rook
red-hat-storage · Oct 2, 2024 · 1ae7d79 · 1ae7d79
2 parents 5f06ecc + fd3927b
commit 1ae7d79
Show file tree

Hide file tree

Showing 14 changed files with 62 additions and 31 deletions.
diff --git a/.github/workflows/daily-nightly-jobs.yml b/.github/workflows/daily-nightly-jobs.yml
@@ -16,8 +16,6 @@ jobs:
   canary-arm64:
     runs-on: [self-hosted, ubuntu-20.04-arm64, ARM64]
     if: github.repository == 'rook/rook'
-    env:
-      BLOCK: /dev/sdb
 
     steps:
       - name: checkout
@@ -91,6 +89,12 @@ jobs:
       - name: wait for ceph to be ready
         run: tests/scripts/github-action-helper.sh wait_for_ceph_to_be_ready "mon,mgr,osd,mds,rgw,rbd_mirror,fs_mirror" 1
 
+      - name: collect common logs
+        if: always()
+        uses: ./.github/workflows/collect-logs
+        with:
+          name: canary-arm64
+
       - name: teardown minikube, docker and kubectl
         if: always()
         run: |
@@ -100,24 +104,12 @@ jobs:
           sudo service docker stop
           sudo rm -rf  /usr/local/bin/minikube
           sudo rm -rf /usr/local/bin/kubectl
+          sudo modprobe -r nbd
 
       - name: remove /usr/bin/yq
         if: always()
         run: sudo rm -rf /usr/bin/yq
 
-      - name: collect common logs
-        if: always()
-        uses: ./.github/workflows/collect-logs
-        with:
-          name: canary-arm64
-
-      - name: upload canary test result
-        uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
-        if: always()
-        with:
-          name: canary-arm64
-          path: test
-
   smoke-suite-quincy-devel:
     if: github.repository == 'rook/rook'
     runs-on: ubuntu-22.04

diff --git a/pkg/daemon/ceph/client/pool.go b/pkg/daemon/ceph/client/pool.go
@@ -178,11 +178,11 @@ func ParsePoolDetails(in []byte) (CephStoragePoolDetails, error) {
 	return poolDetails, nil
 }
 
-func CreatePool(context *clusterd.Context, clusterInfo *ClusterInfo, clusterSpec *cephv1.ClusterSpec, pool cephv1.NamedPoolSpec) error {
+func CreatePool(context *clusterd.Context, clusterInfo *ClusterInfo, clusterSpec *cephv1.ClusterSpec, pool *cephv1.NamedPoolSpec) error {
 	return CreatePoolWithPGs(context, clusterInfo, clusterSpec, pool, DefaultPGCount)
 }
 
-func CreatePoolWithPGs(context *clusterd.Context, clusterInfo *ClusterInfo, clusterSpec *cephv1.ClusterSpec, pool cephv1.NamedPoolSpec, pgCount string) error {
+func CreatePoolWithPGs(context *clusterd.Context, clusterInfo *ClusterInfo, clusterSpec *cephv1.ClusterSpec, pool *cephv1.NamedPoolSpec, pgCount string) error {
 	if pool.Name == "" {
 		return errors.New("pool name must be specified")
 	}
@@ -196,7 +196,7 @@ func CreatePoolWithPGs(context *clusterd.Context, clusterInfo *ClusterInfo, clus
 	}
 
 	if pool.IsReplicated() {
-		return createReplicatedPoolForApp(context, clusterInfo, clusterSpec, pool, pgCount)
+		return createReplicatedPoolForApp(context, clusterInfo, clusterSpec, *pool, pgCount)
 	}
 
 	if !pool.IsErasureCoded() {
@@ -215,7 +215,7 @@ func CreatePoolWithPGs(context *clusterd.Context, clusterInfo *ClusterInfo, clus
 		context,
 		clusterInfo,
 		ecProfileName,
-		pool,
+		*pool,
 		pgCount,
 		true /* enableECOverwrite */)
 }

diff --git a/pkg/operator/ceph/csi/template/cephfs/csi-cephfsplugin-holder.yaml b/pkg/operator/ceph/csi/template/cephfs/csi-cephfsplugin-holder.yaml
@@ -38,6 +38,8 @@ spec:
           # This is necessary for the Bidirectional mount propagation
           securityContext:
             privileged: true
+            capabilities:
+              drop: ["ALL"]
           image: {{ .CSIPluginImage }}
           command:
             - "/bin/sh"

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
@@ -254,6 +254,8 @@ spec:
           {{ if and .Privileged .CSILogRotation }}
           securityContext:
             privileged: true
+            capabilities:
+              drop: ["ALL"]
           {{ end }}
           volumeMounts:
             - name: socket-dir

diff --git a/pkg/operator/ceph/csi/template/csi-logrotate-sidecar.yaml b/pkg/operator/ceph/csi/template/csi-logrotate-sidecar.yaml
@@ -28,6 +28,12 @@ command:
 image: {{ .CSIPluginImage }}
 imagePullPolicy: IfNotPresent
 name: log-collector
+{{ if .Privileged }}
+securityContext:
+  privileged: true
+  capabilities:
+    drop: ["ALL"]
+{{ end }}
 volumeMounts:
   - mountPath: {{ .CsiLogRootPath }}/logrotate-config/{{ .CsiComponentName }}
     name: csi-logs-logrotate

diff --git a/pkg/operator/ceph/csi/template/nfs/csi-nfsplugin-holder.yaml b/pkg/operator/ceph/csi/template/nfs/csi-nfsplugin-holder.yaml
@@ -38,6 +38,8 @@ spec:
           # This is necessary for the Bidirectional mount propagation
           securityContext:
             privileged: true
+            capabilities:
+              drop: ["ALL"]
           image: {{ .CSIPluginImage }}
           command:
             - "/bin/sh"

diff --git a/pkg/operator/ceph/csi/template/nfs/csi-nfsplugin-provisioner-dep.yaml b/pkg/operator/ceph/csi/template/nfs/csi-nfsplugin-provisioner-dep.yaml
@@ -161,6 +161,8 @@ spec:
           {{ if and .Privileged .CSILogRotation }}
           securityContext:
             privileged: true
+            capabilities:
+              drop: ["ALL"]
           {{ end }}
           volumeMounts:
             - name: socket-dir

diff --git a/pkg/operator/ceph/csi/template/rbd/csi-rbdplugin-holder.yaml b/pkg/operator/ceph/csi/template/rbd/csi-rbdplugin-holder.yaml
@@ -38,6 +38,8 @@ spec:
           # This is necessary for the Bidirectional mount propagation
           securityContext:
             privileged: true
+            capabilities:
+              drop: ["ALL"]
           image: {{ .CSIPluginImage }}
           command:
             - "/bin/sh"

diff --git a/pkg/operator/ceph/csi/template/rbd/csi-rbdplugin-provisioner-dep.yaml b/pkg/operator/ceph/csi/template/rbd/csi-rbdplugin-provisioner-dep.yaml
@@ -207,6 +207,8 @@ spec:
           {{ if and .Privileged .CSILogRotation }}
           securityContext:
             privileged: true
+            capabilities:
+              drop: ["ALL"]
           {{ end }}
           volumeMounts:
             - name: socket-dir
@@ -263,6 +265,8 @@ spec:
           {{ if and .Privileged .CSILogRotation }}
           securityContext:
             privileged: true
+            capabilities:
+              drop: ["ALL"]
           {{ end }}
           volumeMounts:
             - name: socket-dir

diff --git a/pkg/operator/ceph/file/filesystem.go b/pkg/operator/ceph/file/filesystem.go
@@ -196,16 +196,17 @@ func createOrUpdatePools(f *Filesystem, context *clusterd.Context, clusterInfo *
 		PoolSpec: spec.MetadataPool,
 	}
 	metadataPool.Application = cephfsApplication
-	err := cephclient.CreatePool(context, clusterInfo, clusterSpec, metadataPool)
+	err := cephclient.CreatePool(context, clusterInfo, clusterSpec, &metadataPool)
 	if err != nil {
 		return errors.Wrapf(err, "failed to update metadata pool %q", metadataPool.Name)
 	}
 	// generating the data pool's name
 	dataPoolNames := generateDataPoolNames(f, spec)
-	for i, dataPool := range spec.DataPools {
+	for i := range spec.DataPools {
+		dataPool := spec.DataPools[i]
 		dataPool.Name = dataPoolNames[i]
 		dataPool.Application = cephfsApplication
-		err := cephclient.CreatePool(context, clusterInfo, clusterSpec, dataPool)
+		err := cephclient.CreatePool(context, clusterInfo, clusterSpec, &dataPool)
 		if err != nil {
 			return errors.Wrapf(err, "failed to update datapool  %q", dataPool.Name)
 		}
@@ -269,18 +270,19 @@ func (f *Filesystem) doFilesystemCreate(context *clusterd.Context, clusterInfo *
 		PoolSpec: spec.MetadataPool,
 	}
 	if _, poolFound := reversedPoolMap[metadataPool.Name]; !poolFound {
-		err = cephclient.CreatePool(context, clusterInfo, clusterSpec, metadataPool)
+		err = cephclient.CreatePool(context, clusterInfo, clusterSpec, &metadataPool)
 		if err != nil {
 			return errors.Wrapf(err, "failed to create metadata pool %q", metadataPool.Name)
 		}
 	}
 
 	dataPoolNames := generateDataPoolNames(f, spec)
-	for i, dataPool := range spec.DataPools {
+	for i := range spec.DataPools {
+		dataPool := spec.DataPools[i]
 		dataPool.Name = dataPoolNames[i]
 		dataPool.Application = cephfsApplication
 		if _, poolFound := reversedPoolMap[dataPool.Name]; !poolFound {
-			err = cephclient.CreatePool(context, clusterInfo, clusterSpec, dataPool)
+			err = cephclient.CreatePool(context, clusterInfo, clusterSpec, &dataPool)
 			if err != nil {
 				return errors.Wrapf(err, "failed to create data pool %q", dataPool.Name)
 			}

diff --git a/pkg/operator/ceph/object/objectstore.go b/pkg/operator/ceph/object/objectstore.go
@@ -1116,7 +1116,7 @@ func createRGWPool(ctx *Context, cluster *cephv1.ClusterSpec, poolSpec cephv1.Po
 		Name:     poolName(ctx.Name, requestedName),
 		PoolSpec: poolSpec,
 	}
-	if err := cephclient.CreatePoolWithPGs(ctx.Context, ctx.clusterInfo, cluster, pool, pgCount); err != nil {
+	if err := cephclient.CreatePoolWithPGs(ctx.Context, ctx.clusterInfo, cluster, &pool, pgCount); err != nil {
 		return errors.Wrapf(err, "failed to create pool %q", pool.Name)
 	}
 	// Set the pg_num_min if not the default so the autoscaler won't immediately increase the pg count

diff --git a/pkg/operator/ceph/pool/controller.go b/pkg/operator/ceph/pool/controller.go
@@ -381,7 +381,7 @@ func createPool(context *clusterd.Context, clusterInfo *cephclient.ClusterInfo,
 	}
 	// create the pool
 	logger.Infof("creating pool %q in namespace %q", p.Name, clusterInfo.Namespace)
-	if err := cephclient.CreatePool(context, clusterInfo, clusterSpec, *p); err != nil {
+	if err := cephclient.CreatePool(context, clusterInfo, clusterSpec, p); err != nil {
 		return errors.Wrapf(err, "failed to configure pool %q", p.Name)
 	}
 

diff --git a/pkg/operator/ceph/pool/controller_test.go b/pkg/operator/ceph/pool/controller_test.go
@@ -52,6 +52,18 @@ func TestCreatePool(t *testing.T) {
 	enabledMgrApp := false
 	clusterInfo := cephclient.AdminTestClusterInfo("mycluster")
 	executor := &exectest.MockExecutor{
+		MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) {
+			logger.Infof("CommandTimeout: %s %v", command, args)
+			if command == "rbd" {
+				if args[0] == "pool" && args[1] == "init" {
+					// assert that `rbd pool init` is only run when application is set to `rbd`
+					assert.Equal(t, "rbd", p.Application)
+					assert.Equal(t, p.Name, args[2])
+					return "{}", nil
+				}
+			}
+			return "", nil
+		},
 		MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
 			logger.Infof("Command: %s %v", command, args)
 			if command == "ceph" {
@@ -69,6 +81,7 @@ func TestCreatePool(t *testing.T) {
 							assert.Equal(t, ".mgr", args[4])
 							assert.Equal(t, "mgr", args[5])
 						} else {
+							fmt.Printf("pool - %v", args)
 							assert.Fail(t, fmt.Sprintf("invalid pool %q", args[4]))
 						}
 					}
@@ -79,14 +92,12 @@ func TestCreatePool(t *testing.T) {
 					return "{}", nil
 				} else if args[0] == "mirror" && args[2] == "disable" {
 					return "", nil
-				} else {
-					assert.Equal(t, []string{"pool", "init", p.Name}, args[0:3])
 				}
-
 			}
 			return "", nil
 		},
 	}
+
 	context := &clusterd.Context{Executor: executor}
 
 	clusterSpec := &cephv1.ClusterSpec{Storage: cephv1.StorageScopeSpec{Config: map[string]string{cephclient.CrushRootConfigKey: "cluster-crush-root"}}}
@@ -95,13 +106,17 @@ func TestCreatePool(t *testing.T) {
 		p.Name = "replicapool"
 		p.Replicated.Size = 1
 		p.Replicated.RequireSafeReplicaSize = false
+		// reset the application name
+		p.Application = ""
 		err := createPool(context, clusterInfo, clusterSpec, p)
 		assert.Nil(t, err)
 		assert.False(t, enabledMetricsApp)
 	})
 
 	t.Run("built-in mgr pool", func(t *testing.T) {
 		p.Name = ".mgr"
+		// reset the application name
+		p.Application = ""
 		err := createPool(context, clusterInfo, clusterSpec, p)
 		assert.Nil(t, err)
 		assert.True(t, enabledMgrApp)
@@ -112,6 +127,8 @@ func TestCreatePool(t *testing.T) {
 		p.Replicated.Size = 0
 		p.ErasureCoded.CodingChunks = 1
 		p.ErasureCoded.DataChunks = 2
+		// reset the application name
+		p.Application = ""
 		err := createPool(context, clusterInfo, clusterSpec, p)
 		assert.Nil(t, err)
 	})

diff --git a/tests/scripts/github-action-helper.sh b/tests/scripts/github-action-helper.sh
@@ -30,7 +30,7 @@ function find_extra_block_dev() {
   boot_dev="$(sudo lsblk --noheading --list --output MOUNTPOINT,PKNAME | grep boot | awk '{print $2}')"
   echo "  == find_extra_block_dev(): boot_dev='$boot_dev'" >/dev/stderr # debug in case of future errors
   # --nodeps ignores partitions
-  extra_dev="$(sudo lsblk --noheading --list --nodeps --output KNAME | grep -v loop | grep -v "$boot_dev" | head -1)"
+  extra_dev="$(sudo lsblk --noheading --list --nodeps --output KNAME | egrep -v "($boot_dev|loop|nbd)" | head -1)"
   echo "  == find_extra_block_dev(): extra_dev='$extra_dev'" >/dev/stderr # debug in case of future errors
   echo "$extra_dev"                                                       # output of function
 }