From 65377cbe633c0ea2ff670bcd977ab04d79c1618e Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Wed, 6 May 2020 17:58:46 -0600 Subject: [PATCH] ceph: osd on pvc node assignment is node selector An OSD on a PVC when portable=false is assigned to a node with a node selector. The same node assignment is expected for the lifetime of the cluster. On subsequent reconciles, the operator was looking up the node assignment from the nodeName on the pod spec, which is not set if the pod is down. The operator needs to retrieve the assignment from the deployment spec node selector. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/cluster/osd/osd.go | 36 +++++++++++++--- pkg/operator/ceph/cluster/osd/osd_test.go | 50 +++++++++++++++++++++++ pkg/operator/ceph/cluster/osd/spec.go | 2 +- pkg/operator/k8sutil/deployment.go | 2 +- 4 files changed, 83 insertions(+), 7 deletions(-) diff --git a/pkg/operator/ceph/cluster/osd/osd.go b/pkg/operator/ceph/cluster/osd/osd.go index 9879ad920..0563e0e73 100644 --- a/pkg/operator/ceph/cluster/osd/osd.go +++ b/pkg/operator/ceph/cluster/osd/osd.go @@ -636,7 +636,8 @@ func (c *Cluster) getOSDPropsForPVC(pvcName string) (osdProperties, error) { portable: volumeSource.Portable, tuneSlowDeviceClass: volumeSource.TuneSlowDeviceClass, } - // If OSD isn't portable, we're getting the host name of the pod where the osd prepare job pod prepared the OSD. + // If OSD isn't portable, we're getting the host name either from the osd deployment that was already initialized + // or from the osd prepare job from initial creation. if !volumeSource.Portable { var err error osdProps.crushHostname, err = c.getPVCHostName(pvcName) @@ -650,21 +651,46 @@ func (c *Cluster) getOSDPropsForPVC(pvcName string) (osdProperties, error) { return osdProperties{}, errors.Errorf("no valid VolumeSource found for pvc %s", pvcName) } +// getPVCHostName finds the node where an OSD pod should be assigned with a node selector. +// First look for the node selector that was previously used for the OSD, or if a new OSD +// check for the assignment of the OSD prepare job. func (c *Cluster) getPVCHostName(pvcName string) (string, error) { listOpts := metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", OSDOverPVCLabelKey, pvcName)} - podList, err := c.context.Clientset.CoreV1().Pods(c.Namespace).List(listOpts) + + // Check for the existence of the OSD deployment where the node selector was applied + // in a previous reconcile. + deployments, err := c.context.Clientset.AppsV1().Deployments(c.Namespace).List(listOpts) if err != nil { - return "", err + return "", errors.Wrapf(err, "failed to get deployment for osd with pvc %q", pvcName) } - for _, pod := range podList.Items { + for _, d := range deployments.Items { + selectors := d.Spec.Template.Spec.NodeSelector + for label, value := range selectors { + if label == v1.LabelHostname { + return value, nil + } + } + } + + // Since the deployment wasn't found it must be a new deployment so look at the node + // assignment of the OSD prepare pod + pods, err := c.context.Clientset.CoreV1().Pods(c.Namespace).List(listOpts) + if err != nil { + return "", errors.Wrapf(err, "failed to get pod for osd with pvc %q", pvcName) + } + for _, pod := range pods.Items { name, err := k8sutil.GetNodeHostName(c.context.Clientset, pod.Spec.NodeName) if err != nil { logger.Warningf("falling back to node name %s since hostname not found for node", pod.Spec.NodeName) name = pod.Spec.NodeName } + if name == "" { + return "", errors.Errorf("node name not found on the osd pod %q", pod.Name) + } return name, nil } - return "", err + + return "", errors.Errorf("node selector not found on deployment for osd with pvc %q", pvcName) } func (c *Cluster) getOSDInfo(d *apps.Deployment) ([]OSDInfo, error) { diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index ddac3edab..801eda2dc 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/pkg/operator/ceph/cluster/osd/osd_test.go @@ -31,6 +31,7 @@ import ( "github.com/rook/rook/pkg/operator/k8sutil" exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" + apps "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -301,6 +302,55 @@ func TestAddNodeFailure(t *testing.T) { assert.NotNil(t, startErr) } +func TestGetPVCHostName(t *testing.T) { + clientset := fake.NewSimpleClientset() + c := &Cluster{context: &clusterd.Context{Clientset: clientset}, Namespace: "ns"} + pvcName := "test-pvc" + + // fail to get the host name when there is no pod or deployment + name, err := c.getPVCHostName(pvcName) + assert.Error(t, err) + assert.Equal(t, "", name) + + // Create a sample osd deployment + osdDeployment := &apps.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osd-23", + Namespace: c.Namespace, + Labels: c.getOSDLabels(23, "", true), + }, + } + k8sutil.AddLabelToDeployment(OSDOverPVCLabelKey, pvcName, osdDeployment) + osdDeployment.Spec.Template.Spec.NodeSelector = map[string]string{v1.LabelHostname: "testnode"} + + _, err = clientset.AppsV1().Deployments(c.Namespace).Create(osdDeployment) + assert.NoError(t, err) + + // get the host name based on the deployment + name, err = c.getPVCHostName(pvcName) + assert.NoError(t, err) + assert.Equal(t, "testnode", name) + + // delete the deployment and get the host name based on the pod + err = clientset.AppsV1().Deployments(c.Namespace).Delete(osdDeployment.Name, &metav1.DeleteOptions{}) + assert.NoError(t, err) + osdPod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osd-23", + Namespace: c.Namespace, + Labels: c.getOSDLabels(23, "", true), + }, + } + osdPod.Labels = map[string]string{OSDOverPVCLabelKey: pvcName} + osdPod.Spec.NodeName = "testnode" + _, err = clientset.CoreV1().Pods(c.Namespace).Create(osdPod) + assert.NoError(t, err) + + name, err = c.getPVCHostName(pvcName) + assert.NoError(t, err) + assert.Equal(t, "testnode", name) +} + func TestGetOSDInfo(t *testing.T) { c := New(&cephconfig.ClusterInfo{}, &clusterd.Context{}, "ns", "myversion", cephv1.CephVersionSpec{}, rookv1.StorageScopeSpec{}, "", rookv1.Placement{}, rookv1.Annotations{}, cephv1.NetworkSpec{}, diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index df2aac6b5..d0d35ba90 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -471,7 +471,7 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo, provisionC }, } if osdProps.onPVC() { - k8sutil.AddLabelToDeployement(OSDOverPVCLabelKey, osdProps.pvc.ClaimName, deployment) + k8sutil.AddLabelToDeployment(OSDOverPVCLabelKey, osdProps.pvc.ClaimName, deployment) k8sutil.AddLabelToPod(OSDOverPVCLabelKey, osdProps.pvc.ClaimName, &deployment.Spec.Template) } if !osdProps.portable { diff --git a/pkg/operator/k8sutil/deployment.go b/pkg/operator/k8sutil/deployment.go index dc9e95f97..c532db2cc 100644 --- a/pkg/operator/k8sutil/deployment.go +++ b/pkg/operator/k8sutil/deployment.go @@ -220,7 +220,7 @@ func AddRookVersionLabelToObjectMeta(meta *metav1.ObjectMeta) { addRookVersionLabel(meta.Labels) } -func AddLabelToDeployement(key, value string, d *v1.Deployment) { +func AddLabelToDeployment(key, value string, d *v1.Deployment) { if d == nil { return }