From ed2d52e56a7a70a4c5284b6ab3d34f83c9408d6c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 15 Jul 2024 15:45:51 +0200 Subject: [PATCH] Ignore terminating nodes (#341) * yawol-cloud-controller: ignore terminating nodes * test that terminating nodes are ignored * address review comments --- .../targetcontroller/node_controller.go | 25 +++- .../targetcontroller/node_controller_test.go | 123 +++++++++++++++--- 2 files changed, 126 insertions(+), 22 deletions(-) diff --git a/controllers/yawol-cloud-controller/targetcontroller/node_controller.go b/controllers/yawol-cloud-controller/targetcontroller/node_controller.go index bb365116..df04745b 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/node_controller.go +++ b/controllers/yawol-cloud-controller/targetcontroller/node_controller.go @@ -21,6 +21,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" ) +// nolint: lll // these are links... +const ( + // From https://github.com/kubernetes/cloud-provider/blob/81e4f58b4d1badd71d633d356faaaf69d971d874/controllers/service/controller.go#L64C2-L64C53 + ToBeDeletedTaint = "ToBeDeletedByClusterAutoscaler" + // From https://github.com/gardener/machine-controller-manager/blob/fc341881a5e71d7c5f240ca73415f967084aa85b/pkg/util/provider/machineutils/utils.go#L61 + NodeTerminationCondition coreV1.NodeConditionType = "Terminating" +) + // NodeReconciler reconciles service Objects with type LoadBalancer type NodeReconciler struct { TargetClient client.Client @@ -178,7 +186,7 @@ func getReadyEndpointsFromNodes( // TODO check if ipFamilies and IPFamilyPolicyType is available? eps := make([]yawolv1beta1.LoadBalancerEndpoint, 0) for i := range nodes { - if !isNodeReady(nodes[i]) { + if !isNodeReady(nodes[i]) || isNodeTerminating(nodes[i]) { continue } eps = append(eps, getLoadBalancerEndpointFromNode(nodes[i], ipFamilies)) @@ -199,3 +207,18 @@ func isNodeReady(node coreV1.Node) bool { } return false } + +func isNodeTerminating(node coreV1.Node) bool { + for _, taint := range node.Spec.Taints { + if taint.Key == ToBeDeletedTaint { + return true + } + } + + for _, condition := range node.Status.Conditions { + if condition.Type == NodeTerminationCondition && condition.Status == coreV1.ConditionTrue { + return true + } + } + return false +} diff --git a/controllers/yawol-cloud-controller/targetcontroller/node_controller_test.go b/controllers/yawol-cloud-controller/targetcontroller/node_controller_test.go index 60c08be3..f1d56022 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/node_controller_test.go +++ b/controllers/yawol-cloud-controller/targetcontroller/node_controller_test.go @@ -19,7 +19,6 @@ import ( ) var _ = Describe("check controller-runtime predicate", func() { - nodeName := "node1" conditionReady := v1.NodeCondition{ Type: v1.NodeReady, Status: v1.ConditionTrue, @@ -39,7 +38,7 @@ var _ = Describe("check controller-runtime predicate", func() { baseNode := v1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: nodeName, + Name: "node1", Namespace: "default"}, Spec: v1.NodeSpec{}, Status: v1.NodeStatus{ @@ -148,12 +147,32 @@ var _ = Describe("Check loadbalancer reconcile", Serial, Ordered, func() { }) + assertLBWithOneEndpoint := func(nodeName string, nodeAddress string) { + GinkgoHelper() + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--node-test1", Namespace: "default"}, &lb) + if err != nil { + return err + } + if lb.Spec.Endpoints == nil || len(lb.Spec.Endpoints) != 1 { + return fmt.Errorf("no or more than one endpoint in LB found: %v", lb.Spec.Endpoints) + } + if len(lb.Spec.Endpoints[0].Addresses) != 1 { + return fmt.Errorf("no or more than one endpoint address in LB found: %v", lb.Spec.Endpoints[0].Addresses) + } + if lb.Spec.Endpoints[0].Name != nodeName || lb.Spec.Endpoints[0].Addresses[0] != nodeAddress { + return helper.ErrEndpointValuesWrong + + } + return nil + }, time.Second*15, time.Millisecond*500).Should(Succeed()) + } + It("Create node and check node", func() { By("create node") - nodeName := "node1" node := v1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: nodeName, + Name: "node1", Namespace: "default"}, Spec: v1.NodeSpec{}, Status: v1.NodeStatus{ @@ -181,23 +200,7 @@ var _ = Describe("Check loadbalancer reconcile", Serial, Ordered, func() { Expect(k8sClient.Create(ctx, &node)).Should(Succeed()) By("check node in LB") - Eventually(func() error { - err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--node-test1", Namespace: "default"}, &lb) - if err != nil { - return err - } - if lb.Spec.Endpoints == nil || len(lb.Spec.Endpoints) != 1 { - return fmt.Errorf("no or more than one endpoint in LB found: %v", lb.Spec.Endpoints) - } - if len(lb.Spec.Endpoints[0].Addresses) != 1 { - return fmt.Errorf("no or more than one endpoint address in LB found: %v", lb.Spec.Endpoints[0].Addresses) - } - if lb.Spec.Endpoints[0].Name == nodeName && - lb.Spec.Endpoints[0].Addresses[0] == "10.10.10.10" { - return nil - } - return helper.ErrEndpointValuesWrong - }, time.Second*15, time.Millisecond*500).Should(Succeed()) + assertLBWithOneEndpoint("node1", "10.10.10.10") By("check event for node sync") Eventually(func() error { eventList := v1.EventList{} @@ -215,6 +218,84 @@ var _ = Describe("Check loadbalancer reconcile", Serial, Ordered, func() { return helper.ErrNoEventFound }, time.Second*5, time.Millisecond*500).Should(Succeed()) }) + It("ignore terminating Node and check", func() { + By("create node") + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nodebrk", + Namespace: "default"}, + Spec: v1.NodeSpec{}, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: metav1.Time{}, + LastTransitionTime: metav1.Time{}, + Reason: "Ready", + Message: "Ready", + }, + { + Type: NodeTerminationCondition, + Status: v1.ConditionTrue, + LastHeartbeatTime: metav1.Time{}, + LastTransitionTime: metav1.Time{}, + Reason: "Terminating", + Message: "Terminating", + }, + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "10.10.10.42", + }, + }, + }, + } + Expect(k8sClient.Create(ctx, &node)).Should(Succeed()) + + By("check node in LB") + assertLBWithOneEndpoint("node1", "10.10.10.10") + }) + It("ignore tainted Node and check", func() { + By("create node") + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nodebrk2", + Namespace: "default"}, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + Effect: v1.TaintEffectNoSchedule, + Value: "123456789", // unix timestamp + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: metav1.Time{}, + LastTransitionTime: metav1.Time{}, + Reason: "Ready", + Message: "Ready", + }, + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "10.10.10.43", + }, + }, + }, + } + Expect(k8sClient.Create(ctx, &node)).Should(Succeed()) + + By("check node in LB") + assertLBWithOneEndpoint("node1", "10.10.10.10") + }) It("add Node and check", func() { By("create node")