diff --git a/internal/kube/site/bindings_test.go b/internal/kube/site/bindings_test.go index ef75b2874..8818256e0 100644 --- a/internal/kube/site/bindings_test.go +++ b/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 +} diff --git a/internal/qdr/port_mapping.go b/internal/qdr/port_mapping.go index 4ca85c551..3ac375679 100644 --- a/internal/qdr/port_mapping.go +++ b/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 diff --git a/internal/qdr/port_mapping_test.go b/internal/qdr/port_mapping_test.go new file mode 100644 index 000000000..e068f723d --- /dev/null +++ b/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) + } +}