diff --git a/pkg/controller/service/service_controller_test.go b/pkg/controller/service/service_controller_test.go index e7515954ef1..52862f52bca 100644 --- a/pkg/controller/service/service_controller_test.go +++ b/pkg/controller/service/service_controller_test.go @@ -618,7 +618,7 @@ func TestProcessServiceCreateOrUpdate(t *testing.T) { } // TestProcessServiceCreateOrUpdateK8sError tests processServiceCreateOrUpdate -// with various kubernetes errors. +// with various kubernetes errors when patching status. func TestProcessServiceCreateOrUpdateK8sError(t *testing.T) { svcName := "svc-k8s-err" conflictErr := apierrors.NewConflict(schema.GroupResource{}, svcName, errors.New("object conflict")) @@ -644,6 +644,8 @@ func TestProcessServiceCreateOrUpdateK8sError(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { svc := newService(svcName, types.UID("123"), v1.ServiceTypeLoadBalancer) + // Preset finalizer so k8s error only happens when patching status. + svc.Finalizers = []string{servicehelper.LoadBalancerCleanupFinalizer} controller, _, client := newController() client.PrependReactor("patch", "services", func(action core.Action) (bool, runtime.Object, error) { return true, nil, tc.k8sErr diff --git a/test/e2e/framework/service/BUILD b/test/e2e/framework/service/BUILD index e3e6ff90801..b73235a7657 100644 --- a/test/e2e/framework/service/BUILD +++ b/test/e2e/framework/service/BUILD @@ -34,6 +34,7 @@ go_library( "//staging/src/k8s.io/client-go/rest:go_default_library", "//staging/src/k8s.io/client-go/tools/cache:go_default_library", "//staging/src/k8s.io/client-go/util/retry:go_default_library", + "//staging/src/k8s.io/cloud-provider/service/helpers:go_default_library", "//test/e2e/framework:go_default_library", "//test/e2e/framework/log:go_default_library", "//test/e2e/framework/node:go_default_library", diff --git a/test/e2e/framework/service/wait.go b/test/e2e/framework/service/wait.go index 760e0f4b09c..8a35f96ea7c 100644 --- a/test/e2e/framework/service/wait.go +++ b/test/e2e/framework/service/wait.go @@ -20,11 +20,15 @@ import ( "context" "fmt" - "github.com/onsi/ginkgo" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" + servicehelper "k8s.io/cloud-provider/service/helpers" "k8s.io/kubernetes/test/e2e/framework" e2elog "k8s.io/kubernetes/test/e2e/framework/log" + + "github.com/onsi/ginkgo" ) // WaitForServiceResponding waits for the service to be responding. @@ -63,3 +67,52 @@ func WaitForServiceResponding(c clientset.Interface, ns, name string) error { return true, nil }) } + +// WaitForServiceDeletedWithFinalizer waits for the service with finalizer to be deleted. +func WaitForServiceDeletedWithFinalizer(cs clientset.Interface, namespace, name string) { + ginkgo.By("Delete service with finalizer") + if err := cs.CoreV1().Services(namespace).Delete(name, nil); err != nil { + e2elog.Failf("Failed to delete service %s/%s", namespace, name) + } + + ginkgo.By("Wait for service to disappear") + if pollErr := wait.PollImmediate(LoadBalancerPollInterval, GetServiceLoadBalancerCreationTimeout(cs), func() (bool, error) { + svc, err := cs.CoreV1().Services(namespace).Get(name, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + e2elog.Logf("Service %s/%s is gone.", namespace, name) + return true, nil + } + return false, err + } + e2elog.Logf("Service %s/%s still exists with finalizers: %v", namespace, name, svc.Finalizers) + return false, nil + }); pollErr != nil { + e2elog.Failf("Failed to wait for service to disappear: %v", pollErr) + } +} + +// WaitForServiceUpdatedWithFinalizer waits for the service to be updated to have or +// don't have a finalizer. +func WaitForServiceUpdatedWithFinalizer(cs clientset.Interface, namespace, name string, hasFinalizer bool) { + ginkgo.By(fmt.Sprintf("Wait for service to hasFinalizer=%t", hasFinalizer)) + if pollErr := wait.PollImmediate(LoadBalancerPollInterval, GetServiceLoadBalancerCreationTimeout(cs), func() (bool, error) { + svc, err := cs.CoreV1().Services(namespace).Get(name, metav1.GetOptions{}) + if err != nil { + return false, err + } + foundFinalizer := false + for _, finalizer := range svc.Finalizers { + if finalizer == servicehelper.LoadBalancerCleanupFinalizer { + foundFinalizer = true + } + } + if foundFinalizer != hasFinalizer { + e2elog.Logf("Service %s/%s hasFinalizer=%t, want %t", namespace, name, foundFinalizer, hasFinalizer) + return false, nil + } + return true, nil + }); pollErr != nil { + e2elog.Failf("Failed to wait for service to hasFinalizer=%t: %v", hasFinalizer, pollErr) + } +} diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index 11ba96d4aa5..a55e1a634b4 100644 --- a/test/e2e/network/service.go +++ b/test/e2e/network/service.go @@ -29,7 +29,6 @@ import ( compute "google.golang.org/api/compute/v1" v1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" @@ -1916,39 +1915,13 @@ var _ = SIGDescribe("Services", func() { } }) - // This test verifies if service load balancer cleanup finalizer can be removed - // when feature gate isn't enabled on the cluster. - // This ensures downgrading from higher version cluster will not break LoadBalancer - // type service. - ginkgo.It("should remove load balancer cleanup finalizer when service is deleted [Slow]", func() { - jig := e2eservice.NewTestJig(cs, "lb-remove-finalizer") - - ginkgo.By("Create load balancer service") - svc := jig.CreateTCPServiceOrFail(f.Namespace.Name, func(svc *v1.Service) { - svc.Spec.Type = v1.ServiceTypeLoadBalancer - }) - - defer func() { - waitForServiceDeletedWithFinalizer(cs, svc.Namespace, svc.Name) - }() - - ginkgo.By("Wait for load balancer to serve traffic") - svc = jig.WaitForLoadBalancerOrFail(svc.Namespace, svc.Name, e2eservice.GetServiceLoadBalancerCreationTimeout(cs)) - - ginkgo.By("Manually add load balancer cleanup finalizer to service") - svc.Finalizers = append(svc.Finalizers, "service.kubernetes.io/load-balancer-cleanup") - if _, err := cs.CoreV1().Services(svc.Namespace).Update(svc); err != nil { - e2elog.Failf("Failed to add finalizer to service %s/%s: %v", svc.Namespace, svc.Name, err) - } - }) - // This test verifies if service load balancer cleanup finalizer is properly // handled during service lifecycle. // 1. Create service with type=LoadBalancer. Finalizer should be added. // 2. Update service to type=ClusterIP. Finalizer should be removed. // 3. Update service to type=LoadBalancer. Finalizer should be added. // 4. Delete service with type=LoadBalancer. Finalizer should be removed. - ginkgo.It("should handle load balancer cleanup finalizer for service [Slow] [Feature:ServiceFinalizer]", func() { + ginkgo.It("should handle load balancer cleanup finalizer for service [Slow]", func() { jig := e2eservice.NewTestJig(cs, "lb-finalizer") ginkgo.By("Create load balancer service") @@ -1957,71 +1930,26 @@ var _ = SIGDescribe("Services", func() { }) defer func() { - waitForServiceDeletedWithFinalizer(cs, svc.Namespace, svc.Name) + ginkgo.By("Check that service can be deleted with finalizer") + e2eservice.WaitForServiceDeletedWithFinalizer(cs, svc.Namespace, svc.Name) }() ginkgo.By("Wait for load balancer to serve traffic") svc = jig.WaitForLoadBalancerOrFail(svc.Namespace, svc.Name, e2eservice.GetServiceLoadBalancerCreationTimeout(cs)) ginkgo.By("Check if finalizer presents on service with type=LoadBalancer") - waitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, true) + e2eservice.WaitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, true) ginkgo.By("Check if finalizer is removed on service after changed to type=ClusterIP") jig.ChangeServiceType(svc.Namespace, svc.Name, v1.ServiceTypeClusterIP, e2eservice.GetServiceLoadBalancerCreationTimeout(cs)) - waitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, false) + e2eservice.WaitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, false) ginkgo.By("Check if finalizer is added back to service after changed to type=LoadBalancer") jig.ChangeServiceType(svc.Namespace, svc.Name, v1.ServiceTypeLoadBalancer, e2eservice.GetServiceLoadBalancerCreationTimeout(cs)) - waitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, true) + e2eservice.WaitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, true) }) }) -func waitForServiceDeletedWithFinalizer(cs clientset.Interface, namespace, name string) { - ginkgo.By("Delete service with finalizer") - if err := cs.CoreV1().Services(namespace).Delete(name, nil); err != nil { - e2elog.Failf("Failed to delete service %s/%s", namespace, name) - } - - ginkgo.By("Wait for service to disappear") - if pollErr := wait.PollImmediate(e2eservice.LoadBalancerPollInterval, e2eservice.GetServiceLoadBalancerCreationTimeout(cs), func() (bool, error) { - svc, err := cs.CoreV1().Services(namespace).Get(name, metav1.GetOptions{}) - if err != nil { - if apierrors.IsNotFound(err) { - e2elog.Logf("Service %s/%s is gone.", namespace, name) - return true, nil - } - return false, err - } - e2elog.Logf("Service %s/%s still exists with finalizers: %v", namespace, name, svc.Finalizers) - return false, nil - }); pollErr != nil { - e2elog.Failf("Failed to wait for service to disappear: %v", pollErr) - } -} - -func waitForServiceUpdatedWithFinalizer(cs clientset.Interface, namespace, name string, hasFinalizer bool) { - ginkgo.By(fmt.Sprintf("Wait for service to hasFinalizer=%t", hasFinalizer)) - if pollErr := wait.PollImmediate(e2eservice.LoadBalancerPollInterval, e2eservice.GetServiceLoadBalancerCreationTimeout(cs), func() (bool, error) { - svc, err := cs.CoreV1().Services(namespace).Get(name, metav1.GetOptions{}) - if err != nil { - return false, err - } - foundFinalizer := false - for _, finalizer := range svc.Finalizers { - if finalizer == "service.kubernetes.io/load-balancer-cleanup" { - foundFinalizer = true - } - } - if foundFinalizer != hasFinalizer { - e2elog.Logf("Service %s/%s hasFinalizer=%t, want %t", namespace, name, foundFinalizer, hasFinalizer) - return false, nil - } - return true, nil - }); pollErr != nil { - e2elog.Failf("Failed to wait for service to hasFinalizer=%t: %v", hasFinalizer, pollErr) - } -} - // TODO: Get rid of [DisabledForLargeClusters] tag when issue #56138 is fixed. var _ = SIGDescribe("ESIPP [Slow] [DisabledForLargeClusters]", func() { f := framework.NewDefaultFramework("esipp") diff --git a/test/e2e/upgrades/services.go b/test/e2e/upgrades/services.go index 6f45fbd4662..257d6e775c7 100644 --- a/test/e2e/upgrades/services.go +++ b/test/e2e/upgrades/services.go @@ -84,12 +84,12 @@ func (t *ServiceUpgradeTest) Setup(f *framework.Framework) { func (t *ServiceUpgradeTest) Test(f *framework.Framework, done <-chan struct{}, upgrade UpgradeType) { switch upgrade { case MasterUpgrade, ClusterUpgrade: - t.test(f, done, true) + t.test(f, done, true, true) case NodeUpgrade: // Node upgrades should test during disruption only on GCE/GKE for now. - t.test(f, done, shouldTestPDBs()) + t.test(f, done, shouldTestPDBs(), false) default: - t.test(f, done, false) + t.test(f, done, false, false) } } @@ -98,7 +98,7 @@ func (t *ServiceUpgradeTest) Teardown(f *framework.Framework) { // rely on the namespace deletion to clean up everything } -func (t *ServiceUpgradeTest) test(f *framework.Framework, done <-chan struct{}, testDuringDisruption bool) { +func (t *ServiceUpgradeTest) test(f *framework.Framework, done <-chan struct{}, testDuringDisruption, testFinalizer bool) { if testDuringDisruption { // Continuous validation ginkgo.By("continuously hitting the pod through the service's LoadBalancer") @@ -115,4 +115,13 @@ func (t *ServiceUpgradeTest) test(f *framework.Framework, done <-chan struct{}, ginkgo.By("hitting the pod through the service's LoadBalancer") t.jig.TestReachableHTTP(t.tcpIngressIP, t.svcPort, e2eservice.LoadBalancerLagTimeoutDefault) t.jig.SanityCheckService(t.tcpService, v1.ServiceTypeLoadBalancer) + + if testFinalizer { + defer func() { + ginkgo.By("Check that service can be deleted with finalizer") + e2eservice.WaitForServiceDeletedWithFinalizer(t.jig.Client, t.tcpService.Namespace, t.tcpService.Name) + }() + ginkgo.By("Check that finalizer is present on loadBalancer type service") + e2eservice.WaitForServiceUpdatedWithFinalizer(t.jig.Client, t.tcpService.Namespace, t.tcpService.Name, true) + } }