From 677efb1a37123254c60558540865b502412d29fc Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 30 Nov 2018 07:58:34 -0500 Subject: [PATCH 1/3] ceph mds: lift setting of max active MDS out of CreateFilesystem This seems a bit misplaced, since CreateFilesystem is all about doing the initial setup of the filesystem, and the daemons are managed in a different block of code. In a later patch, we're going to want to allow the CRD to spin up MDS' for filesystems that already exist. Lift the code that sets the MDS count into the caller. Signed-off-by: Jeff Layton --- pkg/daemon/ceph/client/filesystem.go | 9 +-------- pkg/daemon/ceph/mds/filesystem.go | 2 +- pkg/operator/ceph/file/filesystem.go | 7 +++++++ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/daemon/ceph/client/filesystem.go b/pkg/daemon/ceph/client/filesystem.go index 75f4eb0a2..920506a3a 100644 --- a/pkg/daemon/ceph/client/filesystem.go +++ b/pkg/daemon/ceph/client/filesystem.go @@ -109,7 +109,7 @@ func GetFilesystem(context *clusterd.Context, clusterName string, fsName string) } // CreateFilesystem performs software configuration steps for Ceph to provide a new filesystem. -func CreateFilesystem(context *clusterd.Context, clusterName, name, metadataPool string, dataPools []string, activeMDSCount int32) error { +func CreateFilesystem(context *clusterd.Context, clusterName, name, metadataPool string, dataPools []string) error { if len(dataPools) == 0 { return fmt.Errorf("at least one data pool is required") } @@ -144,13 +144,6 @@ func CreateFilesystem(context *clusterd.Context, clusterName, name, metadataPool } } - // set the number of active mds instances - if activeMDSCount > 1 { - if err = SetNumMDSRanks(context, clusterName, name, activeMDSCount); err != nil { - logger.Warningf("failed setting active mds count to %d. %+v", activeMDSCount, err) - } - } - return nil } diff --git a/pkg/daemon/ceph/mds/filesystem.go b/pkg/daemon/ceph/mds/filesystem.go index 1186e17f6..ed3180f55 100644 --- a/pkg/daemon/ceph/mds/filesystem.go +++ b/pkg/daemon/ceph/mds/filesystem.go @@ -105,7 +105,7 @@ func (f *Filesystem) CreateFilesystem(context *clusterd.Context, clusterName str } // create the filesystem - if err := client.CreateFilesystem(context, clusterName, f.Name, f.metadataPool.Name, dataPoolNames, f.activeMDSCount); err != nil { + if err := client.CreateFilesystem(context, clusterName, f.Name, f.metadataPool.Name, dataPoolNames); err != nil { return err } diff --git a/pkg/operator/ceph/file/filesystem.go b/pkg/operator/ceph/file/filesystem.go index f6450ddbc..332f17d75 100644 --- a/pkg/operator/ceph/file/filesystem.go +++ b/pkg/operator/ceph/file/filesystem.go @@ -55,6 +55,13 @@ func createFilesystem( return fmt.Errorf("failed to get file system %s: %+v", fs.Name, err) } + // set the number of active mds instances + if fs.Spec.MetadataServer.ActiveCount > 1 { + if err = client.SetNumMDSRanks(context, fs.Namespace, fs.Name, fs.Spec.MetadataServer.ActiveCount); err != nil { + logger.Warningf("failed setting active mds count to %d. %+v", fs.Spec.MetadataServer.ActiveCount, err) + } + } + logger.Infof("start running mdses for file system %s", fs.Name) c := newCluster(context, rookVersion, cephVersion, hostNetwork, fs, filesystem, ownerRefs) if err := c.start(); err != nil { From dcd9e1e0ff2b6b0fbdf1c4988778ffd901bc98c2 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 30 Nov 2018 07:49:33 -0500 Subject: [PATCH 2/3] ceph mds: allow filesystem specifications that have no pools design/external-management.md says that we should allow specifying Filesystems that have no pools. When this is done, the CRD should interepret that as a request to just manage the MDS daemons without creating a new filesystem. Don't fail the verification if there are no data pools in the spec, and just skip creating a new filesystem if that's the case. Also, add a testcase for this usage. Signed-off-by: Jeff Layton --- pkg/apis/ceph.rook.io/v1beta1/types.go | 4 +- pkg/operator/ceph/file/filesystem.go | 25 ++++++----- pkg/operator/ceph/file/filesystem_test.go | 51 +++++++++++++++++++++++ 3 files changed, 67 insertions(+), 13 deletions(-) diff --git a/pkg/apis/ceph.rook.io/v1beta1/types.go b/pkg/apis/ceph.rook.io/v1beta1/types.go index 928a012bf..55331ecfe 100755 --- a/pkg/apis/ceph.rook.io/v1beta1/types.go +++ b/pkg/apis/ceph.rook.io/v1beta1/types.go @@ -191,10 +191,10 @@ type FilesystemList struct { // FilesystemSpec represents the spec of a file system type FilesystemSpec struct { // The metadata pool settings - MetadataPool PoolSpec `json:"metadataPool"` + MetadataPool PoolSpec `json:"metadataPool,omitempty"` // The data pool settings - DataPools []PoolSpec `json:"dataPools"` + DataPools []PoolSpec `json:"dataPools,omitempty"` // The mds pod info MetadataServer MetadataServerSpec `json:"metadataServer"` diff --git a/pkg/operator/ceph/file/filesystem.go b/pkg/operator/ceph/file/filesystem.go index 332f17d75..c628ebbce 100644 --- a/pkg/operator/ceph/file/filesystem.go +++ b/pkg/operator/ceph/file/filesystem.go @@ -41,13 +41,15 @@ func createFilesystem( return err } - var dataPools []*model.Pool - for _, p := range fs.Spec.DataPools { - dataPools = append(dataPools, p.ToModel("")) - } - f := mdsdaemon.NewFS(fs.Name, fs.Spec.MetadataPool.ToModel(""), dataPools, fs.Spec.MetadataServer.ActiveCount) - if err := f.CreateFilesystem(context, fs.Namespace); err != nil { - return fmt.Errorf("failed to create file system %s: %+v", fs.Name, err) + if len(fs.Spec.DataPools) != 0 { + var dataPools []*model.Pool + for _, p := range fs.Spec.DataPools { + dataPools = append(dataPools, p.ToModel("")) + } + f := mdsdaemon.NewFS(fs.Name, fs.Spec.MetadataPool.ToModel(""), dataPools, fs.Spec.MetadataServer.ActiveCount) + if err := f.CreateFilesystem(context, fs.Namespace); err != nil { + return fmt.Errorf("failed to create file system %s: %+v", fs.Name, err) + } } filesystem, err := client.GetFilesystem(context, fs.Namespace, fs.Name) @@ -89,8 +91,12 @@ func validateFilesystem(context *clusterd.Context, f cephv1beta1.Filesystem) err if f.Namespace == "" { return fmt.Errorf("missing namespace") } + if f.Spec.MetadataServer.ActiveCount < 1 { + return fmt.Errorf("MetadataServer.ActiveCount must be at least 1") + } + // No data pool means that we expect the fs to exist already if len(f.Spec.DataPools) == 0 { - return fmt.Errorf("at least one data pool required") + return nil } if err := pool.ValidatePoolSpec(context, f.Namespace, &f.Spec.MetadataPool); err != nil { return fmt.Errorf("invalid metadata pool: %+v", err) @@ -100,9 +106,6 @@ func validateFilesystem(context *clusterd.Context, f cephv1beta1.Filesystem) err return fmt.Errorf("Invalid data pool: %+v", err) } } - if f.Spec.MetadataServer.ActiveCount < 1 { - return fmt.Errorf("MetadataServer.ActiveCount must be at least 1") - } return nil } diff --git a/pkg/operator/ceph/file/filesystem_test.go b/pkg/operator/ceph/file/filesystem_test.go index 7ced860c8..babc19962 100644 --- a/pkg/operator/ceph/file/filesystem_test.go +++ b/pkg/operator/ceph/file/filesystem_test.go @@ -134,6 +134,57 @@ func TestCreateFilesystem(t *testing.T) { assert.Equal(t, "failed to create file system myfs: Cannot create multiple filesystems. Enable ROOK_ALLOW_MULTIPLE_FILESYSTEMS env variable to create more than one", err.Error()) } +func TestCreateNopoolFilesystem(t *testing.T) { + configDir, _ := ioutil.TempDir("", "") + // Output to check multiple file system creation + fses := `[{"name":"myfs"}]` + + executor := &exectest.MockExecutor{ + MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + return "{\"key\":\"mysecurekey\"}", nil + }, + MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + if strings.Contains(command, "ceph-authtool") { + cephtest.CreateConfigDir(path.Join(configDir, "ns")) + } + + return "", nil + }, + } + defer os.RemoveAll(configDir) + context := &clusterd.Context{ + Executor: executor, + ConfigDir: configDir, + Clientset: testop.New(3)} + fs := cephv1beta1.Filesystem{ + ObjectMeta: metav1.ObjectMeta{Name: "myfs", Namespace: "ns"}, + Spec: cephv1beta1.FilesystemSpec{ + MetadataServer: cephv1beta1.MetadataServerSpec{ + ActiveCount: 1, + }, + }, + } + + // start a basic cluster + err := createFilesystem(context, fs, "v0.1", cephv1beta1.CephVersionSpec{}, false, []metav1.OwnerReference{}) + assert.Nil(t, err) + validateStart(t, context, fs) + + // starting again should be a no-op + err = createFilesystem(context, fs, "v0.1", cephv1beta1.CephVersionSpec{}, false, []metav1.OwnerReference{}) + assert.Nil(t, err) + validateStart(t, context, fs) + + // Test multiple filesystem creation + executor = &exectest.MockExecutor{ + MockExecuteCommandWithOutputFile: func(debug bool, actionName string, command string, outFileArg string, args ...string) (string, error) { + if contains(args, "ls") { + return fses, nil + } + return "{\"key\":\"mysecurekey\"}", errors.New("multiple fs") + }, + } +} func contains(arr []string, str string) bool { for _, a := range arr { if a == str { From 9945e6c91ba1870c5e67247b318be5e75baf3134 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 30 Nov 2018 14:25:44 -0500 Subject: [PATCH 3/3] ceph mds: only permanently remove the fs if rook created it i.e.: only if there was a data pool specified. Signed-off-by: Jeff Layton --- pkg/daemon/ceph/mds/filesystem.go | 9 ++------- pkg/operator/ceph/file/filesystem.go | 11 +++++++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/daemon/ceph/mds/filesystem.go b/pkg/daemon/ceph/mds/filesystem.go index ed3180f55..79c2bd59a 100644 --- a/pkg/daemon/ceph/mds/filesystem.go +++ b/pkg/daemon/ceph/mds/filesystem.go @@ -113,8 +113,8 @@ func (f *Filesystem) CreateFilesystem(context *clusterd.Context, clusterName str return nil } -// DeleteFilesystem removes the filesystem in Ceph and removes the Ceph file daemons. -func DeleteFilesystem(context *clusterd.Context, clusterName, filesystemName string) error { +// DownFilesystem marks the filesystem as down and the MDS' as failed +func DownFilesystem(context *clusterd.Context, clusterName, filesystemName string) error { logger.Infof("Removing file system %s", filesystemName) // mark the cephFS instance as cluster_down before removing @@ -133,11 +133,6 @@ func DeleteFilesystem(context *clusterd.Context, clusterName, filesystemName str } } - // Permanently remove the file system - if err := client.RemoveFilesystem(context, clusterName, filesystemName); err != nil { - return err - } - logger.Infof("Removed file system %s", filesystemName) return nil } diff --git a/pkg/operator/ceph/file/filesystem.go b/pkg/operator/ceph/file/filesystem.go index c628ebbce..0b06f3a8a 100644 --- a/pkg/operator/ceph/file/filesystem.go +++ b/pkg/operator/ceph/file/filesystem.go @@ -76,9 +76,16 @@ func createFilesystem( // deleteFileSystem deletes the file system and the metadata servers func deleteFilesystem(context *clusterd.Context, fs cephv1beta1.Filesystem) error { // The most important part of deletion is that the filesystem gets removed from Ceph - if err := mdsdaemon.DeleteFilesystem(context, fs.Namespace, fs.Name); err != nil { + if err := mdsdaemon.DownFilesystem(context, fs.Namespace, fs.Name); err != nil { // If the fs isn't deleted from Ceph, leave the daemons so it can still be used. - return fmt.Errorf("failed to delete filesystem %s: %+v", fs.Name, err) + return fmt.Errorf("failed to down filesystem %s: %+v", fs.Name, err) + } + + // Permanently remove the file system if it was created by rook + if len(fs.Spec.DataPools) != 0 { + if err := client.RemoveFilesystem(context, fs.Namespace, fs.Name); err != nil { + return fmt.Errorf("failed to remove filesystem %s: %+v", fs.Name, err) + } } return deleteMdsCluster(context, fs.Namespace, fs.Name)