Skip to content

Commit

Permalink
Ignore terminating nodes (#341)
Browse files Browse the repository at this point in the history 
* yawol-cloud-controller: ignore terminating nodes

* test that terminating nodes are ignored

* address review comments
  • Loading branch information
@MichaelEischer
MichaelEischer authored Jul 15, 2024
1 parent a028aed commit ed2d52e
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 22 deletions.
25 changes: 24 additions & 1 deletion controllers/yawol-cloud-controller/targetcontroller/node_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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
}
123 changes: 102 additions & 21 deletions controllers/yawol-cloud-controller/targetcontroller/node_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
)

var _ = Describe("check controller-runtime predicate", func() {
nodeName := "node1"
conditionReady := v1.NodeCondition{
Type: v1.NodeReady,
Status: v1.ConditionTrue,
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{}
Expand All @@ -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")
Expand Down

0 comments on commit ed2d52e

Please sign in to comment.