Merge pull request #1137 from travisn/crush-hostname

crush: set host properly in the crush map

cmd/rook/osd.go

@@ -18,7 +18,9 @@ package main
 import (
 	"fmt"
 	"os"
+	"strings"
 
+	"github.com/rook/rook/pkg/ceph/client"
 	"github.com/rook/rook/pkg/ceph/mon"
 	"github.com/rook/rook/pkg/ceph/osd"
 	"github.com/rook/rook/pkg/operator/k8sutil"
@@ -95,10 +97,17 @@ func startOSD(cmd *cobra.Command, args []string) error {
 
 	kv := k8sutil.NewConfigMapKVStore(clusterInfo.Name, clientset)
 
+	locArgs, err := client.FormatLocation(cfg.location, cfg.nodeName)
+	if err != nil {
+		fmt.Printf("invalid location. %+v\n", err)
+		os.Exit(1)
+	}
+	crushLocation := strings.Join(locArgs, " ")
+
 	forceFormat := false
 	clusterInfo.Monitors = mon.ParseMonEndpoints(cfg.monEndpoints)
 	agent := osd.NewAgent(dataDevices, usingDeviceFilter, cfg.metadataDevice, cfg.directories, forceFormat,
-		cfg.location, cfg.storeConfig, &clusterInfo, cfg.nodeName, kv)
+		crushLocation, cfg.storeConfig, &clusterInfo, cfg.nodeName, kv)
 
 	err = osd.Run(context, agent)
 	if err != nil {

pkg/ceph/client/crush.go

@@ -200,7 +200,7 @@ func CreateDefaultCrushMap(context *clusterd.Context, clusterName string) (strin
 	return "", nil
 }
 
-func FormatLocation(location string) ([]string, error) {
+func FormatLocation(location, hostName string) ([]string, error) {
 	var pairs []string
 	if location == "" {
 		pairs = []string{}
@@ -218,6 +218,12 @@ func FormatLocation(location string) ([]string, error) {
 	if !isCrushFieldSet("root", pairs) {
 		pairs = append(pairs, formatProperty("root", "default"))
 	}
+	// set the host name
+	if !isCrushFieldSet("host", pairs) {
+		// keep the fully qualified host name in the crush map, but replace the dots with dashes to satisfy ceph
+		hostName = strings.Replace(hostName, ".", "-", -1)
+		pairs = append(pairs, formatProperty("host", hostName))
+	}
 
 	return pairs, nil
 }

pkg/ceph/client/crush_test.go

@@ -249,16 +249,17 @@ func TestCrushLocation(t *testing.T) {
 	loc := "dc=datacenter1"
 
 	// test that root will get filled in with default/runtime values
-	res, err := FormatLocation(loc)
+	res, err := FormatLocation(loc, "my.node")
 	assert.Nil(t, err)
-	assert.Equal(t, 2, len(res))
+	assert.Equal(t, 3, len(res))
 	locSet := util.CreateSet(res)
 	assert.True(t, locSet.Contains("root=default"))
 	assert.True(t, locSet.Contains("dc=datacenter1"))
+	assert.True(t, locSet.Contains("host=my-node"))
 
 	// test that if host name and root are already set they will be honored
 	loc = "root=otherRoot,dc=datacenter2,host=node123"
-	res, err = FormatLocation(loc)
+	res, err = FormatLocation(loc, "othernode")
 	assert.Nil(t, err)
 	assert.Equal(t, 3, len(res))
 	locSet = util.CreateSet(res)
@@ -268,7 +269,7 @@ func TestCrushLocation(t *testing.T) {
 
 	// test an invalid CRUSH location format
 	loc = "root=default,prop:value"
-	_, err = FormatLocation(loc)
+	_, err = FormatLocation(loc, "othernode")
 	assert.NotNil(t, err)
 	assert.Contains(t, err.Error(), "is not in a valid format")
 }

pkg/ceph/mds/daemon.go

@@ -81,7 +81,7 @@ func generateConfigFiles(context *clusterd.Context, config *Config) error {
 
 	keyringPath := getMDSKeyringPath(context.ConfigDir)
 	_, err = mon.GenerateConfigFile(context, config.ClusterInfo, getMDSConfDir(context.ConfigDir),
-		fmt.Sprintf("mds.%s", config.ID), keyringPath, false, nil, settings)
+		fmt.Sprintf("mds.%s", config.ID), keyringPath, nil, settings)
 	if err != nil {
 		return fmt.Errorf("failed to create mds config file. %+v", err)
 	}

pkg/ceph/mgr/daemon.go

@@ -72,7 +72,7 @@ func generateConfigFiles(context *clusterd.Context, config *Config) error {
 	}
 	logger.Infof("Conf files: dir=%s keyring=%s", confDir, keyringPath)
 	_, err := mon.GenerateConfigFile(context, config.ClusterInfo, confDir,
-		username, keyringPath, false, nil, settings)
+		username, keyringPath, nil, settings)
 	if err != nil {
 		return fmt.Errorf("failed to create config file. %+v", err)
 	}

pkg/ceph/mon/config.go

@@ -58,7 +58,7 @@ type cephConfig struct {
 }
 
 type GlobalConfig struct {
-	EnableExperimental       string `ini:"enable experimental unrecoverable data corrupting features"`
+	EnableExperimental       string `ini:"enable experimental unrecoverable data corrupting features,omitempty"`
 	FSID                     string `ini:"fsid,omitempty"`
 	RunDir                   string `ini:"run dir,omitempty"`
 	MonMembers               string `ini:"mon initial members,omitempty"`
@@ -89,6 +89,7 @@ type GlobalConfig struct {
 	OsdMaxObjectNameLen      int    `ini:"osd max object name len,omitempty"`
 	OsdMaxObjectNamespaceLen int    `ini:"osd max object namespace len,omitempty"`
 	OsdObjectStore           string `ini:"osd objectstore"`
+	CrushLocation            string `ini:"crush location,omitempty"`
 	RbdDefaultFeatures       int    `ini:"rbd_default_features,omitempty"`
 	FatalSignalHandlers      string `ini:"fatal signal handlers"`
 }
@@ -114,6 +115,10 @@ func GetConfFilePath(root, clusterName string) string {
 }
 
 func GenerateAdminConnectionConfig(context *clusterd.Context, cluster *ClusterInfo) error {
+	return GenerateAdminConnectionConfigWithSettings(context, cluster, nil)
+}
+
+func GenerateAdminConnectionConfigWithSettings(context *clusterd.Context, cluster *ClusterInfo, settings *cephConfig) error {
 	root := path.Join(context.ConfigDir, cluster.Name)
 	keyring := fmt.Sprintf(AdminKeyringTemplate, cluster.AdminSecret)
 	keyringPath := path.Join(root, fmt.Sprintf("%s.keyring", client.AdminUsername))
@@ -122,7 +127,7 @@ func GenerateAdminConnectionConfig(context *clusterd.Context, cluster *ClusterIn
 		return fmt.Errorf("failed to write keyring to %s. %+v", root, err)
 	}
 
-	if _, err = GenerateConfigFile(context, cluster, root, client.AdminUsername, keyringPath, false, nil, nil); err != nil {
+	if _, err = GenerateConfigFile(context, cluster, root, client.AdminUsername, keyringPath, settings, nil); err != nil {
 		return fmt.Errorf("failed to write config to %s. %+v", root, err)
 	}
 	logger.Infof("generated admin config in %s", root)
@@ -149,12 +154,12 @@ func writeKeyring(keyring, path string) error {
 
 // generates and writes the monitor config file to disk
 func GenerateConnectionConfigFile(context *clusterd.Context, cluster *ClusterInfo, pathRoot, user, keyringPath string) (string, error) {
-	return GenerateConfigFile(context, cluster, pathRoot, user, keyringPath, false, nil, nil)
+	return GenerateConfigFile(context, cluster, pathRoot, user, keyringPath, nil, nil)
 }
 
 // generates and writes the monitor config file to disk
 func GenerateConfigFile(context *clusterd.Context, cluster *ClusterInfo, pathRoot, user, keyringPath string,
-	bluestore bool, userConfig *cephConfig, clientSettings map[string]string) (string, error) {
+	globalConfig *cephConfig, clientSettings map[string]string) (string, error) {
 
 	if pathRoot == "" {
 		pathRoot = getMonRunDirPath(context.ConfigDir, getFirstMonitor(cluster))
@@ -165,7 +170,7 @@ func GenerateConfigFile(context *clusterd.Context, cluster *ClusterInfo, pathRoo
 		logger.Warningf("failed to create config directory at %s: %+v", pathRoot, err)
 	}
 
-	configFile, err := createGlobalConfigFileSection(context, cluster, pathRoot, bluestore, userConfig)
+	configFile, err := createGlobalConfigFileSection(context, cluster, pathRoot, globalConfig)
 	if err != nil {
 		return "", fmt.Errorf("failed to create global config section, %+v", err)
 	}
@@ -248,7 +253,7 @@ func getFirstMonitor(cluster *ClusterInfo) string {
 	return ""
 }
 
-func CreateDefaultCephConfig(context *clusterd.Context, cluster *ClusterInfo, runDir string, bluestore bool) *cephConfig {
+func CreateDefaultCephConfig(context *clusterd.Context, cluster *ClusterInfo, runDir string) *cephConfig {
 	// extract a list of just the monitor names, which will populate the "mon initial members"
 	// global config field
 	monMembers := make([]string, len(cluster.Monitors))
@@ -260,18 +265,10 @@ func CreateDefaultCephConfig(context *clusterd.Context, cluster *ClusterInfo, ru
 		i++
 	}
 
-	experimental := ""
-	store := "filestore"
-	if bluestore {
-		experimental = "bluestore rocksdb"
-		store = "bluestore"
-	}
-
 	cephLogLevel := logLevelToCephLogLevel(context.LogLevel)
 
 	return &cephConfig{
 		GlobalConfig: &GlobalConfig{
-			EnableExperimental:     experimental,
 			FSID:                   cluster.FSID,
 			RunDir:                 runDir,
 			MonMembers:             strings.Join(monMembers, " "),
@@ -299,15 +296,13 @@ func CreateDefaultCephConfig(context *clusterd.Context, cluster *ClusterInfo, ru
 			OsdPoolDefaultMinSize:  1,
 			OsdPoolDefaultPgNum:    100,
 			OsdPoolDefaultPgpNum:   100,
-			OsdObjectStore:         store,
 			RbdDefaultFeatures:     3,
 			FatalSignalHandlers:    "false",
 		},
 	}
 }
 
-func createGlobalConfigFileSection(context *clusterd.Context, cluster *ClusterInfo, runDir string,
-	bluestore bool, userConfig *cephConfig) (*ini.File, error) {
+func createGlobalConfigFileSection(context *clusterd.Context, cluster *ClusterInfo, runDir string, userConfig *cephConfig) (*ini.File, error) {
 
 	var ceph *cephConfig
 
@@ -315,7 +310,7 @@ func createGlobalConfigFileSection(context *clusterd.Context, cluster *ClusterIn
 		// use the user config since it was provided
 		ceph = userConfig
 	} else {
-		ceph = CreateDefaultCephConfig(context, cluster, runDir, bluestore)
+		ceph = CreateDefaultCephConfig(context, cluster, runDir)
 	}
 
 	configFile := ini.Empty()

pkg/ceph/mon/config_test.go

@@ -53,20 +53,14 @@ func TestCreateDefaultCephConfig(t *testing.T) {
 		},
 	}
 
-	cephConfig := CreateDefaultCephConfig(context, clusterInfo, "/var/lib/rook1", false)
-	verifyConfig(t, cephConfig, monMembers, "", "filestore", 0)
-
-	cephConfig = CreateDefaultCephConfig(context, clusterInfo, "/var/lib/rook1", true)
-	verifyConfig(t, cephConfig, monMembers, "bluestore rocksdb", "bluestore", 0)
+	cephConfig := CreateDefaultCephConfig(context, clusterInfo, "/var/lib/rook1")
+	verifyConfig(t, cephConfig, monMembers, 0)
 
 	// now use DEBUG level logging
 	context.LogLevel = capnslog.DEBUG
 
-	cephConfig = CreateDefaultCephConfig(context, clusterInfo, "/var/lib/rook1", false)
-	verifyConfig(t, cephConfig, monMembers, "", "filestore", 10)
-
-	cephConfig = CreateDefaultCephConfig(context, clusterInfo, "/var/lib/rook1", true)
-	verifyConfig(t, cephConfig, monMembers, "bluestore rocksdb", "bluestore", 10)
+	cephConfig = CreateDefaultCephConfig(context, clusterInfo, "/var/lib/rook1")
+	verifyConfig(t, cephConfig, monMembers, 10)
 
 	// verify the network info config
 	assert.Equal(t, "10.1.1.1", cephConfig.PublicAddr)
@@ -108,7 +102,7 @@ debug bluestore = 1234`
 	}
 
 	// generate the config file to disk now
-	configFilePath, err := GenerateConfigFile(context, clusterInfo, configDir, "myuser", filepath.Join(configDir, "mykeyring"), false, nil, nil)
+	configFilePath, err := GenerateConfigFile(context, clusterInfo, configDir, "myuser", filepath.Join(configDir, "mykeyring"), nil, nil)
 	assert.Nil(t, err)
 	assert.Equal(t, filepath.Join(configDir, "foo-cluster.config"), configFilePath)
 
@@ -121,7 +115,7 @@ debug bluestore = 1234`
 	verifyConfigValue(t, actualConf, "global", "debug bluestore", "1234")
 }
 
-func verifyConfig(t *testing.T, cephConfig *cephConfig, expectedMonMembers, experimental, objectStore string, loggingLevel int) {
+func verifyConfig(t *testing.T, cephConfig *cephConfig, expectedMonMembers string, loggingLevel int) {
 
 	for _, expectedMon := range strings.Split(expectedMonMembers, " ") {
 		contained := false
@@ -135,8 +129,6 @@ func verifyConfig(t *testing.T, cephConfig *cephConfig, expectedMonMembers, expe
 		assert.True(t, contained, "expectedMons: %+v, actualMons: %+v", expectedMonMembers, cephConfig.MonMembers)
 	}
 
-	assert.Equal(t, experimental, cephConfig.EnableExperimental)
-	assert.Equal(t, objectStore, cephConfig.OsdObjectStore)
 	assert.Equal(t, loggingLevel, cephConfig.DebugLogDefaultLevel)
 	assert.Equal(t, loggingLevel, cephConfig.DebugLogMonLevel)
 	assert.Equal(t, loggingLevel, cephConfig.DebugLogRadosLevel)

pkg/ceph/osd/agent.go

@@ -361,7 +361,7 @@ func (a *OsdAgent) startOSD(context *clusterd.Context, config *osdConfig) error
 		}
 	} else {
 		// update the osd config file
-		err := writeConfigFile(config, context, a.cluster)
+		err := writeConfigFile(config, context, a.cluster, a.location)
 		if err != nil {
 			logger.Warningf("failed to update config file. %+v", err)
 		}

pkg/ceph/osd/daemon.go

@@ -19,6 +19,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"path"
 	"regexp"
 	"time"
 
@@ -38,13 +39,12 @@ const (
 var logger = capnslog.NewPackageLogger("github.com/rook/rook", "cephosd")
 
 func Run(context *clusterd.Context, agent *OsdAgent) error {
-	if err := setNodeName(context, agent.nodeName); err != nil {
-		// It's best effort so will not block creation of osd if there is a failure.
-		logger.Warningf("failed to set hostname: %+v", err)
-	}
+	// set the crush location in the osd config file
+	cephConfig := mon.CreateDefaultCephConfig(context, agent.cluster, path.Join(context.ConfigDir, agent.cluster.Name))
+	cephConfig.GlobalConfig.CrushLocation = agent.location
 
 	// write the latest config to the config dir
-	if err := mon.GenerateAdminConnectionConfig(context, agent.cluster); err != nil {
+	if err := mon.GenerateAdminConnectionConfigWithSettings(context, agent.cluster, cephConfig); err != nil {
 		return fmt.Errorf("failed to write connection config. %+v", err)
 	}
 
@@ -92,21 +92,6 @@ func Run(context *clusterd.Context, agent *OsdAgent) error {
 	return nil
 }
 
-// Set the name of the node. We don't want the name of the pod,
-// which would change if the pod is re-created.
-func setNodeName(context *clusterd.Context, nodeName string) error {
-	nodeName = strings.Replace(nodeName, ".", "-", -1)
-	if nodeName == "" {
-		return fmt.Errorf("node name is not set")
-	}
-
-	err := context.ProcMan.Run("", "hostname", nodeName)
-	if err != nil {
-		return fmt.Errorf("hostname failed to set: %+v", err)
-	}
-	return nil
-}
-
 func getAvailableDevices(context *clusterd.Context, desiredDevices string, metadataDevice string, usingDeviceFilter bool) (*DeviceOsdMapping, error) {
 
 	var deviceList []string

pkg/ceph/osd/daemon_test.go

@@ -30,18 +30,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestSetNodeName(t *testing.T) {
-	executor := &exectest.MockExecutor{}
-	executor.MockExecuteCommand = func(debug bool, name string, command string, args ...string) error {
-		assert.Equal(t, "hostname", command)
-		assert.Equal(t, args[0], "myhost")
-		return nil
-	}
-
-	context := &clusterd.Context{ProcMan: proc.New(executor), Executor: executor}
-	setNodeName(context, "myhost")
-}
-
 func TestStoreOSDDirMap(t *testing.T) {
 	configDir, _ := ioutil.TempDir("", "")
 	defer os.RemoveAll(configDir)

pkg/ceph/osd/device.go

@@ -471,8 +471,14 @@ func getStoreSettings(config *osdConfig) (map[string]string, error) {
 	return settings, nil
 }
 
-func writeConfigFile(config *osdConfig, context *clusterd.Context, cluster *mon.ClusterInfo) error {
-	cephConfig := mon.CreateDefaultCephConfig(context, cluster, config.rootPath, isBluestore(config))
+func writeConfigFile(config *osdConfig, context *clusterd.Context, cluster *mon.ClusterInfo, location string) error {
+	cephConfig := mon.CreateDefaultCephConfig(context, cluster, config.rootPath)
+	if isBluestore(config) {
+		cephConfig.GlobalConfig.OsdObjectStore = Bluestore
+	} else {
+		cephConfig.GlobalConfig.OsdObjectStore = Filestore
+	}
+	cephConfig.CrushLocation = location
 
 	if config.dir || isFilestoreDevice(config) {
 		// using the local file system requires some config overrides
@@ -489,7 +495,7 @@ func writeConfigFile(config *osdConfig, context *clusterd.Context, cluster *mon.
 
 	// write the OSD config file to disk
 	_, err = mon.GenerateConfigFile(context, cluster, config.rootPath, fmt.Sprintf("osd.%d", config.id),
-		getOSDKeyringPath(config.rootPath), isBluestore(config), cephConfig, settings)
+		getOSDKeyringPath(config.rootPath), cephConfig, settings)
 	if err != nil {
 		return fmt.Errorf("failed to write OSD %d config file: %+v", config.id, err)
 	}
@@ -499,7 +505,7 @@ func writeConfigFile(config *osdConfig, context *clusterd.Context, cluster *mon.
 
 func initializeOSD(config *osdConfig, context *clusterd.Context, cluster *mon.ClusterInfo, location string) error {
 
-	err := writeConfigFile(config, context, cluster)
+	err := writeConfigFile(config, context, cluster, location)
 	if err != nil {
 		return fmt.Errorf("failed to write config file: %+v", err)
 	}
@@ -611,15 +617,10 @@ func addOSDToCrushMap(context *clusterd.Context, config *osdConfig, clusterName,
 	weight, _ = strconv.ParseFloat(fmt.Sprintf("%.4f", weight), 64)
 
 	osdEntity := fmt.Sprintf("osd.%d", osdID)
-	locArgs, err := client.FormatLocation(location)
-	if err != nil {
-		return err
-	}
-
 	logger.Infof("adding %s (%s), bytes: %d, weight: %.4f, to crush map at '%s'",
-		osdEntity, osdDataPath, totalBytes, weight, strings.Join(locArgs, " "))
+		osdEntity, osdDataPath, totalBytes, weight, location)
 	args := []string{"osd", "crush", "create-or-move", strconv.Itoa(osdID), fmt.Sprintf("%.4f", weight)}
-	args = append(args, locArgs...)
+	args = append(args, strings.Split(location, " ")...)
 	_, err = client.ExecuteCephCommand(context, clusterName, args)
 	if err != nil {
 		return fmt.Errorf("failed adding %s to crush map: %+v", osdEntity, err)

pkg/ceph/rgw/daemon.go

@@ -98,7 +98,7 @@ func generateConfigFiles(context *clusterd.Context, config *Config) error {
 		"rgw_zonegroup":                  config.Name,
 	}
 	_, err := mon.GenerateConfigFile(context, config.ClusterInfo, getRGWConfDir(context.ConfigDir),
-		"client.radosgw.gateway", getRGWKeyringPath(context.ConfigDir), false, nil, settings)
+		"client.radosgw.gateway", getRGWKeyringPath(context.ConfigDir), nil, settings)
 	if err != nil {
 		return fmt.Errorf("failed to create config file. %+v", err)
 	}