Skip draining controller pod

during drain operation the controller should skip its own pod to support single-node case where a single node is both control plane and worker. we achieve this by adding an implicit pod selector label to the drain request. Signed-off-by: adrianc <[email protected]>
Mellanox · Jan 22, 2025 · 84a04f5 · 84a04f5
1 parent d25eb29
commit 84a04f5
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 4 deletions.
diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -20,7 +20,7 @@ metadata:
     control-plane: controller-manager
     app.kubernetes.io/name: deployment
     app.kubernetes.io/instance: controller-manager
-    app.kubernetes.io/component: manager
+    app.kubernetes.io/component: maintenance-operator-controller-manager
     app.kubernetes.io/created-by: maintenance-operator
     app.kubernetes.io/part-of: maintenance-operator
     app.kubernetes.io/managed-by: kustomize

diff --git a/deployment/maintenance-operator-chart/templates/deployment.yaml b/deployment/maintenance-operator-chart/templates/deployment.yaml
@@ -4,7 +4,7 @@ metadata:
   name: {{ include "maintenance-operator.fullname" . }}
   namespace: {{ .Release.Namespace }}
   labels:
-    app.kubernetes.io/component: manager
+    app.kubernetes.io/component:  maintenance-operator-controller-manager
     app.kubernetes.io/created-by: maintenance-operator
     app.kubernetes.io/part-of: maintenance-operator
     control-plane: {{ .Release.Name }}-controller-manager
@@ -19,6 +19,7 @@ spec:
     metadata:
       labels:
         control-plane: {{ .Release.Name }}-controller-manager
+        app.kubernetes.io/component:  maintenance-operator-controller-manager
       {{- include "maintenance-operator.selectorLabels" . | nindent 8 }}
       annotations:
         kubectl.kubernetes.io/default-container: manager

diff --git a/internal/drain/request.go b/internal/drain/request.go
@@ -200,7 +200,7 @@ func (dr *drainRequestImpl) createDrainHelper() *drain.Helper {
 		IgnoreAllDaemonSets: true,
 		Timeout:             time.Duration(dr.drainSpec.Spec.TimeoutSecond) * time.Second,
 		DeleteEmptyDirData:  dr.drainSpec.Spec.DeleteEmptyDir,
-		PodSelector:         dr.drainSpec.Spec.PodSelector,
+		PodSelector:         getPodSelectorForDrain(dr.drainSpec.Spec.PodSelector),
 
 		// print args
 		Out:    newLogInfoWriter(dr.log.Info),
@@ -276,3 +276,13 @@ func drainFilterFromPodEvictionFilters(f []maintenancev1.PodEvictionFiterEntry)
 		}
 	}
 }
+
+// getPodSelectorForDrain returns the pod selector to be used for drain operation
+// it appends the default pod selector to skip the controller manager pod
+func getPodSelectorForDrain(ps string) string {
+	defaultSelector := "app.kubernetes.io/component!=maintenance-operator-controller-manager"
+	if ps == "" {
+		return defaultSelector
+	}
+	return fmt.Sprintf("%s,%s", defaultSelector, ps)
+}
diff --git a/internal/drain/request_test.go b/internal/drain/request_test.go
@@ -50,6 +50,8 @@ var _ = Describe("DrainRequest tests", func() {
 		pod = testutils.GetTestPod("test-pod-1", "node-0", map[string]string{"foo": "bar"})
 		podWithResource := testutils.GetTestPod("test-pod-2", "node-0", nil)
 		podWithResource.Spec.Containers[0].Resources.Requests = map[corev1.ResourceName]resource.Quantity{"nvidia.com/rdma-vf": {}}
+		controllerPod := testutils.GetTestPod("node-maintenance-controller", "node-0", map[string]string{
+			"app.kubernetes.io/component": "maintenance-operator-controller-manager"})
 
 		nm = testutils.GetTestNodeMaintenance("test-nm", node.Name, "test.nvidia.com", "")
 		nm.Spec.DrainSpec = &maintenancev1.DrainSpec{
@@ -58,7 +60,7 @@ var _ = Describe("DrainRequest tests", func() {
 			TimeoutSecond: 10,
 		}
 
-		fakeInterface = ks8fake.NewSimpleClientset(node, pod, podWithResource)
+		fakeInterface = ks8fake.NewSimpleClientset(node, pod, podWithResource, controllerPod)
 		// since we use fake client, we need to disable eviction for drain op to succeed.
 		// in addition, fake client doesnt take into account context so no tests that rely
 		// on ctx being cancelled cannot be tested.
@@ -105,6 +107,44 @@ var _ = Describe("DrainRequest tests", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(ds.State).To(Equal(drain.DrainStateSuccess))
 			Expect(ds.PodsToDelete).To(HaveLen(0))
+
+			pl, err := fakeInterface.CoreV1().Pods("default").List(testCtx, metav1.ListOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pl.Items).To(HaveLen(1))
+			// expect node-maintenance-controller pod to remain as we skip it via podSelector implicitly in drain
+			_, err = fakeInterface.CoreV1().Pods("default").Get(testCtx, "node-maintenance-controller", metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
+		})
+	})
+
+	Context("PodSelector", func() {
+		BeforeEach(func() {
+			nm.Spec.DrainSpec.PodSelector = "foo=bar"
+			drainReq = drain.NewDrainRequest(testCtx, ctrllog.Log.WithName("test-drain"), fakeInterface, nm, true)
+		})
+
+		It("Returns expected status", func() {
+			ds, err := drainReq.Status()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(ds.PodsToDelete).To(HaveLen(1))
+			Expect(ds.PodsToDelete).To(ConsistOf("default/test-pod-1"))
+		})
+
+		It("Drains only pods targeted by podSelector", func() {
+			drainReq.StartDrain()
+			Eventually(drainReq.State).WithTimeout(1 * time.Second).WithPolling(100 * time.Millisecond).Should(Equal(drain.DrainStateSuccess))
+			ds, err := drainReq.Status()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(ds.State).To(Equal(drain.DrainStateSuccess))
+			Expect(ds.PodsToDelete).To(HaveLen(0))
+
+			pl, err := fakeInterface.CoreV1().Pods("default").List(testCtx, metav1.ListOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pl.Items).To(HaveLen(2))
+			_, err = fakeInterface.CoreV1().Pods("default").Get(testCtx, "test-pod-2", metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			_, err = fakeInterface.CoreV1().Pods("default").Get(testCtx, "node-maintenance-controller", metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
 		})
 	})
 
@@ -132,8 +172,13 @@ var _ = Describe("DrainRequest tests", func() {
 			Expect(ds.State).To(Equal(drain.DrainStateSuccess))
 			Expect(ds.PodsToDelete).To(HaveLen(0))
 
+			pl, err := fakeInterface.CoreV1().Pods("default").List(testCtx, metav1.ListOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pl.Items).To(HaveLen(2))
 			_, err = fakeInterface.CoreV1().Pods("default").Get(testCtx, "test-pod-1", metav1.GetOptions{})
 			Expect(err).ToNot(HaveOccurred())
+			_, err = fakeInterface.CoreV1().Pods("default").Get(testCtx, "node-maintenance-controller", metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
 		})
 	})