Repository navigation
Gate LB target admission on NodeReady for first admission - #961
Open
skumarc-do wants to merge 1 commit into
Open
skumarc-do wants to merge 1 commit into
skumarc-do wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Backend reconciliation can leave newly Ready nodes excluded and remove previously admitted nodes without ProviderIDs.
2 open findings
What changed in this PR
Updates DigitalOcean load balancer reconciliation to require NodeReady=True before first admission, aiming to prevent scale-up 503s.
Changes:
- Adds readiness filtering and existing-backend admission tracking.
- Adds readiness tests and an unreleased changelog entry.
| File | Description |
|---|---|
| cloud-controller-manager/do/loadbalancers.go | Adds first-admission readiness checks. |
| cloud-controller-manager/do/loadbalancers_test.go | Updates callers and tests readiness filtering. |
| CHANGELOG.md | Documents the admission change. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
|
|
||
| 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] { |
| service.Namespace, service.Name, len(pendingNotReady), formatNodeNames(pendingNotReady, 5)) | ||
| } | ||
|
|
||
| if len(admit) == 0 && len(pendingNotReady) > 0 { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Customer saw bursts of HTTP 503 (
No server is available to handle this request) from the DO LB during node scale-ups.Root cause: Kubernetes service controller admits nodes to
EnsureLoadBalancerunderstableNodeSetPredicates, which does not requireNodeReady=True(kubernetes/kubernetes#90823). DO CCM's node filters (filterAndClassifyNodes,prepareNodesForLBSync) only gate on IP presence, so a NotReady droplet still gets written intolb.DropletIDs.For
externalTrafficPolicy: Clusterthe LB then health-checks:10256/healthz(kube-proxy generic), which can answer 200 while the node is still NotReady the droplet is promoted to UP and traffic lands on a node that cannot serve, producing 503s. (Localpolicy incidentally hides this because its LB probe gates on local Ready pod presence)Reproduced on a 2-node DOKS cluster, new droplet appeared in
lb.DropletIDs10-33s beforeReady=Trueon every scale-up. 503s did not surface on this quiet cluster because the LB's healthy threshold (5 × 3s = 15s) often expires afterReady=True, the premature admission ordering, however, is unambiguous and matches the customer timeline.