Support UDP for LoadBalancer (#98)

# Description  Update primarily loadbalancer.go to support UDP - Removed check preventing creation of UDP LoadBalancer - Modified updateLoadBalancer to make port/protocol combination is used as key instead of only port - Check Port/Protocol combination during NLB deletion - Added healthcheck annotation to make possible to provide a custom healthcheck port - Added example to readme Fixes #79 ## Checklist (For exoscale contributors) * [ ] Changelog updated (under *Unreleased* block) * [ ] Testing ## Testing  Added a test variant for NLB creation with UDP ``` ❯ gmake test go.mk/version.mk:13: warning: overriding recipe for target 'get-version-tag' /Users/denis/Documents/GitHub/exoscale-cloud-controller-manager/go.mk/version.mk:13: warning: ignoring old recipe for target 'get-version-tag' go.mk/version.mk:13: warning: overriding recipe for target 'get-version-tag' /Users/denis/Documents/GitHub/exoscale-cloud-controller-manager/go.mk/version.mk:13: warning: ignoring old recipe for target 'get-version-tag' '/opt/homebrew/bin/go' test \ -race \ -timeout 15s \ \ github.com/exoscale/exoscale-cloud-controller-manager/exoscale ok github.com/exoscale/exoscale-cloud-controller-manager/exoscale 5.832s ``` Manual tests done: - Added and checked UDP services - Modified service ports - Modifying 8080/tcp, 8080/udp to 8081/tcp, 8080/udp works out of the box - Modifying back to the same port/different protocol combination needs `kubectl apply --server-side` because of a long lasting kubectl bug:kubernetes/kubernetes#105610 - Talked successfully to UDP service via netcat, both with externalTrafficPolicy Cluster (and provided healthcheckport in annotation) and with Local (without the sidecar container and annotation) - Deletion of NLB - Classical NLB setup with nginx ingress
exoscale · Dec 19, 2024 · 91f4a67 · 91f4a67
1 parent b4fcd32
commit 91f4a67
Show file tree

Hide file tree

Showing 12 changed files with 354 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* Add UDP Support to LoadBalancer service
 * fix(instances): ensure internal IP is set when there is only public one
 
 ### Improvements

diff --git a/docs/service-loadbalancer.md b/docs/service-loadbalancer.md
@@ -157,6 +157,13 @@ The Exoscale NLB service health checking mode.
 
 Supported values: `tcp` (default), `http`.
 
+#### `service.beta.kubernetes.io/exoscale-loadbalancer-service-healthcheck-port`
+
+Forces an healthcheck port.
+
+Default is `NodePort` of the service when `spec.ExternalTrafficPolicy` is set to `Cluster` (default) or
+`spec.HealthCheckNodePort` when `spec.ExternalTrafficPolicy` is set to `Local`.
+
 
 #### `service.beta.kubernetes.io/exoscale-loadbalancer-service-healthcheck-uri`
 
@@ -201,6 +208,86 @@ the target *Pods*. With `spec.externalTrafficPolicy=Cluster` (the default),
 the CCM uses `spec.ports[].nodePort`.
 
 
+### Configuring a UDP service
+
+When pointing the Exoscale NLB to a UDP service, it still requires a TCP health
+check port to determine if the respective node or application is reachable.
+You can use a sidecar container to provide this functionality.
+
+Also make sure to allow UDP for the NodePort ports in your security group.
+
+```yaml
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: udp-echo-deployment
+  labels:
+    app: udp-echo
+spec:
+  replicas: 2
+  selector:
+    matchLabels:
+      app: udp-echo
+  template:
+    metadata:
+      labels:
+        app: udp-echo
+    spec:
+      containers:
+      - name: udp-echo
+        image: alpine
+        command: ["sh", "-c", "while true; do echo -n 'Echo' | nc -u -l -p 8080 -w 1; done"]
+        ports:
+        - containerPort: 8080
+          protocol: UDP
+      - name: tcp-healthcheck
+        image: k8s.gcr.io/echoserver:1.10
+        ports:
+        - containerPort: 8080
+          protocol: TCP
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: udp-echo-service
+  annotations:
+  # If you use externalTrafficPolicy: Cluster (default) you either have to use the same nodePort for both the
+  #    TCP (healthcheck) and UDP service or define here a healthcheck port which needs to be the same
+  #    as the TCP (healthcheck)'s NodePort
+  # If you use externalTrafficPolicy: local remove that annotation, it will use spec.healthCheckNodePort then
+    service.beta.kubernetes.io/exoscale-loadbalancer-service-healthcheck-port: "31621"
+spec:
+  type: LoadBalancer
+  # externalTrafficPolicy: Local
+  ports:
+    - name: udpapplication
+      port: 8080                 # External UDP Port
+      protocol: UDP
+      targetPort: 8080           # ContainerPort
+    # You can remove the following service + sidecar container when using externalTrafficPolicy: Local
+    - name: udpapphealthcheck
+      port: 8080                 # Health check port (TCP)
+      targetPort: 8080           # ContainerPort for TCP health checks
+      nodePort: 31621            # Must match the annotation above
+      protocol: TCP
+  selector:
+    app: udp-echo
+
+```
+
+**Notes:**
+
+* A [long-standing bug][k8s-same-port-bug] in kubectl prevents adding the same
+  port with different protocols (e.g., TCP and UDP) to an already existing service
+  with kubectl apply, kubectl edit, or similar commands. If you attempt this,
+  Kubernetes may delete the respective second port.  
+  Use `kubectl apply --server-side` to avoid or fix this issue.
+* If `externalTrafficPolicy: Local` is used, the CCM will automatically assign
+  `spec.healthCheckNodePort` (which checks wether a given node is online and holds endpoint for the service),
+  so the explicit annotation for the health check port and the sidecar is unnecessary.  
+  For externalTrafficPolicy: Cluster, ensure the annotation
+  `service.beta.kubernetes.io/exoscale-loadbalancer-service-healthcheck-port` matches the TCP `nodePort`.
+
 ### Using an externally managed NLB instance with the Exoscale CCM
 
 If you prefer to manage the NLB instance yourself using different tools
@@ -236,8 +323,6 @@ spec:
 
 ## ⚠️ Important Notes
 
-* Currently, the Exoscale CCM doesn't support UDP service load balancing due to
-  a [technical limitation in Kubernetes][k8s-issue-no-proto-mix].
 * As `NodePort` created by K8s *Services* are picked randomly [within a defined
   range][k8s-service-nodeport] (by default `30000-32767`), don't forget to
   configure [Security Groups][exo-sg] used by your Compute Instance Pools to
@@ -254,9 +339,9 @@ spec:
 [k8s-assign-pod-node]: https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/
 [k8s-daemonset]: https://kubernetes.io/docs/concepts/workloads/controllers/daemonset/
 [k8s-ingress-controller]: https://kubernetes.io/docs/concepts/services-networking/ingress-controllers/
-[k8s-issue-no-proto-mix]: https://github.com/kubernetes/kubernetes/issues/23880
 [k8s-service-kube-proxy]: https://kubernetes.io/docs/concepts/services-networking/service/#virtual-ips-and-service-proxies
 [k8s-service-nodeport]: https://kubernetes.io/docs/concepts/services-networking/service/#nodeport
 [k8s-service-source-ip]: https://kubernetes.io/docs/tutorials/services/source-ip/#source-ip-for-services-with-type-loadbalancer
 [k8s-service-spec]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#service-v1-core
 [k8s-serviceport-spec]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#serviceport-v1-core
+[k8s-same-port-bug]: https://github.com/kubernetes/kubernetes/issues/105610
diff --git a/exoscale/loadbalancer.go b/exoscale/loadbalancer.go
@@ -28,6 +28,7 @@ const (
 	annotationLoadBalancerServiceDescription         = annotationPrefix + "service-description"
 	annotationLoadBalancerServiceInstancePoolID      = annotationPrefix + "service-instancepool-id"
 	annotationLoadBalancerServiceHealthCheckMode     = annotationPrefix + "service-healthcheck-mode"
+	annotationLoadBalancerServiceHealthCheckPort	 = annotationPrefix + "service-healthcheck-port"
 	annotationLoadBalancerServiceHealthCheckURI      = annotationPrefix + "service-healthcheck-uri"
 	annotationLoadBalancerServiceHealthCheckInterval = annotationPrefix + "service-healthcheck-interval"
 	annotationLoadBalancerServiceHealthCheckTimeout  = annotationPrefix + "service-healthcheck-timeout"
@@ -214,7 +215,7 @@ func (l *loadBalancer) EnsureLoadBalancerDeleted(ctx context.Context, _ string,
 	remainingServices := len(nlb.Services)
 	for _, nlbService := range nlb.Services {
 		for _, servicePort := range service.Spec.Ports {
-			if int32(*nlbService.Port) == servicePort.Port {
+			if int32(*nlbService.Port) == servicePort.Port && strings.ToLower(*nlbService.Protocol) == strings.ToLower(string(servicePort.Protocol)) {
 				infof("deleting NLB service %s/%s", *nlb.Name, *nlbService.Name)
 				if err = l.p.client.DeleteNetworkLoadBalancerService(ctx, l.p.zone, nlb, nlbService); err != nil {
 					return err
@@ -265,18 +266,30 @@ func (l *loadBalancer) updateLoadBalancer(ctx context.Context, service *v1.Servi
 	}
 
 	// Delete the NLB services which port is not present in the updated version.
-	nlbServices := make(map[uint16]*egoscale.NetworkLoadBalancerService)
+	// Info: There is a long standing bug in kubectl where patching a Service towards
+	// the same port tcp/udp and possible even other properties doesn't trigger
+	// It needs then a server side apply or replace
+	// kubectl apply --server-side
+	// https://github.com/kubernetes/kubernetes/issues/39188
+	// https://github.com/kubernetes/kubernetes/issues/105610
+	type ServiceKey struct {
+		Port     uint16
+		Protocol string
+	}
+
+	nlbServices := make(map[ServiceKey]*egoscale.NetworkLoadBalancerService)
 next:
 	for _, nlbServiceCurrent := range nlbCurrent.Services {
+		key := ServiceKey{Port: *nlbServiceCurrent.Port, Protocol: *nlbServiceCurrent.Protocol}
+		debugf("Checking existing NLB service %s/%s - key %v", *nlbCurrent.Name, *nlbServiceCurrent.Name, key)
+
 		for _, nlbServiceUpdate := range nlbUpdate.Services {
-			// If a service exposing the same port already exists,
-			// flag it for update and save its ID for later reference.
-			if *nlbServiceUpdate.Port == *nlbServiceCurrent.Port {
-				debugf("Service port %d already in use by NLB service %s/%s, marking for update",
-					*nlbServiceCurrent.Port,
-					*nlbCurrent.Name,
-					*nlbServiceCurrent.Name)
-				nlbServices[*nlbServiceCurrent.Port] = nlbServiceCurrent
+			updateKey := ServiceKey{Port: *nlbServiceUpdate.Port, Protocol: *nlbServiceUpdate.Protocol}
+
+			if key == updateKey {
+				debugf("Match found for existing service %s/%s with updated service %s/%s",
+                	*nlbCurrent.Name, *nlbServiceCurrent.Name, *nlbUpdate.Name, *nlbServiceUpdate.Name)
+				nlbServices[key] = nlbServiceCurrent
 				continue next
 			}
 		}
@@ -307,7 +320,10 @@ next:
 
 	// Update existing services and add new ones.
 	for _, nlbServiceUpdate := range nlbUpdate.Services {
-		if nlbServiceCurrent, ok := nlbServices[*nlbServiceUpdate.Port]; ok {
+		key := ServiceKey{Port: *nlbServiceUpdate.Port, Protocol: *nlbServiceUpdate.Protocol}
+		debugf("Checking updated NLB service %s/%s - key %v", *nlbUpdate.Name, *nlbServiceUpdate.Name, key)
+
+		if nlbServiceCurrent, ok := nlbServices[key]; ok {
 			nlbServiceUpdate.ID = nlbServiceCurrent.ID
 			if isLoadBalancerServiceUpdated(nlbServiceCurrent, nlbServiceUpdate) {
 				infof("updating NLB service %s/%s", *nlbCurrent.Name, *nlbServiceUpdate.Name)
@@ -537,39 +553,53 @@ func buildLoadBalancerFromAnnotations(service *v1.Service) (*egoscale.NetworkLoa
 	hcRetries := int64(hcRetriesI)
 
 	for _, servicePort := range service.Spec.Ports {
-		// If the Service is configured with externalTrafficPolicy=Local, we use the value of the
-		// healthCheckNodePort property as NLB service healthcheck port as explained in this article:
-		// https://kubernetes.io/docs/tutorials/services/source-ip/#source-ip-for-services-with-type-loadbalancer
-		// TL;DR: this configures the NLB service to ensure only Instance Pool members actually running
-		// an endpoint for the corresponding K8s Service will receive ingress traffic from the NLB, thus
-		// preserving the source IP address information.
-		hcPort := uint16(servicePort.NodePort)
-		if service.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeLocal &&
-			service.Spec.HealthCheckNodePort > 0 {
-			debugf("Service is configured with externalPolicy:Local, "+
-				"using the Service spec.healthCheckNodePort value (%d) instead "+
-				"of NodePort (%d) for NLB service healthcheck port",
-				service.Spec.HealthCheckNodePort,
-				servicePort.NodePort)
-			hcPort = uint16(service.Spec.HealthCheckNodePort)
+		var hcPort uint16
+
+		// If the user specifies a healthcheck port in the Service manifest annotations, we use that
+		// that is important for UDP services, as there the user must specify a TCP nodeport for healthchecks
+		hcPortAnnotation := getAnnotation(service, annotationLoadBalancerServiceHealthCheckPort, "")
+		if hcPortAnnotation != nil {
+			hcPortInt, err := strconv.Atoi(*hcPortAnnotation)
+			if err != nil {
+				return nil, fmt.Errorf("invalid healthcheck port annotation: %s", err)
+			}
+			hcPort = uint16(hcPortInt)
+		} else {
+			// If the Service is configured with externalTrafficPolicy=Local, we use the value of the
+			// healthCheckNodePort property as NLB service healthcheck port as explained in this article:
+			// https://kubernetes.io/docs/tutorials/services/source-ip/#source-ip-for-services-with-type-loadbalancer
+			// TL;DR: this configures the NLB service to ensure only Instance Pool members actually running
+			// an endpoint for the corresponding K8s Service will receive ingress traffic from the NLB, thus
+			// preserving the source IP address information.
+			hcPort = uint16(servicePort.NodePort)
+			if service.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeLocal &&
+				service.Spec.HealthCheckNodePort > 0 {
+				debugf("Service is configured with externalPolicy:Local, "+
+					"using the Service spec.healthCheckNodePort value (%d) instead "+
+					"of NodePort (%d) for NLB service healthcheck port",
+					service.Spec.HealthCheckNodePort,
+					servicePort.NodePort)
+				hcPort = uint16(service.Spec.HealthCheckNodePort)
+			}
 		}
 
-		// Exoscale NLB services can forward both TCP and UDP protocol, however the only supported
-		// healthcheck protocol is TCP (plain TCP or HTTP).
-		// Due to a technical limitation in Kubernetes preventing declaration of mixed protocols in a
-		// service of type LoadBalancer (https://github.com/kubernetes/kubernetes/issues/23880) we only
-		// allow TCP for service ports.
-		if servicePort.Protocol != v1.ProtocolTCP {
-			return nil, errors.New("only TCP is supported as service port protocol")
+		// We support TCP/UDP but not SCTP
+		if servicePort.Protocol != v1.ProtocolTCP && servicePort.Protocol != v1.ProtocolUDP {
+			return nil, errors.New("only TCP and UDP are supported as service port protocols")
 		}
 
 		var (
+			// Name must be unique for updateLoadBalancer to work correctly
 			svcName       = fmt.Sprintf("%s-%d", service.UID, servicePort.Port)
 			svcProtocol   = strings.ToLower(string(servicePort.Protocol))
 			svcPort       = uint16(servicePort.Port)
 			svcTargetPort = uint16(servicePort.NodePort)
 		)
 
+		if servicePort.Protocol != v1.ProtocolTCP { // Add protocol to service name if not TCP
+			svcName += "-" + strings.ToLower(string(servicePort.Protocol))
+		}
+
 		svc := egoscale.NetworkLoadBalancerService{
 			Healthcheck: &egoscale.NetworkLoadBalancerServiceHealthcheck{
 				Mode: getAnnotation(

diff --git a/exoscale/loadbalancer_test.go b/exoscale/loadbalancer_test.go
@@ -35,6 +35,7 @@ var (
 	testNLBServiceInstancePoolID            = new(exoscaleCCMTestSuite).randomID()
 	testNLBServiceName                      = new(exoscaleCCMTestSuite).randomString(10)
 	testNLBServiceProtocol                  = strings.ToLower(string(v1.ProtocolTCP))
+	testNLBServiceProtocolUDP               = strings.ToLower(string(v1.ProtocolUDP))
 	testNLBServiceStrategy                  = "round-robin"
 )
 
@@ -368,7 +369,9 @@ func (ts *exoscaleCCMTestSuite) Test_loadBalancer_EnsureLoadBalancerDeleted() {
 		k8sServiceUID                 = ts.randomID()
 		k8sServicePortPort     uint16 = 80
 		k8sServicePortNodePort uint16 = 32672
+		k8sServicePortProtocol		  = v1.ProtocolTCP
 		nlbServicePortName            = fmt.Sprintf("%s-%d", k8sServiceUID, k8sServicePortPort)
+		nlbServicePortProtocol		  = strings.ToLower(string(v1.ProtocolTCP))
 		nlbDeleted                    = false
 		nlbServiceDeleted             = false
 
@@ -378,6 +381,7 @@ func (ts *exoscaleCCMTestSuite) Test_loadBalancer_EnsureLoadBalancerDeleted() {
 			Services: []*egoscale.NetworkLoadBalancerService{{
 				Name: &nlbServicePortName,
 				Port: &k8sServicePortPort,
+				Protocol: &nlbServicePortProtocol,
 			}},
 		}
 
@@ -393,7 +397,7 @@ func (ts *exoscaleCCMTestSuite) Test_loadBalancer_EnsureLoadBalancerDeleted() {
 			},
 			Spec: v1.ServiceSpec{
 				Ports: []v1.ServicePort{{
-					Protocol: v1.ProtocolTCP,
+					Protocol: k8sServicePortProtocol,
 					Port:     int32(k8sServicePortPort),
 					NodePort: int32(k8sServicePortNodePort),
 				}},
@@ -1030,6 +1034,17 @@ func Test_buildLoadBalancerFromAnnotations(t *testing.T) {
 	actual, err = buildLoadBalancerFromAnnotations(service)
 	require.NoError(t, err)
 	require.Equal(t, expected, actual)
+
+	// Variant: UDP with healthcheck port defined
+	var serviceHealthCheckPort uint16 = 32123
+
+	service.Spec.Ports[0].Protocol = v1.ProtocolUDP
+	service.Annotations[annotationLoadBalancerServiceHealthCheckPort] = fmt.Sprint(serviceHealthCheckPort)
+	expected.Services[0].Protocol = &testNLBServiceProtocolUDP
+	expected.Services[0].Healthcheck.Port = &serviceHealthCheckPort
+	actual, err = buildLoadBalancerFromAnnotations(service)
+	require.NoError(t, err)
+	require.Equal(t, expected, actual)
 }
 
 func Test_isLoadBalancerUpdated(t *testing.T) {

diff --git a/test/README.md b/test/README.md
@@ -71,6 +71,8 @@ Run the tests with minimal verbosity (for successful tests reporting in Pull-Req
 # Set your Exoscale (API) credentials
 export EXOSCALE_API_KEY='EXO...'
 export EXOSCALE_API_SECRET='...'
+# ch-gva-2 currently required
+export EXOSCALE_ZONE="ch-gva-2"
 
 # Run the tests
 pytest

diff --git a/test/conftest.py b/test/conftest.py
@@ -512,3 +512,37 @@ def nlb_hello_ingress(test, tf_control_plane, tf_nodes, nlb_ingress_nginx, logge
             ],
             kubeconfig=tf_control_plane["kubeconfig_admin"],
         )
+
+@pytest.fixture(scope="session")
+def nlb_udp_echo_external(test, tf_control_plane, tf_nodes, ccm, logger):
+    manifest = tf_nodes["manifest_udp_echo"]
+
+    # Apply the UDP Echo application manifest
+    logger.info("[K8s] Applying UDP Echo (external NLB) application manifest ...")
+    kubectl(
+        [
+            "apply",
+            f"--filename={manifest}",
+        ],
+        kubeconfig=tf_control_plane["kubeconfig_admin"],
+        pyexit=True,
+    )
+
+    # Yield the manifest for use in tests
+    yield manifest
+
+    # Teardown
+    if not os.getenv("TEST_CCM_NO_NLB_TEARDOWN"):
+        logger.info(
+            "[K8s] Deleting UDP Echo (external NLB) application manifest (this may take some time) ..."
+        )
+        kubectl(
+            [
+                "delete",
+                f"--filename={manifest}",
+            ],
+            kubeconfig=tf_control_plane["kubeconfig_admin"],
+        )
+    else:
+        logger.info("[K8s] Skipping teardown of UDP Echo manifest due to environment variable.")
+