From aacec0dcda3ad55d0224e540cfdb18963e78e8d1 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Fri, 10 Jan 2020 23:38:13 -0700 Subject: [PATCH 1/2] ceph: simplify arguments to getConfigEnvVars method The method to construct osd environment variables is simplified by passing a struct param instead of individual params Signed-off-by: Travis Nielsen --- pkg/operator/ceph/cluster/osd/spec.go | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index 82c7374e4..c2e02eb0e 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -208,7 +208,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC osdID := strconv.Itoa(osd.ID) tiniEnvVar := v1.EnvVar{Name: "TINI_SUBREAPER", Value: ""} - envVars := append(c.getConfigEnvVars(osdProps.storeConfig, dataDir, osdProps.crushHostname, osdProps.location), []v1.EnvVar{ + envVars := append(c.getConfigEnvVars(osdProps, dataDir), []v1.EnvVar{ tiniEnvVar, }...) envVars = append(envVars, k8sutil.ClusterDaemonEnvVars(c.cephVersion.Image)...) @@ -223,7 +223,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC Key: "mon_host"}}}, {Name: "CEPH_ARGS", Value: "-m $(ROOK_CEPH_MON_HOST)"}, }...) - configEnvVars := append(c.getConfigEnvVars(osdProps.storeConfig, dataDir, osdProps.crushHostname, osdProps.location), []v1.EnvVar{ + configEnvVars := append(c.getConfigEnvVars(osdProps, dataDir), []v1.EnvVar{ tiniEnvVar, {Name: "ROOK_OSD_ID", Value: osdID}, {Name: "ROOK_CEPH_VERSION", Value: c.clusterInfo.CephVersion.CephVersionFormatted()}, @@ -709,9 +709,9 @@ func (c *Cluster) getPVCInitContainer(pvc v1.PersistentVolumeClaimVolumeSource) } } -func (c *Cluster) getConfigEnvVars(storeConfig config.StoreConfig, dataDir, nodeName, location string) []v1.EnvVar { +func (c *Cluster) getConfigEnvVars(osdProps osdProperties, dataDir string) []v1.EnvVar { envVars := []v1.EnvVar{ - nodeNameEnvVar(nodeName), + nodeNameEnvVar(osdProps.crushHostname), {Name: "ROOK_CLUSTER_ID", Value: string(c.ownerRef.UID)}, k8sutil.PodIPEnvVar(k8sutil.PrivateIPEnvVar), k8sutil.PodIPEnvVar(k8sutil.PublicIPEnvVar), @@ -736,23 +736,23 @@ func (c *Cluster) getConfigEnvVars(storeConfig config.StoreConfig, dataDir, node // deliberately skip setting osdStoreEnvVarName (ROOK_OSD_STORE) as a quick means to deprecate // creating new disk-based Filestore OSDs - if storeConfig.DatabaseSizeMB != 0 { - envVars = append(envVars, v1.EnvVar{Name: osdDatabaseSizeEnvVarName, Value: strconv.Itoa(storeConfig.DatabaseSizeMB)}) + if osdProps.storeConfig.DatabaseSizeMB != 0 { + envVars = append(envVars, v1.EnvVar{Name: osdDatabaseSizeEnvVarName, Value: strconv.Itoa(osdProps.storeConfig.DatabaseSizeMB)}) } - if storeConfig.WalSizeMB != 0 { - envVars = append(envVars, v1.EnvVar{Name: osdWalSizeEnvVarName, Value: strconv.Itoa(storeConfig.WalSizeMB)}) + if osdProps.storeConfig.WalSizeMB != 0 { + envVars = append(envVars, v1.EnvVar{Name: osdWalSizeEnvVarName, Value: strconv.Itoa(osdProps.storeConfig.WalSizeMB)}) } - if storeConfig.JournalSizeMB != 0 { - envVars = append(envVars, v1.EnvVar{Name: osdJournalSizeEnvVarName, Value: strconv.Itoa(storeConfig.JournalSizeMB)}) + if osdProps.storeConfig.JournalSizeMB != 0 { + envVars = append(envVars, v1.EnvVar{Name: osdJournalSizeEnvVarName, Value: strconv.Itoa(osdProps.storeConfig.JournalSizeMB)}) } - if storeConfig.OSDsPerDevice != 0 { - envVars = append(envVars, v1.EnvVar{Name: osdsPerDeviceEnvVarName, Value: strconv.Itoa(storeConfig.OSDsPerDevice)}) + if osdProps.storeConfig.OSDsPerDevice != 0 { + envVars = append(envVars, v1.EnvVar{Name: osdsPerDeviceEnvVarName, Value: strconv.Itoa(osdProps.storeConfig.OSDsPerDevice)}) } - if storeConfig.EncryptedDevice { + if osdProps.storeConfig.EncryptedDevice { envVars = append(envVars, v1.EnvVar{Name: encryptedDeviceEnvVarName, Value: "true"}) } @@ -761,7 +761,7 @@ func (c *Cluster) getConfigEnvVars(storeConfig config.StoreConfig, dataDir, node func (c *Cluster) provisionOSDContainer(osdProps osdProperties, copyBinariesMount v1.VolumeMount, provisionConfig *provisionConfig) v1.Container { - envVars := c.getConfigEnvVars(osdProps.storeConfig, k8sutil.DataDir, osdProps.crushHostname, osdProps.location) + envVars := c.getConfigEnvVars(osdProps, k8sutil.DataDir) devMountNeeded := false if osdProps.pvc.ClaimName != "" { From 078e87a72254aae47fba8cb82066bccd9e7fab38 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Fri, 10 Jan 2020 23:41:11 -0700 Subject: [PATCH 2/2] ceph: fix non-portable osd crush host name The host name of an OSD in the CRUSH map should be the real host name for non-portable OSDs. It was incorrectly being set to the PVC name. Now the non-portable OSDs based on PVCs will corretly have the host name set to the node name. Signed-off-by: Travis Nielsen --- cmd/rook/ceph/osd.go | 26 ++++++++++++++++++++------ pkg/operator/ceph/cluster/osd/spec.go | 8 ++++++++ pkg/operator/k8sutil/node.go | 4 ++++ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/cmd/rook/ceph/osd.go b/cmd/rook/ceph/osd.go index 6c73e1085..770e8fff7 100644 --- a/cmd/rook/ceph/osd.go +++ b/cmd/rook/ceph/osd.go @@ -287,18 +287,32 @@ func commonOSDInit(cmd *cobra.Command) { clusterInfo.Monitors = mon.ParseMonEndpoints(cfg.monEndpoints) } +// use zone/region/hostname labels in the crushmap func getLocation(clientset kubernetes.Interface) (string, error) { - // Start with the host name in the CRUSH map - // Keep the fully qualified host name in the crush map, but replace the dots with dashes to satisfy ceph - hostName := client.NormalizeCrushName(cfg.nodeName) - locArgs := []string{"root=default", fmt.Sprintf("host=%s", hostName)} - - // use zone/region/hostname labels in the crushmap + // Get the node based on the immutable k8s node name nodeName := os.Getenv(k8sutil.NodeNameEnvVar) node, err := getNode(clientset, nodeName) if err != nil { return "", errors.Wrapf(err, "could not get the node for topology labels") } + + // get the value the operator instructed to use as the host name in the CRUSH map + hostNameLabel := os.Getenv("ROOK_CRUSHMAP_HOSTNAME") + + // If the operator did not pass a host name, look up the hostname label. + // This happens when the operator doesn't know on what node the osd will be assigned (non-portable PVCs). + if hostNameLabel == "" { + hostNameLabel, err = k8sutil.GetNodeHostNameLabel(node) + if err != nil { + return "", errors.Wrapf(err, "failed to get the host name label for node %q", node.Name) + } + } + + // Start with the host name in the CRUSH map + // Keep the fully qualified host name in the crush map, but replace the dots with dashes to satisfy ceph + hostName := client.NormalizeCrushName(hostNameLabel) + locArgs := []string{"root=default", fmt.Sprintf("host=%s", hostName)} + nodeLabels := node.GetLabels() updateLocationWithNodeLabels(&locArgs, nodeLabels) diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index c2e02eb0e..efb84b16f 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -730,6 +730,14 @@ func (c *Cluster) getConfigEnvVars(osdProps osdProperties, dataDir string) []v1. k8sutil.NodeEnvVar(), } + // Give a hint to the prepare pod for what the host in the CRUSH map should be + crushmapHostname := osdProps.crushHostname + if !osdProps.portable && osdProps.pvc.ClaimName != "" { + // If it's a pvc that's not portable we only know what the host name should be when inside the osd prepare pod + crushmapHostname = "" + } + envVars = append(envVars, v1.EnvVar{Name: "ROOK_CRUSHMAP_HOSTNAME", Value: crushmapHostname}) + // Append ceph-volume environment variables envVars = append(envVars, cephVolumeEnvVar()...) diff --git a/pkg/operator/k8sutil/node.go b/pkg/operator/k8sutil/node.go index 9b374253b..2e32a9192 100644 --- a/pkg/operator/k8sutil/node.go +++ b/pkg/operator/k8sutil/node.go @@ -119,6 +119,10 @@ func GetNodeHostName(clientset kubernetes.Interface, nodeName string) (string, e if err != nil { return "", err } + return GetNodeHostNameLabel(node) +} + +func GetNodeHostNameLabel(node *v1.Node) (string, error) { hostname, ok := node.Labels[v1.LabelHostname] if !ok { return "", fmt.Errorf("hostname not found on the node")