From 20f7ae564c6978107ea59d1a597e144682c2df0a Mon Sep 17 00:00:00 2001 From: skumarc Date: Wed, 7 Oct 2026 20:19:09 +0530 Subject: [PATCH] Gate LB target admission on NodeReady for first admission --- CHANGELOG.md | 2 + cloud-controller-manager/do/loadbalancers.go | 147 +++++++-- .../do/loadbalancers_test.go | 302 +++++++++++++++++- docs/controllers/services/annotations.md | 4 +- 4 files changed, 418 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cb82b250..f7d26a266 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## unreleased +* Gate LB target admission on NodeReady (and, where present, the DOKS critical-components readiness taint) for first admission while preserving the upstream stable node set for already-admitted nodes (@skumarc-do) + ## v0.1.71 (beta) - September 17, 2026 * Update Go to 1.27 (@pyadagiri-do) diff --git a/cloud-controller-manager/do/loadbalancers.go b/cloud-controller-manager/do/loadbalancers.go index 7847028eb..d11bc2d8e 100644 --- a/cloud-controller-manager/do/loadbalancers.go +++ b/cloud-controller-manager/do/loadbalancers.go @@ -240,19 +240,22 @@ func (l *loadBalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri return nil, err } - var lbRequest *godo.LoadBalancerRequest - lbRequest, err = l.buildLoadBalancerRequest(ctx, service, nodes) + lb, lbErr := l.retrieveAndAnnotateLoadBalancer(ctx, service) + if lbErr != nil && !errors.Is(lbErr, errLBNotFound) { + return nil, lbErr + } + admittedIDs := admittedDropletIDSet(lb) + + lbRequest, pendingNotReady, err := l.buildLoadBalancerRequest(ctx, service, nodes, admittedIDs) if err != nil { - // %w keeps ErrNoEligibleBackends inspectable with errors.Is. + // %w keeps ErrNoEligibleBackends (errors.Is) and *api.RetryError (errors.As) inspectable. return nil, fmt.Errorf("failed to build load-balancer request: %w", err) } - var lb *godo.LoadBalancer - lb, err = l.retrieveAndAnnotateLoadBalancer(ctx, service) - switch err { + switch lbErr { case nil: // LB existing - lb, err = l.updateLoadBalancer(ctx, lb, service, nodes) + lb, pendingNotReady, err = l.updateLoadBalancer(ctx, lb, service, nodes) if err != nil { return nil, err } @@ -271,10 +274,6 @@ func (l *loadBalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri updateServiceAnnotation(service, annDOLoadBalancerID, lb.ID) updateServiceAnnotation(service, annDOType, lb.Type) - - default: - // unrecoverable LB retrieval error - return nil, err } if lb.Status == lbStatusNew { @@ -285,6 +284,11 @@ func (l *loadBalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri return nil, fmt.Errorf("load-balancer has unexpected status %q", lb.Status) } + // Backends were written with the currently admissible nodes, retry so nodes + // held back for IP init (pending) or readiness (pendingNotReady: NodeReady + // and critical components) are admitted once eligible, since upstream does + // not re-sync on those transitions. + pending = append(pending, pendingNotReady...) if err := l.retryIfNodesPendingInit(service, pending); err != nil { return nil, err } @@ -369,40 +373,42 @@ func (l *loadBalancers) recordUpdatedLetsEncryptCert(ctx context.Context, servic return nil } -func (l *loadBalancers) updateLoadBalancer(ctx context.Context, lb *godo.LoadBalancer, service *v1.Service, nodes []*v1.Node) (*godo.LoadBalancer, error) { +func (l *loadBalancers) updateLoadBalancer(ctx context.Context, lb *godo.LoadBalancer, service *v1.Service, nodes []*v1.Node) (*godo.LoadBalancer, []*v1.Node, error) { + admittedIDs := admittedDropletIDSet(lb) + // call buildLoadBalancerRequest for its error checking; we have to call it // again just before actually updating the loadbalancer in case // checkAndUpdateLBAndServiceCerts modifies the service - _, err := l.buildLoadBalancerRequest(ctx, service, nodes) + _, _, err := l.buildLoadBalancerRequest(ctx, service, nodes, admittedIDs) if err != nil { - return nil, fmt.Errorf("failed to build load-balancer request: %w", err) + return nil, nil, fmt.Errorf("failed to build load-balancer request: %w", err) } lbCertID := getCertificateIDFromLB(lb) serviceCertID, err := findCertificateID(ctx, service, l.resources.gclient) if err != nil { - return nil, err + return nil, nil, err } err = l.recordUpdatedLetsEncryptCert(ctx, service, lbCertID, serviceCertID) if err != nil { - return nil, err + return nil, nil, err } - lbRequest, err := l.buildLoadBalancerRequest(ctx, service, nodes) + lbRequest, pendingNotReady, err := l.buildLoadBalancerRequest(ctx, service, nodes, admittedIDs) if err != nil { - return nil, fmt.Errorf("failed to build load-balancer request (post-certificate update): %w", err) + return nil, nil, fmt.Errorf("failed to build load-balancer request (post-certificate update): %w", err) } lbID := lb.ID lb, _, err = l.resources.gclient.LoadBalancers.Update(ctx, lb.ID, lbRequest) if err != nil { logLBInfo("UPDATE", lbRequest, 2) - return nil, fmt.Errorf("failed to update load-balancer with ID %s: %s", lbID, err) + return nil, nil, fmt.Errorf("failed to update load-balancer with ID %s: %s", lbID, err) } logLBInfo("UPDATE", lbRequest, 2) - return lb, nil + return lb, pendingNotReady, nil } // UpdateLoadBalancer updates the load balancer for service to balance across @@ -434,11 +440,12 @@ func (l *loadBalancers) UpdateLoadBalancer(ctx context.Context, clusterName stri return err } - _, err = l.updateLoadBalancer(ctx, lb, service, nodes) + _, pendingNotReady, err := l.updateLoadBalancer(ctx, lb, service, nodes) if err != nil { return err } + pending = append(pending, pendingNotReady...) return l.retryIfNodesPendingInit(service, pending) } @@ -692,6 +699,62 @@ func nodeIntendsPublicNetwork(node *v1.Node) bool { return !strings.EqualFold(node.Labels[nodeNetworkTypeLabel], nodeNetworkTypePrivate) } +// isNodeReady reports whether the node currently has a NodeReady condition with Status=True. +func isNodeReady(node *v1.Node) bool { + for _, c := range node.Status.Conditions { + if c.Type == v1.NodeReady { + return c.Status == v1.ConditionTrue + } + } + return false +} + +// doksCriticalComponentsReadyTaintKey is set by the DOKS node-readiness +// controller while a node's critical components are still coming up and removed +// once it can back workloads. It never appears on clusters without node +// readiness enforcement, where the check below is a no-op. +const doksCriticalComponentsReadyTaintKey = "readiness.k8s.io/DOKSCriticalComponentsReady" + +// hasPendingCriticalComponentsTaint reports whether the node still carries the +// DOKS critical-components readiness taint. It gates first admission only, +// already-admitted nodes are kept regardless. +func hasPendingCriticalComponentsTaint(node *v1.Node) bool { + for _, t := range node.Spec.Taints { + if t.Key == doksCriticalComponentsReadyTaintKey { + return true + } + } + return false +} + +// admittedDropletIDSet returns a lookup set of the LB's current backend +// droplet IDs, or nil when lb is nil or has no admitted droplets. +func admittedDropletIDSet(lb *godo.LoadBalancer) map[int]bool { + if lb == nil || len(lb.DropletIDs) == 0 { + return nil + } + set := make(map[int]bool, len(lb.DropletIDs)) + for _, id := range lb.DropletIDs { + set[id] = true + } + return set +} + +func filterByReadiness(nodes []*v1.Node, admittedIDs map[int]bool) (admit, pending []*v1.Node) { + for _, node := range nodes { + if id, err := dropletIDFromProviderID(node.Spec.ProviderID); err == nil && admittedIDs[id] { + admit = append(admit, node) + continue + } + if isNodeReady(node) && !hasPendingCriticalComponentsTaint(node) { + admit = append(admit, node) + continue + } + pending = append(pending, node) + } + return admit, pending +} + // pendingInitNodes splits nodes with no usable public address by declared // network intent: those still expected to gain one (retry) and private-only // nodes that never will (no retry). Both groups are already excluded from the @@ -758,7 +821,7 @@ func (l *loadBalancers) retryIfNodesPendingInit(service *v1.Service, pending []* return nil } msg := fmt.Sprintf( - "service %s/%s: updated backends with currently lb-ready nodes; retrying for %d still initializing: %s", + "service %s/%s: updated backends with the currently admissible node set; retrying for %d node(s) still pending (initializing, not yet NodeReady, or critical components not yet ready): %s", service.Namespace, service.Name, len(pending), formatNodeNames(pending, 5), ) klog.Info(msg) @@ -1063,14 +1126,14 @@ func (l *loadBalancers) emitServiceEvent(service *v1.Service, eventType, reason, // buildLoadBalancerRequest returns a *godo.LoadBalancerRequest to balance // requests for service across nodes. -func (l *loadBalancers) buildLoadBalancerRequest(ctx context.Context, service *v1.Service, nodes []*v1.Node) (*godo.LoadBalancerRequest, error) { +func (l *loadBalancers) buildLoadBalancerRequest(ctx context.Context, service *v1.Service, nodes []*v1.Node, admittedIDs map[int]bool) (*godo.LoadBalancerRequest, []*v1.Node, error) { lbNetwork, err := getNetwork(service) if err != nil { - return nil, err + return nil, nil, err } lbType, err := getType(service, l.defaultLBType) if err != nil { - return nil, err + return nil, nil, err } isInternalLB := lbNetwork == godo.LoadBalancerNetworkTypeInternal @@ -1080,7 +1143,7 @@ func (l *loadBalancers) buildLoadBalancerRequest(ctx context.Context, service *v lbType == godo.LoadBalancerTypeRegionalNetwork && lbNetwork == godo.LoadBalancerNetworkTypeExternal { err := fmt.Errorf("%w: %s", ErrNoEligibleBackends, errNoEligibleBackendsMsg) l.emitServiceEvent(service, v1.EventTypeWarning, "LoadBalancerConfigError", err.Error()) - return nil, err + return nil, nil, err } if nodeState.filteredCount > 0 { @@ -1099,17 +1162,39 @@ func (l *loadBalancers) buildLoadBalancerRequest(ctx context.Context, service *v } } - req, err := buildLoadBalancerRequestWithNodeState(ctx, service, l.resources.gclient, l.defaultLBType, nodeState) + // Already-admitted droplets stay in the target set regardless of transient + // NodeReady flips; new nodes must be Ready and have their critical components + // serving to be first admitted. Nodes that are not yet serving are held back + // and converged via api.RetryError below. + admit, pendingNotReady := filterByReadiness(nodeState.lbReadyNodes, admittedIDs) + if len(pendingNotReady) > 0 { + klog.V(2).Infof("Service %s/%s: holding back %d node(s) pending NodeReady=True and critical components ready on first admission: %s", + service.Namespace, service.Name, len(pendingNotReady), formatNodeNames(pendingNotReady, 5)) + } + + if len(admit) == 0 && len(pendingNotReady) > 0 { + msg := fmt.Sprintf( + "service %s/%s: %d lb-ready-by-IP node(s) are not yet serving (NodeReady=True and critical components ready) and none is already admitted, retrying instead of writing an empty backend set: %s", + service.Namespace, service.Name, len(pendingNotReady), formatNodeNames(pendingNotReady, 5), + ) + klog.Info(msg) + return nil, nil, api.NewRetryError(msg, nodesNotYetLBReadyRetryAfter) + } + + // Derive the request (including NetworkStack) from the admitted nodes only, so + // a held-back node cannot shape config it will not back. + admitState := filterAndClassifyNodes(admit, lbType, lbNetwork) + req, err := buildLoadBalancerRequestWithNodeState(ctx, service, l.resources.gclient, l.defaultLBType, admitState) if err != nil { if errors.Is(err, ErrNetworkStackConfig) { l.emitServiceEvent(service, v1.EventTypeWarning, "LoadBalancerConfigError", err.Error()) } - return nil, err + return nil, nil, err } - dropletIDs, err := l.nodesToDropletIDs(ctx, nodeState.lbReadyNodes) + dropletIDs, err := l.nodesToDropletIDs(ctx, admit) if err != nil { - return nil, err + return nil, nil, err } req.DropletIDs = dropletIDs @@ -1121,7 +1206,7 @@ func (l *loadBalancers) buildLoadBalancerRequest(ctx context.Context, service *v req.Region = l.region req.VPCUUID = l.resources.clusterVPCID - return req, nil + return req, pendingNotReady, nil } // buildHealthChecks returns a godo.HealthCheck for service. diff --git a/cloud-controller-manager/do/loadbalancers_test.go b/cloud-controller-manager/do/loadbalancers_test.go index 769068286..360bc33c9 100644 --- a/cloud-controller-manager/do/loadbalancers_test.go +++ b/cloud-controller-manager/do/loadbalancers_test.go @@ -266,6 +266,17 @@ func withCloudProviderUninitializedTaint(node *v1.Node) *v1.Node { return node } +// withCriticalComponentsPendingTaint marks a node as still waiting for its DOKS +// critical components to become ready (NodeReadiness controller taint). +func withCriticalComponentsPendingTaint(node *v1.Node) *v1.Node { + node.Spec.Taints = append(node.Spec.Taints, v1.Taint{ + Key: doksCriticalComponentsReadyTaintKey, + Value: "pending", + Effect: v1.TaintEffectNoSchedule, + }) + return node +} + // withNodeNetworkType sets doks.digitalocean.com/node-network-type. func withNodeNetworkType(node *v1.Node, networkType string) *v1.Node { if node.Labels == nil { @@ -275,6 +286,32 @@ func withNodeNetworkType(node *v1.Node, networkType string) *v1.Node { return node } +// withProviderID sets Node.Spec.ProviderID using the DO provider format. +func withProviderID(node *v1.Node, dropletID int) *v1.Node { + node.Spec.ProviderID = fmt.Sprintf("%s%d", providerIDPrefix, dropletID) + return node +} + +// withNodeReady overrides the NodeReady condition status on a node constructed +// by newNodeWithIPs / newNodeWithInternalIPOnly. +func withNodeReady(node *v1.Node, ready bool) *v1.Node { + status := v1.ConditionFalse + if ready { + status = v1.ConditionTrue + } + for i, c := range node.Status.Conditions { + if c.Type == v1.NodeReady { + node.Status.Conditions[i].Status = status + return node + } + } + node.Status.Conditions = append(node.Status.Conditions, v1.NodeCondition{ + Type: v1.NodeReady, + Status: status, + }) + return node +} + func Test_getAlgorithm(t *testing.T) { testcases := []struct { name string @@ -4620,7 +4657,7 @@ func Test_buildLoadBalancerRequest(t *testing.T) { test.lbr.Firewall = &godo.LBFirewall{} } - lbr, err := lb.buildLoadBalancerRequest(context.Background(), test.service, test.nodes) + lbr, _, err := lb.buildLoadBalancerRequest(context.Background(), test.service, test.nodes, nil) if !reflect.DeepEqual(lbr, test.lbr) { t.Error("unexpected load balancer request") @@ -4703,7 +4740,7 @@ func Test_buildLoadBalancerRequestWithClusterID(t *testing.T) { defaultLBType: godo.LoadBalancerTypeRegionalNetwork, } - lbr, err := lb.buildLoadBalancerRequest(context.Background(), service, nodes) + lbr, _, err := lb.buildLoadBalancerRequest(context.Background(), service, nodes, nil) if test.err != nil { if err == nil { t.Fatal("expected error but got none") @@ -4774,7 +4811,7 @@ func Test_buildLoadBalancerRequest_SubnetUUID(t *testing.T) { defaultLBType: godo.LoadBalancerTypeRegionalNetwork, } - lbr, err := lb.buildLoadBalancerRequest(context.Background(), service, nodes) + lbr, _, err := lb.buildLoadBalancerRequest(context.Background(), service, nodes, nil) if err != nil { t.Fatalf("got error: %s", err) } @@ -5981,6 +6018,20 @@ func TestEnsureUpdateLoadBalancer_PendingInitNodes(t *testing.T) { wantUpdate: true, wantDropletIDs: []int{100}, }, + { + // node-new is lb-ready (has ExternalIP) but NodeReady=False + // on first admission. We must write the ready backend and still retry + // so node-new is admitted once Ready (upstream won't re-sync on that). + name: "updates ready backend then retries for not-ready lb-ready node", + nodes: []*v1.Node{ + withNodeNetworkType(newNodeWithIPs("node-ready-a", "10.0.0.1", "", true), nodeNetworkTypePublic), + withNodeNetworkType(newNodeWithIPs("node-new", "10.0.0.3", "", false), nodeNetworkTypePublic), + }, + wantRetry: true, + wantRetryAfter: nodesNotYetLBReadyRetryAfter, + wantUpdate: true, + wantDropletIDs: []int{100}, + }, { name: "retries before update when no ready backends yet", nodes: []*v1.Node{ @@ -8068,7 +8119,7 @@ func TestBuildLoadBalancerRequest_EventEmission(t *testing.T) { } // Call buildLoadBalancerRequest - _, err := lb.buildLoadBalancerRequest(context.Background(), test.service, test.nodes) + _, _, err := lb.buildLoadBalancerRequest(context.Background(), test.service, test.nodes, nil) var retryErr *api.RetryError gotRetry := errors.As(err, &retryErr) @@ -8559,3 +8610,246 @@ func Test_buildFirewall(t *testing.T) { }) } } + +func Test_filterByReadiness(t *testing.T) { + readyA := withProviderID(newNodeWithIPs("ready-a", "10.0.0.1", "", true), 100) + readyB := withProviderID(newNodeWithIPs("ready-b", "10.0.0.2", "", true), 101) + notReadyFresh := withProviderID(newNodeWithIPs("fresh-not-ready", "10.0.0.3", "", true), 102) + withNodeReady(notReadyFresh, false) + notReadyStable := withProviderID(newNodeWithIPs("stable-not-ready", "10.0.0.4", "", true), 103) + withNodeReady(notReadyStable, false) + noProviderIDReady := newNodeWithIPs("no-provider-id", "10.0.0.5", "", true) + noProviderIDNotReady := newNodeWithIPs("no-provider-id-not-ready", "10.0.0.6", "", true) + withNodeReady(noProviderIDNotReady, false) + readyTaintPending := withCriticalComponentsPendingTaint(withProviderID(newNodeWithIPs("ready-taint-pending", "10.0.0.7", "", true), 104)) + stableTaintPending := withCriticalComponentsPendingTaint(withProviderID(newNodeWithIPs("stable-taint-pending", "10.0.0.8", "", true), 105)) + + tests := []struct { + name string + nodes []*v1.Node + admittedIDs map[int]bool + wantAdmit []string + wantPending []string + }{ + { + name: "no admissions, all ready", + nodes: []*v1.Node{readyA, readyB}, + admittedIDs: nil, + wantAdmit: []string{"ready-a", "ready-b"}, + wantPending: nil, + }, + { + name: "no admissions, mixed -> only ready admitted", + nodes: []*v1.Node{readyA, notReadyFresh, readyB}, + admittedIDs: nil, + wantAdmit: []string{"ready-a", "ready-b"}, + wantPending: []string{"fresh-not-ready"}, + }, + { + name: "stable node not ready stays admitted (no flap)", + nodes: []*v1.Node{readyA, notReadyStable}, + admittedIDs: map[int]bool{100: true, 103: true}, + wantAdmit: []string{"ready-a", "stable-not-ready"}, + wantPending: nil, + }, + { + name: "node without ProviderID falls through to ready check", + nodes: []*v1.Node{noProviderIDReady, noProviderIDNotReady}, + admittedIDs: nil, + wantAdmit: []string{"no-provider-id"}, + wantPending: []string{"no-provider-id-not-ready"}, + }, + { + name: "ready but critical-components taint pending -> held back", + nodes: []*v1.Node{readyA, readyTaintPending}, + admittedIDs: nil, + wantAdmit: []string{"ready-a"}, + wantPending: []string{"ready-taint-pending"}, + }, + { + name: "stable node with critical-components taint stays admitted", + nodes: []*v1.Node{stableTaintPending}, + admittedIDs: map[int]bool{105: true}, + wantAdmit: []string{"stable-taint-pending"}, + wantPending: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + admit, pending := filterByReadiness(tt.nodes, tt.admittedIDs) + gotAdmit := normalizeEmpty(nodeNames(admit)) + gotPending := normalizeEmpty(nodeNames(pending)) + if !reflect.DeepEqual(gotAdmit, tt.wantAdmit) { + t.Errorf("admit = %v, want %v", gotAdmit, tt.wantAdmit) + } + if !reflect.DeepEqual(gotPending, tt.wantPending) { + t.Errorf("pending = %v, want %v", gotPending, tt.wantPending) + } + }) + } +} + +func normalizeEmpty(s []string) []string { + if len(s) == 0 { + return nil + } + return s +} + +func Test_buildLoadBalancerRequest_readyFilter(t *testing.T) { + service := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc", + Namespace: "default", + UID: "svc-uid", + Annotations: map[string]string{ + annDOProtocol: "http", + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{{ + Name: "http", + Protocol: v1.ProtocolTCP, + Port: 80, + NodePort: 30000, + }}, + }, + } + + readyAdmit := withProviderID(newNodeWithIPs("ready-a", "10.0.0.1", "", true), 100) + freshNotReady := withProviderID(newNodeWithIPs("fresh-not-ready", "10.0.0.2", "", true), 101) + withNodeReady(freshNotReady, false) + stableNotReady := withProviderID(newNodeWithIPs("stable-not-ready", "10.0.0.3", "", true), 102) + withNodeReady(stableNotReady, false) + + tests := []struct { + name string + nodes []*v1.Node + admittedIDs map[int]bool + expectRetry bool + expectDroplets []int + expectPending []string + }{ + { + name: "all fresh not-ready, no admissions -> retry", + nodes: []*v1.Node{freshNotReady}, + admittedIDs: nil, + expectRetry: true, + }, + { + name: "stable not-ready in admitted set -> kept (no flap)", + nodes: []*v1.Node{stableNotReady}, + admittedIDs: map[int]bool{102: true}, + expectRetry: false, + expectDroplets: []int{102}, + }, + { + name: "mixed ready + fresh not-ready -> partial set + pending surfaced", + nodes: []*v1.Node{readyAdmit, freshNotReady}, + admittedIDs: nil, + expectRetry: false, + expectDroplets: []int{100}, + expectPending: []string{"fresh-not-ready"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := newFakeDropletClient(&fakeDropletService{ + listFunc: func(context.Context, *godo.ListOptions) ([]godo.Droplet, *godo.Response, error) { + return []godo.Droplet{ + {ID: 100, Name: "ready-a"}, + {ID: 101, Name: "fresh-not-ready"}, + {ID: 102, Name: "stable-not-ready"}, + }, newFakeOKResponse(), nil + }, + }) + fakeResources := newResources("", "", publicAccessFirewall{}, fakeClient) + fakeResources.kclient = fake.NewSimpleClientset() + if _, err := fakeResources.kclient.CoreV1().Services(service.Namespace).Create( + context.Background(), service, metav1.CreateOptions{}, + ); err != nil { + t.Fatalf("failed to create service: %v", err) + } + lb := &loadBalancers{ + resources: fakeResources, + region: "nyc3", + defaultLBType: godo.LoadBalancerTypeRegional, + } + + req, pendingNotReady, err := lb.buildLoadBalancerRequest(context.Background(), service, tt.nodes, tt.admittedIDs) + + var retryErr *api.RetryError + gotRetry := errors.As(err, &retryErr) + if gotRetry != tt.expectRetry { + t.Fatalf("retry = %v (err=%v), want %v", gotRetry, err, tt.expectRetry) + } + if tt.expectRetry { + if retryErr.RetryAfter() != nodesNotYetLBReadyRetryAfter { + t.Errorf("RetryAfter = %v, want %v", retryErr.RetryAfter(), nodesNotYetLBReadyRetryAfter) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !reflect.DeepEqual(req.DropletIDs, tt.expectDroplets) { + t.Errorf("DropletIDs = %v, want %v", req.DropletIDs, tt.expectDroplets) + } + if !reflect.DeepEqual(normalizeEmpty(nodeNames(pendingNotReady)), normalizeEmpty(tt.expectPending)) { + t.Errorf("pendingNotReady = %v, want %v", nodeNames(pendingNotReady), tt.expectPending) + } + }) + } +} + +func Test_buildLoadBalancerRequest_readyFilterNetworkStack(t *testing.T) { + svc := func(networkStack string) *v1.Service { + annotations := map[string]string{ + annDOProtocol: "http", + annDOType: godo.LoadBalancerTypeRegionalNetwork, + } + if networkStack != "" { + annotations[annDONetworkStack] = networkStack + } + return &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "svc", Namespace: "default", UID: "svc-uid", Annotations: annotations}, + Spec: v1.ServiceSpec{Ports: []v1.ServicePort{{Name: "http", Protocol: v1.ProtocolTCP, Port: 80, NodePort: 30000}}}, + } + } + + readyDualStack := withProviderID(newNodeWithIPs("rn-ready", "10.0.0.1", "2001:db8::1", true), 200) + freshV4NotReady := withProviderID(newNodeWithIPs("rn-fresh-v4", "10.0.0.2", "", true), 201) + withNodeReady(freshV4NotReady, false) + + tests := []struct { + name string + networkStack string + }{ + {name: "implicit default not downgraded by held-back IPv4 node", networkStack: ""}, + {name: "explicit dual-stack not failed by held-back IPv4 node", networkStack: godo.LoadBalancerNetworkStackDualstack}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := newFakeDropletClient(&fakeDropletService{ + listFunc: func(context.Context, *godo.ListOptions) ([]godo.Droplet, *godo.Response, error) { + return []godo.Droplet{{ID: 200, Name: "rn-ready"}, {ID: 201, Name: "rn-fresh-v4"}}, newFakeOKResponse(), nil + }, + }) + fakeResources := newResources("", "", publicAccessFirewall{}, fakeClient) + lb := &loadBalancers{resources: fakeResources, region: "nyc3", defaultLBType: godo.LoadBalancerTypeRegionalNetwork} + + req, _, err := lb.buildLoadBalancerRequest(context.Background(), svc(tt.networkStack), []*v1.Node{readyDualStack, freshV4NotReady}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if req.NetworkStack != godo.LoadBalancerNetworkStackDualstack { + t.Errorf("NetworkStack = %q, want %q", req.NetworkStack, godo.LoadBalancerNetworkStackDualstack) + } + if !reflect.DeepEqual(req.DropletIDs, []int{200}) { + t.Errorf("DropletIDs = %v, want [200]", req.DropletIDs) + } + }) + } +} diff --git a/docs/controllers/services/annotations.md b/docs/controllers/services/annotations.md index 145c085a1..3314de4f1 100644 --- a/docs/controllers/services/annotations.md +++ b/docs/controllers/services/annotations.md @@ -257,12 +257,12 @@ Specifying a subnet UUID is in private preview. Contact DigitalOcean support to Specifies what network addressing will be supported by the load balancer. Options are "IPV4" and "DUALSTACK". "DUALSTACK" is the option to support both IPv4 and IPv6 networking on the load balancer. - If load balancer type is "REGIONAL" (connection-terminating), it will default to "DUALSTACK" regardless of node network stack configuration. -- If load balancer type is "REGIONAL_NETWORK" (pass-through), it will default to "DUALSTACK" when all cluster nodes support dual-stack networking (have both IPv4 and IPv6 external addresses), otherwise it will default to "IPV4". +- If load balancer type is "REGIONAL_NETWORK" (pass-through), it will default to "DUALSTACK" when all nodes attached to the load balancer support dual-stack networking (have both IPv4 and IPv6 external addresses), otherwise it will default to "IPV4". **Node Requirements** The cloud controller manager filters nodes before adding them to load balancers: -- Only nodes with `Ready` status are included in load balancers +- New nodes must be `Ready` before they are added; nodes already attached stay attached if they briefly become `NotReady` - For EXTERNAL load balancers: nodes must have external IP addresses - For INTERNAL load balancers: nodes only need to be Ready (private nodes are allowed) - Nodes with only IPv6 addresses (no IPv4) are currently not supported and will be filtered out