Fixes Controller Port Mapping Recovery

internal/kube/site/bindings_test.go

@@ -1,6 +1,8 @@
 package site
 
 import (
+	"log/slog"
+	"strconv"
 	"testing"
 
 	"github.com/skupperproject/skupper/internal/qdr"
@@ -622,3 +624,98 @@ func TestBindingAdaptor_updateBridgeConfigForListener(t *testing.T) {
 		})
 	}
 }
+
+func TestRecoveredPortMappingMatchesExtendedBindingKeys(t *testing.T) {
+	siteId := "00000000-0000-0000-0000-000000000001"
+	listener := &skupperv2alpha1.Listener{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "backend",
+			Namespace: "test",
+		},
+		Spec: skupperv2alpha1.ListenerSpec{
+			Host:       "backend",
+			Port:       8080,
+			RoutingKey: "backend",
+		},
+	}
+	perTargetListener := &skupperv2alpha1.Listener{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "backend-pods",
+			Namespace: "test",
+		},
+		Spec: skupperv2alpha1.ListenerSpec{
+			Port:             9090,
+			RoutingKey:       "backend-pods",
+			ExposePodsByName: true,
+		},
+	}
+	multiKeyListener := &skupperv2alpha1.MultiKeyListener{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "multi",
+			Namespace: "test",
+		},
+		Spec: skupperv2alpha1.MultiKeyListenerSpec{
+			Host: "multi",
+			Port: 7070,
+			Strategy: skupperv2alpha1.MultiKeyListenerStrategy{
+				Priority: &skupperv2alpha1.PriorityStrategySpec{
+					RoutingKeys: []string{"alpha", "beta"},
+				},
+			},
+		},
+	}
+	network := []skupperv2alpha1.SiteRecord{
+		{
+			Services: []skupperv2alpha1.ServiceRecord{
+				{
+					RoutingKey: "backend-pods.pod-a",
+					Connectors: []string{"backend-pods"},
+				},
+			},
+		},
+	}
+
+	generated := qdr.InitialConfig("router", "site", "test", false, 3)
+	allocator := &ExtendedBindings{
+		context:   NewMockBindingContext(nil),
+		mapping:   qdr.RecoverPortMapping(nil),
+		exposed:   ExposedPorts{},
+		selectors: map[string]TargetSelection{},
+	}
+	allocator.updateBridgeConfigForListener(siteId, listener, &generated.Bridges)
+	perTarget := newPerTargetListener(perTargetListener, slog.Default())
+	_, err := perTarget.extractTargets(network, allocator.mapping, ExposedPorts{}, NewMockBindingContext(nil))
+	assert.NilError(t, err)
+	perTarget.updateBridgeConfig(siteId, &generated.Bridges)
+	allocator.updateBridgeConfigForMultiKeyListener(siteId, multiKeyListener, &generated.Bridges)
+
+	expectedListenerPort := tcpListenerPort(t, generated.Bridges, qdr.TcpListenerNamePrefix+listener.Name)
+	expectedPerTargetPort := tcpListenerPort(t, generated.Bridges, qdr.TcpListenerNamePrefix+perTargetListener.Name+"@pod-a")
+	expectedMultiKeyListenerPort := tcpListenerPort(t, generated.Bridges, "multiAddress/"+multiKeyListener.Name)
+
+	recovered := &ExtendedBindings{
+		context:   NewMockBindingContext(nil),
+		mapping:   qdr.RecoverPortMapping(&generated),
+		exposed:   ExposedPorts{},
+		selectors: map[string]TargetSelection{},
+	}
+	recovered.ListenerUpdated(listener)
+	recovered.multiKeyListenerUpdated(multiKeyListener)
+	recoveredPerTarget := newPerTargetListener(perTargetListener, slog.Default())
+	_, err = recoveredPerTarget.extractTargets(network, recovered.mapping, recovered.exposed, recovered.context)
+	assert.NilError(t, err)
+
+	exposed := recovered.context.(*MockBindingContext).exposed
+	assert.Equal(t, exposed["backend"].Ports["backend"].TargetPort, expectedListenerPort)
+	assert.Equal(t, exposed["pod-a"].Ports["backend-pods"].TargetPort, expectedPerTargetPort)
+	assert.Equal(t, exposed["multi"].Ports["multiaddress-multi"].TargetPort, expectedMultiKeyListenerPort)
+}
+
+func tcpListenerPort(t *testing.T, config qdr.BridgeConfig, name string) int {
+	t.Helper()
+	listener, ok := config.TcpListeners[name]
+	assert.Assert(t, ok, "expected tcpListener %q", name)
+	port, err := strconv.Atoi(listener.Port)
+	assert.NilError(t, err)
+	return port
+}

