Updating EndpointSlice strategy to cover alpha NodeName field

pkg/registry/discovery/endpointslice/strategy.go

@@ -50,7 +50,7 @@ func (endpointSliceStrategy) PrepareForCreate(ctx context.Context, obj runtime.O
 	endpointSlice := obj.(*discovery.EndpointSlice)
 	endpointSlice.Generation = 1
 
-	dropDisabledConditionsOnCreate(endpointSlice)
+	dropDisabledFieldsOnCreate(endpointSlice)
 }
 
 // PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@@ -72,7 +72,7 @@ func (endpointSliceStrategy) PrepareForUpdate(ctx context.Context, obj, old runt
 	newEPS.ObjectMeta = ogNewMeta
 	oldEPS.ObjectMeta = ogOldMeta
 
-	dropDisabledConditionsOnUpdate(oldEPS, newEPS)
+	dropDisabledFieldsOnUpdate(oldEPS, newEPS)
 }
 
 // Validate validates a new EndpointSlice.
@@ -103,41 +103,56 @@ func (endpointSliceStrategy) AllowUnconditionalUpdate() bool {
 	return true
 }
 
-// dropDisabledConditionsOnCreate will drop the terminating condition if the
+// dropDisabledConditionsOnCreate will drop any fields that are disabled.
-// EndpointSliceTerminatingCondition is disabled. Otherwise the field is left untouched.
+func dropDisabledFieldsOnCreate(endpointSlice *discovery.EndpointSlice) {
-func dropDisabledConditionsOnCreate(endpointSlice *discovery.EndpointSlice) {
+	dropNodeName := !utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceNodeName)
-	if utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceTerminatingCondition) {
+	dropTerminating := !utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceTerminatingCondition)
-		return
-	}
 
-	// Always drop the serving/terminating conditions on create when feature gate is disabled.
+	if dropNodeName || dropTerminating {
 		for i := range endpointSlice.Endpoints {
+			if dropNodeName {
+				endpointSlice.Endpoints[i].NodeName = nil
+			}
+			if dropTerminating {
 				endpointSlice.Endpoints[i].Conditions.Serving = nil
 				endpointSlice.Endpoints[i].Conditions.Terminating = nil
 			}
+		}
+	}
 }
 
-// dropDisabledConditionsOnUpdate will drop the terminating condition field if the EndpointSliceTerminatingCondition
+// dropDisabledFieldsOnUpdate will drop any disable fields that have not already
-// feature gate is disabled unless an existing EndpointSlice object has the field already set. This ensures
+// been set on the EndpointSlice.
-// the field is not dropped on rollback.
+func dropDisabledFieldsOnUpdate(oldEPS, newEPS *discovery.EndpointSlice) {
-func dropDisabledConditionsOnUpdate(oldEPS, newEPS *discovery.EndpointSlice) {
+	dropNodeName := !utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceNodeName)
-	if utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceTerminatingCondition) {
+	if dropNodeName {
-		return
-	}
-
-	// Only drop the serving/terminating condition if the existing EndpointSlice doesn't have it set.
-	dropConditions := true
 		for _, ep := range oldEPS.Endpoints {
-		if ep.Conditions.Serving != nil || ep.Conditions.Terminating != nil {
+			if ep.NodeName != nil {
-			dropConditions = false
+				dropNodeName = false
 				break
 			}
 		}
+	}
 
-	if dropConditions {
+	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
+			}
+		}
+	}
+
+	if dropNodeName || dropTerminating {
 		for i := range newEPS.Endpoints {
+			if dropNodeName {
+				newEPS.Endpoints[i].NodeName = nil
+			}
+			if dropTerminating {
 				newEPS.Endpoints[i].Conditions.Serving = nil
 				newEPS.Endpoints[i].Conditions.Terminating = nil
 			}
 		}
+	}
 }

pkg/registry/discovery/endpointslice/strategy_test.go

@@ -27,15 +27,16 @@ import (
 	utilpointer "k8s.io/utils/pointer"
 )
 
-func Test_dropConditionsOnCreate(t *testing.T) {
+func Test_dropDisabledFieldsOnCreate(t *testing.T) {
 	testcases := []struct {
 		name                   string
 		terminatingGateEnabled bool
+		nodeNameGateEnabled    bool
 		eps                    *discovery.EndpointSlice
 		expectedEPS            *discovery.EndpointSlice
 	}{
 		{
-			name:                   "gate enabled, field should be allowed",
+			name:                   "terminating gate enabled, field should be allowed",
 			terminatingGateEnabled: true,
 			eps: &discovery.EndpointSlice{
 				Endpoints: []discovery.Endpoint{
@@ -83,7 +84,7 @@ func Test_dropConditionsOnCreate(t *testing.T) {
 			},
 		},
 		{
-			name:                   "gate disabled, field should be set to nil",
+			name:                   "terminating gate disabled, field should be set to nil",
 			terminatingGateEnabled: false,
 			eps: &discovery.EndpointSlice{
 				Endpoints: []discovery.Endpoint{
@@ -130,13 +131,62 @@ func Test_dropConditionsOnCreate(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:                "node name gate enabled, field should be allowed",
+			nodeNameGateEnabled: true,
+			eps: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2"),
+					},
+				},
+			},
+			expectedEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2"),
+					},
+				},
+			},
+		},
+		{
+			name:                "node name gate disabled, field should be allowed",
+			nodeNameGateEnabled: false,
+			eps: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2"),
+					},
+				},
+			},
+			expectedEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: nil,
+					},
+					{
+						NodeName: nil,
+					},
+				},
+			},
+		},
 	}
 
 	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.EndpointSliceNodeName, testcase.nodeNameGateEnabled)()
 
