From 7240d87f79fbfaedd5f9dfd1066b2af1b511a2d2 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 17 Nov 2021 11:52:57 -0500 Subject: [PATCH 1/4] topology_test.go: fix a test setup bug The "node local endpoints, hints are ignored" test was not actually enabling topology correctly, so it would have gotten the expected result even if the code was wrong. (Which, FTR, it wasn't.) --- pkg/proxy/topology_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index ae9dcdfece2..f5933abd835 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -110,7 +110,7 @@ func TestFilterEndpoints(t *testing.T) { name: "node local endpoints, hints are ignored", hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - serviceInfo: &BaseServiceInfo{nodeLocalExternal: true}, + serviceInfo: &BaseServiceInfo{nodeLocalExternal: true, hintsAnnotation: "auto"}, endpoints: []endpoint{ {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, From e5ba48f7d1f3e4c2e65730465012eedd9485ac33 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 16 Nov 2021 10:33:15 -0500 Subject: [PATCH 2/4] topology_test.go: simplify expected result checking Just check that the actual IP:port of the filtered endpoints is correct; using DeepEqual requires us to copy all the extra endpoint fields (eg, ZoneHints, IsLocal) from endpoints to expectedEndpoints, which just makes the test cases unnecessarily bigger. --- pkg/proxy/topology_test.go | 173 +++++++++++-------------------------- 1 file changed, 51 insertions(+), 122 deletions(-) diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index f5933abd835..03e1d6593eb 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -17,16 +17,34 @@ limitations under the License. package proxy import ( - "reflect" + "fmt" "testing" v1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/features" ) +func checkExpectedEndpoints(expected sets.String, actual []Endpoint) error { + var errs []error + + expectedCopy := sets.NewString(expected.UnsortedList()...) + for _, ep := range actual { + if !expectedCopy.Has(ep.String()) { + errs = append(errs, fmt.Errorf("unexpected endpoint %v", ep)) + } + expectedCopy.Delete(ep.String()) + } + if len(expectedCopy) > 0 { + errs = append(errs, fmt.Errorf("missing endpoints %v", expectedCopy.UnsortedList())) + } + + return kerrors.NewAggregate(errs) +} + func TestFilterEndpoints(t *testing.T) { type endpoint struct { ip string @@ -39,7 +57,7 @@ func TestFilterEndpoints(t *testing.T) { nodeLabels map[string]string serviceInfo ServicePort endpoints []endpoint - expectedEndpoints []endpoint + expectedEndpoints sets.String }{{ name: "hints enabled, hints annotation == auto", hintsEnabled: true, @@ -51,10 +69,7 @@ func TestFilterEndpoints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), }, { name: "hints, hints annotation == disabled, hints ignored", hintsEnabled: true, @@ -66,12 +81,7 @@ func TestFilterEndpoints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "hints, hints annotation == aUto (wrong capitalization), hints ignored", hintsEnabled: true, @@ -83,12 +93,7 @@ func TestFilterEndpoints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "hints, hints annotation empty, hints ignored", hintsEnabled: true, @@ -100,12 +105,7 @@ func TestFilterEndpoints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "node local endpoints, hints are ignored", hintsEnabled: true, @@ -117,42 +117,22 @@ func TestFilterEndpoints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }} - endpointsToStringArray := func(endpoints []Endpoint) []string { - result := make([]string, 0, len(endpoints)) - for _, ep := range endpoints { - result = append(result, ep.String()) - } - return result - } - for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, tc.hintsEnabled)() endpoints := []Endpoint{} for _, ep := range tc.endpoints { - endpoints = append(endpoints, &BaseEndpointInfo{Endpoint: ep.ip, ZoneHints: ep.zoneHints, Ready: !ep.unready}) - } - - expectedEndpoints := []Endpoint{} - for _, ep := range tc.expectedEndpoints { - expectedEndpoints = append(expectedEndpoints, &BaseEndpointInfo{Endpoint: ep.ip, ZoneHints: ep.zoneHints, Ready: !ep.unready}) + endpoints = append(endpoints, &BaseEndpointInfo{Endpoint: ep.ip + ":80", ZoneHints: ep.zoneHints, Ready: !ep.unready}) } filteredEndpoints := FilterEndpoints(endpoints, tc.serviceInfo, tc.nodeLabels) - if len(filteredEndpoints) != len(expectedEndpoints) { - t.Errorf("expected %d filtered endpoints, got %d", len(expectedEndpoints), len(filteredEndpoints)) - } - if !reflect.DeepEqual(filteredEndpoints, expectedEndpoints) { - t.Errorf("expected %v, got %v", endpointsToStringArray(expectedEndpoints), endpointsToStringArray(filteredEndpoints)) + err := checkExpectedEndpoints(tc.expectedEndpoints, filteredEndpoints) + if err != nil { + t.Errorf(err.Error()) } }) } @@ -169,25 +149,25 @@ func Test_filterEndpointsWithHints(t *testing.T) { nodeLabels map[string]string hintsAnnotation string endpoints []endpoint - expectedEndpoints []endpoint + expectedEndpoints sets.String }{{ name: "empty node labels", nodeLabels: map[string]string{}, hintsAnnotation: "auto", endpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, - expectedEndpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, + expectedEndpoints: sets.NewString("10.1.2.3:80"), }, { name: "empty zone label", nodeLabels: map[string]string{v1.LabelTopologyZone: ""}, hintsAnnotation: "auto", endpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, - expectedEndpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, + expectedEndpoints: sets.NewString("10.1.2.3:80"), }, { name: "node in different zone, no endpoint filtering", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-b"}, hintsAnnotation: "auto", endpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, - expectedEndpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, + expectedEndpoints: sets.NewString("10.1.2.3:80"), }, { name: "normal endpoint filtering, auto annotation", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, @@ -198,10 +178,7 @@ func Test_filterEndpointsWithHints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), }, { name: "unready endpoint", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, @@ -212,9 +189,7 @@ func Test_filterEndpointsWithHints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a"), unready: true}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80"), }, { name: "only unready endpoints in same zone (should not filter)", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, @@ -225,12 +200,7 @@ func Test_filterEndpointsWithHints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a"), unready: true}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a"), unready: true}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a"), unready: true}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "normal endpoint filtering, Auto annotation", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, @@ -241,10 +211,7 @@ func Test_filterEndpointsWithHints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), }, { name: "hintsAnnotation empty, no filtering applied", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, @@ -255,12 +222,7 @@ func Test_filterEndpointsWithHints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "hintsAnnotation disabled, no filtering applied", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, @@ -271,12 +233,7 @@ func Test_filterEndpointsWithHints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "missing hints, no filtering applied", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, @@ -287,12 +244,7 @@ func Test_filterEndpointsWithHints(t *testing.T) { {ip: "10.1.2.5"}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5"}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "multiple hints per endpoint, filtering includes any endpoint with zone included", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-c"}, @@ -303,39 +255,20 @@ func Test_filterEndpointsWithHints(t *testing.T) { {ip: "10.1.2.5", zoneHints: sets.NewString("zone-b", "zone-d")}, {ip: "10.1.2.6", zoneHints: sets.NewString("zone-c")}, }, - expectedEndpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a", "zone-b", "zone-c")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b", "zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-c")}, - }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.6:80"), }} - endpointsToStringArray := func(endpoints []Endpoint) []string { - result := make([]string, 0, len(endpoints)) - for _, ep := range endpoints { - result = append(result, ep.String()) - } - return result - } - for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { endpoints := []Endpoint{} for _, ep := range tc.endpoints { - endpoints = append(endpoints, &BaseEndpointInfo{Endpoint: ep.ip, ZoneHints: ep.zoneHints, Ready: !ep.unready}) - } - - expectedEndpoints := []Endpoint{} - for _, ep := range tc.expectedEndpoints { - expectedEndpoints = append(expectedEndpoints, &BaseEndpointInfo{Endpoint: ep.ip, ZoneHints: ep.zoneHints, Ready: !ep.unready}) + endpoints = append(endpoints, &BaseEndpointInfo{Endpoint: ep.ip + ":80", ZoneHints: ep.zoneHints, Ready: !ep.unready}) } filteredEndpoints := filterEndpointsWithHints(endpoints, tc.hintsAnnotation, tc.nodeLabels) - if len(filteredEndpoints) != len(expectedEndpoints) { - t.Errorf("expected %d filtered endpoints, got %d", len(expectedEndpoints), len(filteredEndpoints)) - } - if !reflect.DeepEqual(filteredEndpoints, expectedEndpoints) { - t.Errorf("expected %v, got %v", endpointsToStringArray(expectedEndpoints), endpointsToStringArray(filteredEndpoints)) + err := checkExpectedEndpoints(tc.expectedEndpoints, filteredEndpoints) + if err != nil { + t.Errorf(err.Error()) } }) } @@ -345,7 +278,7 @@ func TestFilterLocalEndpoint(t *testing.T) { testCases := []struct { name string endpoints []Endpoint - expected []Endpoint + expected sets.String }{ { name: "with empty endpoints", @@ -366,10 +299,7 @@ func TestFilterLocalEndpoint(t *testing.T) { &BaseEndpointInfo{Endpoint: "10.0.0.0:80", IsLocal: true}, &BaseEndpointInfo{Endpoint: "10.0.0.1:80", IsLocal: true}, }, - expected: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.0.0.0:80", IsLocal: true}, - &BaseEndpointInfo{Endpoint: "10.0.0.1:80", IsLocal: true}, - }, + expected: sets.NewString("10.0.0.0:80", "10.0.0.1:80"), }, { name: "some endpoints are local", @@ -377,16 +307,15 @@ func TestFilterLocalEndpoint(t *testing.T) { &BaseEndpointInfo{Endpoint: "10.0.0.0:80", IsLocal: true}, &BaseEndpointInfo{Endpoint: "10.0.0.1:80", IsLocal: false}, }, - expected: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.0.0.0:80", IsLocal: true}, - }, + expected: sets.NewString("10.0.0.0:80"), }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { filteredEndpoint := FilterLocalEndpoint(tc.endpoints) - if !reflect.DeepEqual(filteredEndpoint, tc.expected) { - t.Errorf("expected %v, got %v", tc.expected, filteredEndpoint) + err := checkExpectedEndpoints(tc.expected, filteredEndpoint) + if err != nil { + t.Errorf(err.Error()) } }) } From 6caa18a6b7c2c9a5d483ee06b9a50d6737e20dab Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 16 Nov 2021 18:11:47 -0500 Subject: [PATCH 3/4] topology_test.go: remove unnecessary helper type Remove the `endpoint` type, and just use `Endpoint` directly in the testCases. --- pkg/proxy/topology_test.go | 200 +++++++++++++++++-------------------- 1 file changed, 93 insertions(+), 107 deletions(-) diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index 03e1d6593eb..14a09612eee 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -46,28 +46,23 @@ func checkExpectedEndpoints(expected sets.String, actual []Endpoint) error { } func TestFilterEndpoints(t *testing.T) { - type endpoint struct { - ip string - zoneHints sets.String - unready bool - } testCases := []struct { name string hintsEnabled bool nodeLabels map[string]string serviceInfo ServicePort - endpoints []endpoint + endpoints []Endpoint expectedEndpoints sets.String }{{ name: "hints enabled, hints annotation == auto", hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, serviceInfo: &BaseServiceInfo{nodeLocalExternal: false, hintsAnnotation: "auto"}, - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), }, { @@ -75,11 +70,11 @@ func TestFilterEndpoints(t *testing.T) { hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, serviceInfo: &BaseServiceInfo{nodeLocalExternal: false, hintsAnnotation: "disabled"}, - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { @@ -87,11 +82,11 @@ func TestFilterEndpoints(t *testing.T) { hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, serviceInfo: &BaseServiceInfo{nodeLocalExternal: false, hintsAnnotation: "aUto"}, - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { @@ -99,11 +94,11 @@ func TestFilterEndpoints(t *testing.T) { hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, serviceInfo: &BaseServiceInfo{nodeLocalExternal: false}, - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { @@ -111,11 +106,11 @@ func TestFilterEndpoints(t *testing.T) { hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, serviceInfo: &BaseServiceInfo{nodeLocalExternal: true, hintsAnnotation: "auto"}, - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }} @@ -124,12 +119,7 @@ func TestFilterEndpoints(t *testing.T) { t.Run(tc.name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, tc.hintsEnabled)() - endpoints := []Endpoint{} - for _, ep := range tc.endpoints { - endpoints = append(endpoints, &BaseEndpointInfo{Endpoint: ep.ip + ":80", ZoneHints: ep.zoneHints, Ready: !ep.unready}) - } - - filteredEndpoints := FilterEndpoints(endpoints, tc.serviceInfo, tc.nodeLabels) + filteredEndpoints := FilterEndpoints(tc.endpoints, tc.serviceInfo, tc.nodeLabels) err := checkExpectedEndpoints(tc.expectedEndpoints, filteredEndpoints) if err != nil { t.Errorf(err.Error()) @@ -139,133 +129,129 @@ func TestFilterEndpoints(t *testing.T) { } func Test_filterEndpointsWithHints(t *testing.T) { - type endpoint struct { - ip string - zoneHints sets.String - unready bool - } testCases := []struct { name string nodeLabels map[string]string hintsAnnotation string - endpoints []endpoint + endpoints []Endpoint expectedEndpoints sets.String }{{ - name: "empty node labels", - nodeLabels: map[string]string{}, - hintsAnnotation: "auto", - endpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, + name: "empty node labels", + nodeLabels: map[string]string{}, + hintsAnnotation: "auto", + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, expectedEndpoints: sets.NewString("10.1.2.3:80"), }, { - name: "empty zone label", - nodeLabels: map[string]string{v1.LabelTopologyZone: ""}, - hintsAnnotation: "auto", - endpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, + name: "empty zone label", + nodeLabels: map[string]string{v1.LabelTopologyZone: ""}, + hintsAnnotation: "auto", + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, expectedEndpoints: sets.NewString("10.1.2.3:80"), }, { - name: "node in different zone, no endpoint filtering", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-b"}, - hintsAnnotation: "auto", - endpoints: []endpoint{{ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}}, + name: "node in different zone, no endpoint filtering", + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-b"}, + hintsAnnotation: "auto", + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, expectedEndpoints: sets.NewString("10.1.2.3:80"), }, { name: "normal endpoint filtering, auto annotation", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, hintsAnnotation: "auto", - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), }, { name: "unready endpoint", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, hintsAnnotation: "auto", - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a"), unready: true}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, }, expectedEndpoints: sets.NewString("10.1.2.3:80"), }, { name: "only unready endpoints in same zone (should not filter)", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, hintsAnnotation: "auto", - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a"), unready: true}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a"), unready: true}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "normal endpoint filtering, Auto annotation", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, hintsAnnotation: "Auto", - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), }, { name: "hintsAnnotation empty, no filtering applied", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, hintsAnnotation: "", - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "hintsAnnotation disabled, no filtering applied", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, hintsAnnotation: "disabled", - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-c")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "missing hints, no filtering applied", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, hintsAnnotation: "auto", - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b")}, - {ip: "10.1.2.5"}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-a")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: nil, Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), }, { name: "multiple hints per endpoint, filtering includes any endpoint with zone included", nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-c"}, hintsAnnotation: "auto", - endpoints: []endpoint{ - {ip: "10.1.2.3", zoneHints: sets.NewString("zone-a", "zone-b", "zone-c")}, - {ip: "10.1.2.4", zoneHints: sets.NewString("zone-b", "zone-c")}, - {ip: "10.1.2.5", zoneHints: sets.NewString("zone-b", "zone-d")}, - {ip: "10.1.2.6", zoneHints: sets.NewString("zone-c")}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a", "zone-b", "zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b", "zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-b", "zone-d"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.6:80"), }} for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - endpoints := []Endpoint{} - for _, ep := range tc.endpoints { - endpoints = append(endpoints, &BaseEndpointInfo{Endpoint: ep.ip + ":80", ZoneHints: ep.zoneHints, Ready: !ep.unready}) - } - - filteredEndpoints := filterEndpointsWithHints(endpoints, tc.hintsAnnotation, tc.nodeLabels) + filteredEndpoints := filterEndpointsWithHints(tc.endpoints, tc.hintsAnnotation, tc.nodeLabels) err := checkExpectedEndpoints(tc.expectedEndpoints, filteredEndpoints) if err != nil { t.Errorf(err.Error()) @@ -288,24 +274,24 @@ func TestFilterLocalEndpoint(t *testing.T) { { name: "all endpoints not local", endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.0.0.0:80", IsLocal: false}, - &BaseEndpointInfo{Endpoint: "10.0.0.1:80", IsLocal: false}, + &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false}, + &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false}, }, expected: nil, }, { name: "all endpoints are local", endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.0.0.0:80", IsLocal: true}, - &BaseEndpointInfo{Endpoint: "10.0.0.1:80", IsLocal: true}, + &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true}, + &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: true}, }, expected: sets.NewString("10.0.0.0:80", "10.0.0.1:80"), }, { name: "some endpoints are local", endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.0.0.0:80", IsLocal: true}, - &BaseEndpointInfo{Endpoint: "10.0.0.1:80", IsLocal: false}, + &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true}, + &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false}, }, expected: sets.NewString("10.0.0.0:80"), }, From 88a3c6924ec683789916dddd27a1276fd3605889 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 16 Nov 2021 09:55:16 -0500 Subject: [PATCH 4/4] topology_test.go: merge tests into a single test Move all of the tests into TestFilterEndpoints, rather than separately testing some of its internal helper functions (which will be going away). --- pkg/proxy/topology_test.go | 331 +++++++++++++++++-------------------- 1 file changed, 152 insertions(+), 179 deletions(-) diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index 14a09612eee..a151fc2b182 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -113,6 +113,158 @@ func TestFilterEndpoints(t *testing.T) { &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), + }, { + name: "empty node labels", + hintsEnabled: true, + nodeLabels: map[string]string{}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80"), + }, { + name: "empty zone label", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: ""}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80"), + }, { + name: "node in different zone, no endpoint filtering", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-b"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80"), + }, { + name: "normal endpoint filtering, auto annotation", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), + }, { + name: "unready endpoint", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80"), + }, { + name: "only unready endpoints in same zone (should not filter)", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), + }, { + name: "normal endpoint filtering, Auto annotation", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "Auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), + }, { + name: "hintsAnnotation empty, no filtering applied", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: ""}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), + }, { + name: "hintsAnnotation disabled, no filtering applied", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "disabled"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), + }, { + name: "missing hints, no filtering applied", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: nil, Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), + }, { + name: "multiple hints per endpoint, filtering includes any endpoint with zone included", + hintsEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-c"}, + serviceInfo: &BaseServiceInfo{hintsAnnotation: "auto"}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a", "zone-b", "zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b", "zone-c"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-b", "zone-d"), Ready: true}, + &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, + }, + expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.6:80"), + }, { + name: "internalTrafficPolicy: Local, with empty endpoints", + serviceInfo: &BaseServiceInfo{nodeLocalInternal: true}, + endpoints: []Endpoint{}, + expectedEndpoints: nil, + }, { + name: "internalTrafficPolicy: Local, but all endpoints are remote", + serviceInfo: &BaseServiceInfo{nodeLocalInternal: true}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false}, + &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false}, + }, + expectedEndpoints: nil, + }, { + name: "internalTrafficPolicy: Local, all endpoints are local", + serviceInfo: &BaseServiceInfo{nodeLocalInternal: true}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true}, + &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: true}, + }, + expectedEndpoints: sets.NewString("10.0.0.0:80", "10.0.0.1:80"), + }, { + name: "internalTrafficPolicy: Local, some endpoints are local", + serviceInfo: &BaseServiceInfo{nodeLocalInternal: true}, + endpoints: []Endpoint{ + &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true}, + &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false}, + }, + expectedEndpoints: sets.NewString("10.0.0.0:80"), }} for _, tc := range testCases { @@ -127,182 +279,3 @@ func TestFilterEndpoints(t *testing.T) { }) } } - -func Test_filterEndpointsWithHints(t *testing.T) { - testCases := []struct { - name string - nodeLabels map[string]string - hintsAnnotation string - endpoints []Endpoint - expectedEndpoints sets.String - }{{ - name: "empty node labels", - nodeLabels: map[string]string{}, - hintsAnnotation: "auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80"), - }, { - name: "empty zone label", - nodeLabels: map[string]string{v1.LabelTopologyZone: ""}, - hintsAnnotation: "auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80"), - }, { - name: "node in different zone, no endpoint filtering", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-b"}, - hintsAnnotation: "auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80"), - }, { - name: "normal endpoint filtering, auto annotation", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - hintsAnnotation: "auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), - }, { - name: "unready endpoint", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - hintsAnnotation: "auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80"), - }, { - name: "only unready endpoints in same zone (should not filter)", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - hintsAnnotation: "auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, - &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: false}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), - }, { - name: "normal endpoint filtering, Auto annotation", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - hintsAnnotation: "Auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), - }, { - name: "hintsAnnotation empty, no filtering applied", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - hintsAnnotation: "", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), - }, { - name: "hintsAnnotation disabled, no filtering applied", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - hintsAnnotation: "disabled", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), - }, { - name: "missing hints, no filtering applied", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - hintsAnnotation: "auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: nil, Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), - }, { - name: "multiple hints per endpoint, filtering includes any endpoint with zone included", - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-c"}, - hintsAnnotation: "auto", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.1.2.3:80", ZoneHints: sets.NewString("zone-a", "zone-b", "zone-c"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.4:80", ZoneHints: sets.NewString("zone-b", "zone-c"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-b", "zone-d"), Ready: true}, - &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, - }, - expectedEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.6:80"), - }} - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - filteredEndpoints := filterEndpointsWithHints(tc.endpoints, tc.hintsAnnotation, tc.nodeLabels) - err := checkExpectedEndpoints(tc.expectedEndpoints, filteredEndpoints) - if err != nil { - t.Errorf(err.Error()) - } - }) - } -} - -func TestFilterLocalEndpoint(t *testing.T) { - testCases := []struct { - name string - endpoints []Endpoint - expected sets.String - }{ - { - name: "with empty endpoints", - endpoints: []Endpoint{}, - expected: nil, - }, - { - name: "all endpoints not local", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: false}, - &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false}, - }, - expected: nil, - }, - { - name: "all endpoints are local", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true}, - &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: true}, - }, - expected: sets.NewString("10.0.0.0:80", "10.0.0.1:80"), - }, - { - name: "some endpoints are local", - endpoints: []Endpoint{ - &BaseEndpointInfo{Endpoint: "10.0.0.0:80", Ready: true, IsLocal: true}, - &BaseEndpointInfo{Endpoint: "10.0.0.1:80", Ready: true, IsLocal: false}, - }, - expected: sets.NewString("10.0.0.0:80"), - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - filteredEndpoint := FilterLocalEndpoint(tc.endpoints) - err := checkExpectedEndpoints(tc.expected, filteredEndpoint) - if err != nil { - t.Errorf(err.Error()) - } - }) - } -}