diff --git a/CHANGELOG.md b/CHANGELOG.md index 49cdc7628..002b9a33e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ ## unreleased * Add public ipv6 node address handling (@gottwald) +* Add automatic dual-stack support for REGIONAL_NETWORK load balancers when all cluster nodes support dual-stack networking (@gottwald) +* Add node filtering to load balancers: only LB-Ready nodes with appropriate IP addresses are included in load balancers (@gottwald) +* Emit Kubernetes events when load balancer configuration errors occur (e.g., explicit dual-stack annotation with IPv4-only nodes) (@gottwald) ## v0.1.65 (beta) - January 14, 2026 diff --git a/cloud-controller-manager/do/certificates_test.go b/cloud-controller-manager/do/certificates_test.go index e4e28938f..b3f2454f7 100644 --- a/cloud-controller-manager/do/certificates_test.go +++ b/cloud-controller-manager/do/certificates_test.go @@ -312,21 +312,9 @@ func Test_LBaaSCertificateScenarios(t *testing.T) { } nodes := []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), } droplets := []godo.Droplet{ { diff --git a/cloud-controller-manager/do/loadbalancers.go b/cloud-controller-manager/do/loadbalancers.go index 324ab9022..1bf7330b2 100644 --- a/cloud-controller-manager/do/loadbalancers.go +++ b/cloud-controller-manager/do/loadbalancers.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "net/http" + "net/netip" "reflect" "sort" "strconv" @@ -28,6 +29,7 @@ import ( "time" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/kubernetes" cloudprovider "k8s.io/cloud-provider" @@ -56,6 +58,12 @@ const ( // This is the DO-specific tag component prepended to the cluster ID. tagPrefixClusterID = "k8s" +) + +var ( + // ErrNetworkStackConfig indicates an error with network stack configuration + // that requires user intervention (e.g., mismatched dual-stack settings) + ErrNetworkStackConfig = errors.New("network stack configuration error") // Sticky sessions types. stickySessionsTypeNone = "none" @@ -511,6 +519,184 @@ func updateServiceAnnotation(service *v1.Service, annotName, annotValue string) service.ObjectMeta.Annotations[annotName] = annotValue } +// nodeClassification represents the IP stack capability of a node +type nodeClassification string + +const ( + nodeClassSingleStackV4 nodeClassification = "singleStackV4" // Has IPv4, no IPv6 + nodeClassSingleStackV6 nodeClassification = "singleStackV6" // Has IPv6, no IPv4 + nodeClassDualStack nodeClassification = "dualStack" // Has both IPv4 and IPv6 + nodeClassPublicNetUnready nodeClassification = "publicNetUnready" // No external IPs, (yet) +) + +// nodeState contains the classification and filtering results for nodes +type nodeState struct { + // lbReadyNodes are nodes that are ready to serve as LB backends + lbReadyNodes []*v1.Node + filteredCount int + singleStackV4Nodes []*v1.Node + singleStackV6Nodes []*v1.Node + dualStackNodes []*v1.Node +} + +func (ns *nodeState) hasSingleStackV4() bool { + if len(ns.singleStackV4Nodes) > 0 { + return true + } + return false +} + +func (ns *nodeState) hasSingleStackV6() bool { + if len(ns.singleStackV6Nodes) > 0 { + return true + } + return false +} + +func (ns *nodeState) hasDualStackNodes() bool { + if len(ns.dualStackNodes) > 0 { + return true + } + return false +} + +// isAllDualStack reports whether all ready nodes are dual-stack capable. +// For external LBs, IPv6-only nodes are filtered out of readyNodes (and into +// singleStackV6Nodes) by filterAndClassifyNodes, so they are intentionally not +// considered here. This means if the only non-dual-stack nodes are IPv6-only +// (which are unsupported for external LBs anyway), the cluster is still treated +// as all-dual-stack for defaulting purposes. +func (ns *nodeState) isAllDualStack() bool { + return ns.hasDualStackNodes() && !ns.hasSingleStackV4() +} + +// filterAndClassifyNodes filters nodes by available IP stacks and differentiates between +// singleStackV4, singleStackV6, or dualStack. +// +// For INTERNAL load balancers, public IPs not required, private assumed. +// For EXTERNAL load balancers, have at least one external IP, +// and singleStackV6 nodes are filtered out (not supported for external connectivity). +func filterAndClassifyNodes(nodes []*v1.Node, isInternalLB bool) *nodeState { + state := &nodeState{ + lbReadyNodes: make([]*v1.Node, 0, len(nodes)), + singleStackV4Nodes: make([]*v1.Node, 0), + singleStackV6Nodes: make([]*v1.Node, 0), + dualStackNodes: make([]*v1.Node, 0), + } + + for _, node := range nodes { + // For INTERNAL LBs, no need for public IPs - assuming ready + // this is just here so that we can expand in the future when internalLBs support v6 or other features. + if isInternalLB { + state.lbReadyNodes = append(state.lbReadyNodes, node) + continue + } + + // For EXTERNAL LBs, check for external IPs and classify + classification := classifyNode(node) + + if classification == nodeClassPublicNetUnready { + klog.V(4).Infof("Node %s filtered: no external IP addresses (required for EXTERNAL load balancer)", node.Name) + state.filteredCount++ + continue + } + + if classification == nodeClassSingleStackV6 { + klog.V(4).Infof("Node %s filtered: IPv6-only not supported for EXTERNAL load balancer", node.Name) + state.filteredCount++ + state.singleStackV6Nodes = append(state.singleStackV6Nodes, node) + continue + } + + // Node passed filters + state.lbReadyNodes = append(state.lbReadyNodes, node) + + // Classify node by IP stack capability + switch classification { + case nodeClassSingleStackV4: + state.singleStackV4Nodes = append(state.singleStackV4Nodes, node) + case nodeClassDualStack: + state.dualStackNodes = append(state.dualStackNodes, node) + } + } + + return state +} + +// classifyNode determines node's IP stack classification +func classifyNode(node *v1.Node) nodeClassification { + hasIPv4 := false + hasIPv6 := false + + for _, addr := range node.Status.Addresses { + if addr.Type != v1.NodeExternalIP { + continue + } + + if isIPv6(addr.Address) { + hasIPv6 = true + } else { + hasIPv4 = true + } + } + + // Determine classification based on IP presence + switch { + case hasIPv4 && hasIPv6: + return nodeClassDualStack + case hasIPv4 && !hasIPv6: + return nodeClassSingleStackV4 + case !hasIPv4 && hasIPv6: + return nodeClassSingleStackV6 + default: + // No external IPs found (!hasIPv4 && !hasIPv6) + return nodeClassPublicNetUnready + } +} + +// isIPv6 checks if an IP address string is IPv6 +func isIPv6(addr string) bool { + ip, err := netip.ParseAddr(addr) + return err == nil && ip.Is6() +} + +// nodeNames extracts a slice of node names from a slice of node pointers +func nodeNames(nodes []*v1.Node) []string { + names := make([]string, len(nodes)) + for i, node := range nodes { + names[i] = node.Name + } + return names +} + +// formatNodeNames formats a list of node names for logging, truncating if necessary. +// Shows at most maxNodes (sorted), appending "[+N more]" if the list is longer. +func formatNodeNames(nodes []*v1.Node, maxNodes int) string { + names := nodeNames(nodes) + return formatNodeNameList(names, maxNodes) +} + +// formatNodeNameList formats a list of node name strings for logging, truncating if necessary. +// Shows at most maxNodes (sorted), appending "[+N more]" if the list is longer. +func formatNodeNameList(names []string, maxNodes int) string { + if len(names) == 0 { + return "" + } + + // Sort for deterministic output + sorted := make([]string, len(names)) + copy(sorted, names) + sort.Strings(sorted) + + if len(sorted) <= maxNodes { + return strings.Join(sorted, ", ") + } + + truncated := sorted[:maxNodes] + remaining := len(sorted) - maxNodes + return fmt.Sprintf("%s [+%d more]", strings.Join(truncated, ", "), remaining) +} + // nodesToDropletID returns a []int containing ids of all droplets identified by name in nodes. // // Node names are assumed to match droplet names. @@ -569,13 +755,28 @@ func (l *loadBalancers) nodesToDropletIDs(ctx context.Context, nodes []*v1.Node) } sort.Strings(missingNames) - klog.Errorf("Failed to find droplets for nodes %s", strings.Join(missingNames, " ")) + klog.Errorf("Failed to find droplets for nodes: %s", formatNodeNameList(missingNames, 3)) } return dropletIDs, nil } +// buildLoadBalancerRequest returns a *godo.LoadBalancerRequest without requiring node state. +// This is used by the admission handler for validation where node information is not available. +// The request will have all configuration except DropletIDs (which must be set separately if needed). func buildLoadBalancerRequest(ctx context.Context, service *v1.Service, godoClient *godo.Client, defaultLBType string) (*godo.LoadBalancerRequest, error) { + // Call the node-aware version with nil nodeState + // This will skip node-based defaulting and validation + return buildLoadBalancerRequestWithNodeState(ctx, service, godoClient, defaultLBType, nil) +} + +// buildLoadBalancerRequestWithNodeState returns a *godo.LoadBalancerRequest with node-aware +// configuration. The nodeState parameter can be nil for validation scenarios where node +// information is not available (e.g., admission webhooks). When nodeState is nil: +// - Node-based network stack defaulting is skipped (REGIONAL_NETWORK defaults to IPV4) +// - Node capability validation is skipped +// - DropletIDs are not set (must be set by caller if needed) +func buildLoadBalancerRequestWithNodeState(ctx context.Context, service *v1.Service, godoClient *godo.Client, defaultLBType string, nodeState *nodeState) (*godo.LoadBalancerRequest, error) { lbName := getLoadBalancerName(service) lbType, err := getType(service, defaultLBType) @@ -586,7 +787,7 @@ func buildLoadBalancerRequest(ctx context.Context, service *v1.Service, godoClie if err != nil { return nil, err } - lbNetworkStack, err := getNetworkStack(service, lbType, lbNetwork) + lbNetworkStack, err := getNetworkStack(service, lbType, lbNetwork, nodeState) if err != nil { return nil, err } @@ -679,15 +880,78 @@ func buildLoadBalancerRequest(ctx context.Context, service *v1.Service, godoClie }, nil } +// emitServiceEvent records an event on the service object +func (l *loadBalancers) emitServiceEvent(service *v1.Service, eventType, reason, message string) { + l.resources.kclient.CoreV1().Events(service.Namespace).Create( + context.Background(), + &v1.Event{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s.%x", service.Name, time.Now().UnixNano()), + Namespace: service.Namespace, + }, + InvolvedObject: v1.ObjectReference{ + Kind: "Service", + Namespace: service.Namespace, + Name: service.Name, + UID: service.UID, + APIVersion: "v1", + }, + Reason: reason, + Message: message, + Type: eventType, + Source: v1.EventSource{ + Component: "digitalocean-cloud-controller-manager", + }, + FirstTimestamp: metav1.Now(), + LastTimestamp: metav1.Now(), + Count: 1, + }, + metav1.CreateOptions{}, + ) +} + // 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) { - req, err := buildLoadBalancerRequest(ctx, service, l.resources.gclient, l.defaultLBType) + // Determine if this is an INTERNAL LB (affects node filtering) + lbNetwork, err := getNetwork(service) + if err != nil { + return nil, err + } + isInternalLB := (lbNetwork == godo.LoadBalancerNetworkTypeInternal) + + // Filter and classify nodes FIRST + nodeState := filterAndClassifyNodes(nodes, isInternalLB) + + // Log filtering results + if nodeState.filteredCount > 0 { + if isInternalLB { + klog.V(2).Infof("Service %s/%s: Filtered out %d non-lb-ready nodes, %d lb-ready nodes remaining (INTERNAL LB)", + service.Namespace, service.Name, nodeState.filteredCount, len(nodeState.lbReadyNodes)) + } else { + klog.V(2).Infof("Service %s/%s: Filtered out %d non-lb-ready/non-public nodes, %d lb-ready nodes remaining (%d dualStack, %d singleStackV4)", + service.Namespace, service.Name, nodeState.filteredCount, + len(nodeState.lbReadyNodes), len(nodeState.dualStackNodes), len(nodeState.singleStackV4Nodes)) + + // Log if IPv6-only nodes were filtered + if nodeState.hasSingleStackV6() { + klog.V(4).Infof("Service %s/%s: Filtered out %d IPv6-only nodes (not supported): %s", + service.Namespace, service.Name, len(nodeState.singleStackV6Nodes), formatNodeNames(nodeState.singleStackV6Nodes, 3)) + } + } + } + + // Build request with filtered nodes + req, err := buildLoadBalancerRequestWithNodeState(ctx, service, l.resources.gclient, l.defaultLBType, nodeState) if err != nil { + // Emit Kubernetes event for network stack configuration errors + if errors.Is(err, ErrNetworkStackConfig) { + l.emitServiceEvent(service, v1.EventTypeWarning, "LoadBalancerConfigError", err.Error()) + } return nil, err } - dropletIDs, err := l.nodesToDropletIDs(ctx, nodes) + dropletIDs, err := l.nodesToDropletIDs(ctx, nodeState.lbReadyNodes) if err != nil { return nil, err } @@ -886,9 +1150,9 @@ func buildRegionalNetworkForwardingRule(service *v1.Service) ([]godo.ForwardingR for _, port := range service.Spec.Ports { var protocol string switch port.Protocol { - case portProtocolTCP: + case v1.ProtocolTCP: protocol = protocolTCP - case portProtocolUDP: + case v1.ProtocolUDP: protocol = protocolUDP default: return nil, fmt.Errorf("only TCP or UDP protocol is supported, got: %q", port.Protocol) @@ -907,7 +1171,7 @@ func buildForwardingRule(service *v1.Service, port *v1.ServicePort, protocol, ce var forwardingRule godo.ForwardingRule switch port.Protocol { - case portProtocolTCP, portProtocolUDP: + case v1.ProtocolTCP, v1.ProtocolUDP: default: return nil, fmt.Errorf("only TCP or UDP protocol is supported, got: %q", port.Protocol) } @@ -1458,23 +1722,90 @@ func getNetwork(service *v1.Service) (string, error) { return network, nil } -func getNetworkStack(service *v1.Service, lbType string, network string) (string, error) { +// getNetworkStack determines the network stack for the load balancer based on +// service annotations, LB type, network type, and node IPv6 capabilities. +// +// For INTERNAL load balancers, this always returns IPV4 (IPv6 not supported). +// For REGIONAL load balancers, defaults to DUALSTACK (connection termination allows IPv6→IPv4 translation). +// For REGIONAL_NETWORK load balancers, considers node IPv6 capability. +func getNetworkStack(service *v1.Service, lbType string, network string, nodeState *nodeState) (string, error) { + serviceName := fmt.Sprintf("%s/%s", service.Namespace, service.Name) + + // When nodeState is nil (admission validation path), we can't check node readiness + // or node capabilities. We rely on explicit annotations and safe defaults. + hasNodeState := (nodeState != nil) + + // Check if we have any ready nodes (only when we have node state) + if hasNodeState && len(nodeState.lbReadyNodes) == 0 { + return "", fmt.Errorf("no ready nodes available for load balancer") + } + + // INTERNAL load balancers don't support IPv6 + isInternalLB := (network == godo.LoadBalancerNetworkTypeInternal) + + // Get desired network stack from annotation or determine default networkStack := service.Annotations[annDONetworkStack] + explicitAnnotation := (networkStack != "") + if networkStack == "" { - if network != godo.LoadBalancerNetworkTypeInternal && lbType != godo.LoadBalancerTypeRegionalNetwork { - return godo.LoadBalancerNetworkStackDualstack, nil + // Determine default based on LB type and node capabilities + if isInternalLB { + networkStack = godo.LoadBalancerNetworkStackIPv4 + } else if lbType == godo.LoadBalancerTypeRegionalNetwork { + // REGIONAL_NETWORK: default to DUALSTACK only if ALL nodes are dual-stack + if hasNodeState && nodeState.isAllDualStack() { + klog.V(2).Infof("Service %s: REGIONAL_NETWORK load balancer defaulting to dual-stack (all %d nodes have IPv6)", + serviceName, len(nodeState.lbReadyNodes)) + networkStack = godo.LoadBalancerNetworkStackDualstack + } else { + // Either no node info OR mixed/IPv4-only nodes - default to IPv4 + if hasNodeState { + klog.V(2).Infof("Service %s: REGIONAL_NETWORK load balancer defaulting to IPv4 (%d singleStackV4 nodes)", + serviceName, len(nodeState.singleStackV4Nodes)) + } else { + klog.V(4).Infof("Service %s: REGIONAL_NETWORK load balancer defaulting to IPv4 (no node state available for admission validation)", + serviceName) + } + networkStack = godo.LoadBalancerNetworkStackIPv4 + } + } else { + // REGIONAL: default to DUALSTACK (connection termination allows IPv6→IPv4 translation) + // Node IPv6 capability doesn't matter for this LB type + networkStack = godo.LoadBalancerNetworkStackDualstack } - return godo.LoadBalancerNetworkStackIPv4, nil } + + // Validate network stack value if !(networkStack == godo.LoadBalancerNetworkStackIPv4 || networkStack == godo.LoadBalancerNetworkStackDualstack) { - return "", fmt.Errorf("only LB network stacks supported are (%s, %s)", godo.LoadBalancerNetworkStackIPv4, godo.LoadBalancerNetworkStackDualstack) + return "", fmt.Errorf("only LB network stacks supported are (%s, %s)", + godo.LoadBalancerNetworkStackIPv4, godo.LoadBalancerNetworkStackDualstack) } - if lbType == godo.LoadBalancerTypeRegionalNetwork && networkStack == godo.LoadBalancerNetworkStackDualstack { - return "", fmt.Errorf("dual stack networking is not supported for regional network LB with kubernetes at this time") - } - if network == godo.LoadBalancerNetworkTypeInternal && networkStack == godo.LoadBalancerNetworkStackDualstack { - return "", fmt.Errorf("dual stack networking is not supported for internal load balancer") + + // Check restrictions on dual-stack + if networkStack == godo.LoadBalancerNetworkStackDualstack { + // INTERNAL network cannot use dual-stack + if isInternalLB { + return "", fmt.Errorf("%w: dual stack networking is not supported for internal load balancer", ErrNetworkStackConfig) + } + + // REGIONAL_NETWORK with explicit dual-stack annotation requires validation + // (only when we have node state - admission path skips this) + if hasNodeState && lbType == godo.LoadBalancerTypeRegionalNetwork { + if nodeState.hasSingleStackV4() { + if explicitAnnotation { + // User explicitly requested dual-stack but has singleStackV4 nodes - PERMANENT ERROR + return "", fmt.Errorf("%w: dual-stack networking requested but %d nodes lack IPv6 addresses: %s. REGIONAL_NETWORK load balancers require all nodes to have IPv6 for dual-stack mode", + ErrNetworkStackConfig, len(nodeState.singleStackV4Nodes), formatNodeNames(nodeState.singleStackV4Nodes, 3)) + } + // Should not reach here due to default logic above, but handle defensively + return "", fmt.Errorf("BUG detected: cluster has single stack v4 nodes but was not defaulted to single stack REGIONAL NETWORK lb") + } + } + + // Note: REGIONAL type doesn't need IPv6 capability check because it terminates + // connections and can translate IPv6→IPv4 implicitly at the LB layer } + return networkStack, nil } diff --git a/cloud-controller-manager/do/loadbalancers_test.go b/cloud-controller-manager/do/loadbalancers_test.go index 4fa35359a..360f05cfe 100644 --- a/cloud-controller-manager/do/loadbalancers_test.go +++ b/cloud-controller-manager/do/loadbalancers_test.go @@ -196,6 +196,67 @@ func healthCheck(protocol string, port int, path string, proxyProtocol *bool) *g } } +// Test helpers for node creation + +// newNodeWithIPs creates a test node with specified IP addresses and ready state +func newNodeWithIPs(name string, ipv4, ipv6 string, ready bool) *v1.Node { + node := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + Addresses: []v1.NodeAddress{}, + }, + } + + if !ready { + node.Status.Conditions[0].Status = v1.ConditionFalse + } + + if ipv4 != "" { + node.Status.Addresses = append(node.Status.Addresses, + v1.NodeAddress{Type: v1.NodeExternalIP, Address: ipv4}) + } + + if ipv6 != "" { + node.Status.Addresses = append(node.Status.Addresses, + v1.NodeAddress{Type: v1.NodeExternalIP, Address: ipv6}) + } + + return node +} + +// newNodeWithInternalIPOnly creates a test node with only internal IPs (for INTERNAL LB tests) +func newNodeWithInternalIPOnly(name string, internalIP string, ready bool) *v1.Node { + node := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + Addresses: []v1.NodeAddress{}, + }, + } + + if !ready { + node.Status.Conditions[0].Status = v1.ConditionFalse + } + + if internalIP != "" { + node.Status.Addresses = append(node.Status.Addresses, + v1.NodeAddress{Type: v1.NodeInternalIP, Address: internalIP}) + } + + return node +} + func Test_getAlgorithm(t *testing.T) { testcases := []struct { name string @@ -3663,21 +3724,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -3743,21 +3792,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -3829,21 +3866,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -3914,21 +3939,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, nil, fmt.Errorf("no health check port of protocol TCP found"), @@ -3972,21 +3985,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -4052,21 +4053,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -4132,21 +4121,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -4213,21 +4190,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -4294,21 +4259,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, nil, fmt.Errorf("only one of LB size slug and size unit can be provided"), @@ -4353,21 +4306,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -4438,21 +4379,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -4529,21 +4458,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -4625,21 +4542,9 @@ func Test_buildLoadBalancerRequest(t *testing.T) { }, }, []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, &godo.LoadBalancerRequest{ Name: "afoobar123", @@ -4755,11 +4660,7 @@ func Test_buildLoadBalancerRequestWithClusterID(t *testing.T) { }, } nodes := []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), } fakeClient := newFakeDropletClient( &fakeDropletService{ @@ -4825,21 +4726,9 @@ func Test_nodeToDropletIDs(t *testing.T) { { name: "node to droplet ids", nodes: []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, droplets: []godo.Droplet{ { @@ -4860,30 +4749,18 @@ func Test_nodeToDropletIDs(t *testing.T) { { name: "node to droplet ID with droplets not in cluster", nodes: []*v1.Node{ + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), + }, + droplets: []godo.Droplet{ { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, + ID: 100, + Name: "node-1", }, { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, - }, - droplets: []godo.Droplet{ - { - ID: 100, - Name: "node-1", - }, - { - ID: 101, - Name: "node-2", + ID: 101, + Name: "node-2", }, { ID: 102, @@ -5010,7 +4887,7 @@ func Test_nodeToDropletIDs(t *testing.T) { if len(test.missingNames) > 0 { klog.Flush() - wantErrMsg := fmt.Sprintf("Failed to find droplets for nodes %s", strings.Join(test.missingNames, " ")) + wantErrMsg := fmt.Sprintf("Failed to find droplets for nodes: %s", formatNodeNameList(test.missingNames, 3)) gotErrMsg := logBuf.String() if !strings.Contains(gotErrMsg, wantErrMsg) { t.Errorf("got missing nodes error message %q, want %q contained", gotErrMsg, wantErrMsg) @@ -5356,21 +5233,9 @@ func Test_EnsureLoadBalancer(t *testing.T) { }, }, nodes: []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, lbStatus: &v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ @@ -5433,21 +5298,9 @@ func Test_EnsureLoadBalancer(t *testing.T) { }, }, nodes: []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, lbStatus: &v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ @@ -5509,21 +5362,9 @@ func Test_EnsureLoadBalancer(t *testing.T) { }, }, nodes: []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, lbStatus: &v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ @@ -5589,21 +5430,9 @@ func Test_EnsureLoadBalancer(t *testing.T) { }, newLoadBalancerID: stringP("other-load-balancer-id"), nodes: []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, lbStatus: &v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ @@ -5671,21 +5500,9 @@ func Test_EnsureLoadBalancer(t *testing.T) { }, newLoadBalancerType: stringP(godo.LoadBalancerTypeRegionalNetwork), nodes: []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, lbStatus: &v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ @@ -5733,21 +5550,9 @@ func Test_EnsureLoadBalancer(t *testing.T) { }, }, nodes: []*v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-2", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-3", - }, - }, + newNodeWithIPs("node-1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node-2", "10.0.0.2", "2001:db8::2", true), + newNodeWithIPs("node-3", "10.0.0.3", "2001:db8::3", true), }, lbStatus: nil, err: utilerrors.NewAggregate([]error{api.NewRetryError("load-balancer is currently being created", 15*time.Second)}), @@ -6422,34 +6227,78 @@ func Test_getNetworkStack(t *testing.T) { ipv4 = godo.LoadBalancerNetworkStackIPv4 dualstack = godo.LoadBalancerNetworkStackDualstack ) + + // Helper to create nodeState + nodeStateAllDualStack := &nodeState{ + lbReadyNodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true)}, + dualStackNodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true)}, + } + + nodeStateAllSingleStackV4 := &nodeState{ + lbReadyNodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "", true)}, + singleStackV4Nodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "", true)}, + } + + nodeStateMixed := &nodeState{ + lbReadyNodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node2", "10.0.0.2", "", true), + }, + dualStackNodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true)}, + singleStackV4Nodes: []*v1.Node{newNodeWithIPs("node2", "10.0.0.2", "", true)}, + } + testcases := []struct { name string service *v1.Service lbType string lbNetwork string + nodeState *nodeState wantErr bool expected *string }{ { - name: "lb type regional network with no annotation defaults to IPV4", - service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}}, + name: "REGIONAL_NETWORK with all dualStack nodes defaults to DUALSTACK", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllDualStack, + wantErr: false, + expected: &dualstack, + }, + { + name: "REGIONAL_NETWORK with singleStackV4 nodes defaults to IPV4", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllSingleStackV4, + wantErr: false, + expected: &ipv4, + }, + { + name: "REGIONAL_NETWORK with mixed nodes defaults to IPV4", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, lbType: godo.LoadBalancerTypeRegionalNetwork, lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateMixed, wantErr: false, expected: &ipv4, }, { - name: "lb type regional with no annotation defaults to DUALSTACK", - service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}}, + name: "REGIONAL with any nodes defaults to DUALSTACK", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, lbType: godo.LoadBalancerTypeRegional, lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllSingleStackV4, wantErr: false, expected: &dualstack, }, { - name: "lb type regional with annotation set to IPV4", + name: "REGIONAL with explicit IPV4 annotation", service: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", Annotations: map[string]string{ annDONetworkStack: godo.LoadBalancerNetworkStackIPv4, }, @@ -6457,13 +6306,16 @@ func Test_getNetworkStack(t *testing.T) { }, lbType: godo.LoadBalancerTypeRegional, lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllDualStack, wantErr: false, expected: &ipv4, }, { - name: "lb type regional with annotation set to DUALSTACK", + name: "REGIONAL with explicit DUALSTACK annotation", service: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", Annotations: map[string]string{ annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, }, @@ -6471,13 +6323,16 @@ func Test_getNetworkStack(t *testing.T) { }, lbType: godo.LoadBalancerTypeRegional, lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllSingleStackV4, wantErr: false, expected: &dualstack, }, { - name: "lb type regional network with annotation set to IPV4", + name: "REGIONAL_NETWORK with explicit IPV4 annotation", service: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", Annotations: map[string]string{ annDONetworkStack: godo.LoadBalancerNetworkStackIPv4, }, @@ -6485,13 +6340,50 @@ func Test_getNetworkStack(t *testing.T) { }, lbType: godo.LoadBalancerTypeRegionalNetwork, lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllDualStack, wantErr: false, expected: &ipv4, }, { - name: "lb type regional network with annotation set to DUALSTACK", + name: "REGIONAL_NETWORK with explicit DUALSTACK and all dualStack nodes", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + }, + }, + }, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllDualStack, + wantErr: false, + expected: &dualstack, + }, + { + name: "REGIONAL_NETWORK with explicit DUALSTACK but singleStackV4 nodes - ERROR", service: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + }, + }, + }, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllSingleStackV4, + wantErr: true, + expected: nil, + }, + { + name: "REGIONAL_NETWORK with explicit DUALSTACK but mixed nodes - ERROR", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", Annotations: map[string]string{ annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, }, @@ -6499,21 +6391,25 @@ func Test_getNetworkStack(t *testing.T) { }, lbType: godo.LoadBalancerTypeRegionalNetwork, lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateMixed, wantErr: true, expected: nil, }, { - name: "internal regional lb defaults to IPV4", - service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}}, + name: "INTERNAL LB defaults to IPV4", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, lbType: godo.LoadBalancerTypeRegional, lbNetwork: godo.LoadBalancerNetworkTypeInternal, + nodeState: nodeStateAllDualStack, wantErr: false, expected: &ipv4, }, { - name: "internal regional lb with annotation set to DUALSTACK", + name: "INTERNAL LB with DUALSTACK annotation - ERROR", service: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", Annotations: map[string]string{ annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, }, @@ -6521,13 +6417,16 @@ func Test_getNetworkStack(t *testing.T) { }, lbType: godo.LoadBalancerTypeRegional, lbNetwork: godo.LoadBalancerNetworkTypeInternal, + nodeState: nodeStateAllDualStack, wantErr: true, expected: nil, }, { - name: "illegal value", + name: "illegal network stack value", service: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", Annotations: map[string]string{ annDONetworkStack: "foo", }, @@ -6535,13 +6434,22 @@ func Test_getNetworkStack(t *testing.T) { }, lbType: godo.LoadBalancerTypeRegionalNetwork, lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: nodeStateAllDualStack, + wantErr: true, + }, + { + name: "no ready nodes - ERROR", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, + lbType: godo.LoadBalancerTypeRegional, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: &nodeState{lbReadyNodes: []*v1.Node{}}, wantErr: true, }, } for _, test := range testcases { t.Run(test.name, func(t *testing.T) { - stack, err := getNetworkStack(test.service, test.lbType, test.lbNetwork) + stack, err := getNetworkStack(test.service, test.lbType, test.lbNetwork, test.nodeState) if test.wantErr != (err != nil) { t.Errorf("got error %q, want error: %t", err, test.wantErr) } @@ -6555,6 +6463,1048 @@ func Test_getNetworkStack(t *testing.T) { } } +func TestGetNetworkStackWithNilNodeState(t *testing.T) { + var ( + ipv4 = godo.LoadBalancerNetworkStackIPv4 + dualstack = godo.LoadBalancerNetworkStackDualstack + ) + + testcases := []struct { + name string + service *v1.Service + lbType string + lbNetwork string + wantErr bool + expected *string + }{ + { + name: "INTERNAL LB with nil nodeState defaults to IPV4", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeInternal, + wantErr: false, + expected: &ipv4, + }, + { + name: "REGIONAL LB with nil nodeState defaults to DUALSTACK", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, + lbType: godo.LoadBalancerTypeRegional, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + wantErr: false, + expected: &dualstack, + }, + { + name: "REGIONAL_NETWORK with nil nodeState defaults to IPV4", + service: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default", Annotations: map[string]string{}}}, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + wantErr: false, + expected: &ipv4, + }, + { + name: "REGIONAL_NETWORK with explicit IPV4 annotation and nil nodeState", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackIPv4, + }, + }, + }, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + wantErr: false, + expected: &ipv4, + }, + { + name: "REGIONAL_NETWORK with explicit DUALSTACK annotation and nil nodeState - allowed (no validation)", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + }, + }, + }, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + wantErr: false, + expected: &dualstack, + }, + { + name: "INTERNAL with explicit DUALSTACK annotation - error even with nil nodeState", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + }, + }, + }, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeInternal, + wantErr: true, + expected: nil, + }, + { + name: "REGIONAL with explicit DUALSTACK annotation and nil nodeState", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + }, + }, + }, + lbType: godo.LoadBalancerTypeRegional, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + wantErr: false, + expected: &dualstack, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + // Pass nil for nodeState to test admission path behavior + stack, err := getNetworkStack(test.service, test.lbType, test.lbNetwork, nil) + if test.wantErr != (err != nil) { + t.Errorf("got error %q, want error: %t", err, test.wantErr) + } + if test.expected != nil && stack != *test.expected { + t.Fatalf("got network stack %v, want %v", stack, *test.expected) + } + if test.expected == nil && stack != "" { + t.Fatalf("expected nil/empty network stack, got %v", stack) + } + }) + } +} + +func TestClassifyNode(t *testing.T) { + testcases := []struct { + name string + node *v1.Node + expected nodeClassification + }{ + { + name: "single stack IPv4 node", + node: newNodeWithIPs("node1", "10.0.0.1", "", true), + expected: nodeClassSingleStackV4, + }, + { + name: "single stack IPv6 node", + node: newNodeWithIPs("node1", "", "2001:db8::1", true), + expected: nodeClassSingleStackV6, + }, + { + name: "dual stack node", + node: newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + expected: nodeClassDualStack, + }, + { + name: "node without external IPs", + node: newNodeWithInternalIPOnly("node1", "192.168.1.1", true), + expected: nodeClassPublicNetUnready, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + result := classifyNode(test.node) + if result != test.expected { + t.Errorf("got %v, want %v", result, test.expected) + } + }) + } +} + +func TestFormatNodeNames(t *testing.T) { + testcases := []struct { + name string + nodes []*v1.Node + maxNodes int + expected string + }{ + { + name: "empty list", + nodes: []*v1.Node{}, + maxNodes: 3, + expected: "", + }, + { + name: "single node", + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "", true), + }, + maxNodes: 3, + expected: "node1", + }, + { + name: "exactly at limit (3 nodes)", + nodes: []*v1.Node{ + newNodeWithIPs("node3", "10.0.0.3", "", true), + newNodeWithIPs("node1", "10.0.0.1", "", true), + newNodeWithIPs("node2", "10.0.0.2", "", true), + }, + maxNodes: 3, + expected: "node1, node2, node3", // Should be sorted + }, + { + name: "one over limit (4 nodes, max 3)", + nodes: []*v1.Node{ + newNodeWithIPs("node4", "10.0.0.4", "", true), + newNodeWithIPs("node2", "10.0.0.2", "", true), + newNodeWithIPs("node1", "10.0.0.1", "", true), + newNodeWithIPs("node3", "10.0.0.3", "", true), + }, + maxNodes: 3, + expected: "node1, node2, node3 [+1 more]", + }, + { + name: "many over limit (10 nodes, max 3)", + nodes: []*v1.Node{ + newNodeWithIPs("node10", "10.0.0.10", "", true), + newNodeWithIPs("node5", "10.0.0.5", "", true), + newNodeWithIPs("node1", "10.0.0.1", "", true), + newNodeWithIPs("node7", "10.0.0.7", "", true), + newNodeWithIPs("node3", "10.0.0.3", "", true), + newNodeWithIPs("node2", "10.0.0.2", "", true), + newNodeWithIPs("node9", "10.0.0.9", "", true), + newNodeWithIPs("node4", "10.0.0.4", "", true), + newNodeWithIPs("node6", "10.0.0.6", "", true), + newNodeWithIPs("node8", "10.0.0.8", "", true), + }, + maxNodes: 3, + expected: "node1, node10, node2 [+7 more]", // Sorted alphabetically + }, + { + name: "maxNodes of 0", + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "", true), + newNodeWithIPs("node2", "10.0.0.2", "", true), + }, + maxNodes: 0, + expected: " [+2 more]", // Edge case - should show all are hidden + }, + { + name: "maxNodes of 1", + nodes: []*v1.Node{ + newNodeWithIPs("node2", "10.0.0.2", "", true), + newNodeWithIPs("node1", "10.0.0.1", "", true), + }, + maxNodes: 1, + expected: "node1 [+1 more]", + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + result := formatNodeNames(test.nodes, test.maxNodes) + if result != test.expected { + t.Errorf("got %q, want %q", result, test.expected) + } + }) + } +} + +func TestFormatNodeNameList(t *testing.T) { + testcases := []struct { + name string + names []string + maxNodes int + expected string + }{ + { + name: "empty list", + names: []string{}, + maxNodes: 3, + expected: "", + }, + { + name: "single name", + names: []string{"node1"}, + maxNodes: 3, + expected: "node1", + }, + { + name: "exactly at limit", + names: []string{"node3", "node1", "node2"}, + maxNodes: 3, + expected: "node1, node2, node3", // Should be sorted + }, + { + name: "over limit", + names: []string{"node4", "node2", "node1", "node3"}, + maxNodes: 3, + expected: "node1, node2, node3 [+1 more]", + }, + { + name: "many over limit", + names: []string{"node10", "node5", "node1", "node7", "node3"}, + maxNodes: 3, + expected: "node1, node10, node3 [+2 more]", + }, + { + name: "already sorted input", + names: []string{"node1", "node2", "node3", "node4"}, + maxNodes: 2, + expected: "node1, node2 [+2 more]", + }, + { + name: "with numeric sorting (lexicographic)", + names: []string{"node100", "node2", "node10", "node1", "node20"}, + maxNodes: 3, + expected: "node1, node10, node100 [+2 more]", // Lexicographic sort + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + result := formatNodeNameList(test.names, test.maxNodes) + if result != test.expected { + t.Errorf("got %q, want %q", result, test.expected) + } + }) + } +} + +func TestFilterAndClassifyNodes_ExternalLB(t *testing.T) { + testcases := []struct { + name string + nodes []*v1.Node + isInternalLB bool + expectedReadyCount int + expectedFilteredCount int + expectedAllDualStack bool + expectedSingleStackV4 []string + expectedDualStack []string + expectedSingleStackV6 []string + }{ + { + name: "all dual stack nodes", + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node2", "10.0.0.2", "2001:db8::2", true), + }, + isInternalLB: false, + expectedReadyCount: 2, + expectedFilteredCount: 0, + expectedAllDualStack: true, + expectedDualStack: []string{"node1", "node2"}, + }, + { + name: "all single stack v4 nodes", + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "", true), + newNodeWithIPs("node2", "10.0.0.2", "", true), + }, + isInternalLB: false, + expectedReadyCount: 2, + expectedFilteredCount: 0, + expectedAllDualStack: false, + expectedSingleStackV4: []string{"node1", "node2"}, + }, + { + name: "mixed nodes", + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node2", "10.0.0.2", "", true), + }, + isInternalLB: false, + expectedReadyCount: 2, + expectedFilteredCount: 0, + expectedAllDualStack: false, + expectedSingleStackV4: []string{"node2"}, + expectedDualStack: []string{"node1"}, + }, + { + name: "nodes without external IPs filtered", + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + newNodeWithInternalIPOnly("node2", "192.168.1.1", true), + }, + isInternalLB: false, + expectedReadyCount: 1, + expectedFilteredCount: 1, + expectedAllDualStack: true, + expectedDualStack: []string{"node1"}, + }, + { + name: "IPv6-only nodes filtered", + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node2", "", "2001:db8::2", true), + }, + isInternalLB: false, + expectedReadyCount: 1, + expectedFilteredCount: 1, + expectedAllDualStack: true, + expectedDualStack: []string{"node1"}, + expectedSingleStackV6: []string{"node2"}, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + result := filterAndClassifyNodes(test.nodes, test.isInternalLB) + + if len(result.lbReadyNodes) != test.expectedReadyCount { + t.Errorf("ready nodes: got %d, want %d", len(result.lbReadyNodes), test.expectedReadyCount) + } + if result.filteredCount != test.expectedFilteredCount { + t.Errorf("filtered count: got %d, want %d", result.filteredCount, test.expectedFilteredCount) + } + if result.isAllDualStack() != test.expectedAllDualStack { + t.Errorf("isAllDualStack: got %v, want %v", result.isAllDualStack(), test.expectedAllDualStack) + } + // Compare slices, handling nil vs empty + if len(test.expectedSingleStackV4) > 0 && !reflect.DeepEqual(nodeNames(result.singleStackV4Nodes), test.expectedSingleStackV4) { + t.Errorf("singleStackV4Nodes: got %v, want %v", nodeNames(result.singleStackV4Nodes), test.expectedSingleStackV4) + } + if len(test.expectedDualStack) > 0 && !reflect.DeepEqual(nodeNames(result.dualStackNodes), test.expectedDualStack) { + t.Errorf("dualStackNodes: got %v, want %v", nodeNames(result.dualStackNodes), test.expectedDualStack) + } + if len(test.expectedSingleStackV6) > 0 && !reflect.DeepEqual(nodeNames(result.singleStackV6Nodes), test.expectedSingleStackV6) { + t.Errorf("singleStackV6Nodes: got %v, want %v", nodeNames(result.singleStackV6Nodes), test.expectedSingleStackV6) + } + }) + } +} + +func TestFilterAndClassifyNodes_InternalLB(t *testing.T) { + testcases := []struct { + name string + nodes []*v1.Node + expectedReadyCount int + expectedFilteredCount int + }{ + { + name: "ready nodes with public IPs allowed", + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + newNodeWithIPs("node2", "10.0.0.2", "", true), + }, + expectedReadyCount: 2, + expectedFilteredCount: 0, + }, + { + name: "ready nodes without public IPs allowed", + nodes: []*v1.Node{ + newNodeWithInternalIPOnly("node1", "192.168.1.1", true), + newNodeWithInternalIPOnly("node2", "192.168.1.2", true), + }, + expectedReadyCount: 2, + expectedFilteredCount: 0, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + result := filterAndClassifyNodes(test.nodes, true) // isInternalLB = true + + if len(result.lbReadyNodes) != test.expectedReadyCount { + t.Errorf("ready nodes: got %d, want %d", len(result.lbReadyNodes), test.expectedReadyCount) + } + if result.filteredCount != test.expectedFilteredCount { + t.Errorf("filtered count: got %d, want %d", result.filteredCount, test.expectedFilteredCount) + } + }) + } +} + +func TestNodeStateMethods(t *testing.T) { + node := &v1.Node{} + + testcases := []struct { + name string + state *nodeState + wantSingleStackV4 bool + wantSingleStackV6 bool + wantDualStackNodes bool + wantAllDualStack bool + }{ + { + name: "zero value nodeState", + state: &nodeState{}, + wantSingleStackV4: false, + wantSingleStackV6: false, + wantDualStackNodes: false, + wantAllDualStack: false, + }, + { + name: "only single-stack v4 nodes", + state: &nodeState{ + singleStackV4Nodes: []*v1.Node{node}, + }, + wantSingleStackV4: true, + wantSingleStackV6: false, + wantDualStackNodes: false, + wantAllDualStack: false, + }, + { + name: "only single-stack v6 nodes", + state: &nodeState{ + singleStackV6Nodes: []*v1.Node{node}, + }, + wantSingleStackV4: false, + wantSingleStackV6: true, + wantDualStackNodes: false, + wantAllDualStack: false, + }, + { + name: "only dual-stack nodes", + state: &nodeState{ + dualStackNodes: []*v1.Node{node}, + }, + wantSingleStackV4: false, + wantSingleStackV6: false, + wantDualStackNodes: true, + wantAllDualStack: true, + }, + { + name: "mixed v4 and dual-stack nodes", + state: &nodeState{ + singleStackV4Nodes: []*v1.Node{node}, + dualStackNodes: []*v1.Node{node}, + }, + wantSingleStackV4: true, + wantSingleStackV6: false, + wantDualStackNodes: true, + wantAllDualStack: false, + }, + { + name: "mixed v6 and dual-stack nodes — isAllDualStack true because v6-only nodes are filtered for external LBs", + state: &nodeState{ + singleStackV6Nodes: []*v1.Node{node}, + dualStackNodes: []*v1.Node{node}, + }, + wantSingleStackV4: false, + wantSingleStackV6: true, + wantDualStackNodes: true, + wantAllDualStack: true, + }, + { + name: "all three node types present", + state: &nodeState{ + singleStackV4Nodes: []*v1.Node{node}, + singleStackV6Nodes: []*v1.Node{node}, + dualStackNodes: []*v1.Node{node}, + }, + wantSingleStackV4: true, + wantSingleStackV6: true, + wantDualStackNodes: true, + wantAllDualStack: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if got := tc.state.hasSingleStackV4(); got != tc.wantSingleStackV4 { + t.Errorf("hasSingleStackV4() = %v, want %v", got, tc.wantSingleStackV4) + } + if got := tc.state.hasSingleStackV6(); got != tc.wantSingleStackV6 { + t.Errorf("hasSingleStackV6() = %v, want %v", got, tc.wantSingleStackV6) + } + if got := tc.state.hasDualStackNodes(); got != tc.wantDualStackNodes { + t.Errorf("hasDualStackNodes() = %v, want %v", got, tc.wantDualStackNodes) + } + if got := tc.state.isAllDualStack(); got != tc.wantAllDualStack { + t.Errorf("isAllDualStack() = %v, want %v", got, tc.wantAllDualStack) + } + }) + } +} + +func TestBuildLoadBalancerRequest_EventEmission(t *testing.T) { + testcases := []struct { + name string + service *v1.Service + nodes []*v1.Node + expectEvent bool + expectedEventReason string + expectedErrorType error + }{ + { + name: "network stack config error emits event - INTERNAL LB with DUALSTACK", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: "test-uid", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + annDONetwork: godo.LoadBalancerNetworkTypeInternal, + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "http", + Protocol: v1.ProtocolTCP, + Port: 80, + NodePort: 30000, + }, + }, + }, + }, + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + }, + expectEvent: true, + expectedEventReason: "LoadBalancerConfigError", + expectedErrorType: ErrNetworkStackConfig, + }, + { + name: "network stack config error emits event - REGIONAL_NETWORK with explicit DUALSTACK but IPv4-only nodes", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: "test-uid", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + annDOType: godo.LoadBalancerTypeRegionalNetwork, + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "http", + Protocol: v1.ProtocolTCP, + Port: 80, + NodePort: 30000, + }, + }, + }, + }, + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "", true), + }, + expectEvent: true, + expectedEventReason: "LoadBalancerConfigError", + expectedErrorType: ErrNetworkStackConfig, + }, + { + name: "non-config error does not emit event - no ready nodes", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: "test-uid", + Annotations: map[string]string{ + annDOProtocol: "http", + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "http", + Protocol: v1.ProtocolTCP, + Port: 80, + NodePort: 30000, + }, + }, + }, + }, + nodes: []*v1.Node{}, + expectEvent: false, + expectedEventReason: "", + expectedErrorType: nil, + }, + { + name: "success case does not emit event", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: "test-uid", + Annotations: map[string]string{ + annDOProtocol: "http", + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: "http", + Protocol: v1.ProtocolTCP, + Port: 80, + NodePort: 30000, + }, + }, + }, + }, + nodes: []*v1.Node{ + newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true), + }, + expectEvent: false, + expectedEventReason: "", + expectedErrorType: nil, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + // Create fake Kubernetes client + fakeClient := newFakeDropletClient(&fakeDropletService{ + listFunc: func(context.Context, *godo.ListOptions) ([]godo.Droplet, *godo.Response, error) { + return []godo.Droplet{ + {ID: 100, Name: "node1"}, + }, newFakeOKResponse(), nil + }, + }) + fakeResources := newResources("", "", publicAccessFirewall{}, fakeClient) + fakeResources.kclient = fake.NewSimpleClientset() + + // Create the service in the fake clientset + if _, err := fakeResources.kclient.CoreV1().Services(test.service.Namespace).Create( + context.Background(), test.service, metav1.CreateOptions{}); err != nil { + t.Fatalf("failed to create service: %v", err) + } + + lb := &loadBalancers{ + resources: fakeResources, + region: "nyc3", + lbActiveTimeout: 2, + lbActiveCheckTick: 1, + defaultLBType: godo.LoadBalancerTypeRegionalNetwork, + } + + // Call buildLoadBalancerRequest + _, err := lb.buildLoadBalancerRequest(context.Background(), test.service, test.nodes) + + // Check error type + if test.expectedErrorType != nil { + if err == nil { + t.Fatalf("expected error of type %v, got nil", test.expectedErrorType) + } + if !errors.Is(err, test.expectedErrorType) { + t.Errorf("expected error to wrap %v, got: %v", test.expectedErrorType, err) + } + } + + // Check event emission + events, err := fakeResources.kclient.CoreV1().Events(test.service.Namespace).List( + context.Background(), metav1.ListOptions{}) + if err != nil { + t.Fatalf("failed to list events: %v", err) + } + + if test.expectEvent { + if len(events.Items) == 0 { + t.Fatalf("expected event to be created, but none found") + } + + foundEvent := false + for _, event := range events.Items { + if event.Reason == test.expectedEventReason && + event.InvolvedObject.Name == test.service.Name && + event.Type == v1.EventTypeWarning { + foundEvent = true + // Verify event details + if event.InvolvedObject.Kind != "Service" { + t.Errorf("event involved object kind: got %s, want Service", event.InvolvedObject.Kind) + } + if event.InvolvedObject.UID != test.service.UID { + t.Errorf("event involved object UID: got %s, want %s", event.InvolvedObject.UID, test.service.UID) + } + if event.Source.Component != "digitalocean-cloud-controller-manager" { + t.Errorf("event source component: got %s, want digitalocean-cloud-controller-manager", event.Source.Component) + } + // Verify the error message is in the event + if !strings.Contains(event.Message, "dual") { + t.Errorf("event message should contain error details, got: %s", event.Message) + } + break + } + } + if !foundEvent { + t.Errorf("expected event with reason %s not found. Events: %+v", test.expectedEventReason, events.Items) + } + } else { + if len(events.Items) > 0 { + t.Errorf("expected no events, but found %d: %+v", len(events.Items), events.Items) + } + } + }) + } +} + +func TestBuildLoadBalancerRequest_NodeLess(t *testing.T) { + testcases := []struct { + name string + service *v1.Service + defaultLBType string + expectError bool + expectedLBType string + expectedNetwork string + expectedNetStack string + expectDropletIDs bool + }{ + { + name: "REGIONAL_NETWORK service without annotation defaults to IPV4", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-svc", + Namespace: "default", + UID: "abc123", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000, Protocol: v1.ProtocolTCP}, + }, + }, + }, + defaultLBType: godo.LoadBalancerTypeRegionalNetwork, + expectError: false, + expectedLBType: godo.LoadBalancerTypeRegionalNetwork, + expectedNetwork: godo.LoadBalancerNetworkTypeExternal, + expectedNetStack: godo.LoadBalancerNetworkStackIPv4, + expectDropletIDs: false, + }, + { + name: "REGIONAL service defaults to DUALSTACK", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-svc", + Namespace: "default", + UID: "abc123", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000, Protocol: v1.ProtocolTCP}, + }, + }, + }, + defaultLBType: godo.LoadBalancerTypeRegional, + expectError: false, + expectedLBType: godo.LoadBalancerTypeRegional, + expectedNetwork: godo.LoadBalancerNetworkTypeExternal, + expectedNetStack: godo.LoadBalancerNetworkStackDualstack, + expectDropletIDs: false, + }, + { + name: "REGIONAL_NETWORK with explicit DUALSTACK annotation - allowed without validation", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-svc", + Namespace: "default", + UID: "abc123", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + }, + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000, Protocol: v1.ProtocolTCP}, + }, + }, + }, + defaultLBType: godo.LoadBalancerTypeRegionalNetwork, + expectError: false, + expectedLBType: godo.LoadBalancerTypeRegionalNetwork, + expectedNetwork: godo.LoadBalancerNetworkTypeExternal, + expectedNetStack: godo.LoadBalancerNetworkStackDualstack, + expectDropletIDs: false, + }, + { + name: "INTERNAL LB defaults to IPV4", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-svc", + Namespace: "default", + UID: "abc123", + Annotations: map[string]string{ + annDONetwork: godo.LoadBalancerNetworkTypeInternal, + }, + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + Ports: []v1.ServicePort{ + {Port: 80, NodePort: 30000, Protocol: v1.ProtocolTCP}, + }, + }, + }, + defaultLBType: godo.LoadBalancerTypeRegionalNetwork, + expectError: false, + expectedLBType: godo.LoadBalancerTypeRegionalNetwork, + expectedNetwork: godo.LoadBalancerNetworkTypeInternal, + expectedNetStack: godo.LoadBalancerNetworkStackIPv4, + expectDropletIDs: false, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + godoClient := newFakeClient(nil, &fakeLBService{}, nil) + + lbReq, err := buildLoadBalancerRequest(context.Background(), test.service, godoClient, test.defaultLBType) + + if test.expectError { + if err == nil { + t.Fatal("expected error but got none") + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if lbReq == nil { + t.Fatal("expected non-nil LoadBalancerRequest") + } + + // Verify LB type + if lbReq.Type != test.expectedLBType { + t.Errorf("expected LB type %s, got %s", test.expectedLBType, lbReq.Type) + } + + // Verify network + if lbReq.Network != test.expectedNetwork { + t.Errorf("expected network %s, got %s", test.expectedNetwork, lbReq.Network) + } + + // Verify network stack + if lbReq.NetworkStack != test.expectedNetStack { + t.Errorf("expected network stack %s, got %s", test.expectedNetStack, lbReq.NetworkStack) + } + + // Verify DropletIDs are not set (should be empty for node-less path) + if test.expectDropletIDs { + if len(lbReq.DropletIDs) == 0 { + t.Error("expected DropletIDs to be set but got empty") + } + } else { + if len(lbReq.DropletIDs) > 0 { + t.Errorf("expected DropletIDs to be empty but got: %v", lbReq.DropletIDs) + } + } + }) + } +} + +func Test_ErrorWrapping_ErrNetworkStackConfig(t *testing.T) { + testcases := []struct { + name string + service *v1.Service + lbType string + lbNetwork string + nodeState *nodeState + expectError bool + expectSentinelErr bool + }{ + { + name: "INTERNAL LB with DUALSTACK - should wrap sentinel error", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + }, + }, + }, + lbType: godo.LoadBalancerTypeRegional, + lbNetwork: godo.LoadBalancerNetworkTypeInternal, + nodeState: &nodeState{ + lbReadyNodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true)}, + }, + expectError: true, + expectSentinelErr: true, + }, + { + name: "REGIONAL_NETWORK with explicit DUALSTACK and IPv4-only nodes - should wrap sentinel error", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + annDONetworkStack: godo.LoadBalancerNetworkStackDualstack, + }, + }, + }, + lbType: godo.LoadBalancerTypeRegionalNetwork, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: &nodeState{ + lbReadyNodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "", true)}, + singleStackV4Nodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "", true)}, + }, + expectError: true, + expectSentinelErr: true, + }, + { + name: "no ready nodes - should NOT wrap sentinel error", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{}, + }, + }, + lbType: godo.LoadBalancerTypeRegional, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: &nodeState{lbReadyNodes: []*v1.Node{}}, + expectError: true, + expectSentinelErr: false, + }, + { + name: "valid configuration - no error", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{}, + }, + }, + lbType: godo.LoadBalancerTypeRegional, + lbNetwork: godo.LoadBalancerNetworkTypeExternal, + nodeState: &nodeState{ + lbReadyNodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true)}, + dualStackNodes: []*v1.Node{newNodeWithIPs("node1", "10.0.0.1", "2001:db8::1", true)}, + }, + expectError: false, + expectSentinelErr: false, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + _, err := getNetworkStack(test.service, test.lbType, test.lbNetwork, test.nodeState) + + if test.expectError { + if err == nil { + t.Fatal("expected an error, got nil") + } + + // Test errors.Is() functionality + isSentinelErr := errors.Is(err, ErrNetworkStackConfig) + if test.expectSentinelErr && !isSentinelErr { + t.Errorf("expected error to wrap ErrNetworkStackConfig, but errors.Is() returned false. Error: %v", err) + } + if !test.expectSentinelErr && isSentinelErr { + t.Errorf("expected error NOT to wrap ErrNetworkStackConfig, but errors.Is() returned true. Error: %v", err) + } + + // Verify error message contains useful information + if test.expectSentinelErr && !strings.Contains(err.Error(), "network stack configuration error") { + t.Errorf("expected error message to contain sentinel error text, got: %v", err) + } + } else { + if err != nil { + t.Errorf("expected no error, got: %v", err) + } + } + }) + } +} + func Test_buildFirewall(t *testing.T) { testcases := []struct { name string diff --git a/docs/controllers/services/annotations.md b/docs/controllers/services/annotations.md index a9e05aa68..e086e0b7f 100644 --- a/docs/controllers/services/annotations.md +++ b/docs/controllers/services/annotations.md @@ -238,16 +238,36 @@ External load balancer will be accessible via the public internet. Internal load ## service.beta.kubernetes.io/do-loadbalancer-network-stack -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" then it will default to "DUALSTACK". If load balancer type is "REGIONAL_NETWORK" then it will default to "IPV4". +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". + +**Node Requirements** + +The cloud controller manager filters nodes before adding them to load balancers: +- Only nodes with `Ready` status are included in load balancers +- 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 **Note** - "INTERNAL" load balancer does not support "DUALSTACK" networking -- "REGIONAL_NETWORK" load balancer for managed kubernetes does not support "DUALSTACK" networking +- For "REGIONAL_NETWORK" load balancers, explicitly setting "DUALSTACK" when some nodes only support IPv4 will result in an error + +For Load balancer IPv6 support, you have two options: -For Load balancer IPv6 support with managed kubernetes, please use: +1. Use connection-terminating load balancer (always supports dual-stack): ``` do-loadbalancer-type: "REGIONAL" do-loadbalancer-network: "EXTERNAL" -do-loadbalancer-network-stack: "DUALSTACK" +do-loadbalancer-network-stack: "DUALSTACK" # Optional, this is the default +``` + +2. Use pass-through load balancer with dual-stack nodes (automatic dual-stack when all nodes support it): +``` +do-loadbalancer-type: "REGIONAL_NETWORK" +do-loadbalancer-network: "EXTERNAL" +# do-loadbalancer-network-stack will automatically be set to "DUALSTACK" if all nodes have both IPv4 and IPv6 external addresses ```