Merge pull request #1779 from travisn/name-ceph-pods

Name mgr and mds pods more consistently

Documentation/upgrade.md

@@ -276,15 +276,13 @@ kubectl -n rook get pod -l app=rook-ceph-osd -o jsonpath='{range .items[*]}{.met
 Remember after each OSD pod to verify the cluster health using the instructions found in the [health verification section](#health-verification).
 
 ### Ceph Manager
-Similar to the Rook operator, the Ceph manager pods are managed by a deployment.
-We will edit the deployment to use the new image version of `rook/ceph:master`:
-```bash
-kubectl -n rook set image deploy/rook-ceph-mgr0 rook-ceph-mgr0=rook/ceph:master
-```
+The ceph manager has been renamed in 0.8. The new manager will be started automatically by the operator. 
+The old manager and its secret can simply be deleted.
 
-To verify that the manager pod is `Running` and on the new version, use the following:
+To delete the 0.7 manager, run the following:
 ```bash
-kubectl -n rook get pod -l app=rook-ceph-mgr -o jsonpath='{range .items[*]}{.metadata.name}{" "}{.status.phase}{" "}{.spec.containers[0].image}{"\n"}{end}'
+kubectl -n rook delete deploy rook-ceph-mgr0
+kubectl -n rook delete secret rook-ceph-mgr0
 ```
 
 ### Legacy Custom Resource Definitions (CRDs)

cmd/rook/ceph/mds.go

@@ -21,6 +21,7 @@ import (
 	"github.com/rook/rook/cmd/rook/rook"
 	"github.com/rook/rook/pkg/daemon/ceph/mds"
 	"github.com/rook/rook/pkg/daemon/ceph/mon"
+	"github.com/rook/rook/pkg/operator/ceph/file"
 	"github.com/rook/rook/pkg/util/flags"
 	"github.com/spf13/cobra"
 )
@@ -58,14 +59,7 @@ func startMDS(cmd *cobra.Command, args []string) error {
 
 	rook.LogStartupInfo(mdsCmd.Flags())
 
-	// the MDS ID is the last part of the pod name
-	id := podName
-	dashIndex := strings.LastIndex(podName, "-")
-	if dashIndex > 0 {
-		id = podName[dashIndex+1:]
-	}
-	// ensure the id has a non-numerical prefix
-	id = "m" + id
+	id := extractMdsID(podName)
 
 	clusterInfo.Monitors = mon.ParseMonEndpoints(cfg.monEndpoints)
 	config := &mds.Config{
@@ -82,3 +76,14 @@ func startMDS(cmd *cobra.Command, args []string) error {
 
 	return nil
 }
+
+func extractMdsID(mdsName string) string {
+	prefix := file.AppName + "-"
+	if strings.Index(mdsName, prefix) == 0 && len(mdsName) > len(prefix) {
+		// remove the prefix from the mds name
+		return mdsName[len(prefix):]
+	}
+
+	// return the original name if we did not find the prefix
+	return mdsName
+}

cmd/rook/ceph/mds_test.go

@@ -0,0 +1,39 @@
+/*
+Copyright 2018 The Rook Authors. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package ceph
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestExtractMdsName(t *testing.T) {
+	name := extractMdsID("random")
+	assert.Equal(t, "random", name)
+
+	name = extractMdsID("rook-ceph-mds")
+	assert.Equal(t, "rook-ceph-mds", name)
+
+	name = extractMdsID("rook-ceph-mds-a")
+	assert.Equal(t, "a", name)
+
+	name = extractMdsID("rook-ceph-mds-rook-ceph-mds-foo")
+	assert.Equal(t, "rook-ceph-mds-foo", name)
+
+	name = extractMdsID("rook-ceph-mds-myfs-64b66569f6-5tz2s")
+	assert.Equal(t, "myfs-64b66569f6-5tz2s", name)
+}

pkg/daemon/ceph/mgr/daemon.go

@@ -109,7 +109,7 @@ func startMgr(context *clusterd.Context, config *Config) error {
 }
 
 func getMgrConfDir(dir, name string) string {
-	return path.Join(dir, fmt.Sprintf("mgr%s", name))
+	return path.Join(dir, fmt.Sprintf("mgr-%s", name))
 }
 
 func getMgrConfFilePath(dir, name, clusterName string) string {

pkg/operator/ceph/cluster/mgr/mgr.go

@@ -37,15 +37,16 @@ import (
 var logger = capnslog.NewPackageLogger("github.com/rook/rook", "op-mgr")
 
 const (
-	appName     = "rook-ceph-mgr"
-	keyringName = "keyring"
-
+	appName              = "rook-ceph-mgr"
+	keyringName          = "keyring"
 	prometheusModuleName = "prometheus"
 	dashboardModuleName  = "dashboard"
 	metricsPort          = 9283
 	dashboardPort        = 7000
 )
 
+var mgrNames = []string{"a", "b"}
+
 // Cluster is the ceph mgr manager
 type Cluster struct {
 	Namespace   string
@@ -82,16 +83,21 @@ func (c *Cluster) Start() error {
 	logger.Infof("start running mgr")
 
 	for i := 0; i < c.Replicas; i++ {
-		name := fmt.Sprintf("%s%d", appName, i)
-		if err := c.createKeyring(c.Namespace, name); err != nil {
-			return fmt.Errorf("failed to create mgr keyring. %+v", err)
+		if i >= len(mgrNames) {
+			logger.Errorf("cannot have more than %d mgrs", len(mgrNames))
+			break
+		}
+		daemonName := mgrNames[i]
+		name := fmt.Sprintf("%s-%s", appName, daemonName)
+		if err := c.createKeyring(c.Namespace, name, daemonName); err != nil {
+			return fmt.Errorf("failed to create %s keyring. %+v", name, err)
 		}
 
 		// start the deployment
-		deployment := c.makeDeployment(name)
+		deployment := c.makeDeployment(name, daemonName)
 		if _, err := c.context.Clientset.ExtensionsV1beta1().Deployments(c.Namespace).Create(deployment); err != nil {
 			if !errors.IsAlreadyExists(err) {
-				return fmt.Errorf("failed to create mgr deployment. %+v", err)
+				return fmt.Errorf("failed to create %s deployment. %+v", name, err)
 			}
 			logger.Infof("%s deployment already exists", name)
 		} else {
@@ -104,7 +110,7 @@ func (c *Cluster) Start() error {
 	}
 
 	// create the metrics service
-	service := c.makeService(appName)
+	service := c.makeMetricsService(appName)
 	if _, err := c.context.Clientset.CoreV1().Services(c.Namespace).Create(service); err != nil {
 		if !errors.IsAlreadyExists(err) {
 			return fmt.Errorf("failed to create mgr service. %+v", err)
@@ -119,7 +125,7 @@ func (c *Cluster) Start() error {
 
 func (c *Cluster) configureDashboard() error {
 	// enable or disable the dashboard module
-	if err := c.enableDashboardModule(c.Namespace, c.dashboard.Enabled); err != nil {
+	if err := c.configureDashboardModule(c.Namespace, c.dashboard.Enabled); err != nil {
 		return fmt.Errorf("failed to enable mgr dashboard module. %+v", err)
 	}
 
@@ -145,7 +151,7 @@ func (c *Cluster) configureDashboard() error {
 	return nil
 }
 
-func (c *Cluster) makeService(name string) *v1.Service {
+func (c *Cluster) makeMetricsService(name string) *v1.Service {
 	labels := c.getLabels()
 	return &v1.Service{
 		ObjectMeta: metav1.ObjectMeta{
@@ -191,17 +197,17 @@ func (c *Cluster) makeDashboardService(name string) *v1.Service {
 	}
 }
 
-func (c *Cluster) makeDeployment(name string) *extensions.Deployment {
+func (c *Cluster) makeDeployment(name, daemonName string) *extensions.Deployment {
 
 	podSpec := v1.PodTemplateSpec{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:   name,
-			Labels: c.getLabels(),
+			Labels: c.getDaemonLabels(daemonName),
 			Annotations: map[string]string{"prometheus.io/scrape": "true",
 				"prometheus.io/port": strconv.Itoa(metricsPort)},
 		},
 		Spec: v1.PodSpec{
-			Containers:    []v1.Container{c.mgrContainer(name)},
+			Containers:    []v1.Container{c.mgrContainer(name, daemonName)},
 			RestartPolicy: v1.RestartPolicyAlways,
 			Volumes: []v1.Volume{
 				{Name: k8sutil.DataDirVolume, VolumeSource: v1.VolumeSource{EmptyDir: &v1.EmptyDirVolumeSource{}}},
@@ -226,7 +232,7 @@ func (c *Cluster) makeDeployment(name string) *extensions.Deployment {
 	}
 }
 
-func (c *Cluster) mgrContainer(name string) v1.Container {
+func (c *Cluster) mgrContainer(name, daemonName string) v1.Container {
 
 	return v1.Container{
 		Args: []string{
@@ -241,7 +247,7 @@ func (c *Cluster) mgrContainer(name string) v1.Container {
 			k8sutil.ConfigOverrideMount(),
 		},
 		Env: []v1.EnvVar{
-			{Name: "ROOK_MGR_NAME", Value: name},
+			{Name: "ROOK_MGR_NAME", Value: daemonName},
 			{Name: "ROOK_MGR_KEYRING", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: name}, Key: keyringName}}},
 			k8sutil.PodIPEnvVar(k8sutil.PrivateIPEnvVar),
 			k8sutil.PodIPEnvVar(k8sutil.PublicIPEnvVar),
@@ -263,6 +269,11 @@ func (c *Cluster) mgrContainer(name string) v1.Container {
 				ContainerPort: int32(metricsPort),
 				Protocol:      v1.ProtocolTCP,
 			},
+			{
+				Name:          "dashboard",
+				ContainerPort: int32(dashboardPort),
+				Protocol:      v1.ProtocolTCP,
+			},
 		},
 	}
 }
@@ -274,7 +285,13 @@ func (c *Cluster) getLabels() map[string]string {
 	}
 }
 
-func (c *Cluster) createKeyring(clusterName, name string) error {
+func (c *Cluster) getDaemonLabels(daemonName string) map[string]string {
+	labels := c.getLabels()
+	labels["instance"] = daemonName
+	return labels
+}
+
+func (c *Cluster) createKeyring(clusterName, name, daemonName string) error {
 	_, err := c.context.Clientset.CoreV1().Secrets(c.Namespace).Get(name, metav1.GetOptions{})
 	if err == nil {
 		logger.Infof("the mgr keyring was already generated")
@@ -285,7 +302,7 @@ func (c *Cluster) createKeyring(clusterName, name string) error {
 	}
 
 	// get-or-create-key for the user account
-	keyring, err := createKeyring(c.context, clusterName, name)
+	keyring, err := createKeyring(c.context, clusterName, daemonName)
 	if err != nil {
 		return fmt.Errorf("failed to create mgr keyring. %+v", err)
 	}
@@ -320,7 +337,7 @@ func (c *Cluster) enablePrometheusModule(clusterName string) error {
 }
 
 // Ceph docs about the dashboard module: http://docs.ceph.com/docs/luminous/mgr/dashboard/
-func (c *Cluster) enableDashboardModule(clusterName string, enable bool) error {
+func (c *Cluster) configureDashboardModule(clusterName string, enable bool) error {
 	if enable {
 		if err := client.MgrEnableModule(c.context, clusterName, dashboardModuleName, true); err != nil {
 			return fmt.Errorf("failed to enable mgr dashboard module. %+v", err)

pkg/operator/ceph/cluster/mgr/mgr_test.go

@@ -66,8 +66,12 @@ func TestStartMGR(t *testing.T) {
 func validateStart(t *testing.T, c *Cluster) {
 
 	for i := 0; i < c.Replicas; i++ {
+		if i == len(mgrNames) {
+			break
+		}
 		logger.Infof("Looking for cephmgr replica %d", i)
-		_, err := c.context.Clientset.ExtensionsV1beta1().Deployments(c.Namespace).Get(fmt.Sprintf("rook-ceph-mgr%d", i), metav1.GetOptions{})
+		daemonName := mgrNames[i]
+		_, err := c.context.Clientset.ExtensionsV1beta1().Deployments(c.Namespace).Get(fmt.Sprintf("rook-ceph-mgr-%s", daemonName), metav1.GetOptions{})
 		assert.Nil(t, err)
 	}
 
@@ -92,15 +96,14 @@ func TestPodSpec(t *testing.T) {
 		},
 	}, metav1.OwnerReference{})
 
-	d := c.makeDeployment("mgr1")
+	d := c.makeDeployment("mgr-a", "a")
 	assert.NotNil(t, d)
-	assert.Equal(t, "mgr1", d.Name)
+	assert.Equal(t, "mgr-a", d.Name)
 	assert.Equal(t, v1.RestartPolicyAlways, d.Spec.Template.Spec.RestartPolicy)
 	assert.Equal(t, 2, len(d.Spec.Template.Spec.Volumes))
-	assert.Equal(t, 2, len(d.Spec.Template.Spec.Containers[0].Ports))
+	assert.Equal(t, 3, len(d.Spec.Template.Spec.Containers[0].Ports))
 	assert.Equal(t, "rook-data", d.Spec.Template.Spec.Volumes[0].Name)
-
-	assert.Equal(t, "mgr1", d.ObjectMeta.Name)
+	assert.Equal(t, "mgr-a", d.ObjectMeta.Name)
 	assert.Equal(t, appName, d.Spec.Template.ObjectMeta.Labels["app"])
 	assert.Equal(t, c.Namespace, d.Spec.Template.ObjectMeta.Labels["rook_cluster"])
 	assert.Equal(t, 0, len(d.ObjectMeta.Annotations))
@@ -124,7 +127,7 @@ func TestPodSpec(t *testing.T) {
 func TestServiceSpec(t *testing.T) {
 	c := New(nil, "ns", "myversion", rookalpha.Placement{}, false, cephv1alpha1.DashboardSpec{}, v1.ResourceRequirements{}, metav1.OwnerReference{})
 
-	s := c.makeService("rook-mgr")
+	s := c.makeMetricsService("rook-mgr")
 	assert.NotNil(t, s)
 	assert.Equal(t, "rook-mgr", s.Name)
 	assert.Equal(t, 1, len(s.Spec.Ports))
@@ -133,7 +136,7 @@ func TestServiceSpec(t *testing.T) {
 func TestHostNetwork(t *testing.T) {
 	c := New(nil, "ns", "myversion", rookalpha.Placement{}, true, cephv1alpha1.DashboardSpec{}, v1.ResourceRequirements{}, metav1.OwnerReference{})
 
-	d := c.makeDeployment("mgr1")
+	d := c.makeDeployment("mgr-a", "a")
 	assert.NotNil(t, d)
 
 	assert.Equal(t, true, d.Spec.Template.Spec.HostNetwork)

pkg/operator/ceph/file/mds.go

@@ -36,7 +36,7 @@ import (
 )
 
 const (
-	appName = "rook-ceph-mds"
+	AppName = "rook-ceph-mds"
 )
 
 // Create the file system
@@ -97,7 +97,7 @@ func DeleteFilesystem(context *clusterd.Context, fs cephv1alpha1.Filesystem) err
 }
 
 func instanceName(fs cephv1alpha1.Filesystem) string {
-	return fmt.Sprintf("%s-%s", appName, fs.Name)
+	return fmt.Sprintf("%s-%s", AppName, fs.Name)
 }
 
 func makeDeployment(fs cephv1alpha1.Filesystem, filesystemID, version string, hostNetwork bool, ownerRefs []metav1.OwnerReference) *extensions.Deployment {
@@ -171,7 +171,7 @@ func mdsContainer(fs cephv1alpha1.Filesystem, filesystemID, version string) v1.C
 
 func getLabels(fs cephv1alpha1.Filesystem) map[string]string {
 	return map[string]string{
-		k8sutil.AppAttr:     appName,
+		k8sutil.AppAttr:     AppName,
 		k8sutil.ClusterAttr: fs.Namespace,
 		"rook_file_system":  fs.Name,
 	}

pkg/operator/ceph/file/mds_test.go

@@ -119,7 +119,7 @@ func validateStart(t *testing.T, context *clusterd.Context, fs cephv1alpha1.File
 
 	r, err := context.Clientset.ExtensionsV1beta1().Deployments(fs.Namespace).Get("rook-ceph-mds-myfs", metav1.GetOptions{})
 	assert.Nil(t, err)
-	assert.Equal(t, appName+"-myfs", r.Name)
+	assert.Equal(t, AppName+"-myfs", r.Name)
 }
 
 func TestPodSpecs(t *testing.T) {
@@ -143,13 +143,13 @@ func TestPodSpecs(t *testing.T) {
 
 	d := makeDeployment(fs, mdsID, "rook/rook:myversion", false, []metav1.OwnerReference{})
 	assert.NotNil(t, d)
-	assert.Equal(t, appName+"-myfs", d.Name)
+	assert.Equal(t, AppName+"-myfs", d.Name)
 	assert.Equal(t, v1.RestartPolicyAlways, d.Spec.Template.Spec.RestartPolicy)
 	assert.Equal(t, 2, len(d.Spec.Template.Spec.Volumes))
 	assert.Equal(t, "rook-data", d.Spec.Template.Spec.Volumes[0].Name)
 
-	assert.Equal(t, appName+"-myfs", d.ObjectMeta.Name)
-	assert.Equal(t, appName, d.Spec.Template.ObjectMeta.Labels["app"])
+	assert.Equal(t, AppName+"-myfs", d.ObjectMeta.Name)
+	assert.Equal(t, AppName, d.Spec.Template.ObjectMeta.Labels["app"])
 	assert.Equal(t, fs.Namespace, d.Spec.Template.ObjectMeta.Labels["rook_cluster"])
 	assert.Equal(t, 0, len(d.ObjectMeta.Annotations))