diff --git a/pkg/apis/core/validation/conditional_validation.go b/pkg/apis/core/validation/conditional_validation.go deleted file mode 100644 index 43266c16ccb..00000000000 --- a/pkg/apis/core/validation/conditional_validation.go +++ /dev/null @@ -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 -} diff --git a/pkg/apis/core/validation/conditional_validation_test.go b/pkg/apis/core/validation/conditional_validation_test.go deleted file mode 100644 index 12d0b2edb94..00000000000 --- a/pkg/apis/core/validation/conditional_validation_test.go +++ /dev/null @@ -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]) - } - } - }) - } -} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 6ae2eb086bd..20f8f2de705 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -568,6 +568,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" @@ -969,7 +970,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}, diff --git a/pkg/registry/core/service/storage/storage_test.go b/pkg/registry/core/service/storage/storage_test.go index bbff651b0fa..1345ed519fa 100644 --- a/pkg/registry/core/service/storage/storage_test.go +++ b/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 - svc *api.Service - expectError bool - expectNodePorts bool - gateMixedProtocolLBService bool + name string + svc *api.Service + expectError bool + expectNodePorts 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_MixedProtocolLBService:off", + name: "type:LoadBalancer_multiport_multiproto_unspecified", 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, - expectError: true, + expectNodePorts: true, }, { - // When the MixedProtocolLBService gate is locked, this can be removed. - name: "type:LoadBalancer_multiport_multiproto_specified_MixedProtocolLBService:off", + name: "type:LoadBalancer_multiport_multiproto_specified", 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, - expectError: true, + expectNodePorts: true, }, { - // When the MixedProtocolLBService gate is locked, this can be removed. - name: "type:LoadBalancer_multiport_multiproto_same_MixedProtocolLBService:off", + name: "type:LoadBalancer_multiport_multiproto_same", 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, - 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, + 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{}) diff --git a/pkg/registry/core/service/strategy.go b/pkg/registry/core/service/strategy.go index e76855795f8..d2ab2d80c51 100644 --- a/pkg/registry/core/service/strategy.go +++ b/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 diff --git a/pkg/registry/core/service/strategy_test.go b/pkg/registry/core/service/strategy_test.go index 7769a428b22..55d13d4adf2 100644 --- a/pkg/registry/core/service/strategy_test.go +++ b/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", - enableMixedProtocol: false, - svc: makeServiceWithConditions(nil), - oldSvc: makeServiceWithConditions(nil), - compareSvc: makeServiceWithConditions(nil), + name: "mixed protocol enabled, field not used in old, not used in new", + svc: makeServiceWithConditions(nil), + oldSvc: makeServiceWithConditions(nil), + compareSvc: makeServiceWithConditions(nil), }, { - name: "mixed protocol not enabled, field used in old and in new", - enableMixedProtocol: false, - svc: makeServiceWithConditions([]metav1.Condition{}), - oldSvc: makeServiceWithConditions([]metav1.Condition{}), - compareSvc: makeServiceWithConditions([]metav1.Condition{}), + name: "mixed protocol enabled, field used in old and in new", + svc: makeServiceWithConditions([]metav1.Condition{}), + oldSvc: makeServiceWithConditions([]metav1.Condition{}), + compareSvc: makeServiceWithConditions([]metav1.Condition{}), }, { - name: "mixed protocol not enabled, field not used in old, used in new", - enableMixedProtocol: false, - svc: makeServiceWithConditions([]metav1.Condition{}), - oldSvc: makeServiceWithConditions(nil), - compareSvc: makeServiceWithConditions(nil), + name: "mixed protocol enabled, field not used in old, used in new", + svc: makeServiceWithConditions([]metav1.Condition{}), + oldSvc: makeServiceWithConditions(nil), + compareSvc: makeServiceWithConditions([]metav1.Condition{}), }, { - name: "mixed protocol not enabled, field used in old, not used in new", - enableMixedProtocol: false, - svc: makeServiceWithConditions(nil), - oldSvc: makeServiceWithConditions([]metav1.Condition{}), - 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), + name: "mixed protocol enabled, field used in old, not used in new", + 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", - enableMixedProtocol: false, - svc: makeServiceWithPorts(nil), - oldSvc: makeServiceWithPorts(nil), - compareSvc: makeServiceWithPorts(nil), + name: "mixed protocol enabled, field not used in old, not used in new", + svc: makeServiceWithPorts(nil), + oldSvc: makeServiceWithPorts(nil), + compareSvc: makeServiceWithPorts(nil), }, { - name: "mixed protocol not enabled, field used in old and in new", - enableMixedProtocol: false, - svc: makeServiceWithPorts([]api.PortStatus{}), - oldSvc: makeServiceWithPorts([]api.PortStatus{}), - compareSvc: makeServiceWithPorts([]api.PortStatus{}), + name: "mixed protocol enabled, field used in old and in new", + svc: makeServiceWithPorts([]api.PortStatus{}), + oldSvc: makeServiceWithPorts([]api.PortStatus{}), + compareSvc: makeServiceWithPorts([]api.PortStatus{}), }, { - name: "mixed protocol not enabled, field not used in old, used in new", - enableMixedProtocol: false, - svc: makeServiceWithPorts([]api.PortStatus{}), - oldSvc: makeServiceWithPorts(nil), - compareSvc: makeServiceWithPorts(nil), + name: "mixed protocol enabled, field not used in old, used in new", + svc: makeServiceWithPorts([]api.PortStatus{}), + oldSvc: makeServiceWithPorts(nil), + compareSvc: makeServiceWithPorts([]api.PortStatus{}), }, { - name: "mixed protocol not enabled, field used in old, not used in new", - enableMixedProtocol: false, - svc: makeServiceWithPorts(nil), - oldSvc: makeServiceWithPorts([]api.PortStatus{}), - 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), + name: "mixed protocol enabled, field used in old, not used in new", + 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() diff --git a/test/e2e/framework/service/jig.go b/test/e2e/framework/service/jig.go index 6e8a39bfec1..5e7b5ec7eb0 100644 --- a/test/e2e/framework/service/jig.go +++ b/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,36 @@ 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(timeout time.Duration, 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) + } + _, 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) + } + + ginkgo.By("waiting for cluster IP for loadbalancer service " + j.Namespace + "/" + j.Name) + return j.WaitForLoadBalancerClusterIP(timeout) +} + +// WaitForLoadBalancerClusterIP waits the given LoadBalancer service to have a ClusterIP, or returns an error after the given timeout +func (j *TestJig) WaitForLoadBalancerClusterIP(timeout time.Duration) (*v1.Service, error) { + framework.Logf("Waiting up to %v for LoadBalancer service %q to have a ClusterIP", timeout, j.Name) + service, err := j.waitForCondition(timeout, "have a ClusterIP", func(svc *v1.Service) bool { + return len(svc.Spec.ClusterIP) > 0 + }) + if err != nil { + return nil, err + } + + return j.sanityCheckService(service, v1.ServiceTypeLoadBalancer) +} diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index e196d805dcc..bebe2265af2 100644 --- a/test/e2e/network/service.go +++ b/test/e2e/network/service.go @@ -3731,6 +3731,78 @@ 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(2*time.Minute, func(service *v1.Service) { + service.Spec.Ports = []v1.ServicePort{ + { + Name: "portname1", + Port: 80, + TargetPort: intstr.FromString(svc1port), + Protocol: v1.ProtocolTCP, + }, + { + Name: "portname2", + Port: 81, + TargetPort: intstr.FromString(svc2port), + Protocol: v1.ProtocolUDP, + }, + } + }) + framework.ExpectNoError(err) + + port1 := 100 + port2 := 101 + + 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(port1), + Protocol: v1.ProtocolTCP, + }, + { + Name: svc2port, + ContainerPort: int32(port2), + Protocol: v1.ProtocolUDP, + }, + } + + podname1 := "pod1" + + createPodOrFail(f, ns, podname1, jig.Labels, containerPorts, "netexec", "--http-port", strconv.Itoa(port1), "--udp-port", strconv.Itoa(port2)) + validateEndpointsPortsOrFail(cs, ns, serviceName, portsByPodName{podname1: {port1, port2}}) + + 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) + }) }) diff --git a/test/integration/service/loadbalancer_test.go b/test/integration/service/loadbalancer_test.go index 63ce4d73f0f..04fecc25d98 100644 --- a/test/integration/service/loadbalancer_test.go +++ b/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)