From 5ca805fdeeb31db31805aaa02d96dc0a0c3cfe15 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Tue, 25 Oct 2022 21:48:09 -0400 Subject: [PATCH] pkg/registry/discovery: remove references to EndpointSliceTerminatingCondition feature gate when dropping disabled fields Signed-off-by: Andrew Sy Kim --- .../discovery/endpointslice/strategy.go | 23 +- .../discovery/endpointslice/strategy_test.go | 395 +----------------- 2 files changed, 11 insertions(+), 407 deletions(-) diff --git a/pkg/registry/discovery/endpointslice/strategy.go b/pkg/registry/discovery/endpointslice/strategy.go index 1473563ce10..6bb8359f1e4 100644 --- a/pkg/registry/discovery/endpointslice/strategy.go +++ b/pkg/registry/discovery/endpointslice/strategy.go @@ -123,15 +123,10 @@ func (endpointSliceStrategy) AllowUnconditionalUpdate() bool { // dropDisabledConditionsOnCreate will drop any fields that are disabled. func dropDisabledFieldsOnCreate(endpointSlice *discovery.EndpointSlice) { - dropTerminating := !utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceTerminatingCondition) dropHints := !utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) - if dropHints || dropTerminating { + if dropHints { for i := range endpointSlice.Endpoints { - if dropTerminating { - endpointSlice.Endpoints[i].Conditions.Serving = nil - endpointSlice.Endpoints[i].Conditions.Terminating = nil - } if dropHints { endpointSlice.Endpoints[i].Hints = nil } @@ -142,16 +137,6 @@ func dropDisabledFieldsOnCreate(endpointSlice *discovery.EndpointSlice) { // dropDisabledFieldsOnUpdate will drop any disable fields that have not already // been set on the EndpointSlice. func dropDisabledFieldsOnUpdate(oldEPS, newEPS *discovery.EndpointSlice) { - dropTerminating := !utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceTerminatingCondition) - if dropTerminating { - for _, ep := range oldEPS.Endpoints { - if ep.Conditions.Serving != nil || ep.Conditions.Terminating != nil { - dropTerminating = false - break - } - } - } - dropHints := !utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) if dropHints { for _, ep := range oldEPS.Endpoints { @@ -162,12 +147,8 @@ func dropDisabledFieldsOnUpdate(oldEPS, newEPS *discovery.EndpointSlice) { } } - if dropHints || dropTerminating { + if dropHints { for i := range newEPS.Endpoints { - if dropTerminating { - newEPS.Endpoints[i].Conditions.Serving = nil - newEPS.Endpoints[i].Conditions.Terminating = nil - } if dropHints { newEPS.Endpoints[i].Hints = nil } diff --git a/pkg/registry/discovery/endpointslice/strategy_test.go b/pkg/registry/discovery/endpointslice/strategy_test.go index 8dd6a6edb11..6204fe5af62 100644 --- a/pkg/registry/discovery/endpointslice/strategy_test.go +++ b/pkg/registry/discovery/endpointslice/strategy_test.go @@ -34,108 +34,11 @@ import ( func Test_dropDisabledFieldsOnCreate(t *testing.T) { testcases := []struct { - name string - terminatingGateEnabled bool - hintsGateEnabled bool - eps *discovery.EndpointSlice - expectedEPS *discovery.EndpointSlice + name string + hintsGateEnabled bool + eps *discovery.EndpointSlice + expectedEPS *discovery.EndpointSlice }{ - { - name: "terminating gate enabled, field should be allowed", - terminatingGateEnabled: true, - eps: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - expectedEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - }, - { - name: "terminating gate disabled, field should be set to nil", - terminatingGateEnabled: false, - eps: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - expectedEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - }, { name: "node name gate enabled, field should be allowed", eps: &discovery.EndpointSlice{ @@ -163,7 +66,6 @@ func Test_dropDisabledFieldsOnCreate(t *testing.T) { for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EndpointSliceTerminatingCondition, testcase.terminatingGateEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, testcase.hintsGateEnabled)() dropDisabledFieldsOnCreate(testcase.eps) @@ -178,290 +80,12 @@ func Test_dropDisabledFieldsOnCreate(t *testing.T) { func Test_dropDisabledFieldsOnUpdate(t *testing.T) { testcases := []struct { - name string - terminatingGateEnabled bool - hintsGateEnabled bool - oldEPS *discovery.EndpointSlice - newEPS *discovery.EndpointSlice - expectedEPS *discovery.EndpointSlice + name string + hintsGateEnabled bool + oldEPS *discovery.EndpointSlice + newEPS *discovery.EndpointSlice + expectedEPS *discovery.EndpointSlice }{ - { - name: "terminating gate enabled, field should be allowed", - terminatingGateEnabled: true, - oldEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - newEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - expectedEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - }, - { - name: "terminating gate disabled, and not set on existing EPS", - terminatingGateEnabled: false, - oldEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - newEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - expectedEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - }, - { - name: "terminating gate disabled, and set on existing EPS", - terminatingGateEnabled: false, - oldEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - newEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - expectedEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: nil, - Terminating: nil, - }, - }, - }, - }, - }, - { - name: "terminating gate disabled, and set on existing EPS with new values", - terminatingGateEnabled: false, - oldEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(false), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Terminating: nil, - }, - }, - }, - }, - newEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(false), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Terminating: utilpointer.BoolPtr(false), - }, - }, - }, - }, - expectedEPS: &discovery.EndpointSlice{ - Endpoints: []discovery.Endpoint{ - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(true), - Terminating: utilpointer.BoolPtr(true), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Serving: utilpointer.BoolPtr(false), - Terminating: utilpointer.BoolPtr(false), - }, - }, - { - Conditions: discovery.EndpointConditions{ - Terminating: utilpointer.BoolPtr(false), - }, - }, - }, - }, - }, { name: "node name gate enabled, set on new EPS", oldEPS: &discovery.EndpointSlice{ @@ -658,7 +282,6 @@ func Test_dropDisabledFieldsOnUpdate(t *testing.T) { for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EndpointSliceTerminatingCondition, testcase.terminatingGateEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, testcase.hintsGateEnabled)() dropDisabledFieldsOnUpdate(testcase.oldEPS, testcase.newEPS)