diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index ec105a41f44..0598bcc7237 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3945,7 +3945,6 @@ type ServiceSpec struct { // dropping the traffic if there are no local endpoints. The default value, // "Cluster", uses the standard behavior of routing to all endpoints evenly // (possibly modified by topology and other features). - // +featureGate=ServiceInternalTrafficPolicy // +optional InternalTrafficPolicy *ServiceInternalTrafficPolicyType } diff --git a/pkg/apis/core/v1/defaults.go b/pkg/apis/core/v1/defaults.go index 0d16a8a1bda..b42221691fa 100644 --- a/pkg/apis/core/v1/defaults.go +++ b/pkg/apis/core/v1/defaults.go @@ -22,8 +22,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/util/parsers" "k8s.io/utils/pointer" ) @@ -130,14 +128,11 @@ func SetDefaults_Service(obj *v1.Service) { obj.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeCluster } - if utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) { - if obj.Spec.InternalTrafficPolicy == nil { - if obj.Spec.Type == v1.ServiceTypeNodePort || obj.Spec.Type == v1.ServiceTypeLoadBalancer || obj.Spec.Type == v1.ServiceTypeClusterIP { - serviceInternalTrafficPolicyCluster := v1.ServiceInternalTrafficPolicyCluster - obj.Spec.InternalTrafficPolicy = &serviceInternalTrafficPolicyCluster - } + if obj.Spec.InternalTrafficPolicy == nil { + if obj.Spec.Type == v1.ServiceTypeNodePort || obj.Spec.Type == v1.ServiceTypeLoadBalancer || obj.Spec.Type == v1.ServiceTypeClusterIP { + serviceInternalTrafficPolicyCluster := v1.ServiceInternalTrafficPolicyCluster + obj.Spec.InternalTrafficPolicy = &serviceInternalTrafficPolicyCluster } - } if obj.Spec.Type == v1.ServiceTypeLoadBalancer { diff --git a/pkg/apis/core/v1/defaults_test.go b/pkg/apis/core/v1/defaults_test.go index 983ee8318c0..3a93eff021b 100644 --- a/pkg/apis/core/v1/defaults_test.go +++ b/pkg/apis/core/v1/defaults_test.go @@ -33,7 +33,6 @@ import ( featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/api/legacyscheme" corev1 "k8s.io/kubernetes/pkg/apis/core/v1" - "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" // ensure types are installed @@ -1862,13 +1861,11 @@ func TestSetDefaultServiceInternalTrafficPolicy(t *testing.T) { name string expectedInternalTrafficPolicy *v1.ServiceInternalTrafficPolicyType svc v1.Service - featureGateOn bool }{ { name: "must set default internalTrafficPolicy", expectedInternalTrafficPolicy: &cluster, svc: v1.Service{}, - featureGateOn: true, }, { name: "must not set default internalTrafficPolicy when it's cluster", @@ -1878,7 +1875,6 @@ func TestSetDefaultServiceInternalTrafficPolicy(t *testing.T) { InternalTrafficPolicy: &cluster, }, }, - featureGateOn: true, }, { name: "must not set default internalTrafficPolicy when type is ExternalName", @@ -1888,7 +1884,6 @@ func TestSetDefaultServiceInternalTrafficPolicy(t *testing.T) { Type: v1.ServiceTypeExternalName, }, }, - featureGateOn: true, }, { name: "must not set default internalTrafficPolicy when it's local", @@ -1898,18 +1893,10 @@ func TestSetDefaultServiceInternalTrafficPolicy(t *testing.T) { InternalTrafficPolicy: &local, }, }, - featureGateOn: true, - }, - { - name: "must not set default internalTrafficPolicy when gate is disabled", - expectedInternalTrafficPolicy: nil, - svc: v1.Service{}, - featureGateOn: false, }, } for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, test.featureGateOn)() obj := roundTrip(t, runtime.Object(&test.svc)) svc := obj.(*v1.Service) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 53ae4988a8c..7b46b7b9498 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4876,14 +4876,12 @@ func validateServiceExternalTrafficFieldsUpdate(before, after *core.Service) fie func validateServiceInternalTrafficFieldsValue(service *core.Service) field.ErrorList { allErrs := field.ErrorList{} - if utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) { - if service.Spec.InternalTrafficPolicy == nil { - // We do not forbid internalTrafficPolicy on other Service types because of historical reasons. - // We did not check that before it went beta and we don't want to invalidate existing stored objects. - if service.Spec.Type == core.ServiceTypeNodePort || - service.Spec.Type == core.ServiceTypeLoadBalancer || service.Spec.Type == core.ServiceTypeClusterIP { - allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("internalTrafficPolicy"), "")) - } + if service.Spec.InternalTrafficPolicy == nil { + // We do not forbid internalTrafficPolicy on other Service types because of historical reasons. + // We did not check that before it went beta and we don't want to invalidate existing stored objects. + if service.Spec.Type == core.ServiceTypeNodePort || + service.Spec.Type == core.ServiceTypeLoadBalancer || service.Spec.Type == core.ServiceTypeClusterIP { + allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("internalTrafficPolicy"), "")) } } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 29c985a216a..c935e6f7384 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -13335,8 +13335,7 @@ func TestValidateServiceCreate(t *testing.T) { tweakSvc: func(s *core.Service) { s.Spec.InternalTrafficPolicy = nil }, - featureGates: []featuregate.Feature{features.ServiceInternalTrafficPolicy}, - numErrs: 1, + numErrs: 1, }, { name: "internalTrafficPolicy field nil when type is ExternalName", @@ -13345,8 +13344,7 @@ func TestValidateServiceCreate(t *testing.T) { s.Spec.Type = core.ServiceTypeExternalName s.Spec.ExternalName = "foo.bar.com" }, - featureGates: []featuregate.Feature{features.ServiceInternalTrafficPolicy}, - numErrs: 0, + numErrs: 0, }, { // Typically this should fail validation, but in v1.22 we have existing clusters @@ -13360,8 +13358,7 @@ func TestValidateServiceCreate(t *testing.T) { s.Spec.Type = core.ServiceTypeExternalName s.Spec.ExternalName = "foo.bar.com" }, - featureGates: []featuregate.Feature{features.ServiceInternalTrafficPolicy}, - numErrs: 0, + numErrs: 0, }, { name: "invalid internalTraffic field", diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 947b481d7d4..6150cffa7cb 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -720,6 +720,7 @@ const ( // kep: https://kep.k8s.io/2086 // alpha: v1.21 // beta: v1.22 + // GA: v1.26 // // Enables node-local routing for Service internal traffic ServiceInternalTrafficPolicy featuregate.Feature = "ServiceInternalTrafficPolicy" @@ -1017,7 +1018,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS ServiceIPStaticSubrange: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28 - ServiceInternalTrafficPolicy: {Default: true, PreRelease: featuregate.Beta}, + ServiceInternalTrafficPolicy: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28 SizeMemoryBackedVolumes: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 6e965f7a7b0..351c01c18a8 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -5275,34 +5275,10 @@ func TestInternalTrafficPolicyE2E(t *testing.T) { }, }, }, - { - name: "Local internalTrafficPolicy is ignored when feature gate is off", - line: getLine(), - internalTrafficPolicy: &local, - featureGateOn: false, - endpoints: []endpoint{ - {"10.0.1.1", testHostname}, - {"10.0.1.2", "host1"}, - {"10.0.1.3", "host2"}, - }, - expectEndpointRule: false, - expectedIPTablesWithSlice: clusterExpectedIPTables, - flowTests: []packetFlowTest{ - { - name: "pod to ClusterIP hits all endpoints", - sourceIP: "10.0.0.2", - destIP: "172.30.1.1", - destPort: 80, - output: "10.0.1.1:80, 10.0.1.2:80, 10.0.1.3:80", - masq: false, - }, - }, - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, tc.featureGateOn)() ipt := iptablestest.NewFake() fp := NewFakeProxier(ipt) fp.OnServiceSynced() @@ -7434,8 +7410,6 @@ func TestInternalExternalMasquerade(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, true)() - ipt := iptablestest.NewFake() fp := NewFakeProxier(ipt) fp.masqueradeAll = tc.masqueradeAll @@ -8274,7 +8248,6 @@ func TestNoEndpointsMetric(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, true)() ipt := iptablestest.NewFake() fp := NewFakeProxier(ipt) fp.OnServiceSynced() diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 6991bd92a37..6f013cac743 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -42,10 +42,8 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/events" utilsysctl "k8s.io/component-helpers/node/util/sysctl" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/proxy" "k8s.io/kubernetes/pkg/proxy/healthcheck" "k8s.io/kubernetes/pkg/proxy/metaproxier" @@ -1239,7 +1237,7 @@ func (proxier *Proxier) syncProxyRules() { // ExternalTrafficPolicy only works for NodePort and external LB traffic, does not affect ClusterIP // So we still need clusterIP rules in onlyNodeLocalEndpoints mode. internalNodeLocal := false - if utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) && svcInfo.InternalPolicyLocal() { + if svcInfo.InternalPolicyLocal() { internalNodeLocal = true } if err := proxier.syncEndpoint(svcPortName, internalNodeLocal, serv); err != nil { @@ -2018,7 +2016,7 @@ func (proxier *Proxier) syncEndpoint(svcPortName proxy.ServicePortName, onlyNode // will have the POD address and will be discarded. endpoints = clusterEndpoints - if hasAnyEndpoints && svcInfo.InternalPolicyLocal() && utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) { + if hasAnyEndpoints && svcInfo.InternalPolicyLocal() { proxier.serviceNoLocalEndpointsInternal.Insert(svcPortName.NamespacedName.String()) } diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 6db5ef1a51e..c486415dacf 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -4876,8 +4876,6 @@ func TestTestInternalTrafficPolicyE2E(t *testing.T) { }, } for _, tc := range testCases { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, true)() - ipt := iptablestest.NewFake() ipvs := ipvstest.NewFake() ipset := ipsettest.NewFake(testIPSetVersion) @@ -6025,8 +6023,6 @@ func TestNoEndpointsMetric(t *testing.T) { }, } for _, tc := range testCases { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, true)() - ipt := iptablestest.NewFake() ipvs := ipvstest.NewFake() ipset := ipsettest.NewFake(testIPSetVersion) diff --git a/pkg/proxy/service.go b/pkg/proxy/service.go index a9a66e02785..a53a299f548 100644 --- a/pkg/proxy/service.go +++ b/pkg/proxy/service.go @@ -30,9 +30,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - utilfeature "k8s.io/apiserver/pkg/util/feature" apiservice "k8s.io/kubernetes/pkg/api/v1/service" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/proxy/metrics" utilproxy "k8s.io/kubernetes/pkg/proxy/util" ) @@ -158,14 +156,9 @@ func (bsvcPortInfo *BaseServicePortInfo) UsesLocalEndpoints() bool { } func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, service *v1.Service) *BaseServicePortInfo { - externalPolicyLocal := false - if apiservice.ExternalPolicyLocal(service) { - externalPolicyLocal = true - } - internalPolicyLocal := false - if utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) { - internalPolicyLocal = apiservice.InternalPolicyLocal(service) - } + externalPolicyLocal := apiservice.ExternalPolicyLocal(service) + internalPolicyLocal := apiservice.InternalPolicyLocal(service) + var stickyMaxAgeSeconds int if service.Spec.SessionAffinity == v1.ServiceAffinityClientIP { // Kube-apiserver side guarantees SessionAffinityConfig won't be nil when session affinity type is ClientIP diff --git a/pkg/registry/core/service/strategy.go b/pkg/registry/core/service/strategy.go index d2ab2d80c51..fbbf7ce7a73 100644 --- a/pkg/registry/core/service/strategy.go +++ b/pkg/registry/core/service/strategy.go @@ -24,11 +24,9 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/validation" - "k8s.io/kubernetes/pkg/features" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -117,19 +115,6 @@ func (svcStrategy) AllowUnconditionalUpdate() bool { // } func dropServiceDisabledFields(newSvc *api.Service, oldSvc *api.Service) { - // Clear InternalTrafficPolicy if not enabled - if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) { - if !serviceInternalTrafficPolicyInUse(oldSvc) { - newSvc.Spec.InternalTrafficPolicy = nil - } - } -} - -func serviceInternalTrafficPolicyInUse(svc *api.Service) bool { - if svc == nil { - return false - } - return svc.Spec.InternalTrafficPolicy != nil } type serviceStatusStrategy struct { diff --git a/pkg/registry/core/service/strategy_test.go b/pkg/registry/core/service/strategy_test.go index 55d13d4adf2..20abb014d48 100644 --- a/pkg/registry/core/service/strategy_test.go +++ b/pkg/registry/core/service/strategy_test.go @@ -26,11 +26,8 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" _ "k8s.io/kubernetes/pkg/apis/core/install" - "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" ) @@ -153,23 +150,12 @@ func makeServiceWithPorts(ports []api.PortStatus) *api.Service { } } -func makeServiceWithInternalTrafficPolicy(policy *api.ServiceInternalTrafficPolicyType) *api.Service { - return &api.Service{ - Spec: api.ServiceSpec{ - InternalTrafficPolicy: policy, - }, - } -} - func TestDropDisabledField(t *testing.T) { - localInternalTrafficPolicy := api.ServiceInternalTrafficPolicyLocal - testCases := []struct { - name string - enableInternalTrafficPolicy bool - svc *api.Service - oldSvc *api.Service - compareSvc *api.Service + name string + svc *api.Service + oldSvc *api.Service + compareSvc *api.Service }{ /* svc.Status.Conditions */ { @@ -221,33 +207,10 @@ func TestDropDisabledField(t *testing.T) { oldSvc: makeServiceWithPorts([]api.PortStatus{}), compareSvc: makeServiceWithPorts(nil), }, - /* svc.spec.internalTrafficPolicy */ - { - name: "internal traffic policy not enabled, field used in old, not used in new", - enableInternalTrafficPolicy: false, - svc: makeServiceWithInternalTrafficPolicy(nil), - oldSvc: makeServiceWithInternalTrafficPolicy(&localInternalTrafficPolicy), - compareSvc: makeServiceWithInternalTrafficPolicy(nil), - }, - { - name: "internal traffic policy not enabled, field not used in old, used in new", - enableInternalTrafficPolicy: false, - svc: makeServiceWithInternalTrafficPolicy(&localInternalTrafficPolicy), - oldSvc: makeServiceWithInternalTrafficPolicy(nil), - compareSvc: makeServiceWithInternalTrafficPolicy(nil), - }, - { - name: "internal traffic policy enabled, field not used in old, used in new", - enableInternalTrafficPolicy: true, - svc: makeServiceWithInternalTrafficPolicy(&localInternalTrafficPolicy), - oldSvc: makeServiceWithInternalTrafficPolicy(nil), - compareSvc: makeServiceWithInternalTrafficPolicy(&localInternalTrafficPolicy), - }, /* add more tests for other dropped fields as needed */ } for _, tc := range testCases { func() { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, tc.enableInternalTrafficPolicy)() old := tc.oldSvc.DeepCopy() // to test against user using IPFamily not set on cluster diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index c43dbc3bace..c073776195b 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -5212,7 +5212,6 @@ message ServiceSpec { // dropping the traffic if there are no local endpoints. The default value, // "Cluster", uses the standard behavior of routing to all endpoints evenly // (possibly modified by topology and other features). - // +featureGate=ServiceInternalTrafficPolicy // +optional optional string internalTrafficPolicy = 22; } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index adfad114e4a..22156917971 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4591,7 +4591,6 @@ type ServiceSpec struct { // dropping the traffic if there are no local endpoints. The default value, // "Cluster", uses the standard behavior of routing to all endpoints evenly // (possibly modified by topology and other features). - // +featureGate=ServiceInternalTrafficPolicy // +optional InternalTrafficPolicy *ServiceInternalTrafficPolicyType `json:"internalTrafficPolicy,omitempty" protobuf:"bytes,22,opt,name=internalTrafficPolicy"` }