Collapse services sharing ports

KNE may require you to specify different services with the same port to
satisfy Ondatra. This created an issue because both gNMI and gNOI use
port 6030. Now, if two services share a service, the second is just
ignored.

Makefile

@@ -3,7 +3,7 @@
 # To re-generate a bundle for another specific version without changing the standard setup, you can:
 # - use the VERSION as arg of the bundle target (e.g make bundle VERSION=0.0.2)
 # - use environment variables to overwrite this value (e.g export VERSION=0.0.2)
-VERSION ?= 2.0.1
+VERSION ?= 2.0.2
 
 # Image URL to use all building/pushing image targets
 IMG ?= ghcr.io/aristanetworks/arista-ceoslab-operator

config/kustomized/manifest.yaml

@@ -487,7 +487,7 @@ spec:
         - --leader-elect
         command:
         - /manager
-        image: ghcr.io/aristanetworks/arista-ceoslab-operator:v2.0.1
+        image: ghcr.io/aristanetworks/arista-ceoslab-operator:v2.0.2
         livenessProbe:
           httpGet:
             path: /healthz

config/manager/kustomization.yaml

@@ -13,4 +13,4 @@ kind: Kustomization
 images:
 - name: controller
   newName: ghcr.io/aristanetworks/arista-ceoslab-operator
-  newTag: v2.0.1
+  newTag: v2.0.2

controllers/ceoslabdevice_controller.go

@@ -296,7 +296,10 @@ func (r *CEosLabDeviceReconciler) Reconcile(ctx_ context.Context, req ctrl.Reque
 	servicesConfed := len(device.Spec.Services) > 0
 	if servicesConfed {
 		getServiceObject := func(object client.Object) error {
-			getService(object.(*corev1.Service), device)
+			err := getService(object.(*corev1.Service), device)
+			if err != nil {
+				return err
+			}
 			return nil
 		}
 		isNewObject, err := r.getOrCreateObject(device, serviceName, getServiceObject, deviceService)
@@ -578,7 +581,10 @@ func (r *CEosLabDeviceReconciler) Reconcile(ctx_ context.Context, req ctrl.Reque
 	}
 
 	// Ensure services are the same as the spec
-	specServiceMap := getServiceMap(device)
+	specServiceMap, err := getServiceMap(device)
+	if err != nil {
+		return noRequeue, err
+	}
 	containerServiceMap := getServiceMapFromK8sAPI(deviceService)
 	if !reflect.DeepEqual(specServiceMap, containerServiceMap) {
 		msg := fmt.Sprintf("Updating CEosLabDevice %s pod services, new: %v, old: %v",
@@ -1091,8 +1097,11 @@ func getWaitForAgentsFromK8sAPI(startupProbe *corev1.Probe) []string {
 
 // Services
 
-func getService(service *corev1.Service, device *ceoslabv1alpha1.CEosLabDevice) {
-	serviceMap := getServiceMap(device)
+func getService(service *corev1.Service, device *ceoslabv1alpha1.CEosLabDevice) error {
+	serviceMap, err := getServiceMap(device)
+	if err != nil {
+		return err
+	}
 	servicePorts := getServicePortsAPI(serviceMap)
 	service.ObjectMeta.Labels = map[string]string{"pod": device.Name}
 	service.Spec = corev1.ServiceSpec{
@@ -1102,23 +1111,52 @@ func getService(service *corev1.Service, device *ceoslabv1alpha1.CEosLabDevice)
 		},
 		Type: "LoadBalancer",
 	}
-	return
+	return nil
 }
 
-func getServiceMap(device *ceoslabv1alpha1.CEosLabDevice) map[string]ceoslabv1alpha1.PortConfig {
+func getServiceMap(device *ceoslabv1alpha1.CEosLabDevice) (map[string]ceoslabv1alpha1.PortConfig, error) {
 	serviceMap := map[string]ceoslabv1alpha1.PortConfig{}
-	for service, serviceConfig := range device.Spec.Services {
+	// Handle services in a deterministic order in case we ignore a service that shares a port with
+	// another. Otherwise a future reconcilation may elide a different one and create a discrepancy.
+	serviceNames := []string{}
+	for service := range device.Spec.Services {
+		serviceNames = append(serviceNames, service)
+	}
+	sort.Strings(serviceNames)
+	portMappingOutIn := map[int32]int32{}
+	portMappingInOut := map[int32]int32{}
+	for _, service := range serviceNames {
+		serviceConfig := device.Spec.Services[service]
 		for _, portConfig := range serviceConfig.TCPPorts {
 			effectivePortConfig := portConfig
 			if out := portConfig.Out; out == 0 {
 				// Outside port not set, default to inside.
 				effectivePortConfig.Out = portConfig.In
 			}
-			serviceName := strings.ToLower(fmt.Sprintf("%s%d", service, portConfig.In))
+			// Check if we've already forwarded this port
+			in, present := portMappingOutIn[effectivePortConfig.Out]
+			if present && in != effectivePortConfig.In {
+				// We already mapped this outside port to a different inside port
+				return nil, fmt.Errorf("Service %s wants to map outside port %d to %d but it's already mapped to %d",
+					service, effectivePortConfig.Out, effectivePortConfig.In, in)
+			}
+			out, present := portMappingInOut[effectivePortConfig.In]
+			if present && out != effectivePortConfig.Out {
+				// Wr aleady mapped this inside port to a different outside port
+				return nil, fmt.Errorf("Service %s wants to map inside port %d to %d but it's already mapped to %d",
+					service, effectivePortConfig.In, effectivePortConfig.Out, out)
+			}
+			if present {
+				// Two services have the same port mapping. This is OK, we just need to ignore this one.
+				continue
+			}
+			portMappingOutIn[effectivePortConfig.Out] = effectivePortConfig.In
+			portMappingInOut[effectivePortConfig.In] = effectivePortConfig.Out
+			serviceName := strings.ToLower(fmt.Sprintf("%s%d", service, effectivePortConfig.In))
 			serviceMap[serviceName] = effectivePortConfig
 		}
 	}
-	return serviceMap
+	return serviceMap, nil
 }
 
 func getServiceMapFromK8sAPI(service *corev1.Service) map[string]ceoslabv1alpha1.PortConfig {

kuttl-test.yaml

@@ -7,7 +7,7 @@ testDirs:
 manifestDirs:
 - config/kustomized
 kindContainers:
-- ghcr.io/aristanetworks/arista-ceoslab-operator:v2.0.1
+- ghcr.io/aristanetworks/arista-ceoslab-operator:v2.0.2
 - ceos:latest
 - ceos-2:latest
 - networkop/init-wait:latest

test/reconcile-services/01-assert.yaml

@@ -41,6 +41,11 @@ spec:
     port: 25565
     protocol: TCP
     targetPort: 25565
+  - name: gnmi6030
+    port: 6030
+    protocol: TCP
+    targetPort: 6030
+    # gNOI port ignored because it's the same as gNMI
   selector:
     app: ceoslab-device
   type: LoadBalancer

test/reconcile-services/01-new-services.yaml

@@ -9,3 +9,9 @@ spec:
         - in: 22
           out: 2022
         - in: 25565
+    gnmi:
+      tcpports:
+        - in: 6030
+    gnoi:
+      tcpports:
+        - in: 6030