Promote ServiceInternalTrafficPolicy to GA

This commit is contained in:
Andy Voltz 2022-10-28 23:03:45 -04:00
parent c98aef484d
commit 29f4862ed8
14 changed files with 24 additions and 141 deletions

View File

@ -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
}

View File

@ -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 {

View File

@ -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)

View File

@ -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"), ""))
}
}

View File

@ -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",

View File

@ -719,6 +719,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"
@ -1016,7 +1017,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},

View File

@ -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()

View File

@ -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())
}

View File

@ -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)

View File

@ -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

View File

@ -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 {

View File

@ -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

View File

@ -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;
}

View File

@ -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"`
}