diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index cbb35119c22..bd676f15232 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -625,6 +625,7 @@ const ( // kep: http://kep.k8s.io/1959 // alpha: v1.21 // beta: v1.22 + // GA: v1.24 // // Enable support multiple Service "type: LoadBalancer" implementations in a cluster by specifying LoadBalancerClass ServiceLoadBalancerClass featuregate.Feature = "ServiceLoadBalancerClass" @@ -917,7 +918,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS StatefulSetAutoDeletePVC: {Default: false, PreRelease: featuregate.Alpha}, TopologyAwareHints: {Default: false, PreRelease: featuregate.Beta}, PodAffinityNamespaceSelector: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.26 - ServiceLoadBalancerClass: {Default: true, PreRelease: featuregate.Beta}, + ServiceLoadBalancerClass: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.26 IngressClassNamespacedParams: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.25 ServiceInternalTrafficPolicy: {Default: true, PreRelease: featuregate.Beta}, LogarithmicScaleDown: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/registry/core/service/strategy.go b/pkg/registry/core/service/strategy.go index e714c1386dd..6dc188d4c59 100644 --- a/pkg/registry/core/service/strategy.go +++ b/pkg/registry/core/service/strategy.go @@ -174,13 +174,6 @@ func dropServiceDisabledFields(newSvc *api.Service, oldSvc *api.Service) { } } - // Drop LoadBalancerClass if LoadBalancerClass is not enabled - if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceLoadBalancerClass) { - if !loadBalancerClassInUse(oldSvc) { - newSvc.Spec.LoadBalancerClass = nil - } - } - // Clear InternalTrafficPolicy if not enabled if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) { if !serviceInternalTrafficPolicyInUse(oldSvc) { @@ -210,14 +203,6 @@ func loadBalancerPortsInUse(svc *api.Service) bool { return false } -// returns true if svc.Spec.LoadBalancerClass field is in use -func loadBalancerClassInUse(svc *api.Service) bool { - if svc == nil { - return false - } - return svc.Spec.LoadBalancerClass != nil -} - 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 b9270a72414..292a87ba784 100644 --- a/pkg/registry/core/service/strategy_test.go +++ b/pkg/registry/core/service/strategy_test.go @@ -166,14 +166,6 @@ func makeServiceWithPorts(ports []api.PortStatus) *api.Service { } } -func makeServiceWithLoadBalancerClass(loadBalancerClass *string) *api.Service { - return &api.Service{ - Spec: api.ServiceSpec{ - LoadBalancerClass: loadBalancerClass, - }, - } -} - func makeServiceWithInternalTrafficPolicy(policy *api.ServiceInternalTrafficPolicyType) *api.Service { return &api.Service{ Spec: api.ServiceSpec{ @@ -188,7 +180,6 @@ func TestDropDisabledField(t *testing.T) { testCases := []struct { name string enableMixedProtocol bool - enableLoadBalancerClass bool enableInternalTrafficPolicy bool svc *api.Service oldSvc *api.Service @@ -308,63 +299,6 @@ func TestDropDisabledField(t *testing.T) { oldSvc: makeServiceWithPorts([]api.PortStatus{}), compareSvc: makeServiceWithPorts(nil), }, - /* svc.Spec.LoadBalancerClass */ - { - name: "loadBalancerClass not enabled, field not used in old, not used in new", - enableLoadBalancerClass: false, - svc: makeServiceWithLoadBalancerClass(nil), - oldSvc: makeServiceWithLoadBalancerClass(nil), - compareSvc: makeServiceWithLoadBalancerClass(nil), - }, - { - name: "loadBalancerClass not enabled, field used in old and in new", - enableLoadBalancerClass: false, - svc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - oldSvc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - compareSvc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - }, - { - name: "loadBalancerClass not enabled, field not used in old, used in new", - enableLoadBalancerClass: false, - svc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - oldSvc: makeServiceWithLoadBalancerClass(nil), - compareSvc: makeServiceWithLoadBalancerClass(nil), - }, - { - name: "loadBalancerClass not enabled, field used in old, not used in new", - enableLoadBalancerClass: false, - svc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - oldSvc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - compareSvc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - }, - { - name: "loadBalancerClass enabled, field not used in old, not used in new", - enableLoadBalancerClass: true, - svc: makeServiceWithLoadBalancerClass(nil), - oldSvc: makeServiceWithLoadBalancerClass(nil), - compareSvc: makeServiceWithLoadBalancerClass(nil), - }, - { - name: "loadBalancerClass enabled, field used in old and in new", - enableLoadBalancerClass: true, - svc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - oldSvc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - compareSvc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - }, - { - name: "loadBalancerClass enabled, field not used in old, used in new", - enableLoadBalancerClass: true, - svc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - oldSvc: makeServiceWithLoadBalancerClass(nil), - compareSvc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - }, - { - name: "loadBalancerClass enabled, field used in old, not used in new", - enableLoadBalancerClass: true, - svc: makeServiceWithLoadBalancerClass(nil), - oldSvc: makeServiceWithLoadBalancerClass(utilpointer.StringPtr("test.com/test")), - compareSvc: makeServiceWithLoadBalancerClass(nil), - }, /* svc.spec.internalTrafficPolicy */ { name: "internal traffic policy not enabled, field used in old, not used in new", @@ -392,7 +326,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.ServiceLoadBalancerClass, tc.enableLoadBalancerClass)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceInternalTrafficPolicy, tc.enableInternalTrafficPolicy)() old := tc.oldSvc.DeepCopy() diff --git a/test/integration/service/loadbalancer_test.go b/test/integration/service/loadbalancer_test.go index 3baf89ac973..a07a316f30f 100644 --- a/test/integration/service/loadbalancer_test.go +++ b/test/integration/service/loadbalancer_test.go @@ -19,6 +19,7 @@ package service import ( "context" "testing" + "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,8 +29,6 @@ import ( restclient "k8s.io/client-go/rest" servicecontroller "k8s.io/cloud-provider/controllers/service" fakecloud "k8s.io/cloud-provider/fake" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/test/integration/framework" utilpointer "k8s.io/utils/pointer" ) @@ -249,7 +248,6 @@ func serviceHasNodePorts(svc *corev1.Service) bool { // Test_ServiceLoadBalancerEnableLoadBalancerClass tests that when a LoadBalancer // type of service has spec.LoadBalancerClass set, cloud provider should not create default load balancer. func Test_ServiceLoadBalancerEnableLoadBalancerClass(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceLoadBalancerClass, true)() controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig() _, server, closeFn := framework.RunAnAPIServer(controlPlaneConfig) @@ -289,16 +287,16 @@ func Test_ServiceLoadBalancerEnableLoadBalancerClass(t *testing.T) { 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("Unexpected cloud provider calls: %v", cloud.Calls) } } -// Test_ServiceLoadBalancerEnableLoadBalancerClassThenUpdateLoadBalancerClass tests that when a LoadBalancer +// Test_SetLoadBalancerClassThenUpdateLoadBalancerClass tests that when a LoadBalancer // type of service has spec.LoadBalancerClass set, it should be immutable as long as the service type // is still LoadBalancer. -func Test_ServiceLoadBalancerEnableLoadBalancerClassThenUpdateLoadBalancerClass(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceLoadBalancerClass, true)() +func Test_SetLoadBalancerClassThenUpdateLoadBalancerClass(t *testing.T) { controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig() _, server, closeFn := framework.RunAnAPIServer(controlPlaneConfig) @@ -338,21 +336,71 @@ func Test_ServiceLoadBalancerEnableLoadBalancerClassThenUpdateLoadBalancerClass( t.Fatalf("Error creating test service: %v", err) } - if len(cloud.Calls) > 0 { - t.Errorf("Unexpected cloud provider calls: %v", cloud.Calls) - } - service.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/update") _, err = client.CoreV1().Services(ns.Name).Update(ctx, service, metav1.UpdateOptions{}) if err == nil { - t.Fatal("Error updating test service load balancer class should throw error") + t.Fatal("Error: updating test service load balancer class should throw error, field is immutable") } + time.Sleep(5 * time.Second) // sleep 5 second to wait for the service controller reconcile if len(cloud.Calls) > 0 { t.Errorf("Unexpected cloud provider calls: %v", cloud.Calls) } } +// Test_UpdateLoadBalancerWithLoadBalancerClass tests that when a Load Balancer type of Service that +// is updated from non loadBalancerClass set to loadBalancerClass set, it should be not allowed. +func Test_UpdateLoadBalancerWithLoadBalancerClass(t *testing.T) { + + controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig() + _, server, closeFn := framework.RunAnAPIServer(controlPlaneConfig) + defer closeFn() + + config := restclient.Config{Host: server.URL} + client, err := clientset.NewForConfig(&config) + if err != nil { + t.Fatalf("Error creating clientset: %v", err) + } + + ns := framework.CreateTestingNamespace("test-service-update-load-balancer-class", server, t) + defer framework.DeleteTestingNamespace(ns, server, t) + + controller, cloud, informer := newServiceController(t, client) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + informer.Start(ctx.Done()) + go controller.Run(ctx, 1) + + service := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-update-load-balancer-class", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{{ + Port: int32(80), + }}, + }, + } + + service, err = client.CoreV1().Services(ns.Name).Create(ctx, service, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Error creating test service: %v", err) + } + + service.Spec.LoadBalancerClass = utilpointer.StringPtr("test.com/test") + _, err = client.CoreV1().Services(ns.Name).Update(ctx, service, metav1.UpdateOptions{}) + if err == nil { + t.Fatal("Error: updating test service load balancer class should throw error, field is immutable") + } + + 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) @@ -368,6 +416,6 @@ func newServiceController(t *testing.T, client *clientset.Clientset) (*serviceco if err != nil { t.Fatalf("Error creating service controller: %v", err) } - cloud.Calls = nil // ignore any cloud calls made in init() + cloud.ClearCalls() // ignore any cloud calls made in init() return controller, cloud, informerFactory }