From 5dc909fd9358c3aa7a98ae7bc9d1ab6efe3f65d1 Mon Sep 17 00:00:00 2001 From: Tomy Guichard Date: Wed, 22 Jan 2025 15:56:25 +0100 Subject: [PATCH] fix: updateLoadBalancer only when all nodes are initialized --- scaleway/loadbalancers.go | 24 +++++++++++ scaleway/loadbalancers_test.go | 79 ++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/scaleway/loadbalancers.go b/scaleway/loadbalancers.go index 28d5656..fbf2ef0 100644 --- a/scaleway/loadbalancers.go +++ b/scaleway/loadbalancers.go @@ -28,6 +28,7 @@ import ( "golang.org/x/exp/slices" v1 "k8s.io/api/core/v1" + "k8s.io/cloud-provider/api" "k8s.io/klog/v2" scwipam "github.com/scaleway/scaleway-sdk-go/api/ipam/v1" @@ -582,6 +583,10 @@ func (l *loadbalancers) updateLoadBalancer(ctx context.Context, loadbalancer *sc return fmt.Errorf("invalid value for annotation %s: expected boolean", serviceAnnotationLoadBalancerExternallyManaged) } + if err := nodesInitialized(nodes); err != nil { + return err + } + nodes = filterNodes(service, nodes) if l.pnID != "" { respPN, err := l.api.ListLBPrivateNetworks(&scwlb.ZonedAPIListLBPrivateNetworksRequest{ @@ -1727,3 +1732,22 @@ func hasEqualLoadBalancerStaticIPs(service *v1.Service, lb *scwlb.LB) bool { return true } + +// nodesInitialized verifies that all nodes are initialized before using them as LoadBalancer targets. +func nodesInitialized(nodes []*v1.Node) error { + for _, node := range nodes { + // If node was created more than 3 minutes ago, we ignore it to + // avoid blocking callers indefinitely. + if time.Since(node.CreationTimestamp.Time) > 3*time.Minute { + continue + } + + if slices.ContainsFunc(node.Spec.Taints, func(taint v1.Taint) bool { + return taint.Key == api.TaintExternalCloudProvider + }) { + return fmt.Errorf("node %s is not yet initialized", node.Name) + } + } + + return nil +} diff --git a/scaleway/loadbalancers_test.go b/scaleway/loadbalancers_test.go index 26d5a2e..ee8d2e4 100644 --- a/scaleway/loadbalancers_test.go +++ b/scaleway/loadbalancers_test.go @@ -27,6 +27,7 @@ import ( "github.com/scaleway/scaleway-sdk-go/scw" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/cloud-provider/api" ) func TestGetValueForPort(t *testing.T) { @@ -1401,3 +1402,81 @@ func Test_hasEqualLoadBalancerStaticIPs(t *testing.T) { }) } } + +func Test_nodesInitialized(t *testing.T) { + type args struct { + nodes []*v1.Node + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "node initialized", + args: args{ + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + }, + }, + wantErr: false, + }, + { + name: "node not initialized but created 5 minutes ago", + args: args{ + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + CreationTimestamp: metav1.NewTime(time.Now().Add(-5 * time.Minute)), + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: api.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "node not initialized, created 10 seconds ago", + args: args{ + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + CreationTimestamp: metav1.NewTime(time.Now().Add(-10 * time.Second)), + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: api.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := nodesInitialized(tt.args.nodes); (err != nil) != tt.wantErr { + t.Errorf("nodesInitialized() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}