Skip to content

Commit 0a7c085

Browse files
MrHohnMrHohn
committed
Check service finalizer on upgrade test and fix-up test cases
1 parent 60290dd commit 0a7c085

File tree

5 files changed

+77
-84
lines changed

5 files changed

+77
-84
lines changed

‎pkg/controller/service/service_controller_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,7 @@ func TestProcessServiceCreateOrUpdate(t *testing.T) {
618618
}
619619

620620
// TestProcessServiceCreateOrUpdateK8sError tests processServiceCreateOrUpdate
621-
// with various kubernetes errors.
621+
// with various kubernetes errors when patching status.
622622
func TestProcessServiceCreateOrUpdateK8sError(t *testing.T) {
623623
svcName := "svc-k8s-err"
624624
conflictErr := apierrors.NewConflict(schema.GroupResource{}, svcName, errors.New("object conflict"))
@@ -644,6 +644,8 @@ func TestProcessServiceCreateOrUpdateK8sError(t *testing.T) {
644644
for _, tc := range testCases {
645645
t.Run(tc.desc, func(t *testing.T) {
646646
svc := newService(svcName, types.UID("123"), v1.ServiceTypeLoadBalancer)
647+
// Preset finalizer so k8s error only happens when patching status.
648+
svc.Finalizers = []string{servicehelper.LoadBalancerCleanupFinalizer}
647649
controller, _, client := newController()
648650
client.PrependReactor("patch", "services", func(action core.Action) (bool, runtime.Object, error) {
649651
return true, nil, tc.k8sErr

‎test/e2e/framework/service/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ go_library(
3434
"//staging/src/k8s.io/client-go/rest:go_default_library",
3535
"//staging/src/k8s.io/client-go/tools/cache:go_default_library",
3636
"//staging/src/k8s.io/client-go/util/retry:go_default_library",
37+
"//staging/src/k8s.io/cloud-provider/service/helpers:go_default_library",
3738
"//test/e2e/framework:go_default_library",
3839
"//test/e2e/framework/log:go_default_library",
3940
"//test/e2e/framework/node:go_default_library",

‎test/e2e/framework/service/wait.go‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,15 @@ import (
2020
"context"
2121
"fmt"
2222

23-
"github.com/onsi/ginkgo"
23+
apierrors "k8s.io/apimachinery/pkg/api/errors"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2425
"k8s.io/apimachinery/pkg/util/wait"
2526
clientset "k8s.io/client-go/kubernetes"
27+
servicehelper "k8s.io/cloud-provider/service/helpers"
2628
"k8s.io/kubernetes/test/e2e/framework"
2729
e2elog "k8s.io/kubernetes/test/e2e/framework/log"
30+
31+
"github.com/onsi/ginkgo"
2832
)
2933

3034
// WaitForServiceResponding waits for the service to be responding.
@@ -63,3 +67,52 @@ func WaitForServiceResponding(c clientset.Interface, ns, name string) error {
6367
return true, nil
6468
})
6569
}
70+
71+
// WaitForServiceDeletedWithFinalizer waits for the service with finalizer to be deleted.
72+
func WaitForServiceDeletedWithFinalizer(cs clientset.Interface, namespace, name string) {
73+
ginkgo.By("Delete service with finalizer")
74+
if err := cs.CoreV1().Services(namespace).Delete(name, nil); err != nil {
75+
e2elog.Failf("Failed to delete service %s/%s", namespace, name)
76+
}
77+
78+
ginkgo.By("Wait for service to disappear")
79+
if pollErr := wait.PollImmediate(LoadBalancerPollInterval, GetServiceLoadBalancerCreationTimeout(cs), func() (bool, error) {
80+
svc, err := cs.CoreV1().Services(namespace).Get(name, metav1.GetOptions{})
81+
if err != nil {
82+
if apierrors.IsNotFound(err) {
83+
e2elog.Logf("Service %s/%s is gone.", namespace, name)
84+
return true, nil
85+
}
86+
return false, err
87+
}
88+
e2elog.Logf("Service %s/%s still exists with finalizers: %v", namespace, name, svc.Finalizers)
89+
return false, nil
90+
}); pollErr != nil {
91+
e2elog.Failf("Failed to wait for service to disappear: %v", pollErr)
92+
}
93+
}
94+
95+
// WaitForServiceUpdatedWithFinalizer waits for the service to be updated to have or
96+
// don't have a finalizer.
97+
func WaitForServiceUpdatedWithFinalizer(cs clientset.Interface, namespace, name string, hasFinalizer bool) {
98+
ginkgo.By(fmt.Sprintf("Wait for service to hasFinalizer=%t", hasFinalizer))
99+
if pollErr := wait.PollImmediate(LoadBalancerPollInterval, GetServiceLoadBalancerCreationTimeout(cs), func() (bool, error) {
100+
svc, err := cs.CoreV1().Services(namespace).Get(name, metav1.GetOptions{})
101+
if err != nil {
102+
return false, err
103+
}
104+
foundFinalizer := false
105+
for _, finalizer := range svc.Finalizers {
106+
if finalizer == servicehelper.LoadBalancerCleanupFinalizer {
107+
foundFinalizer = true
108+
}
109+
}
110+
if foundFinalizer != hasFinalizer {
111+
e2elog.Logf("Service %s/%s hasFinalizer=%t, want %t", namespace, name, foundFinalizer, hasFinalizer)
112+
return false, nil
113+
}
114+
return true, nil
115+
}); pollErr != nil {
116+
e2elog.Failf("Failed to wait for service to hasFinalizer=%t: %v", hasFinalizer, pollErr)
117+
}
118+
}

‎test/e2e/network/service.go‎

Lines changed: 6 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
compute "google.golang.org/api/compute/v1"
3030

3131
v1 "k8s.io/api/core/v1"
32-
apierrors "k8s.io/apimachinery/pkg/api/errors"
3332
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3433
"k8s.io/apimachinery/pkg/labels"
3534
"k8s.io/apimachinery/pkg/util/intstr"
@@ -1916,39 +1915,13 @@ var _ = SIGDescribe("Services", func() {
19161915
}
19171916
})
19181917

1919-
// This test verifies if service load balancer cleanup finalizer can be removed
1920-
// when feature gate isn't enabled on the cluster.
1921-
// This ensures downgrading from higher version cluster will not break LoadBalancer
1922-
// type service.
1923-
ginkgo.It("should remove load balancer cleanup finalizer when service is deleted [Slow]", func() {
1924-
jig := e2eservice.NewTestJig(cs, "lb-remove-finalizer")
1925-
1926-
ginkgo.By("Create load balancer service")
1927-
svc := jig.CreateTCPServiceOrFail(f.Namespace.Name, func(svc *v1.Service) {
1928-
svc.Spec.Type = v1.ServiceTypeLoadBalancer
1929-
})
1930-
1931-
defer func() {
1932-
waitForServiceDeletedWithFinalizer(cs, svc.Namespace, svc.Name)
1933-
}()
1934-
1935-
ginkgo.By("Wait for load balancer to serve traffic")
1936-
svc = jig.WaitForLoadBalancerOrFail(svc.Namespace, svc.Name, e2eservice.GetServiceLoadBalancerCreationTimeout(cs))
1937-
1938-
ginkgo.By("Manually add load balancer cleanup finalizer to service")
1939-
svc.Finalizers = append(svc.Finalizers, "service.kubernetes.io/load-balancer-cleanup")
1940-
if _, err := cs.CoreV1().Services(svc.Namespace).Update(svc); err != nil {
1941-
e2elog.Failf("Failed to add finalizer to service %s/%s: %v", svc.Namespace, svc.Name, err)
1942-
}
1943-
})
1944-
19451918
// This test verifies if service load balancer cleanup finalizer is properly
19461919
// handled during service lifecycle.
19471920
// 1. Create service with type=LoadBalancer. Finalizer should be added.
19481921
// 2. Update service to type=ClusterIP. Finalizer should be removed.
19491922
// 3. Update service to type=LoadBalancer. Finalizer should be added.
19501923
// 4. Delete service with type=LoadBalancer. Finalizer should be removed.
1951-
ginkgo.It("should handle load balancer cleanup finalizer for service [Slow] [Feature:ServiceFinalizer]", func() {
1924+
ginkgo.It("should handle load balancer cleanup finalizer for service [Slow]", func() {
19521925
jig := e2eservice.NewTestJig(cs, "lb-finalizer")
19531926

19541927
ginkgo.By("Create load balancer service")
@@ -1957,71 +1930,26 @@ var _ = SIGDescribe("Services", func() {
19571930
})
19581931

19591932
defer func() {
1960-
waitForServiceDeletedWithFinalizer(cs, svc.Namespace, svc.Name)
1933+
ginkgo.By("Check that service can be deleted with finalizer")
1934+
e2eservice.WaitForServiceDeletedWithFinalizer(cs, svc.Namespace, svc.Name)
19611935
}()
19621936

19631937
ginkgo.By("Wait for load balancer to serve traffic")
19641938
svc = jig.WaitForLoadBalancerOrFail(svc.Namespace, svc.Name, e2eservice.GetServiceLoadBalancerCreationTimeout(cs))
19651939

19661940
ginkgo.By("Check if finalizer presents on service with type=LoadBalancer")
1967-
waitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, true)
1941+
e2eservice.WaitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, true)
19681942

19691943
ginkgo.By("Check if finalizer is removed on service after changed to type=ClusterIP")
19701944
jig.ChangeServiceType(svc.Namespace, svc.Name, v1.ServiceTypeClusterIP, e2eservice.GetServiceLoadBalancerCreationTimeout(cs))
1971-
waitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, false)
1945+
e2eservice.WaitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, false)
19721946

19731947
ginkgo.By("Check if finalizer is added back to service after changed to type=LoadBalancer")
19741948
jig.ChangeServiceType(svc.Namespace, svc.Name, v1.ServiceTypeLoadBalancer, e2eservice.GetServiceLoadBalancerCreationTimeout(cs))
1975-
waitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, true)
1949+
e2eservice.WaitForServiceUpdatedWithFinalizer(cs, svc.Namespace, svc.Name, true)
19761950
})
19771951
})
19781952

