From 82ce61afc7bcbddb0f1f3d86145d46811410b33a Mon Sep 17 00:00:00 2001 From: Laszlo Janosi Date: Thu, 6 Oct 2022 11:31:57 +0200 Subject: [PATCH 1/2] KEP-1435 Mixed Protocol values in LoadBalancer Service GA Removed the unit tests that test the cases when the MixedProtocolLBService feature flag was false - the feature flag is locked to true with GA Added an integration test to test whether the API server accepts an LB Service with different protocols. Added an e2e test to test whether a service which is exposed by a multi-protocol LB Service is accessible via both ports. Removed the conditional validation that compared the new and the old Service definitions during an update - the feature flag is locked to true with GA. --- .../core/validation/conditional_validation.go | 61 ---- .../validation/conditional_validation_test.go | 277 ------------------ pkg/features/kube_features.go | 3 +- .../core/service/storage/storage_test.go | 61 +--- pkg/registry/core/service/strategy.go | 34 --- pkg/registry/core/service/strategy_test.go | 130 ++------ test/e2e/framework/service/jig.go | 35 +++ test/e2e/network/service.go | 72 +++++ test/integration/service/loadbalancer_test.go | 53 ++++ 9 files changed, 204 insertions(+), 522 deletions(-) delete mode 100644 pkg/apis/core/validation/conditional_validation.go delete mode 100644 pkg/apis/core/validation/conditional_validation_test.go 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) From 9d75c958cedbf354668bba5a6f89393ac661e858 Mon Sep 17 00:00:00 2001 From: Laszlo Janosi Date: Thu, 3 Nov 2022 10:54:14 +0200 Subject: [PATCH 2/2] Fix review comments. Implement endpoint port validation that verifies the protocol, too. --- test/e2e/framework/endpoints/ports.go | 24 +++++ test/e2e/framework/endpointslice/ports.go | 29 +++++ test/e2e/framework/service/jig.go | 20 +--- test/e2e/network/service.go | 123 ++++++++++++++++++++-- 4 files changed, 171 insertions(+), 25 deletions(-) diff --git a/test/e2e/framework/endpoints/ports.go b/test/e2e/framework/endpoints/ports.go index efd960d93f4..8c116b031a5 100644 --- a/test/e2e/framework/endpoints/ports.go +++ b/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 +} diff --git a/test/e2e/framework/endpointslice/ports.go b/test/e2e/framework/endpointslice/ports.go index c61230c7d2a..f51162b0083 100644 --- a/test/e2e/framework/endpointslice/ports.go +++ b/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 +} diff --git a/test/e2e/framework/service/jig.go b/test/e2e/framework/service/jig.go index 5e7b5ec7eb0..202c872ae12 100644 --- a/test/e2e/framework/service/jig.go +++ b/test/e2e/framework/service/jig.go @@ -1070,7 +1070,7 @@ func (j *TestJig) CreateSCTPServiceWithPort(tweak func(svc *v1.Service), port in // 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) { +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 @@ -1079,24 +1079,10 @@ func (j *TestJig) CreateLoadBalancerServiceWaitForClusterIPOnly(timeout time.Dur if tweak != nil { tweak(svc) } - _, err := j.Client.CoreV1().Services(j.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{}) + 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) } - 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) + return j.sanityCheckService(result, v1.ServiceTypeLoadBalancer) } diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index bebe2265af2..1a0ef2ff601 100644 --- a/test/e2e/network/service.go +++ b/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) { @@ -3750,7 +3756,7 @@ var _ = common.SIGDescribe("Services", func() { svc2port := "svc2" ginkgo.By("creating service " + serviceName + " in namespace " + ns) - svc, err := jig.CreateLoadBalancerServiceWaitForClusterIPOnly(2*time.Minute, func(service *v1.Service) { + svc, err := jig.CreateLoadBalancerServiceWaitForClusterIPOnly(func(service *v1.Service) { service.Spec.Ports = []v1.ServicePort{ { Name: "portname1", @@ -3760,7 +3766,7 @@ var _ = common.SIGDescribe("Services", func() { }, { Name: "portname2", - Port: 81, + Port: 80, TargetPort: intstr.FromString(svc2port), Protocol: v1.ProtocolUDP, }, @@ -3768,8 +3774,7 @@ var _ = common.SIGDescribe("Services", func() { }) framework.ExpectNoError(err) - port1 := 100 - port2 := 101 + containerPort := 100 names := map[string]bool{} defer func() { @@ -3782,20 +3787,20 @@ var _ = common.SIGDescribe("Services", func() { containerPorts := []v1.ContainerPort{ { Name: svc1port, - ContainerPort: int32(port1), + ContainerPort: int32(containerPort), Protocol: v1.ProtocolTCP, }, { Name: svc2port, - ContainerPort: int32(port2), + ContainerPort: int32(containerPort), 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}}) + 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) @@ -4236,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