From 3c22575e66743a8ef5210a19eedd639ef620a99f Mon Sep 17 00:00:00 2001 From: wccsama Date: Wed, 18 Mar 2020 13:50:36 +0800 Subject: [PATCH 1/2] clean up the awkward pattern in service_controller_test --- pkg/controller/service/controller_test.go | 31 +++++++++++------------ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/pkg/controller/service/controller_test.go b/pkg/controller/service/controller_test.go index ab0da4aaa30..c08ea363ec7 100644 --- a/pkg/controller/service/controller_test.go +++ b/pkg/controller/service/controller_test.go @@ -345,23 +345,22 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) { if !createCallFound { t.Errorf("Got no create call for load balancer, expected one") } - // TODO(@MrHohn): Clean up the awkward pattern here. - var balancer *fakecloud.Balancer - for k := range cloud.Balancers { - if balancer == nil { - b := cloud.Balancers[k] - balancer = &b - } else { - t.Errorf("Got load balancer %v, expected one to be created", cloud.Balancers) + + var isFound bool + for _, balancer := range cloud.Balancers { + if !reflect.DeepEqual(balancer, fakecloud.Balancer{}) { + isFound = true + if balancer.Name != controller.balancer.GetLoadBalancerName(context.Background(), "", tc.service) || + balancer.Region != region || + balancer.Ports[0].Port != tc.service.Spec.Ports[0].Port { + t.Errorf("Created load balancer has incorrect parameters: %v", balancer) + } break } } - if balancer == nil { - t.Errorf("Got no load balancer, expected one to be created") - } else if balancer.Name != controller.balancer.GetLoadBalancerName(context.Background(), "", tc.service) || - balancer.Region != region || - balancer.Ports[0].Port != tc.service.Spec.Ports[0].Port { - t.Errorf("Created load balancer has incorrect parameters: %v", balancer) + + if !isFound { + t.Errorf("Got no load balancer: %v, expected one to be created", cloud.Balancers) } } if tc.expectDeleteAttempt { @@ -472,8 +471,8 @@ func TestUpdateNodesInExternalLoadBalancer(t *testing.T) { var services []*v1.Service services = append(services, item.services...) - if err := controller.updateLoadBalancerHosts(services, nodes); err != nil { - t.Errorf("unexpected error: %v", err) + if servicesToRetry := controller.updateLoadBalancerHosts(services, nodes); servicesToRetry != nil { + t.Errorf("unexpected servicesToRetry: %v", servicesToRetry) } if !reflect.DeepEqual(item.expectedUpdateCalls, cloud.UpdateCalls) { t.Errorf("expected update calls mismatch, expected %+v, got %+v", item.expectedUpdateCalls, cloud.UpdateCalls) From 8daf9ec2d422f3c9a27a23c8de325c34297846cd Mon Sep 17 00:00:00 2001 From: wccsama Date: Thu, 2 Apr 2020 15:04:58 +0800 Subject: [PATCH 2/2] clean up the pattern --- pkg/controller/service/controller_test.go | 25 ++++++++++------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/pkg/controller/service/controller_test.go b/pkg/controller/service/controller_test.go index c08ea363ec7..61433a8ca82 100644 --- a/pkg/controller/service/controller_test.go +++ b/pkg/controller/service/controller_test.go @@ -41,6 +41,7 @@ import ( fakecloud "k8s.io/cloud-provider/fake" servicehelper "k8s.io/cloud-provider/service/helpers" featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/controller" ) @@ -346,23 +347,19 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) { t.Errorf("Got no create call for load balancer, expected one") } - var isFound bool - for _, balancer := range cloud.Balancers { - if !reflect.DeepEqual(balancer, fakecloud.Balancer{}) { - isFound = true - if balancer.Name != controller.balancer.GetLoadBalancerName(context.Background(), "", tc.service) || - balancer.Region != region || - balancer.Ports[0].Port != tc.service.Spec.Ports[0].Port { - t.Errorf("Created load balancer has incorrect parameters: %v", balancer) - } - break - } - } - - if !isFound { + if len(cloud.Balancers) == 0 { t.Errorf("Got no load balancer: %v, expected one to be created", cloud.Balancers) } + + for _, balancer := range cloud.Balancers { + if balancer.Name != controller.balancer.GetLoadBalancerName(context.Background(), "", tc.service) || + balancer.Region != region || + balancer.Ports[0].Port != tc.service.Spec.Ports[0].Port { + t.Errorf("Created load balancer has incorrect parameters: %v", balancer) + } + } } + if tc.expectDeleteAttempt { deleteCallFound := false for _, call := range cloud.Calls {