From 5d2db6a9f19dc77de897a6b5185b59df3aeb76db Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Fri, 20 Mar 2020 11:18:18 -0600 Subject: [PATCH] ceph: allow creation of object store with pre-existing pools The pools may already be created by the mgr module. If the pool specs are not specified in the object CR, skip pool creation for the object store. The pools are required to exist and the reconcile will fail until they do exist. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/object/controller.go | 12 +++---- pkg/operator/ceph/object/objectstore.go | 34 +++++++++++++++++--- pkg/operator/ceph/object/objectstore_test.go | 25 ++++++++++---- pkg/operator/ceph/object/rgw.go | 22 ++++++++++--- pkg/operator/ceph/object/rgw_test.go | 13 ++++++++ 5 files changed, 84 insertions(+), 22 deletions(-) diff --git a/pkg/operator/ceph/object/controller.go b/pkg/operator/ceph/object/controller.go index ec9c417bb..d042eed1a 100644 --- a/pkg/operator/ceph/object/controller.go +++ b/pkg/operator/ceph/object/controller.go @@ -236,25 +236,26 @@ func (r *ReconcileCephObjectStore) reconcile(request reconcile.Request) (reconci return r.setFailedStatus(cephObjectStore, "failed to reconcile service", err) } - // RECONCILE POOLS - logger.Debug("reconciling object store pools") objContext := NewContext(r.context, cephObjectStore.Name, cephObjectStore.Namespace) + + // RECONCILE POOLS + logger.Info("reconciling object store pools") metadataPoolToModel := *cephObjectStore.Spec.MetadataPool.ToModel("") dataPoolToModel := *cephObjectStore.Spec.DataPool.ToModel("") - err = createPools(objContext, metadataPoolToModel, dataPoolToModel) + err = createPools(objContext, cephObjectStore.Spec, metadataPoolToModel, dataPoolToModel) if err != nil { return r.setFailedStatus(cephObjectStore, "failed to create object pools", err) } // RECONCILE REALM - logger.Debug("reconciling object store realms") + logger.Info("reconciling object store realms") err = reconcileRealm(objContext, serviceIP, cephObjectStore.Spec.Gateway.Port) if err != nil { return r.setFailedStatus(cephObjectStore, "failed to create object store realm", err) } // CREATE/UPDATE - logger.Debug("reconciling object store deployments") + logger.Info("reconciling object store deployments") reconcileResponse, err = r.reconcileCreateObjectStore(cephObjectStore) if err != nil { return r.setFailedStatus(cephObjectStore, "failed to create object store deployments", err) @@ -270,7 +271,6 @@ func (r *ReconcileCephObjectStore) reconcile(request reconcile.Request) (reconci // Return and do not requeue logger.Debug("done reconciling") return reconcile.Result{}, nil - } func (r *ReconcileCephObjectStore) reconcileCreateObjectStore(cephObjectStore *cephv1.CephObjectStore) (reconcile.Result, error) { diff --git a/pkg/operator/ceph/object/objectstore.go b/pkg/operator/ceph/object/objectstore.go index 6e033f753..129f97c8a 100644 --- a/pkg/operator/ceph/object/objectstore.go +++ b/pkg/operator/ceph/object/objectstore.go @@ -23,6 +23,7 @@ import ( "strings" "github.com/pkg/errors" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" ceph "github.com/rook/rook/pkg/daemon/ceph/client" "github.com/rook/rook/pkg/daemon/ceph/model" ) @@ -56,7 +57,7 @@ type realmType struct { Realms []string `json:"realms"` } -func deleteRealmAndPools(context *Context, preservePoolsOnDelete bool) error { +func deleteRealmAndPools(context *Context, spec cephv1.ObjectStoreSpec) error { stores, err := getObjectStores(context) if err != nil { return errors.Wrapf(err, "failed to detect object stores during deletion") @@ -73,8 +74,8 @@ func deleteRealmAndPools(context *Context, preservePoolsOnDelete bool) error { lastStore = true } - if !preservePoolsOnDelete { - err = deletePools(context, lastStore) + if !spec.PreservePoolsOnDelete { + err = deletePools(context, spec, lastStore) if err != nil { return errors.Wrapf(err, "failed to delete object store pools") } @@ -203,7 +204,12 @@ func getObjectStores(context *Context) ([]string, error) { return r.Realms, nil } -func deletePools(context *Context, lastStore bool) error { +func deletePools(context *Context, spec cephv1.ObjectStoreSpec, lastStore bool) error { + if emptyPool(spec.DataPool) && emptyPool(spec.MetadataPool) { + logger.Info("skipping removal of pools since not specified in the object store") + return nil + } + pools := append(metadataPools, dataPools...) if lastStore { pools = append(pools, rootPool) @@ -235,7 +241,25 @@ func deletePools(context *Context, lastStore bool) error { return nil } -func createPools(context *Context, metadataSpec, dataSpec model.Pool) error { +func createPools(context *Context, spec cephv1.ObjectStoreSpec, metadataSpec, dataSpec model.Pool) error { + if emptyPool(spec.DataPool) && emptyPool(spec.MetadataPool) { + logger.Info("no pools specified for the object store, checking for their existence...") + pools := append(metadataPools, dataPools...) + pools = append(pools, rootPool) + var missingPools []string + for _, pool := range pools { + poolName := poolName(context.Name, pool) + _, err := ceph.GetPoolDetails(context.Context, context.ClusterName, poolName) + if err != nil { + logger.Debugf("failed to find pool %q. %v", poolName, err) + missingPools = append(missingPools, poolName) + } + } + if len(missingPools) > 0 { + return fmt.Errorf("object store pools are missing: %v", missingPools) + } + } + if err := createSimilarPools(context, append(metadataPools, rootPool), metadataSpec); err != nil { return errors.Wrapf(err, "failed to create metadata pools") } diff --git a/pkg/operator/ceph/object/objectstore_test.go b/pkg/operator/ceph/object/objectstore_test.go index d5a11050f..9ca160eb1 100644 --- a/pkg/operator/ceph/object/objectstore_test.go +++ b/pkg/operator/ceph/object/objectstore_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/pkg/errors" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/clusterd" exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" @@ -152,18 +153,30 @@ func deleteStore(t *testing.T, name string, existingStores string, expectedDelet executor.MockExecuteCommandWithCombinedOutput = executorFunc context := &Context{Context: &clusterd.Context{Executor: executor}, Name: "myobj", ClusterName: "ns"} - // Delete an object store - err := deleteRealmAndPools(context, false) + // Delete an object store without deleting the pools + spec := cephv1.ObjectStoreSpec{} + err := deleteRealmAndPools(context, spec) assert.Nil(t, err) - expectedPoolsDeleted := 6 - if expectedDeleteRootPool { - expectedPoolsDeleted++ - } + expectedPoolsDeleted := 0 assert.Equal(t, expectedPoolsDeleted, poolsDeleted) assert.Equal(t, expectedPoolsDeleted, rulesDeleted) assert.True(t, realmDeleted) assert.True(t, zoneGroupDeleted) assert.True(t, zoneDeleted) + assert.Equal(t, false, deletedErasureCodeProfile) + + // Delete an object store with the pools + spec = cephv1.ObjectStoreSpec{ + MetadataPool: cephv1.PoolSpec{Replicated: cephv1.ReplicatedSpec{Size: 1}}, + DataPool: cephv1.PoolSpec{Replicated: cephv1.ReplicatedSpec{Size: 1}}, + } + err = deleteRealmAndPools(context, spec) + assert.Nil(t, err) + expectedPoolsDeleted = 6 + if expectedDeleteRootPool { + expectedPoolsDeleted++ + } + assert.Equal(t, expectedPoolsDeleted, poolsDeleted) assert.Equal(t, expectedDeleteRootPool, deletedRootPool) assert.Equal(t, true, deletedErasureCodeProfile) } diff --git a/pkg/operator/ceph/object/rgw.go b/pkg/operator/ceph/object/rgw.go index 1f3d9f99a..0e2b5fd1a 100644 --- a/pkg/operator/ceph/object/rgw.go +++ b/pkg/operator/ceph/object/rgw.go @@ -19,6 +19,7 @@ package object import ( "fmt" + "reflect" "strings" "github.com/banzaicloud/k8s-objectmatcher/patch" @@ -281,7 +282,7 @@ func (c *clusterConfig) deleteStore() error { // Delete the realm and pools objContext := NewContext(c.context, c.store.Name, c.store.Namespace) - err := deleteRealmAndPools(objContext, c.store.Spec.PreservePoolsOnDelete) + err := deleteRealmAndPools(objContext, c.store.Spec) if err != nil { return errors.Wrap(err, "failed to delete the realm and pools") } @@ -310,11 +311,18 @@ func validateStore(context *clusterd.Context, s *cephv1.CephObjectStore) error { if securePort < 0 || securePort > 65535 { return errors.Errorf("securePort value of %d must be between 0 and 65535", securePort) } - if err := pool.ValidatePoolSpec(context, s.Namespace, &s.Spec.MetadataPool); err != nil { - return errors.Wrapf(err, "invalid metadata pool spec") + + // Validate the pool settings, but allow for empty pools specs in case they have already been created + // such as by the ceph mgr + if !emptyPool(s.Spec.MetadataPool) { + if err := pool.ValidatePoolSpec(context, s.Namespace, &s.Spec.MetadataPool); err != nil { + return errors.Wrap(err, "invalid metadata pool spec") + } } - if err := pool.ValidatePoolSpec(context, s.Namespace, &s.Spec.DataPool); err != nil { - return errors.Wrapf(err, "invalid data pool spec") + if !emptyPool(s.Spec.DataPool) { + if err := pool.ValidatePoolSpec(context, s.Namespace, &s.Spec.DataPool); err != nil { + return errors.Wrap(err, "invalid data pool spec") + } } return nil @@ -323,3 +331,7 @@ func validateStore(context *clusterd.Context, s *cephv1.CephObjectStore) error { func (c *clusterConfig) generateSecretName(id string) string { return fmt.Sprintf("%s-%s-%s-keyring", AppName, c.store.Name, id) } + +func emptyPool(pool cephv1.PoolSpec) bool { + return reflect.DeepEqual(pool, cephv1.PoolSpec{}) +} diff --git a/pkg/operator/ceph/object/rgw_test.go b/pkg/operator/ceph/object/rgw_test.go index f51a8e215..5e9e52692 100644 --- a/pkg/operator/ceph/object/rgw_test.go +++ b/pkg/operator/ceph/object/rgw_test.go @@ -140,3 +140,16 @@ func TestGenerateSecretName(t *testing.T) { secret := c.generateSecretName("a") assert.Equal(t, "rook-ceph-rgw-default-a-keyring", secret) } + +func TestEmptyPoolSpec(t *testing.T) { + assert.True(t, emptyPool(cephv1.PoolSpec{})) + + p := cephv1.PoolSpec{FailureDomain: "foo"} + assert.False(t, emptyPool(p)) + + p = cephv1.PoolSpec{Replicated: cephv1.ReplicatedSpec{Size: 1}} + assert.False(t, emptyPool(p)) + + p = cephv1.PoolSpec{ErasureCoded: cephv1.ErasureCodedSpec{CodingChunks: 1}} + assert.False(t, emptyPool(p)) +}