From b1434d1ea3cd7ab2e8984dbe5c388241a40847a2 Mon Sep 17 00:00:00 2001 From: Rob Scott Date: Wed, 3 Mar 2021 19:05:23 -0800 Subject: [PATCH] Incrementing EndpointSlice generation when labels change EndpointSlice labels can be quite meaningful. They are used to indicate the controller they are managed by and the Service they are associated with. Changing these labels can have significant affects on how the EndpointSlice is consumed so incrementing generation seems appropriate. --- .../discovery/endpointslice/strategy.go | 2 +- .../discovery/endpointslice/strategy_test.go | 93 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/pkg/registry/discovery/endpointslice/strategy.go b/pkg/registry/discovery/endpointslice/strategy.go index 92cb2e65e06..7fc26e2cd4b 100644 --- a/pkg/registry/discovery/endpointslice/strategy.go +++ b/pkg/registry/discovery/endpointslice/strategy.go @@ -65,7 +65,7 @@ func (endpointSliceStrategy) PrepareForUpdate(ctx context.Context, obj, old runt newEPS.ObjectMeta = v1.ObjectMeta{} oldEPS.ObjectMeta = v1.ObjectMeta{} - if !apiequality.Semantic.DeepEqual(newEPS, oldEPS) { + if !apiequality.Semantic.DeepEqual(newEPS, oldEPS) || !apiequality.Semantic.DeepEqual(ogNewMeta.Labels, ogOldMeta.Labels) { ogNewMeta.Generation = ogOldMeta.Generation + 1 } diff --git a/pkg/registry/discovery/endpointslice/strategy_test.go b/pkg/registry/discovery/endpointslice/strategy_test.go index fce0bf76231..e9e2c3d0eec 100644 --- a/pkg/registry/discovery/endpointslice/strategy_test.go +++ b/pkg/registry/discovery/endpointslice/strategy_test.go @@ -17,9 +17,11 @@ limitations under the License. package endpointslice import ( + "context" "testing" apiequality "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilfeature "k8s.io/apiserver/pkg/util/feature" featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/apis/discovery" @@ -600,3 +602,94 @@ func Test_dropDisabledFieldsOnUpdate(t *testing.T) { }) } } + +func TestPrepareForUpdate(t *testing.T) { + testCases := []struct { + name string + oldEPS *discovery.EndpointSlice + newEPS *discovery.EndpointSlice + expectedEPS *discovery.EndpointSlice + }{ + { + name: "unchanged EPS should not increment generation", + oldEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.4"}, + }}, + }, + newEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.4"}, + }}, + }, + expectedEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.4"}, + }}, + }, + }, + { + name: "changed endpoints should increment generation", + oldEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.4"}, + }}, + }, + newEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.5"}, + }}, + }, + expectedEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{Generation: 2}, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.5"}, + }}, + }, + }, + { + name: "changed labels should increment generation", + oldEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + Labels: map[string]string{"example": "one"}, + }, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.4"}, + }}, + }, + newEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + Labels: map[string]string{"example": "two"}, + }, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.4"}, + }}, + }, + expectedEPS: &discovery.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 2, + Labels: map[string]string{"example": "two"}, + }, + Endpoints: []discovery.Endpoint{{ + Addresses: []string{"1.2.3.4"}, + }}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + Strategy.PrepareForUpdate(context.TODO(), tc.newEPS, tc.oldEPS) + if !apiequality.Semantic.DeepEqual(tc.newEPS, tc.expectedEPS) { + t.Errorf("Expected %+v\nGot: %+v", tc.expectedEPS, tc.newEPS) + } + }) + } +}