-			dropDisabledConditionsOnCreate(testcase.eps)
+			dropDisabledFieldsOnCreate(testcase.eps)
 			if !apiequality.Semantic.DeepEqual(testcase.eps, testcase.expectedEPS) {
 				t.Logf("actual endpointslice: %v", testcase.eps)
 				t.Logf("expected endpointslice: %v", testcase.expectedEPS)
@@ -146,16 +196,17 @@ func Test_dropConditionsOnCreate(t *testing.T) {
 	}
 }
 
-func Test_dropTerminatingConditionOnUpdate(t *testing.T) {
+func Test_dropDisabledFieldsOnUpdate(t *testing.T) {
 	testcases := []struct {
 		name                   string
 		terminatingGateEnabled bool
+		nodeNameGateEnabled    bool
 		oldEPS                 *discovery.EndpointSlice
 		newEPS                 *discovery.EndpointSlice
 		expectedEPS            *discovery.EndpointSlice
 	}{
 		{
-			name:                   "gate enabled, field should be allowed",
+			name:                   "terminating gate enabled, field should be allowed",
 			terminatingGateEnabled: true,
 			oldEPS: &discovery.EndpointSlice{
 				Endpoints: []discovery.Endpoint{
@@ -225,7 +276,7 @@ func Test_dropTerminatingConditionOnUpdate(t *testing.T) {
 			},
 		},
 		{
-			name:                   "gate disabled, and not set on existing EPS",
+			name:                   "terminating gate disabled, and not set on existing EPS",
 			terminatingGateEnabled: false,
 			oldEPS: &discovery.EndpointSlice{
 				Endpoints: []discovery.Endpoint{
@@ -295,7 +346,7 @@ func Test_dropTerminatingConditionOnUpdate(t *testing.T) {
 			},
 		},
 		{
-			name:                   "gate disabled, and set on existing EPS",
+			name:                   "terminating gate disabled, and set on existing EPS",
 			terminatingGateEnabled: false,
 			oldEPS: &discovery.EndpointSlice{
 				Endpoints: []discovery.Endpoint{
@@ -365,7 +416,7 @@ func Test_dropTerminatingConditionOnUpdate(t *testing.T) {
 			},
 		},
 		{
-			name:                   "gate disabled, and set on existing EPS with new values",
+			name:                   "terminating gate disabled, and set on existing EPS with new values",
 			terminatingGateEnabled: false,
 			oldEPS: &discovery.EndpointSlice{
 				Endpoints: []discovery.Endpoint{
@@ -431,13 +482,116 @@ func Test_dropTerminatingConditionOnUpdate(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:                "node name gate enabled, set on new EPS",
+			nodeNameGateEnabled: true,
+			oldEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: nil,
+					},
+					{
+						NodeName: nil,
+					},
+				},
+			},
+			newEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2"),
+					},
+				},
+			},
+			expectedEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2"),
+					},
+				},
+			},
+		},
+		{
+			name:                "node name gate disabled, set on new EPS",
+			nodeNameGateEnabled: false,
+			oldEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: nil,
+					},
+					{
+						NodeName: nil,
+					},
+				},
+			},
+			newEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2"),
+					},
+				},
+			},
+			expectedEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: nil,
+					},
+					{
+						NodeName: nil,
+					},
+				},
+			},
+		},
+		{
+			name:                "node name gate disabled, set on old and updated EPS",
+			nodeNameGateEnabled: false,
+			oldEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1-old"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2-old"),
+					},
+				},
+			},
+			newEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2"),
+					},
+				},
+			},
+			expectedEPS: &discovery.EndpointSlice{
+				Endpoints: []discovery.Endpoint{
+					{
+						NodeName: utilpointer.StringPtr("node-1"),
+					},
+					{
+						NodeName: utilpointer.StringPtr("node-2"),
+					},
+				},
+			},
+		},
 	}
 
 	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.EndpointSliceNodeName, testcase.nodeNameGateEnabled)()
 
-			dropDisabledConditionsOnUpdate(testcase.oldEPS, testcase.newEPS)
+			dropDisabledFieldsOnUpdate(testcase.oldEPS, testcase.newEPS)
 			if !apiequality.Semantic.DeepEqual(testcase.newEPS, testcase.expectedEPS) {
 				t.Logf("actual endpointslice: %v", testcase.newEPS)
 				t.Logf("expected endpointslice: %v", testcase.expectedEPS)