From c3accdcc3aa7b7900d3fbddcf9cd7e56d7c1206c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 23 Nov 2021 11:07:43 +0100 Subject: [PATCH 1/2] nfs: only set the pool size when it exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For CRD not using the new nfs spec that includes the pool settings, applying the "size" property won't work since it is set to 0. The pool still gets created but returns an error. The loop is re-queued but on the second run the pool is detected so no further configuration is done. Closes: https://github.com/rook/rook/issues/9205 Signed-off-by: Sébastien Han --- pkg/daemon/ceph/client/pool.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/daemon/ceph/client/pool.go b/pkg/daemon/ceph/client/pool.go index 5813c3df5c92..421f2e2a5a1c 100644 --- a/pkg/daemon/ceph/client/pool.go +++ b/pkg/daemon/ceph/client/pool.go @@ -408,8 +408,11 @@ func CreateReplicatedPoolForApp(context *clusterd.Context, clusterInfo *ClusterI if !clusterSpec.IsStretchCluster() { // the pool is type replicated, set the size for the pool now that it's been created - if err := SetPoolReplicatedSizeProperty(context, clusterInfo, poolName, strconv.FormatUint(uint64(pool.Replicated.Size), 10)); err != nil { - return errors.Wrapf(err, "failed to set size property to replicated pool %q to %d", poolName, pool.Replicated.Size) + // Only set the size if not 0, otherwise ceph will fail to set size to 0 + if pool.Replicated.Size > 0 { + if err := SetPoolReplicatedSizeProperty(context, clusterInfo, poolName, strconv.FormatUint(uint64(pool.Replicated.Size), 10)); err != nil { + return errors.Wrapf(err, "failed to set size property to replicated pool %q to %d", poolName, pool.Replicated.Size) + } } } From f3142847d391e075b59ab3a100227e74f57f2bcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 23 Nov 2021 11:25:11 +0100 Subject: [PATCH 2/2] nfs: always run default pool creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, if the pool was present we would not run the pool creation again. This is a problem if the pool spec changes, the new settings will never be applied. Signed-off-by: Sébastien Han --- pkg/operator/ceph/nfs/controller.go | 7 +++-- pkg/operator/ceph/nfs/controller_test.go | 33 +++++++----------------- pkg/operator/ceph/nfs/nfs.go | 17 ------------ 3 files changed, 15 insertions(+), 42 deletions(-) diff --git a/pkg/operator/ceph/nfs/controller.go b/pkg/operator/ceph/nfs/controller.go index 0107e2ef1e47..66118feb2489 100644 --- a/pkg/operator/ceph/nfs/controller.go +++ b/pkg/operator/ceph/nfs/controller.go @@ -278,8 +278,11 @@ func (r *ReconcileCephNFS) reconcile(request reconcile.Request) (reconcile.Resul if err := validateGanesha(r.context, r.clusterInfo, cephNFS); err != nil { return reconcile.Result{}, errors.Wrapf(err, "invalid ceph nfs %q arguments", cephNFS.Name) } - if err := r.fetchOrCreatePool(cephNFS); err != nil { - return reconcile.Result{}, errors.Wrap(err, "failed to fetch or create RADOS pool") + + // Always create the default pool + err = r.createDefaultNFSRADOSPool(cephNFS) + if err != nil { + return reconcile.Result{}, errors.Wrapf(err, "failed to create default pool %q", cephNFS.Spec.RADOS.Pool) } // CREATE/UPDATE diff --git a/pkg/operator/ceph/nfs/controller_test.go b/pkg/operator/ceph/nfs/controller_test.go index 33c815157236..140fecb7c636 100644 --- a/pkg/operator/ceph/nfs/controller_test.go +++ b/pkg/operator/ceph/nfs/controller_test.go @@ -19,11 +19,11 @@ package nfs import ( "context" - "errors" "os" "testing" "github.com/coreos/pkg/capnslog" + "github.com/pkg/errors" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" rookclient "github.com/rook/rook/pkg/client/clientset/versioned/fake" "github.com/rook/rook/pkg/client/clientset/versioned/scheme" @@ -46,25 +46,6 @@ var ( name = "my-nfs" namespace = "rook-ceph" nfsCephAuthGetOrCreateKey = `{"key":"AQCvzWBeIV9lFRAAninzm+8XFxbSfTiPwoX50g=="}` - poolDetails = `{ - "pool": "foo", - "pool_id": 1, - "size": 3, - "min_size": 2, - "pg_num": 8, - "pgp_num": 8, - "crush_rule": "replicated_rule", - "hashpspool": true, - "nodelete": false, - "nopgchange": false, - "nosizechange": false, - "write_fadvise_dontneed": false, - "noscrub": false, - "nodeep-scrub": false, - "use_gmt_hitset": true, - "fast_read": 0, - "pg_autoscale_mode": "on" - }` ) func TestCephNFSController(t *testing.T) { @@ -202,10 +183,16 @@ func TestCephNFSController(t *testing.T) { if args[0] == "auth" && args[1] == "get-or-create-key" { return nfsCephAuthGetOrCreateKey, nil } - if args[0] == "osd" && args[1] == "pool" && args[2] == "get" { - return poolDetails, nil + if args[0] == "osd" && args[1] == "pool" && args[2] == "create" { + return "", nil } - return "", errors.New("unknown command") + if args[0] == "osd" && args[1] == "crush" && args[2] == "rule" { + return "", nil + } + if args[0] == "osd" && args[1] == "pool" && args[2] == "application" { + return "", nil + } + return "", errors.Errorf("unknown command %q %v", command, args) }, MockExecuteCommand: func(command string, args ...string) error { if command == "rados" { diff --git a/pkg/operator/ceph/nfs/nfs.go b/pkg/operator/ceph/nfs/nfs.go index 937895bd1f25..88fb364d88ea 100644 --- a/pkg/operator/ceph/nfs/nfs.go +++ b/pkg/operator/ceph/nfs/nfs.go @@ -19,7 +19,6 @@ package nfs import ( "fmt" - "strings" "github.com/banzaicloud/k8s-objectmatcher/patch" "github.com/pkg/errors" @@ -296,19 +295,3 @@ func (r *ReconcileCephNFS) createDefaultNFSRADOSPool(n *cephv1.CephNFS) error { return nil } - -func (r *ReconcileCephNFS) fetchOrCreatePool(n *cephv1.CephNFS) error { - // The existence of the pool provided in n.Spec.RADOS.Pool is necessary otherwise addRADOSConfigFile() will fail - _, err := cephclient.GetPoolDetails(r.context, r.clusterInfo, n.Spec.RADOS.Pool) - if err != nil { - if strings.Contains(err.Error(), "unrecognized pool") && r.clusterInfo.CephVersion.IsAtLeastPacific() { - err := r.createDefaultNFSRADOSPool(n) - if err != nil { - return errors.Wrapf(err, "failed to find %q pool and unable to create it", n.Spec.RADOS.Pool) - } - return nil - } - return errors.Wrapf(err, "pool %q not found", n.Spec.RADOS.Pool) - } - return nil -}