internal/qdr/port_mapping.go

@@ -3,6 +3,7 @@ package qdr
 import (
 	"log/slog"
 	"strconv"
+	"strings"
 
 	"github.com/skupperproject/skupper/internal/ports"
 )
@@ -41,10 +42,20 @@ func (p *PortMapping) recovered(key string, portstr string) {
 	}
 	p.pool.InUse(port)
 	p.mappings[key] = port
-	if existing, ok := p.mappings[key]; ok {
-		p.pool.Release(existing)
-		delete(p.mappings, key)
+}
+
+func portMappingKey(listener TcpEndpoint) string {
+	if strings.HasPrefix(listener.Name, TcpListenerNamePrefix) {
+		name := strings.TrimPrefix(listener.Name, TcpListenerNamePrefix)
+		if strings.Contains(name, "@") && listener.Address != "" {
+			return listener.Address
+		}
+		return name
+	}
+	if strings.HasPrefix(listener.Name, "multiAddress/") {
+		return "multiaddress-" + strings.TrimPrefix(listener.Name, "multiAddress/")
 	}
+	return listener.Name
 }
 
 func RecoverPortMapping(config *RouterConfig) *PortMapping {
@@ -58,8 +69,8 @@ func RecoverPortMapping(config *RouterConfig) *PortMapping {
 			mapping.pool.InUse(int(listener.Port))
 		}
 
-		for key, listener := range config.Bridges.TcpListeners {
-			mapping.recovered(key, listener.Port)
+		for _, listener := range config.Bridges.TcpListeners {
+			mapping.recovered(portMappingKey(listener), listener.Port)
 		}
 	}
 	return mapping

internal/qdr/port_mapping_test.go

@@ -0,0 +1,44 @@
+package qdr
+
+import "testing"
+
+func TestRecoverPortMappingPreservesListenerPorts(t *testing.T) {
+	config := InitialConfig("router", "site", "test", false, 3)
+	config.Bridges.AddTcpListener(TcpEndpoint{
+		Name:    TcpListenerNamePrefix + "frontend",
+		Port:    "12001",
+		Address: "frontend",
+	})
+	config.Bridges.AddTcpListener(TcpEndpoint{
+		Name:    TcpListenerNamePrefix + "backend",
+		Port:    "12002",
+		Address: "backend",
+	})
+	config.Bridges.AddTcpListener(TcpEndpoint{
+		Name:    TcpListenerNamePrefix + "per-target@pod-a",
+		Port:    "12003",
+		Address: "per-target.pod-a",
+	})
+	config.Bridges.AddTcpListener(TcpEndpoint{
+		Name: "multiAddress/multi",
+		Port: "12004",
+	})
+
+	mapping := RecoverPortMapping(&config)
+
+	assertPortForKey(t, mapping, "frontend", 12001)
+	assertPortForKey(t, mapping, "backend", 12002)
+	assertPortForKey(t, mapping, "per-target.pod-a", 12003)
+	assertPortForKey(t, mapping, "multiaddress-multi", 12004)
+}
+
+func assertPortForKey(t *testing.T, mapping *PortMapping, key string, expected int) {
+	t.Helper()
+	actual, err := mapping.GetPortForKey(key)
+	if err != nil {
+		t.Fatalf("GetPortForKey(%q) returned error: %v", key, err)
+	}
+	if actual != expected {
+		t.Fatalf("GetPortForKey(%q) = %d, want %d", key, actual, expected)
+	}
+}