From 760a8d3d2c40efc322cce1d72cbc6a7db1c5c110 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 (cherry picked from commit c3accdcc3aa7b7900d3fbddcf9cd7e56d7c1206c) --- 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 1d70afd2fc4891cbccf1f4c9fc7b2b3270b9e926 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 (cherry picked from commit f3142847d391e075b59ab3a100227e74f57f2bcc) --- 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 fbbe57d9c8a2..eb7335368f70 100644 --- a/pkg/operator/ceph/nfs/controller.go +++ b/pkg/operator/ceph/nfs/controller.go @@ -252,8 +252,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 c013e26d5d47..55499f4f65fd 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" @@ -51,25 +51,6 @@ var ( "ceph version 14.2.8 (3a54b2b6d167d4a2a19e003a705696d4fe619afc) nautilus (stable)": 3 } }` - 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) { @@ -217,10 +198,16 @@ func TestCephNFSController(t *testing.T) { if args[0] == "versions" { return dummyVersionsRaw, 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 c41ef8e391da..197eb70c8703 100644 --- a/pkg/operator/ceph/nfs/nfs.go +++ b/pkg/operator/ceph/nfs/nfs.go @@ -20,7 +20,6 @@ package nfs import ( "context" "fmt" - "strings" "github.com/banzaicloud/k8s-objectmatcher/patch" "github.com/pkg/errors" @@ -300,19 +299,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 -}