extend lending limit unit tests to cover node deletion

Author: dgrove-oss

internal/controller/appwrapper/node_health_monitor_test.go

@@ -37,19 +37,26 @@ var _ = Describe("NodeMonitor Controller", func() {
 	var cqMonitor *SlackClusterQueueMonitor
 	nodeGPUs := v1.ResourceList{v1.ResourceName("nvidia.com/gpu"): resource.MustParse("4")}
 
-	BeforeEach(func() {
-		for _, nodeName := range []string{node1Name.Name, node2Name.Name} {
-			node := &v1.Node{
-				TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Node"},
-				ObjectMeta: metav1.ObjectMeta{Name: nodeName, Labels: map[string]string{"key1": "value1"}},
-			}
-			Expect(k8sClient.Create(ctx, node)).To(Succeed())
-			node = getNode(nodeName)
-			node.Status.Capacity = nodeGPUs
-			Expect(k8sClient.Status().Update(ctx, node)).To(Succeed())
+	createNode := func(nodeName string) {
+		node := &v1.Node{
+			TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Node"},
+			ObjectMeta: metav1.ObjectMeta{Name: nodeName, Labels: map[string]string{"key1": "value1"}},
 		}
+		Expect(k8sClient.Create(ctx, node)).To(Succeed())
+		node = getNode(nodeName)
+		node.Status.Capacity = nodeGPUs
+		Expect(k8sClient.Status().Update(ctx, node)).To(Succeed())
+	}
+
+	deleteNode := func(nodeName string) {
+		Expect(k8sClient.Delete(ctx, &v1.Node{
+			TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Node"},
+			ObjectMeta: metav1.ObjectMeta{Name: nodeName},
+		})).To(Succeed())
+	}
 
-		// Create reconciller
+	BeforeEach(func() {
+		// Create reconcillers
 		awConfig := config.NewAppWrapperConfig()
 		awConfig.SlackQueueName = slackQueueName
 		conduit := make(chan event.GenericEvent, 1)
@@ -66,18 +73,14 @@ var _ = Describe("NodeMonitor Controller", func() {
 	})
 
 	AfterEach(func() {
-		Expect(k8sClient.Delete(ctx, &v1.Node{
-			TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Node"},
-			ObjectMeta: metav1.ObjectMeta{Name: node1Name.Name},
-		})).To(Succeed())
-		Expect(k8sClient.Delete(ctx, &v1.Node{
-			TypeMeta:   metav1.TypeMeta{APIVersion: "v1", Kind: "Node"},
-			ObjectMeta: metav1.ObjectMeta{Name: node2Name.Name},
-		})).To(Succeed())
 		nodeMonitor = nil
+		cqMonitor = nil
 	})
 
 	It("Autopilot Monitoring", func() {
+		createNode(node1Name.Name)
+		createNode(node2Name.Name)
+
 		_, err := nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node1Name})
 		Expect(err).NotTo(HaveOccurred())
 		_, err = nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node2Name})
@@ -113,9 +116,15 @@ var _ = Describe("NodeMonitor Controller", func() {
 		_, err = nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node1Name})
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(noExecuteNodes)).Should(Equal(0))
+
+		deleteNode(node1Name.Name)
+		deleteNode(node2Name.Name)
 	})
 
 	It("ClusterQueue Lending Adjustment", func() {
+		createNode(node1Name.Name)
+		createNode(node2Name.Name)
+
 		_, err := nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node1Name})
 		Expect(err).NotTo(HaveOccurred())
 		_, err = nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node2Name})
@@ -188,7 +197,7 @@ var _ = Describe("NodeMonitor Controller", func() {
 		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: slackQueueName}, queue)).Should(Succeed())
 		Expect(queue.Spec.ResourceGroups[0].Flavors[0].Resources[0].LendingLimit.Value()).Should(Equal(int64(2)))
 
-		// Increase the slack cluster queue's quota by 2 and expect LedningLimit to increase by 2 to become 4
+		// Increase the slack cluster queue's quota by 2 and expect LendngLimit to increase by 2 to become 4
 		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: slackQueueName}, queue)).Should(Succeed())
 		queue.Spec.ResourceGroups[0].Flavors[0].Resources[0].NominalQuota = resource.MustParse("8")
 		Expect(k8sClient.Update(ctx, queue)).Should(Succeed())
@@ -198,6 +207,26 @@ var _ = Describe("NodeMonitor Controller", func() {
 		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: slackQueueName}, queue)).Should(Succeed())
 		Expect(queue.Spec.ResourceGroups[0].Flavors[0].Resources[0].LendingLimit.Value()).Should(Equal(int64(4)))
 
+		// Deleting a noncordoned node should not change the lending limit
+		deleteNode(node2Name.Name)
+		_, err = nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node2Name})
+		Expect(err).NotTo(HaveOccurred())
+		_, err = cqMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: dispatch})
+		Expect(err).NotTo(HaveOccurred())
+
+		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: slackQueueName}, queue)).Should(Succeed())
+		Expect(queue.Spec.ResourceGroups[0].Flavors[0].Resources[0].LendingLimit.Value()).Should(Equal(int64(4)))
+
+		// Delete the cordoned node; lending limit should now by nil
+		deleteNode(node1Name.Name)
+		_, err = nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node1Name})
+		Expect(err).NotTo(HaveOccurred())
+		_, err = cqMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: dispatch})
+		Expect(err).NotTo(HaveOccurred())
+
+		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: slackQueueName}, queue)).Should(Succeed())
+		Expect(queue.Spec.ResourceGroups[0].Flavors[0].Resources[0].LendingLimit).Should(BeNil())
+
 		Expect(k8sClient.Delete(ctx, queue)).To(Succeed())
 	})
 })