From 1a95d6f5338e7284d1ab526fcc35fda1ab3ca137 Mon Sep 17 00:00:00 2001 From: Santosh Pillai Date: Mon, 23 Sep 2024 10:53:19 +0530 Subject: [PATCH] core: preserve pool application name change default application name is updated inside the `CreatePool` method. Send pool spec as address in order to preserve this change. Signed-off-by: Santosh Pillai --- pkg/daemon/ceph/client/pool.go | 8 ++++---- pkg/operator/ceph/file/filesystem.go | 14 ++++++++------ pkg/operator/ceph/object/objectstore.go | 2 +- pkg/operator/ceph/pool/controller.go | 2 +- pkg/operator/ceph/pool/controller_test.go | 23 ++++++++++++++++++++--- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/pkg/daemon/ceph/client/pool.go b/pkg/daemon/ceph/client/pool.go index 82eae5e77..536f5ff90 100644 --- 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/file/filesystem.go b/pkg/operator/ceph/file/filesystem.go index 8a35ab1a0..b8e6b36cf 100644 --- 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 index 794419ac8..b34642938 100644 --- 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 index 24d080ebf..14f57c341 100644 --- 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 index 29a277370..74afcc2d7 100644 --- 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,6 +106,8 @@ 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) @@ -102,6 +115,8 @@ func TestCreatePool(t *testing.T) { 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) })