ceph: judge whether an OSD is on PVC or not by function

There are many duplicated code to judge whether an OSD is on PVC
or not. Instead, let's introduce a function for this purpose.
It can also be applied to the logic of judging whether an OSD
on PVC has metadata device or not.

Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>

pkg/operator/ceph/cluster/osd/osd.go

@@ -177,6 +177,14 @@ type osdProperties struct {
 	crushDeviceClass    string
 }
 
+func (osdProps osdProperties) onPVC() bool {
+	return osdProps.pvc.ClaimName != ""
+}
+
+func (osdProps osdProperties) onPVCWithMetadata() bool {
+	return osdProps.metadataPVC.ClaimName != ""
+}
+
 // Start the osd management
 func (c *Cluster) Start() error {
 	config := c.newProvisionConfig()

pkg/operator/ceph/cluster/osd/osd_test.go

@@ -40,6 +40,23 @@ import (
 	k8stesting "k8s.io/client-go/testing"
 )
 
+func TestOSDProperties(t *testing.T) {
+	osdProps := []osdProperties{
+		{pvc: v1.PersistentVolumeClaimVolumeSource{ClaimName: "claim"},
+			metadataPVC: v1.PersistentVolumeClaimVolumeSource{ClaimName: "claim"}},
+		{pvc: v1.PersistentVolumeClaimVolumeSource{ClaimName: ""},
+			metadataPVC: v1.PersistentVolumeClaimVolumeSource{ClaimName: ""}},
+	}
+	expected := [][2]bool{
+		{true, true},
+		{false, false},
+	}
+	for i, p := range osdProps {
+		actual := [2]bool{p.onPVC(), p.onPVCWithMetadata()}
+		assert.Equal(t, expected[i], actual, "detected a problem in `expected[%d]`", i)
+	}
+}
+
 func TestStart(t *testing.T) {
 	clientset := fake.NewSimpleClientset()
 	clusterInfo := &cephconfig.ClusterInfo{

pkg/operator/ceph/cluster/osd/spec.go

@@ -125,21 +125,18 @@ const (
 )
 
 func (c *Cluster) makeJob(osdProps osdProperties, provisionConfig *provisionConfig) (*batch.Job, error) {
-	osdOnPVC := osdProps.pvc.ClaimName != ""
-	osdOnPVCMetadata := osdProps.metadataPVC.ClaimName != ""
-
 	podSpec, err := c.provisionPodTemplateSpec(osdProps, v1.RestartPolicyOnFailure, provisionConfig)
 	if err != nil {
 		return nil, err
 	}
 
-	if !osdOnPVC {
+	if !osdProps.onPVC() {
 		podSpec.Spec.NodeSelector = map[string]string{v1.LabelHostname: osdProps.crushHostname}
 	} else {
 		// This is not needed in raw mode and 14.2.8 brings it
 		// but we still want to do this not to lose backward compatibility with lvm based OSDs...
 		podSpec.Spec.InitContainers = append(podSpec.Spec.InitContainers, c.getPVCInitContainer(osdProps))
-		if osdOnPVCMetadata {
+		if osdProps.onPVCWithMetadata() {
 			podSpec.Spec.InitContainers = append(podSpec.Spec.InitContainers, c.getPVCMetadataInitContainer("/srv", osdProps))
 		}
 	}
@@ -158,7 +155,7 @@ func (c *Cluster) makeJob(osdProps osdProperties, provisionConfig *provisionConf
 		},
 	}
 
-	if osdOnPVC {
+	if osdProps.onPVC() {
 		k8sutil.AddLabelToJob(OSDOverPVCLabelKey, osdProps.pvc.ClaimName, job)
 	}
 
@@ -175,10 +172,8 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	replicaCount := int32(1)
 	volumeMounts := controller.CephVolumeMounts(provisionConfig.DataPathMap, false)
 	configVolumeMounts := controller.RookVolumeMounts(provisionConfig.DataPathMap, false)
-	osdOnPVC := osdProps.pvc.ClaimName != ""
-	osdOnPVCMetadata := osdProps.metadataPVC.ClaimName != ""
 	// When running on PVC, OSDs don't need a bindmount on dataDirHostPath, only the monitors do
-	if osdOnPVC {
+	if osdProps.onPVC() {
 		c.dataDirHostPath = ""
 	}
 	volumes := controller.PodVolumes(provisionConfig.DataPathMap, c.dataDirHostPath, false)
@@ -205,7 +200,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	}
 
 	// If the OSD runs on PVC
-	if osdOnPVC {
+	if osdProps.onPVC() {
 		// Create volume config for PVCs
 		volumes = append(volumes, getPVCOSDVolumes(&osdProps)...)
 	}
@@ -242,7 +237,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	var commonArgs []string
 
 	// If the OSD runs on PVC
-	if osdOnPVC && osdProps.tuneSlowDeviceClass {
+	if osdProps.onPVC() && osdProps.tuneSlowDeviceClass {
 		// Append tuning flag if necessary
 		err := c.osdRunFlagTuningOnPVC(osd.ID)
 		if err != nil {
@@ -262,7 +257,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	var command []string
 	var args []string
 	// If the OSD was prepared with ceph-volume and running on PVC and using the LVM mode
-	if osdOnPVC && osd.CVMode == "lvm" {
+	if osdProps.onPVC() && osd.CVMode == "lvm" {
 		// if the osd was provisioned by ceph-volume, we need to launch it with rook as the parent process
 		command = []string{path.Join(rookBinariesMountPath, "tini")}
 		args = []string{
@@ -277,7 +272,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 			"--setgroup", "ceph",
 			fmt.Sprintf("--crush-location=%s", osd.Location),
 		}
-	} else if osdOnPVC && osd.CVMode == "raw" {
+	} else if osdProps.onPVC() && osd.CVMode == "raw" {
 		doBinaryCopyInit = false
 		doConfigInit = false
 		command = []string{"ceph-osd"}
@@ -329,7 +324,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	args = append(args, commonArgs...)
 
 	osdDataDirPath := activateOSDMountPath + osdID
-	if osdOnPVC && osd.CVMode == "lvm" {
+	if osdProps.onPVC() && osd.CVMode == "lvm" {
 		// Let's use the old bridge for these lvm based pvc osds
 		volumeMounts = append(volumeMounts, getPvcOSDBridgeMount(osdProps.pvc.ClaimName))
 		envVars = append(envVars, pvcBackedOSDEnvVar("true"))
@@ -338,7 +333,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 		envVars = append(envVars, lvBackedPVEnvVar(strconv.FormatBool(osd.LVBackedPV)))
 	}
 
-	if osdOnPVC && osd.CVMode == "raw" {
+	if osdProps.onPVC() && osd.CVMode == "raw" {
 		volumeMounts = append(volumeMounts, getPvcOSDBridgeMountActivate(osdDataDirPath, osdProps.pvc.ClaimName))
 		envVars = append(envVars, pvcBackedOSDEnvVar("true"))
 		envVars = append(envVars, blockPathEnvVariable(osd.BlockPath))
@@ -376,13 +371,13 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	if doBinaryCopyInit {
 		initContainers = append(initContainers, *copyBinariesContainer)
 	}
-	if osdOnPVC && osd.CVMode == "lvm" {
+	if osdProps.onPVC() && osd.CVMode == "lvm" {
 		initContainers = append(initContainers, c.getPVCInitContainer(osdProps))
 	}
 
-	if osdOnPVC && osd.CVMode == "raw" {
+	if osdProps.onPVC() && osd.CVMode == "raw" {
 		initContainers = append(initContainers, c.getPVCInitContainerActivate(osdDataDirPath, osdProps))
-		if osdOnPVCMetadata {
+		if osdProps.onPVCWithMetadata() {
 			initContainers = append(initContainers, c.getPVCMetadataInitContainerActivate(osdDataDirPath, osdProps))
 		}
 		initContainers = append(initContainers, c.getActivatePVCInitContainer(osdProps, osdID))
@@ -400,7 +395,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	dataPath := ""
 
 	// Raw mode on PVC needs this path so that OSD's metadata files can be chown after 'ceph-bluestore-tool' ran
-	if osd.CVMode == "raw" && osdOnPVC {
+	if osd.CVMode == "raw" && osdProps.onPVC() {
 		dataPath = activateOSDMountPath + osdID
 	}
 
@@ -475,7 +470,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 			Replicas: &replicaCount,
 		},
 	}
-	if osdOnPVC {
+	if osdProps.onPVC() {
 		k8sutil.AddLabelToDeployement(OSDOverPVCLabelKey, osdProps.pvc.ClaimName, deployment)
 		k8sutil.AddLabelToPod(OSDOverPVCLabelKey, osdProps.pvc.ClaimName, &deployment.Spec.Template)
 	}
@@ -483,7 +478,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 		deployment.Spec.Template.Spec.NodeSelector = map[string]string{v1.LabelHostname: osdProps.crushHostname}
 	}
 	// Replace default unreachable node toleration if the osd pod is portable and based in PVC
-	if osdOnPVC && osdProps.portable {
+	if osdProps.onPVC() && osdProps.portable {
 		k8sutil.AddUnreachableNodeToleration(&deployment.Spec.Template.Spec)
 	}
 
@@ -493,7 +488,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC
 	controller.AddCephVersionLabelToDeployment(c.clusterInfo.CephVersion, deployment)
 	controller.AddCephVersionLabelToDeployment(c.clusterInfo.CephVersion, deployment)
 	k8sutil.SetOwnerRef(&deployment.ObjectMeta, &c.ownerRef)
-	if !osdOnPVC {
+	if !osdProps.onPVC() {
 		c.placement.ApplyToPodSpec(&deployment.Spec.Template.Spec)
 	} else {
 		osdProps.placement.ApplyToPodSpec(&deployment.Spec.Template.Spec)
@@ -521,7 +516,6 @@ func (c *Cluster) getCopyBinariesContainer() (v1.Volume, *v1.Container) {
 
 // This container runs all the actions needed to activate an OSD before we can run the OSD process
 func (c *Cluster) getActivateOSDInitContainer(osdID string, osdInfo OSDInfo, osdProps osdProperties) (v1.Volume, *v1.Container) {
-	osdOnPVC := osdProps.pvc.ClaimName != ""
 	volume := v1.Volume{Name: activateOSDVolumeName, VolumeSource: v1.VolumeSource{EmptyDir: &v1.EmptyDirVolumeSource{}}}
 	envVars := osdActivateEnvVar()
 	osdStore := "--bluestore"
@@ -535,7 +529,7 @@ func (c *Cluster) getActivateOSDInitContainer(osdID string, osdInfo OSDInfo, osd
 		{Name: k8sutil.ConfigOverrideName, ReadOnly: true, MountPath: opconfig.EtcCephDir},
 	}
 
-	if osdOnPVC {
+	if osdProps.onPVC() {
 		volMounts = append(volMounts, getPvcOSDBridgeMount(osdProps.pvc.ClaimName))
 	}
 
@@ -557,7 +551,6 @@ func (c *Cluster) getActivateOSDInitContainer(osdID string, osdInfo OSDInfo, osd
 }
 
 func (c *Cluster) provisionPodTemplateSpec(osdProps osdProperties, restart v1.RestartPolicy, provisionConfig *provisionConfig) (*v1.PodTemplateSpec, error) {
-	osdOnPVC := osdProps.pvc.ClaimName != ""
 	copyBinariesVolume, copyBinariesContainer := c.getCopyBinariesContainer()
 
 	// ceph-volume is currently set up to use /etc/ceph/ceph.conf; this means no user config
@@ -570,7 +563,7 @@ func (c *Cluster) provisionPodTemplateSpec(osdProps osdProperties, restart v1.Re
 	udevVolume := v1.Volume{Name: "udev", VolumeSource: v1.VolumeSource{HostPath: &v1.HostPathVolumeSource{Path: "/run/udev"}}}
 	volumes = append(volumes, udevVolume)
 
-	if osdOnPVC {
+	if osdProps.onPVC() {
 		// Create volume config for PVCs
 		volumes = append(volumes, getPVCOSDVolumes(&osdProps)...)
 	}
@@ -595,7 +588,7 @@ func (c *Cluster) provisionPodTemplateSpec(osdProps osdProperties, restart v1.Re
 	if c.Network.IsHost() {
 		podSpec.DNSPolicy = v1.DNSClusterFirstWithHostNet
 	}
-	if !osdOnPVC {
+	if !osdProps.onPVC() {
 		c.placement.ApplyToPodSpec(&podSpec)
 	} else {
 		osdProps.placement.ApplyToPodSpec(&podSpec)
@@ -772,7 +765,6 @@ func (c *Cluster) getExpandPVCInitContainer(osdProps osdProperties, osdID string
 	return container
 }
 func (c *Cluster) getConfigEnvVars(osdProps osdProperties, dataDir string) []v1.EnvVar {
-	osdOnPVC := osdProps.pvc.ClaimName != ""
 	envVars := []v1.EnvVar{
 		nodeNameEnvVar(osdProps.crushHostname),
 		{Name: "ROOK_CLUSTER_ID", Value: string(c.ownerRef.UID)},
@@ -795,7 +787,7 @@ func (c *Cluster) getConfigEnvVars(osdProps osdProperties, dataDir string) []v1.
 
 	// Give a hint to the prepare pod for what the host in the CRUSH map should be
 	crushmapHostname := osdProps.crushHostname
-	if !osdProps.portable && osdOnPVC {
+	if !osdProps.portable && osdProps.onPVC() {
 		// 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 = ""
 	}
@@ -824,8 +816,6 @@ func (c *Cluster) getConfigEnvVars(osdProps osdProperties, dataDir string) []v1.
 }
 
 func (c *Cluster) provisionOSDContainer(osdProps osdProperties, copyBinariesMount v1.VolumeMount, provisionConfig *provisionConfig) v1.Container {
-	osdOnPVC := osdProps.pvc.ClaimName != ""
-	osdOnPVCMetadata := osdProps.metadataPVC.ClaimName != ""
 	envVars := c.getConfigEnvVars(osdProps, k8sutil.DataDir)
 
 	// enable debug logging in the prepare job
@@ -887,11 +877,11 @@ func (c *Cluster) provisionOSDContainer(osdProps osdProperties, copyBinariesMoun
 	}...)
 
 	// If the OSD runs on PVC
-	if osdOnPVC {
+	if osdProps.onPVC() {
 		volumeMounts = append(volumeMounts, getPvcOSDBridgeMount(osdProps.pvc.ClaimName))
 		// The device list is read by the Rook CLI via environment variables so let's add them
 		dev := []string{fmt.Sprintf("/mnt/%s", osdProps.pvc.ClaimName)}
-		if osdOnPVCMetadata {
+		if osdProps.onPVCWithMetadata() {
 			volumeMounts = append(volumeMounts, getPvcMetadataOSDBridgeMount(osdProps.metadataPVC.ClaimName))
 			dev = append(dev, fmt.Sprintf("/srv/%s", osdProps.metadataPVC.ClaimName))
 		}
@@ -965,7 +955,7 @@ func getPVCOSDVolumes(osdProps *osdProperties) []v1.Volume {
 	}
 
 	// If we have a metadata PVC let's add it
-	if osdProps.metadataPVC.ClaimName != "" {
+	if osdProps.onPVCWithMetadata() {
 		metadataPVCVolume := []v1.Volume{
 			{
 				Name: osdProps.metadataPVC.ClaimName,