1979-
func waitForServiceDeletedWithFinalizer(cs clientset.Interface, namespace, name string) {
1980-
ginkgo.By("Delete service with finalizer")
1981-
if err := cs.CoreV1().Services(namespace).Delete(name, nil); err != nil {
1982-
e2elog.Failf("Failed to delete service %s/%s", namespace, name)
1983-
}
1984-
1985-
ginkgo.By("Wait for service to disappear")
1986-
if pollErr := wait.PollImmediate(e2eservice.LoadBalancerPollInterval, e2eservice.GetServiceLoadBalancerCreationTimeout(cs), func() (bool, error) {
1987-
svc, err := cs.CoreV1().Services(namespace).Get(name, metav1.GetOptions{})
1988-
if err != nil {
1989-
if apierrors.IsNotFound(err) {
1990-
e2elog.Logf("Service %s/%s is gone.", namespace, name)
1991-
return true, nil
1992-
}
1993-
return false, err
1994-
}
1995-
e2elog.Logf("Service %s/%s still exists with finalizers: %v", namespace, name, svc.Finalizers)
1996-
return false, nil
1997-
}); pollErr != nil {
1998-
e2elog.Failf("Failed to wait for service to disappear: %v", pollErr)
1999-
}
2000-
}
2001-
2002-
func waitForServiceUpdatedWithFinalizer(cs clientset.Interface, namespace, name string, hasFinalizer bool) {
2003-
ginkgo.By(fmt.Sprintf("Wait for service to hasFinalizer=%t", hasFinalizer))
2004-
if pollErr := wait.PollImmediate(e2eservice.LoadBalancerPollInterval, e2eservice.GetServiceLoadBalancerCreationTimeout(cs), func() (bool, error) {
2005-
svc, err := cs.CoreV1().Services(namespace).Get(name, metav1.GetOptions{})
2006-
if err != nil {
2007-
return false, err
2008-
}
2009-
foundFinalizer := false
2010-
for _, finalizer := range svc.Finalizers {
2011-
if finalizer == "service.kubernetes.io/load-balancer-cleanup" {
2012-
foundFinalizer = true
2013-
}
2014-
}
2015-
if foundFinalizer != hasFinalizer {
2016-
e2elog.Logf("Service %s/%s hasFinalizer=%t, want %t", namespace, name, foundFinalizer, hasFinalizer)
2017-
return false, nil
2018-
}
2019-
return true, nil
2020-
}); pollErr != nil {
2021-
e2elog.Failf("Failed to wait for service to hasFinalizer=%t: %v", hasFinalizer, pollErr)
2022-
}
2023-
}
2024-
20251953
// TODO: Get rid of [DisabledForLargeClusters] tag when issue #56138 is fixed.
20261954
var _ = SIGDescribe("ESIPP [Slow] [DisabledForLargeClusters]", func() {
20271955
f := framework.NewDefaultFramework("esipp")

‎test/e2e/upgrades/services.go‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ func (t *ServiceUpgradeTest) Setup(f *framework.Framework) {
8484
func (t *ServiceUpgradeTest) Test(f *framework.Framework, done <-chan struct{}, upgrade UpgradeType) {
8585
switch upgrade {
8686
case MasterUpgrade, ClusterUpgrade:
87-
t.test(f, done, true)
87+
t.test(f, done, true, true)
8888
case NodeUpgrade:
8989
// Node upgrades should test during disruption only on GCE/GKE for now.
90-
t.test(f, done, shouldTestPDBs())
90+
t.test(f, done, shouldTestPDBs(), false)
9191
default:
92-
t.test(f, done, false)
92+
t.test(f, done, false, false)
9393
}
9494
}
9595

@@ -98,7 +98,7 @@ func (t *ServiceUpgradeTest) Teardown(f *framework.Framework) {
9898
// rely on the namespace deletion to clean up everything
9999
}
100100

101-
func (t *ServiceUpgradeTest) test(f *framework.Framework, done <-chan struct{}, testDuringDisruption bool) {
101+
func (t *ServiceUpgradeTest) test(f *framework.Framework, done <-chan struct{}, testDuringDisruption, testFinalizer bool) {
102102
if testDuringDisruption {
103103
// Continuous validation
104104
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{},
115115
ginkgo.By("hitting the pod through the service's LoadBalancer")
116116
t.jig.TestReachableHTTP(t.tcpIngressIP, t.svcPort, e2eservice.LoadBalancerLagTimeoutDefault)
117117
t.jig.SanityCheckService(t.tcpService, v1.ServiceTypeLoadBalancer)
118+
119+
if testFinalizer {
120+
defer func() {
121+
ginkgo.By("Check that service can be deleted with finalizer")
122+
e2eservice.WaitForServiceDeletedWithFinalizer(t.jig.Client, t.tcpService.Namespace, t.tcpService.Name)
123+
}()
124+
ginkgo.By("Check that finalizer is present on loadBalancer type service")
125+
e2eservice.WaitForServiceUpdatedWithFinalizer(t.jig.Client, t.tcpService.Namespace, t.tcpService.Name, true)
126+
}
118127
}

0 commit comments

Comments
 (0)