nfs: remove poolConfig #9356
nfs: remove poolConfig #9356
Conversation
BlaineEXE
force-pushed
the
nfs-remove-poolconfig
branch
from
3d883e4
to
99ee08c
Compare
| @@ -4,12 +4,9 @@ metadata: | |||
| name: my-nfs | |||
| namespace: rook-ceph # namespace:cluster | |||
| spec: | |||
| # rados settings aren't necessary in Ceph Versions equal to or greater than Pacific 16.2.7 | |||
| rados: | |||
There was a problem hiding this comment.
cluster-test.yaml uses 16.2.7 now, shall we still leave a comment here about rados being ignored in that case?
BlaineEXE
force-pushed
the
nfs-remove-poolconfig
branch
from
99ee08c
to
be49053
Compare
BlaineEXE
force-pushed
the
nfs-remove-poolconfig
branch
from
be49053
to
5e2fc50
Compare
| func (r *ReconcileCephNFS) createDefaultNFSRADOSPool(n *cephv1.CephNFS) error { | ||
| poolName := n.Spec.RADOS.Pool | ||
| // Settings are not always declared and CreateReplicatedPoolForApp does not accept a pointer for | ||
| // the pool spec | ||
| if n.Spec.RADOS.PoolConfig == nil { | ||
| n.Spec.RADOS.PoolConfig = &cephv1.PoolSpec{} | ||
|
|
||
| args := []string{"osd", "pool", "create", poolName} | ||
| output, err := cephclient.NewCephCommand(r.context, r.clusterInfo, args).Run() | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to create default NFS pool %q. %s", poolName, string(output)) | ||
| } | ||
| err := cephclient.CreateReplicatedPoolForApp(r.context, r.clusterInfo, r.cephClusterSpec, poolName, *n.Spec.RADOS.PoolConfig, cephclient.DefaultPGCount, "nfs") | ||
|
|
||
| args = []string{"osd", "pool", "application", "enable", poolName, "nfs", "--yes-i-really-mean-it"} | ||
| _, err = cephclient.NewCephCommand(r.context, r.clusterInfo, args).Run() | ||
| if err != nil { | ||
| return err | ||
| return errors.Wrapf(err, "failed to enable application 'nfs' on pool %q", poolName) | ||
| } |
There was a problem hiding this comment.
Forgot to update this function before. With manual testing, this all looks good. The .nfs pool is created with the nfs application, and I verified that it doesn't fail if the operator is restarted and this is run again.
Remove the poolConfig in CephNFS to prevent different CephNFS CRs from contending to change the replication spec of the default NFS pool. Resolves rook#9355 Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>
BlaineEXE
force-pushed
the
nfs-remove-poolconfig
branch
from
5e2fc50
to
aaecc93
Compare
| @@ -293,14 +293,17 @@ func validateGanesha(context *clusterd.Context, clusterInfo *cephclient.ClusterI | |||
| // create and enable default RADOS pool | |||
| func (r *ReconcileCephNFS) createDefaultNFSRADOSPool(n *cephv1.CephNFS) error { | |||
| poolName := n.Spec.RADOS.Pool | |||
| // Settings are not always declared and CreateReplicatedPoolForApp does not accept a pointer for | |||
There was a problem hiding this comment.
If rook doesn't create this pool, will ceph create it automatically? If so, seems like we could stop creating it explicitly since after #9209 is available, they should be creating the .nfs pool with the pool CR.
There was a problem hiding this comment.
I don't believe ceph creates it automatically, no. At least it doesn't for all versions.
The operation is idempotent though AFAIK. If the pool exists, Ceph won't fail the command. Same for setting the application. Since the command doesn't specify a crush rule, I do wonder if it might reset the pool to defaults or not. That would be an integration we'd have to investigate a bit I think.
There was a problem hiding this comment.
Ok, let's leave this for now to ensure the pool is created by default. With #9209 perhaps we require that they define the .nfs CephBlockPool.
nfs: remove poolConfig (backport #9356)
Remove the poolConfig in CephNFS to prevent different CephNFS CRs from
contending to change the replication spec of the default NFS pool.
Resolves #9355
Signed-off-by: Blaine Gardner blaine.gardner@redhat.com
Description of your changes:
Which issue is resolved by this Pull Request:
Resolves #
Checklist:
make codegen) has been run to update object specifications, if necessary.