From 413af836b381b763957b3bfe1c77070028915b5d Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 13 Mar 2025 11:25:27 -0400 Subject: [PATCH] Minor updates to traffic distribution unit tests Merge TestReconcileHints_trafficDistribution_is_PreferClose and TestReconcileHints_trafficDistribution_is_nil_or_empty together. Change the `trafficDistribution: ""` test to `trafficDistribution: Unknown`, since `""` is not actually a possible value (but we should still test that unknown values are ignored, to prevent weird skew bugs). Fill in the NodeName field in the endpoints. It's not needed yet but it will be. --- .../trafficdist/trafficdist_test.go | 88 +++++++++++-------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go index 61b4f54f52a..32d1d3c644b 100644 --- a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go +++ b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go @@ -26,7 +26,7 @@ import ( "k8s.io/utils/ptr" ) -func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { +func TestReconcileHints(t *testing.T) { testCases := []struct { name string @@ -40,7 +40,8 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { wantSlicesUnchanged []*discoveryv1.EndpointSlice }{ { - name: "should set same zone hints", + name: "should set zone hints with PreferClose", + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose), slicesToCreate: []*discoveryv1.EndpointSlice{ { @@ -48,11 +49,13 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.1"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, { Addresses: []string{"10.0.0.2"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -62,11 +65,13 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.3"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-3"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, { Addresses: []string{"10.0.0.4"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-4"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -78,11 +83,13 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.5"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, { Addresses: []string{"10.0.0.6"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-6"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -92,11 +99,13 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.7"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-7"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, { Addresses: []string{"10.0.0.8"}, Zone: ptr.To("zone-c"), + NodeName: ptr.To("node-8"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -108,12 +117,14 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.1"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, { Addresses: []string{"10.0.0.2"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, }, @@ -124,12 +135,14 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.3"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-3"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, { Addresses: []string{"10.0.0.4"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-4"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, }, @@ -142,12 +155,14 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.5"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, { Addresses: []string{"10.0.0.6"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-6"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, @@ -158,12 +173,14 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.7"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-7"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, }, { Addresses: []string{"10.0.0.8"}, Zone: ptr.To("zone-c"), + NodeName: ptr.To("node-8"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-c"}}}, }, @@ -172,7 +189,8 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { }, }, { - name: "incorrect hints should be corrected", + name: "should correct incorrect hints with PreferClose", + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose), slicesToUpdate: []*discoveryv1.EndpointSlice{ { @@ -180,6 +198,7 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.1"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, // incorrect hint as per new heuristic }, @@ -192,6 +211,7 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.2"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-c"}}}, }, @@ -202,6 +222,7 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.3"}, Zone: ptr.To("zone-c"), + NodeName: ptr.To("node-3"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -213,6 +234,7 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.1"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, @@ -223,6 +245,7 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.2"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, }, @@ -233,6 +256,7 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.3"}, Zone: ptr.To("zone-c"), + NodeName: ptr.To("node-3"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-c"}}}, }, @@ -241,7 +265,8 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { }, }, { - name: "unready endpoints should not trigger updates", + name: "unready endpoints should not trigger updates", + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose), slicesUnchanged: []*discoveryv1.EndpointSlice{ { @@ -249,6 +274,7 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.2"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(false)}, // endpoint is not ready }, }, @@ -260,59 +286,31 @@ func TestReconcileHints_trafficDistribution_is_PreferClose(t *testing.T) { { Addresses: []string{"10.0.0.2"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(false)}, }, }, }, }, }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - gotSlicesToCreate, gotSlicesToUpdate, gotSlicesUnchanged := ReconcileHints(tc.trafficDistribution, tc.slicesToCreate, tc.slicesToUpdate, tc.slicesUnchanged) - - if diff := cmp.Diff(tc.wantSlicesToCreate, gotSlicesToCreate, cmpopts.EquateEmpty()); diff != "" { - t.Errorf("ReconcileHints(...) returned unexpected diff in 'slicesToCreate': (-want, +got)\n%v", diff) - } - if diff := cmp.Diff(tc.wantSlicesToUpdate, gotSlicesToUpdate, cmpopts.EquateEmpty()); diff != "" { - t.Errorf("ReconcileHints(...) returned unexpected diff in 'slicesToUpdate': (-want, +got)\n%v", diff) - } - if diff := cmp.Diff(tc.wantSlicesUnchanged, gotSlicesUnchanged, cmpopts.EquateEmpty()); diff != "" { - t.Errorf("ReconcileHints(...) returned unexpected diff in 'slicesUnchanged': (-want, +got)\n%v", diff) - } - }) - } -} - -func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { - testCases := []struct { - name string - - trafficDistribution *string - slicesToCreate []*discoveryv1.EndpointSlice - slicesToUpdate []*discoveryv1.EndpointSlice - slicesUnchanged []*discoveryv1.EndpointSlice - - wantSlicesToCreate []*discoveryv1.EndpointSlice - wantSlicesToUpdate []*discoveryv1.EndpointSlice - wantSlicesUnchanged []*discoveryv1.EndpointSlice - }{ { - name: "trafficDistribution='' should remove zone hints", - trafficDistribution: ptr.To(""), + name: "should remove hints when trafficDistribution is unrecognized", + + trafficDistribution: ptr.To("Unknown"), slicesToCreate: []*discoveryv1.EndpointSlice{ { Endpoints: []discoveryv1.Endpoint{ { Addresses: []string{"10.0.0.1"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, { Addresses: []string{"10.0.0.2"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, }, @@ -323,6 +321,7 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.3"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-3"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, @@ -335,6 +334,7 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.5"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, @@ -347,11 +347,13 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.1"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, { Addresses: []string{"10.0.0.2"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -361,6 +363,7 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.3"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-3"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -372,6 +375,7 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.5"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -379,7 +383,8 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { }, }, { - name: "trafficDistribution=nil should remove zone hints", + name: "should remove hints when trafficDistribution is unset", + trafficDistribution: nil, slicesToUpdate: []*discoveryv1.EndpointSlice{ { @@ -387,6 +392,7 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.5"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, }, @@ -399,6 +405,7 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.6"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-6"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, }, @@ -411,6 +418,7 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.5"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -420,6 +428,7 @@ func TestReconcileHints_trafficDistribution_is_nil_or_empty(t *testing.T) { { Addresses: []string{"10.0.0.6"}, Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-6"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, }, @@ -452,6 +461,7 @@ func TestReconcileHints_doesNotMutateUnchangedSlices(t *testing.T) { { Addresses: []string{"10.0.0.1"}, Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, }, },