Merge pull request #112895 from nokia/kep-1435-GA

KEP-1435 Mixed Protocol values in LoadBalancer Service GA

pkg/apis/core/validation/conditional_validation.go

@@ -1,61 +0,0 @@
-/*
-Copyright 2019 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package validation
-
-import (
-	"k8s.io/apimachinery/pkg/util/validation/field"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	api "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/features"
-)
-
-// ValidateConditionalService validates conditionally valid fields.
-func ValidateConditionalService(service, oldService *api.Service) field.ErrorList {
-	var errs field.ErrorList
-
-	errs = append(errs, validateMixedProtocolLBService(service, oldService)...)
-
-	return errs
-}
-
-// validateMixedProtocolLBService checks if the old Service has type=LoadBalancer and whether the Service has different Protocols
-// on its ports. If the MixedProtocolLBService feature flag is disabled the usage of different Protocols in the new Service is
-// valid only if the old Service has different Protocols, too.
-func validateMixedProtocolLBService(service, oldService *api.Service) (errs field.ErrorList) {
-	if service.Spec.Type != api.ServiceTypeLoadBalancer {
-		return
-	}
-	if utilfeature.DefaultFeatureGate.Enabled(features.MixedProtocolLBService) {
-		return
-	}
-
-	if serviceHasMixedProtocols(service) && !serviceHasMixedProtocols(oldService) {
-		errs = append(errs, field.Invalid(field.NewPath("spec", "ports"), service.Spec.Ports, "may not contain more than 1 protocol when type is 'LoadBalancer'"))
-	}
-	return
-}
-
-func serviceHasMixedProtocols(service *api.Service) bool {
-	if service == nil {
-		return false
-	}
-	protos := map[string]bool{}
-	for _, port := range service.Spec.Ports {
-		protos[string(port.Protocol)] = true
-	}
-	return len(protos) > 1
-}

pkg/apis/core/validation/conditional_validation_test.go

@@ -1,277 +0,0 @@
-/*
-Copyright 2019 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package validation
-
-import (
-	"strings"
-	"testing"
-
-	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/features"
-)
-
-func TestValidateMixedProtocolLBService(t *testing.T) {
-	newLBServiceDifferentProtocols := &api.Service{
-		Spec: api.ServiceSpec{
-			Ports: []api.ServicePort{
-				{
-					Protocol: api.ProtocolTCP,
-				},
-				{
-					Protocol: api.ProtocolUDP,
-				},
-			},
-			Type: api.ServiceTypeLoadBalancer,
-		},
-	}
-	newLBServiceSameProtocols := &api.Service{
-		Spec: api.ServiceSpec{
-			Ports: []api.ServicePort{
-				{
-					Protocol: api.ProtocolTCP,
-				},
-				{
-					Protocol: api.ProtocolTCP,
-				},
-			},
-			Type: api.ServiceTypeLoadBalancer,
-		},
-	}
-	newNonLBServiceDifferentProtocols := &api.Service{
-		Spec: api.ServiceSpec{
-			Ports: []api.ServicePort{
-				{
-					Protocol: api.ProtocolTCP,
-				},
-				{
-					Protocol: api.ProtocolUDP,
-				},
-			},
-			Type: api.ServiceTypeNodePort,
-		},
-	}
-	newNonLBServiceSameProtocols := &api.Service{
-		Spec: api.ServiceSpec{
-			Ports: []api.ServicePort{
-				{
-					Protocol: api.ProtocolUDP,
-				},
-				{
-					Protocol: api.ProtocolUDP,
-				},
-			},
-			Type: api.ServiceTypeNodePort,
-		},
-	}
-	oldLBServiceDifferentProtocols := &api.Service{
-		Spec: api.ServiceSpec{
-			Ports: []api.ServicePort{
-				{
-					Protocol: api.ProtocolTCP,
-				},
-				{
-					Protocol: api.ProtocolUDP,
-				},
-			},
-			Type: api.ServiceTypeLoadBalancer,
-		},
-	}
-	oldLBServiceSameProtocols := &api.Service{
-		Spec: api.ServiceSpec{
-			Ports: []api.ServicePort{
-				{
-					Protocol: api.ProtocolTCP,
-				},
-				{
-					Protocol: api.ProtocolTCP,
-				},
-			},
-			Type: api.ServiceTypeLoadBalancer,
-		},
-	}
-	oldNonLBServiceDifferentProtocols := &api.Service{
-		Spec: api.ServiceSpec{
-			Ports: []api.ServicePort{
-				{
-					Protocol: api.ProtocolTCP,
-				},
-				{
-					Protocol: api.ProtocolUDP,
-				},
-			},
-			Type: api.ServiceTypeNodePort,
-		},
-	}
-	oldNonLBServiceSameProtocols := &api.Service{
-		Spec: api.ServiceSpec{
-			Ports: []api.ServicePort{
-				{
-					Protocol: api.ProtocolUDP,
-				},
-				{
-					Protocol: api.ProtocolUDP,
-				},
-			},
-			Type: api.ServiceTypeNodePort,
-		},
-	}
-	cases := map[string]struct {
-		oldService    *api.Service
-		newService    *api.Service
-		fgEnabled     bool
-		expectedError []string
-	}{
-		"Old service is nil, new service has different protocols, feature gate false": {
-			oldService:    nil,
-			newService:    newLBServiceDifferentProtocols,
-			fgEnabled:     false,
-			expectedError: []string{`spec.ports: Invalid value: []core.ServicePort{core.ServicePort{Name:"", Protocol:"TCP", AppProtocol:(*string)(nil), Port:0, TargetPort:intstr.IntOrString{Type:0, IntVal:0, StrVal:""}, NodePort:0}, core.ServicePort{Name:"", Protocol:"UDP", AppProtocol:(*string)(nil), Port:0, TargetPort:intstr.IntOrString{Type:0, IntVal:0, StrVal:""}, NodePort:0}}: may not contain more than 1 protocol when type is 'LoadBalancer'`},
-		},
-		"Old service is nil, new service has different protocols, feature gate true": {
-			oldService: nil,
-			newService: newLBServiceDifferentProtocols,
-			fgEnabled:  true,
-		},
-		"Old service is nil, new service does not have different protocols, feature gate false": {
-			oldService: nil,
-			newService: newLBServiceSameProtocols,
-			fgEnabled:  false,
-		},
-		"Old service is nil, new service does not have different protocols, feature gate true": {
-			oldService: nil,
-			newService: newLBServiceSameProtocols,
-			fgEnabled:  true,
-		},
-		"Old service is nil, new non-LB service has different protocols, feature gate false": {
-			oldService: nil,
-			newService: newNonLBServiceDifferentProtocols,
-			fgEnabled:  false,
-		},
-		"Old service is nil, new non-LB service has different protocols, feature gate true": {
-			oldService: nil,
-			newService: newNonLBServiceDifferentProtocols,
-			fgEnabled:  true,
-		},
-		"Old service is nil, new non-LB service does not have different protocols, feature gate false": {
-			oldService: nil,
-			newService: newNonLBServiceSameProtocols,
-			fgEnabled:  false,
-		},
-		"Old service is nil, new non-LB service does not have different protocols, feature gate true": {
-			oldService: nil,
-			newService: newNonLBServiceSameProtocols,
-			fgEnabled:  true,
-		},
-		"Non-LB services, both services have different protocols, feature gate false": {
-			oldService: oldNonLBServiceDifferentProtocols,
-			newService: newNonLBServiceDifferentProtocols,
-			fgEnabled:  false,
-		},
-		"Non-LB services, old service has same protocols, new service has different protocols, feature gate false": {
-			oldService: oldNonLBServiceSameProtocols,
-			newService: newNonLBServiceDifferentProtocols,
-			fgEnabled:  false,
-		},
-		"Non-LB services, old service has different protocols, new service has identical protocols, feature gate false": {
-			oldService: oldNonLBServiceDifferentProtocols,
-			newService: newNonLBServiceSameProtocols,
-			fgEnabled:  false,
-		},
-		"Non-LB services, both services have same protocols, feature gate false": {
-			oldService: oldNonLBServiceSameProtocols,
-			newService: newNonLBServiceSameProtocols,
-			fgEnabled:  false,
-		},
-		"Non-LB services, both services have different protocols, feature gate true": {
-			oldService: oldNonLBServiceDifferentProtocols,
-			newService: newNonLBServiceDifferentProtocols,
-			fgEnabled:  true,
-		},
-		"Non-LB services, old service has same protocols, new service has different protocols, feature gate true": {
-			oldService: oldNonLBServiceSameProtocols,
-			newService: newNonLBServiceDifferentProtocols,
-			fgEnabled:  true,
-		},
-		"Non-LB services, old service has different protocols, new service has identical protocols, feature gate true": {
-			oldService: oldNonLBServiceDifferentProtocols,
-			newService: newNonLBServiceSameProtocols,
-			fgEnabled:  true,
-		},
-		"Non-LB services, both services have same protocols, feature gate true": {
-			oldService: oldNonLBServiceSameProtocols,
-			newService: newNonLBServiceSameProtocols,
-			fgEnabled:  true,
-		},
-		"LB service, neither service has different protocols, feature gate false": {
-			oldService: oldLBServiceSameProtocols,
-			newService: newLBServiceSameProtocols,
-			fgEnabled:  false,
-		},
-		"LB service, old service does not have different protocols, new service has different protocols, feature gate false": {
-			oldService:    oldLBServiceSameProtocols,
-			newService:    newLBServiceDifferentProtocols,
-			fgEnabled:     false,
-			expectedError: []string{`spec.ports: Invalid value: []core.ServicePort{core.ServicePort{Name:"", Protocol:"TCP", AppProtocol:(*string)(nil), Port:0, TargetPort:intstr.IntOrString{Type:0, IntVal:0, StrVal:""}, NodePort:0}, core.ServicePort{Name:"", Protocol:"UDP", AppProtocol:(*string)(nil), Port:0, TargetPort:intstr.IntOrString{Type:0, IntVal:0, StrVal:""}, NodePort:0}}: may not contain more than 1 protocol when type is 'LoadBalancer'`},
-		},
-		"LB service, old service has different protocols, new service does not have different protocols, feature gate false": {
-			oldService: oldLBServiceDifferentProtocols,
-			newService: newLBServiceSameProtocols,
-			fgEnabled:  false,
-		},
-		"LB service, both services have different protocols, feature gate false": {
-			oldService: oldLBServiceDifferentProtocols,
-			newService: newLBServiceDifferentProtocols,
-			fgEnabled:  false,
-		},
-		"LB service, neither service has different protocols, feature gate true": {
-			oldService: oldLBServiceSameProtocols,
-			newService: newLBServiceSameProtocols,
-			fgEnabled:  true,
-		},
-		"LB service, old service has different protocols, new service does not have different protocols, feature gate true": {
-			oldService: oldLBServiceDifferentProtocols,
-			newService: newLBServiceSameProtocols,
-			fgEnabled:  true,
-		},
-		"LB service, old service does not have different protocols, new service has different protocols, feature gate true": {
-			oldService: oldLBServiceSameProtocols,
-			newService: newLBServiceDifferentProtocols,
-			fgEnabled:  true,
-		},
-		"LB service, both services have different protocols, feature gate true": {
-			oldService: oldLBServiceDifferentProtocols,
-			newService: newLBServiceDifferentProtocols,
-			fgEnabled:  true,
-		},
-	}
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.MixedProtocolLBService, tc.fgEnabled)()
-			errs := validateMixedProtocolLBService(tc.newService, tc.oldService)
-			if len(errs) != len(tc.expectedError) {
-				t.Fatalf("unexpected number of errors: %v", errs)
-			}
-			for i := range errs {
-				if !strings.Contains(errs[i].Error(), tc.expectedError[i]) {
-					t.Errorf("unexpected error %d: %v", i, errs[i])
-				}
-			}
-		})
-	}
-}

pkg/features/kube_features.go

@@ -571,6 +571,7 @@ const (
 	// kep: https://kep.k8s.io/1435
 	// alpha: v1.20
 	// beta: v1.24
+	// ga: v1.26
 	//
 	// Enables the usage of different protocols in the same Service with type=LoadBalancer
 	MixedProtocolLBService featuregate.Feature = "MixedProtocolLBService"
@@ -973,7 +974,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	MinimizeIPTablesRestore: {Default: false, PreRelease: featuregate.Alpha},
 
-	MixedProtocolLBService: {Default: true, PreRelease: featuregate.Beta},
+	MixedProtocolLBService: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28
 
 	MultiCIDRRangeAllocator: {Default: false, PreRelease: featuregate.Alpha},
 

pkg/registry/core/service/storage/storage_test.go

@@ -39,12 +39,9 @@ import (
 	genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing"
 	"k8s.io/apiserver/pkg/registry/rest"
 	etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	epstest "k8s.io/kubernetes/pkg/api/endpoints/testing"
 	svctest "k8s.io/kubernetes/pkg/api/service/testing"
 	api "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/features"
 	endpointstore "k8s.io/kubernetes/pkg/registry/core/endpoint/storage"
 	podstore "k8s.io/kubernetes/pkg/registry/core/pod/storage"
 	"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
@@ -6113,11 +6110,10 @@ func TestCreateDeleteReuse(t *testing.T) {
 
 func TestCreateInitNodePorts(t *testing.T) {
 	testCases := []struct {
-		name                       string
+		name            string
-		svc                        *api.Service
+		svc             *api.Service
-		expectError                bool
+		expectError     bool
-		expectNodePorts            bool
+		expectNodePorts bool
-		gateMixedProtocolLBService bool
 	}{{
 		name:            "type:ExternalName",
 		svc:             svctest.MakeService("foo"),
@@ -6281,66 +6277,31 @@ func TestCreateInitNodePorts(t *testing.T) {
 			svctest.SetNodePorts(30080, 30080)),
 		expectError: true,
 	}, {
-		// When the MixedProtocolLBService gate is locked, this can be removed.
+		name: "type:LoadBalancer_multiport_multiproto_unspecified",
-		name: "type:LoadBalancer_multiport_multiproto_unspecified_MixedProtocolLBService:off",
 		svc: svctest.MakeService("foo",
 			svctest.SetTypeLoadBalancer,
 			svctest.SetPorts(
 				svctest.MakeServicePort("p", 53, intstr.FromInt(53), api.ProtocolTCP),
 				svctest.MakeServicePort("q", 53, intstr.FromInt(53), api.ProtocolUDP))),
-		gateMixedProtocolLBService: false,
+		expectNodePorts: true,
-		expectError:                true,
 	}, {
-		// When the MixedProtocolLBService gate is locked, this can be removed.
+		name: "type:LoadBalancer_multiport_multiproto_specified",
-		name: "type:LoadBalancer_multiport_multiproto_specified_MixedProtocolLBService:off",
 		svc: svctest.MakeService("foo",
 			svctest.SetTypeLoadBalancer,
 			svctest.SetPorts(
 				svctest.MakeServicePort("p", 53, intstr.FromInt(53), api.ProtocolTCP),
 				svctest.MakeServicePort("q", 53, intstr.FromInt(53), api.ProtocolUDP)),
 			svctest.SetUniqueNodePorts),
-		gateMixedProtocolLBService: false,
+		expectNodePorts: true,
-		expectError:                true,
 	}, {
-		// When the MixedProtocolLBService gate is locked, this can be removed.
+		name: "type:LoadBalancer_multiport_multiproto_same",
-		name: "type:LoadBalancer_multiport_multiproto_same_MixedProtocolLBService:off",
 		svc: svctest.MakeService("foo",
 			svctest.SetTypeLoadBalancer,
 			svctest.SetPorts(
 				svctest.MakeServicePort("p", 53, intstr.FromInt(53), api.ProtocolTCP),
 				svctest.MakeServicePort("q", 53, intstr.FromInt(53), api.ProtocolUDP)),
 			svctest.SetNodePorts(30053, 30053)),
-		gateMixedProtocolLBService: false,
+		expectNodePorts: true,
-		expectError:                true,
-	}, {
-		name: "type:LoadBalancer_multiport_multiproto_unspecified_MixedProtocolLBService:on",
-		svc: svctest.MakeService("foo",
-			svctest.SetTypeLoadBalancer,
-			svctest.SetPorts(
-				svctest.MakeServicePort("p", 53, intstr.FromInt(53), api.ProtocolTCP),
-				svctest.MakeServicePort("q", 53, intstr.FromInt(53), api.ProtocolUDP))),
-		gateMixedProtocolLBService: true,
-		expectNodePorts:            true,
-	}, {
-		name: "type:LoadBalancer_multiport_multiproto_specified_MixedProtocolLBService:on",
-		svc: svctest.MakeService("foo",
-			svctest.SetTypeLoadBalancer,
-			svctest.SetPorts(
-				svctest.MakeServicePort("p", 53, intstr.FromInt(53), api.ProtocolTCP),
-				svctest.MakeServicePort("q", 53, intstr.FromInt(53), api.ProtocolUDP)),
-			svctest.SetUniqueNodePorts),
-		gateMixedProtocolLBService: true,
-		expectNodePorts:            true,
-	}, {
-		name: "type:LoadBalancer_multiport_multiproto_same_MixedProtocolLBService:on",
-		svc: svctest.MakeService("foo",
-			svctest.SetTypeLoadBalancer,
-			svctest.SetPorts(
-				svctest.MakeServicePort("p", 53, intstr.FromInt(53), api.ProtocolTCP),
-				svctest.MakeServicePort("q", 53, intstr.FromInt(53), api.ProtocolUDP)),
-			svctest.SetNodePorts(30053, 30053)),
-		gateMixedProtocolLBService: true,
-		expectNodePorts:            true,
 	}, {
 		name: "type:LoadBalancer_multiport_multiproto_conflict",
 		svc: svctest.MakeService("foo",
@@ -6358,8 +6319,6 @@ func TestCreateInitNodePorts(t *testing.T) {
 	defer storage.Store.DestroyFunc()
 
 	for _, tc := range testCases {
-		defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.MixedProtocolLBService, tc.gateMixedProtocolLBService)()
-
 		t.Run(tc.name, func(t *testing.T) {
 			ctx := genericapirequest.NewDefaultContext()
 			createdObj, err := storage.Create(ctx, tc.svc, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})

pkg/registry/core/service/strategy.go

@@ -81,7 +81,6 @@ func (svcStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object
 func (svcStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
 	service := obj.(*api.Service)
 	allErrs := validation.ValidateServiceCreate(service)
-	allErrs = append(allErrs, validation.ValidateConditionalService(service, nil)...)
 	return allErrs
 }
 
@@ -98,7 +97,6 @@ func (svcStrategy) AllowCreateOnUpdate() bool {
 
 func (strategy svcStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
 	allErrs := validation.ValidateServiceUpdate(obj.(*api.Service), old.(*api.Service))
-	allErrs = append(allErrs, validation.ValidateConditionalService(obj.(*api.Service), old.(*api.Service))...)
 	return allErrs
 }
 
@@ -119,17 +117,6 @@ func (svcStrategy) AllowUnconditionalUpdate() bool {
 //	}
 func dropServiceDisabledFields(newSvc *api.Service, oldSvc *api.Service) {
 
-	if !utilfeature.DefaultFeatureGate.Enabled(features.MixedProtocolLBService) {
-		if !serviceConditionsInUse(oldSvc) {
-			newSvc.Status.Conditions = nil
-		}
-		if !loadBalancerPortsInUse(oldSvc) {
-			for i := range newSvc.Status.LoadBalancer.Ingress {
-				newSvc.Status.LoadBalancer.Ingress[i].Ports = nil
-			}
-		}
-	}
-
 	// Clear InternalTrafficPolicy if not enabled
 	if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) {
 		if !serviceInternalTrafficPolicyInUse(oldSvc) {
@@ -138,27 +125,6 @@ func dropServiceDisabledFields(newSvc *api.Service, oldSvc *api.Service) {
 	}
 }
 
-// returns true when the svc.Status.Conditions field is in use.
-func serviceConditionsInUse(svc *api.Service) bool {
-	if svc == nil {
-		return false
-	}
-	return svc.Status.Conditions != nil
-}
-
-// returns true when the svc.Status.LoadBalancer.Ingress.Ports field is in use.
-func loadBalancerPortsInUse(svc *api.Service) bool {
-	if svc == nil {
-		return false
-	}
-	for _, ing := range svc.Status.LoadBalancer.Ingress {
-		if ing.Ports != nil {
-			return true
-		}
-	}
-	return false
-}
-
 func serviceInternalTrafficPolicyInUse(svc *api.Service) bool {
 	if svc == nil {
 		return false

pkg/registry/core/service/strategy_test.go

@@ -166,7 +166,6 @@ func TestDropDisabledField(t *testing.T) {
 
 	testCases := []struct {
 		name                        string
-		enableMixedProtocol         bool
 		enableInternalTrafficPolicy bool
 		svc                         *api.Service
 		oldSvc                      *api.Service
@@ -174,117 +173,53 @@ func TestDropDisabledField(t *testing.T) {
 	}{
 		/* svc.Status.Conditions */
 		{
-			name:                "mixed protocol not enabled, field not used in old, not used in new",
+			name:       "mixed protocol enabled, field not used in old, not used in new",
-			enableMixedProtocol: false,
+			svc:        makeServiceWithConditions(nil),
-			svc:                 makeServiceWithConditions(nil),
+			oldSvc:     makeServiceWithConditions(nil),
-			oldSvc:              makeServiceWithConditions(nil),
+			compareSvc: makeServiceWithConditions(nil),
-			compareSvc:          makeServiceWithConditions(nil),
 		},
 		{
-			name:                "mixed protocol not enabled, field used in old and in new",
+			name:       "mixed protocol enabled, field used in old and in new",
-			enableMixedProtocol: false,
+			svc:        makeServiceWithConditions([]metav1.Condition{}),
-			svc:                 makeServiceWithConditions([]metav1.Condition{}),
+			oldSvc:     makeServiceWithConditions([]metav1.Condition{}),
-			oldSvc:              makeServiceWithConditions([]metav1.Condition{}),
+			compareSvc: makeServiceWithConditions([]metav1.Condition{}),
-			compareSvc:          makeServiceWithConditions([]metav1.Condition{}),
 		},
 		{
-			name:                "mixed protocol not enabled, field not used in old, used in new",
+			name:       "mixed protocol enabled, field not used in old, used in new",
-			enableMixedProtocol: false,
+			svc:        makeServiceWithConditions([]metav1.Condition{}),
-			svc:                 makeServiceWithConditions([]metav1.Condition{}),
+			oldSvc:     makeServiceWithConditions(nil),
-			oldSvc:              makeServiceWithConditions(nil),
+			compareSvc: makeServiceWithConditions([]metav1.Condition{}),
-			compareSvc:          makeServiceWithConditions(nil),
 		},
 		{
-			name:                "mixed protocol not enabled, field used in old, not used in new",
+			name:       "mixed protocol enabled, field used in old, not used in new",
-			enableMixedProtocol: false,
+			svc:        makeServiceWithConditions(nil),
-			svc:                 makeServiceWithConditions(nil),
+			oldSvc:     makeServiceWithConditions([]metav1.Condition{}),
-			oldSvc:              makeServiceWithConditions([]metav1.Condition{}),
+			compareSvc: makeServiceWithConditions(nil),
-			compareSvc:          makeServiceWithConditions(nil),
-		},
-		{
-			name:                "mixed protocol enabled, field not used in old, not used in new",
-			enableMixedProtocol: true,
-			svc:                 makeServiceWithConditions(nil),
-			oldSvc:              makeServiceWithConditions(nil),
-			compareSvc:          makeServiceWithConditions(nil),
-		},
-		{
-			name:                "mixed protocol enabled, field used in old and in new",
-			enableMixedProtocol: true,
-			svc:                 makeServiceWithConditions([]metav1.Condition{}),
-			oldSvc:              makeServiceWithConditions([]metav1.Condition{}),
-			compareSvc:          makeServiceWithConditions([]metav1.Condition{}),
-		},
-		{
-			name:                "mixed protocol enabled, field not used in old, used in new",
-			enableMixedProtocol: true,
-			svc:                 makeServiceWithConditions([]metav1.Condition{}),
-			oldSvc:              makeServiceWithConditions(nil),
-			compareSvc:          makeServiceWithConditions([]metav1.Condition{}),
-		},
-		{
-			name:                "mixed protocol enabled, field used in old, not used in new",
-			enableMixedProtocol: true,
-			svc:                 makeServiceWithConditions(nil),
-			oldSvc:              makeServiceWithConditions([]metav1.Condition{}),
-			compareSvc:          makeServiceWithConditions(nil),
 		},
 		/* svc.Status.LoadBalancer.Ingress.Ports */
 		{
-			name:                "mixed protocol not enabled, field not used in old, not used in new",
+			name:       "mixed protocol enabled, field not used in old, not used in new",
-			enableMixedProtocol: false,
+			svc:        makeServiceWithPorts(nil),
-			svc:                 makeServiceWithPorts(nil),
+			oldSvc:     makeServiceWithPorts(nil),
-			oldSvc:              makeServiceWithPorts(nil),
+			compareSvc: makeServiceWithPorts(nil),
-			compareSvc:          makeServiceWithPorts(nil),
 		},
 		{
-			name:                "mixed protocol not enabled, field used in old and in new",
+			name:       "mixed protocol enabled, field used in old and in new",
-			enableMixedProtocol: false,
+			svc:        makeServiceWithPorts([]api.PortStatus{}),
-			svc:                 makeServiceWithPorts([]api.PortStatus{}),
+			oldSvc:     makeServiceWithPorts([]api.PortStatus{}),
-			oldSvc:              makeServiceWithPorts([]api.PortStatus{}),
+			compareSvc: makeServiceWithPorts([]api.PortStatus{}),
-			compareSvc:          makeServiceWithPorts([]api.PortStatus{}),
 		},
 		{
-			name:                "mixed protocol not enabled, field not used in old, used in new",
+			name:       "mixed protocol enabled, field not used in old, used in new",
-			enableMixedProtocol: false,
+			svc:        makeServiceWithPorts([]api.PortStatus{}),
-			svc:                 makeServiceWithPorts([]api.PortStatus{}),
+			oldSvc:     makeServiceWithPorts(nil),
-			oldSvc:              makeServiceWithPorts(nil),
+			compareSvc: makeServiceWithPorts([]api.PortStatus{}),
-			compareSvc:          makeServiceWithPorts(nil),
 		},
 		{
-			name:                "mixed protocol not enabled, field used in old, not used in new",
+			name:       "mixed protocol enabled, field used in old, not used in new",
-			enableMixedProtocol: false,
+			svc:        makeServiceWithPorts(nil),
-			svc:                 makeServiceWithPorts(nil),
+			oldSvc:     makeServiceWithPorts([]api.PortStatus{}),
-			oldSvc:              makeServiceWithPorts([]api.PortStatus{}),
+			compareSvc: makeServiceWithPorts(nil),
-			compareSvc:          makeServiceWithPorts(nil),
-		},
-		{
-			name:                "mixed protocol enabled, field not used in old, not used in new",
-			enableMixedProtocol: true,
-			svc:                 makeServiceWithPorts(nil),
-			oldSvc:              makeServiceWithPorts(nil),
-			compareSvc:          makeServiceWithPorts(nil),
-		},
-		{
-			name:                "mixed protocol enabled, field used in old and in new",
-			enableMixedProtocol: true,
-			svc:                 makeServiceWithPorts([]api.PortStatus{}),
-			oldSvc:              makeServiceWithPorts([]api.PortStatus{}),
-			compareSvc:          makeServiceWithPorts([]api.PortStatus{}),
-		},
-		{
-			name:                "mixed protocol enabled, field not used in old, used in new",
-			enableMixedProtocol: true,
-			svc:                 makeServiceWithPorts([]api.PortStatus{}),
-			oldSvc:              makeServiceWithPorts(nil),
-			compareSvc:          makeServiceWithPorts([]api.PortStatus{}),
-		},
-		{
-			name:                "mixed protocol enabled, field used in old, not used in new",
-			enableMixedProtocol: true,
-			svc:                 makeServiceWithPorts(nil),
-			oldSvc:              makeServiceWithPorts([]api.PortStatus{}),
-			compareSvc:          makeServiceWithPorts(nil),
 		},
 		/* svc.spec.internalTrafficPolicy */
 		{
@@ -312,7 +247,6 @@ func TestDropDisabledField(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		func() {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.MixedProtocolLBService, tc.enableMixedProtocol)()
 			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, tc.enableInternalTrafficPolicy)()
 			old := tc.oldSvc.DeepCopy()
 

test/e2e/framework/endpoints/ports.go

@@ -24,6 +24,9 @@ import (
 // PortsByPodUID is a map that maps pod UID to container ports.
 type PortsByPodUID map[types.UID][]int
 
+// FullPortsByPodUID is a map that maps pod UID to container ports.
+type FullPortsByPodUID map[types.UID][]v1.ContainerPort
+
 // GetContainerPortsByPodUID returns a PortsByPodUID map on the given endpoints.
 func GetContainerPortsByPodUID(ep *v1.Endpoints) PortsByPodUID {
 	m := PortsByPodUID{}
@@ -40,3 +43,24 @@ func GetContainerPortsByPodUID(ep *v1.Endpoints) PortsByPodUID {
 	}
 	return m
 }
+
+// GetFullContainerPortsByPodUID returns a FullPortsByPodUID map on the given endpoints with all the port data.
+func GetFullContainerPortsByPodUID(ep *v1.Endpoints) FullPortsByPodUID {
+	m := FullPortsByPodUID{}
+	for _, ss := range ep.Subsets {
+		for _, port := range ss.Ports {
+			containerPort := v1.ContainerPort{
+				Name:          port.Name,
+				ContainerPort: port.Port,
+				Protocol:      port.Protocol,
+			}
+			for _, addr := range ss.Addresses {
+				if _, ok := m[addr.TargetRef.UID]; !ok {
+					m[addr.TargetRef.UID] = make([]v1.ContainerPort, 0)
+				}
+				m[addr.TargetRef.UID] = append(m[addr.TargetRef.UID], containerPort)
+			}
+		}
+	}
+	return m
+}

test/e2e/framework/endpointslice/ports.go

@@ -17,6 +17,7 @@ limitations under the License.
 package endpointslice
 
 import (
+	v1 "k8s.io/api/core/v1"
 	discoveryv1 "k8s.io/api/discovery/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -24,6 +25,9 @@ import (
 // PortsByPodUID is a map that maps pod UID to container ports.
 type PortsByPodUID map[types.UID][]int
 
+// FullPortsByPodUID is a map that maps pod UID to container ports.
+type FullPortsByPodUID map[types.UID][]v1.ContainerPort
+
 // GetContainerPortsByPodUID returns a PortsByPodUID map on the given endpoints.
 func GetContainerPortsByPodUID(eps []discoveryv1.EndpointSlice) PortsByPodUID {
 	m := PortsByPodUID{}
@@ -44,3 +48,28 @@ func GetContainerPortsByPodUID(eps []discoveryv1.EndpointSlice) PortsByPodUID {
 	}
 	return m
 }
+
+// GetFullContainerPortsByPodUID returns a PortsByPodUID map on the given endpoints.
+func GetFullContainerPortsByPodUID(eps []discoveryv1.EndpointSlice) FullPortsByPodUID {
+	m := FullPortsByPodUID{}
+
+	for _, es := range eps {
+		for _, port := range es.Ports {
+			if port.Port == nil {
+				continue
+			}
+			containerPort := v1.ContainerPort{
+				Name:          *port.Name,
+				ContainerPort: *port.Port,
+				Protocol:      *port.Protocol,
+			}
+			for _, ep := range es.Endpoints {
+				if _, ok := m[ep.TargetRef.UID]; !ok {
+					m[ep.TargetRef.UID] = make([]v1.ContainerPort, 0)
+				}
+				m[ep.TargetRef.UID] = append(m[ep.TargetRef.UID], containerPort)
+			}
+		}
+	}
+	return m
+}

test/e2e/framework/service/jig.go

@@ -1030,6 +1030,8 @@ func (j *TestJig) CheckServiceReachability(svc *v1.Service, pod *v1.Pod) error {
 		return j.checkNodePortServiceReachability(svc, pod)
 	case v1.ServiceTypeExternalName:
 		return j.checkExternalServiceReachability(svc, pod)
+	case v1.ServiceTypeLoadBalancer:
+		return j.checkClusterIPServiceReachability(svc, pod)
 	default:
 		return fmt.Errorf("unsupported service type \"%s\" to verify service reachability for \"%s\" service. This may due to diverse implementation of the service type", svcType, svc.Name)
 	}
@@ -1065,3 +1067,22 @@ func (j *TestJig) CreateSCTPServiceWithPort(tweak func(svc *v1.Service), port in
 	}
 	return j.sanityCheckService(result, svc.Spec.Type)
 }
+
+// CreateLoadBalancerServiceWaitForClusterIPOnly creates a loadbalancer service and waits
+// for it to acquire a cluster IP
+func (j *TestJig) CreateLoadBalancerServiceWaitForClusterIPOnly(tweak func(svc *v1.Service)) (*v1.Service, error) {
+	ginkgo.By("creating a service " + j.Namespace + "/" + j.Name + " with type=LoadBalancer")
+	svc := j.newServiceTemplate(v1.ProtocolTCP, 80)
+	svc.Spec.Type = v1.ServiceTypeLoadBalancer
+	// We need to turn affinity off for our LB distribution tests
+	svc.Spec.SessionAffinity = v1.ServiceAffinityNone
+	if tweak != nil {
+		tweak(svc)
+	}
+	result, err := j.Client.CoreV1().Services(j.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create LoadBalancer Service %q: %v", svc.Name, err)
+	}
+
+	return j.sanityCheckService(result, v1.ServiceTypeLoadBalancer)
+}

test/e2e/network/service.go

@@ -125,6 +125,12 @@ type portsByPodName map[string][]int
 // portsByPodUID is a map that maps pod name to container ports.
 type portsByPodUID map[types.UID][]int
 
+// fullPortsByPodName is a map that maps pod name to container ports including their protocols.
+type fullPortsByPodName map[string][]v1.ContainerPort
+
+// fullPortsByPodUID is a map that maps pod name to container ports.
+type fullPortsByPodUID map[types.UID][]v1.ContainerPort
+
 // affinityCheckFromPod returns interval, timeout and function pinging the service and
 // returning pinged hosts for pinging the service from execPod.
 func affinityCheckFromPod(execPod *v1.Pod, serviceIP string, servicePort int) (time.Duration, time.Duration, func() []string) {
@@ -3731,6 +3737,77 @@ var _ = common.SIGDescribe("Services", func() {
 
 		framework.Logf("Collection of services has been deleted")
 	})
+	/*
+		Release: v1.26
+		Testname: Service, same ports with different protocols on a Load Balancer Service
+		Description: Create a LoadBalancer service with two ports that have the same value but use different protocols. Add a Pod that listens on both ports. The Pod must be reachable via the ClusterIP and both ports
+	*/
+	ginkgo.It("should serve endpoints on same port and different protocol for internal traffic on Type LoadBalancer ", func() {
+		serviceName := "multiprotocol-lb-test"
+		ns := f.Namespace.Name
+		jig := e2eservice.NewTestJig(cs, ns, serviceName)
+
+		defer func() {
+			err := cs.CoreV1().Services(ns).Delete(context.TODO(), serviceName, metav1.DeleteOptions{})
+			framework.ExpectNoError(err, "failed to delete service: %s in namespace: %s", serviceName, ns)
+		}()
+
+		svc1port := "svc1"
+		svc2port := "svc2"
+
+		ginkgo.By("creating service " + serviceName + " in namespace " + ns)
+		svc, err := jig.CreateLoadBalancerServiceWaitForClusterIPOnly(func(service *v1.Service) {
+			service.Spec.Ports = []v1.ServicePort{
+				{
+					Name:       "portname1",
+					Port:       80,
+					TargetPort: intstr.FromString(svc1port),
+					Protocol:   v1.ProtocolTCP,
+				},
+				{
+					Name:       "portname2",
+					Port:       80,
+					TargetPort: intstr.FromString(svc2port),
+					Protocol:   v1.ProtocolUDP,
+				},
+			}
+		})
+		framework.ExpectNoError(err)
+
+		containerPort := 100
+
+		names := map[string]bool{}
+		defer func() {
+			for name := range names {
+				err := cs.CoreV1().Pods(ns).Delete(context.TODO(), name, metav1.DeleteOptions{})
+				framework.ExpectNoError(err, "failed to delete pod: %s in namespace: %s", name, ns)
+			}
+		}()
+
+		containerPorts := []v1.ContainerPort{
+			{
+				Name:          svc1port,
+				ContainerPort: int32(containerPort),
+				Protocol:      v1.ProtocolTCP,
+			},
+			{
+				Name:          svc2port,
+				ContainerPort: int32(containerPort),
+				Protocol:      v1.ProtocolUDP,
+			},
+		}
+
+		podname1 := "pod1"
+
+		createPodOrFail(f, ns, podname1, jig.Labels, containerPorts, "netexec", "--http-port", strconv.Itoa(containerPort), "--udp-port", strconv.Itoa(containerPort))
+		validateEndpointsPortsWithProtocolsOrFail(cs, ns, serviceName, fullPortsByPodName{podname1: containerPorts})
+
+		ginkgo.By("Checking if the Service forwards traffic to pods")
+		execPod := e2epod.CreateExecPodOrFail(cs, ns, "execpod", nil)
+		err = jig.CheckServiceReachability(svc, execPod)
+		framework.ExpectNoError(err)
+		e2epod.DeletePodOrFail(cs, ns, podname1)
+	})
 
 })
 
@@ -4164,6 +4241,108 @@ func restartComponent(cs clientset.Interface, cName, ns string, matchLabels map[
 	return err
 }
 
+// validateEndpointsPortsWithProtocolsOrFail validates that the given service exists and is served by the given expectedEndpoints.
+func validateEndpointsPortsWithProtocolsOrFail(c clientset.Interface, namespace, serviceName string, expectedEndpoints fullPortsByPodName) {
+	ginkgo.By(fmt.Sprintf("waiting up to %v for service %s in namespace %s to expose endpoints %v", framework.ServiceStartTimeout, serviceName, namespace, expectedEndpoints))
+	expectedPortsByPodUID, err := translatePortsByPodNameToPortsByPodUID(c, namespace, expectedEndpoints)
+	framework.ExpectNoError(err, "failed to translate pod name to UID, ns:%s, expectedEndpoints:%v", namespace, expectedEndpoints)
+
+	var (
+		pollErr error
+		i       = 0
+	)
+	if pollErr = wait.PollImmediate(time.Second, framework.ServiceStartTimeout, func() (bool, error) {
+		i++
+
+		ep, err := c.CoreV1().Endpoints(namespace).Get(context.TODO(), serviceName, metav1.GetOptions{})
+		if err != nil {
+			framework.Logf("Failed go get Endpoints object: %v", err)
+			// Retry the error
+			return false, nil
+		}
+		portsByUID := fullPortsByPodUID(e2eendpoints.GetFullContainerPortsByPodUID(ep))
+		if err := validatePortsAndProtocols(portsByUID, expectedPortsByPodUID); err != nil {
+			if i%5 == 0 {
+				framework.Logf("Unexpected endpoints: found %v, expected %v, will retry", portsByUID, expectedEndpoints)
+			}
+			return false, nil
+		}
+
+		// If EndpointSlice API is enabled, then validate if appropriate EndpointSlice objects
+		// were also create/updated/deleted.
+		if _, err := c.Discovery().ServerResourcesForGroupVersion(discoveryv1.SchemeGroupVersion.String()); err == nil {
+			opts := metav1.ListOptions{
+				LabelSelector: "kubernetes.io/service-name=" + serviceName,
+			}
+			es, err := c.DiscoveryV1().EndpointSlices(namespace).List(context.TODO(), opts)
+			if err != nil {
+				framework.Logf("Failed go list EndpointSlice objects: %v", err)
+				// Retry the error
+				return false, nil
+			}
+			portsByUID = fullPortsByPodUID(e2eendpointslice.GetFullContainerPortsByPodUID(es.Items))
+			if err := validatePortsAndProtocols(portsByUID, expectedPortsByPodUID); err != nil {
+				if i%5 == 0 {
+					framework.Logf("Unexpected endpoint slices: found %v, expected %v, will retry", portsByUID, expectedEndpoints)
+				}
+				return false, nil
+			}
+		}
+		framework.Logf("successfully validated that service %s in namespace %s exposes endpoints %v",
+			serviceName, namespace, expectedEndpoints)
+		return true, nil
+	}); pollErr != nil {
+		if pods, err := c.CoreV1().Pods(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{}); err == nil {
+			for _, pod := range pods.Items {
+				framework.Logf("Pod %s\t%s\t%s\t%s", pod.Namespace, pod.Name, pod.Spec.NodeName, pod.DeletionTimestamp)
+			}
+		} else {
+			framework.Logf("Can't list pod debug info: %v", err)
+		}
+	}
+	framework.ExpectNoError(pollErr, "error waithing for service %s in namespace %s to expose endpoints %v: %v", serviceName, namespace, expectedEndpoints)
+}
+
+func translatePortsByPodNameToPortsByPodUID(c clientset.Interface, ns string, expectedEndpoints fullPortsByPodName) (fullPortsByPodUID, error) {
+	portsByUID := make(fullPortsByPodUID)
+	for name, portList := range expectedEndpoints {
+		pod, err := c.CoreV1().Pods(ns).Get(context.TODO(), name, metav1.GetOptions{})
+		if err != nil {
+			return nil, fmt.Errorf("failed to get pod %s, that's pretty weird. validation failed: %s", name, err)
+		}
+		portsByUID[pod.ObjectMeta.UID] = portList
+	}
+	return portsByUID, nil
+}
+
+func validatePortsAndProtocols(ep, expectedEndpoints fullPortsByPodUID) error {
+	if len(ep) != len(expectedEndpoints) {
+		// should not happen because we check this condition before
+		return fmt.Errorf("invalid number of endpoints got %v, expected %v", ep, expectedEndpoints)
+	}
+	for podUID := range expectedEndpoints {
+		if _, ok := ep[podUID]; !ok {
+			return fmt.Errorf("endpoint %v not found", podUID)
+		}
+		if len(ep[podUID]) != len(expectedEndpoints[podUID]) {
+			return fmt.Errorf("invalid list of ports for uid %v. Got %v, expected %v", podUID, ep[podUID], expectedEndpoints[podUID])
+		}
+		var match bool
+		for _, epPort := range ep[podUID] {
+			match = false
+			for _, expectedPort := range expectedEndpoints[podUID] {
+				if epPort.ContainerPort == expectedPort.ContainerPort && epPort.Protocol == expectedPort.Protocol {
+					match = true
+				}
+			}
+			if !match {
+				return fmt.Errorf("invalid list of ports for uid %v. Got %v, expected %v", podUID, ep[podUID], expectedEndpoints[podUID])
+			}
+		}
+	}
+	return nil
+}
+
 var _ = common.SIGDescribe("SCTP [LinuxOnly]", func() {
 	f := framework.NewDefaultFramework("sctp")
 	f.NamespacePodSecurityEnforceLevel = admissionapi.LevelPrivileged

test/integration/service/loadbalancer_test.go

@@ -381,6 +381,59 @@ func Test_UpdateLoadBalancerWithLoadBalancerClass(t *testing.T) {
 	}
 }
 
+// Test_ServiceLoadBalancerMixedProtocolSetup tests that a LoadBalancer Service with different protocol values
+// can be created.
+func Test_ServiceLoadBalancerMixedProtocolSetup(t *testing.T) {
+	server := kubeapiservertesting.StartTestServerOrDie(t, nil, nil, framework.SharedEtcd())
+	defer server.TearDownFn()
+
+	client, err := clientset.NewForConfig(server.ClientConfig)
+	if err != nil {
+		t.Fatalf("Error creating clientset: %v", err)
+	}
+
+	ns := framework.CreateNamespaceOrDie(client, "test-service-mixed-protocols", t)
+	defer framework.DeleteNamespaceOrDie(client, ns, t)
+
+	controller, cloud, informer := newServiceController(t, client)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	informer.Start(ctx.Done())
+	go controller.Run(ctx, 1, controllersmetrics.NewControllerManagerMetrics("loadbalancer-test"))
+
+	service := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-123",
+		},
+		Spec: corev1.ServiceSpec{
+			Type: corev1.ServiceTypeLoadBalancer,
+			Ports: []corev1.ServicePort{
+				{
+					Name:     "tcpport",
+					Port:     int32(53),
+					Protocol: corev1.ProtocolTCP,
+				},
+				{
+					Name:     "udpport",
+					Port:     int32(53),
+					Protocol: corev1.ProtocolUDP,
+				},
+			},
+		},
+	}
+
+	_, err = client.CoreV1().Services(ns.Name).Create(context.TODO(), service, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating test service: %v", err)
+	}
+
+	time.Sleep(5 * time.Second) // sleep 5 second to wait for the service controller reconcile
+	if len(cloud.Calls) == 0 {
+		t.Errorf("expected cloud provider calls to create load balancer")
+	}
+}
+
 func newServiceController(t *testing.T, client *clientset.Clientset) (*servicecontroller.Controller, *fakecloud.Cloud, informers.SharedInformerFactory) {
 	cloud := &fakecloud.Cloud{}
 	informerFactory := informers.NewSharedInformerFactory(client, 0)