From 708a051399ba85aa96ce0557445ee2a63dcb87c9 Mon Sep 17 00:00:00 2001 From: Ashley Gau Date: Fri, 13 Apr 2018 14:34:31 -0700 Subject: [PATCH 1/5] refactor - create new apiService per test. encapsulate resource create/delete checks. --- .../gce/gce_loadbalancer_external_test.go | 184 ++++++---------- .../gce/gce_loadbalancer_internal_test.go | 172 +++++---------- .../gce/gce_loadbalancer_utils_test.go | 198 ++++++++++++++++-- 3 files changed, 304 insertions(+), 250 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go index ec4af22e15c..c6772780056 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go @@ -268,7 +268,7 @@ func TestDeleteAddressWithWrongTier(t *testing.T) { } } -func createExternalLoadBalancer(gce *GCECloud, nodeNames []string, clusterName, clusterID, zoneName string) (*v1.LoadBalancerStatus, error) { +func createExternalLoadBalancer(gce *GCECloud, apiService *v1.Service, nodeNames []string, clusterName, clusterID, zoneName string) (*v1.LoadBalancerStatus, error) { nodes, err := createAndInsertNodes(gce, nodeNames, zoneName) if err != nil { return nil, err @@ -277,7 +277,7 @@ func createExternalLoadBalancer(gce *GCECloud, nodeNames []string, clusterName, return gce.ensureExternalLoadBalancer( clusterName, clusterID, - fakeApiService, + apiService, nil, nodes, ) @@ -285,49 +285,17 @@ func createExternalLoadBalancer(gce *GCECloud, nodeNames []string, clusterName, func TestEnsureExternalLoadBalancer(t *testing.T) { vals := DefaultTestClusterValues() - nodeName := "test-node-1" + nodeNames := []string{"test-node-1"} gce, err := fakeGCECloud(vals) require.NoError(t, err) - status, err := createExternalLoadBalancer(gce, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + status, err := createExternalLoadBalancer(gce, apiService, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) - hcName := MakeNodesHealthCheckName(vals.ClusterID) - - // Check that Firewalls are created for the LoadBalancer and the HealthCheck - fwNames := []string{ - MakeFirewallName(lbName), - MakeHealthCheckFirewallName(vals.ClusterID, hcName, true), - } - - for _, fwName := range fwNames { - firewall, err := gce.GetFirewall(fwName) - require.NoError(t, err) - assert.Equal(t, []string{nodeName}, firewall.TargetTags) - assert.NotEmpty(t, firewall.SourceRanges) - } - - // Check that TargetPool is Created - pool, err := gce.GetTargetPool(lbName, gce.region) - require.NoError(t, err) - assert.Equal(t, lbName, pool.Name) - assert.NotEmpty(t, pool.HealthChecks) - assert.Equal(t, 1, len(pool.Instances)) - - // Check that HealthCheck is created - healthcheck, err := gce.GetHttpHealthCheck(hcName) - require.NoError(t, err) - assert.Equal(t, hcName, healthcheck.Name) - - // Check that ForwardingRule is created - fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region) - require.NoError(t, err) - assert.Equal(t, lbName, fwdRule.Name) - assert.Equal(t, "TCP", fwdRule.IPProtocol) - assert.Equal(t, "123-123", fwdRule.PortRange) + assertExternalLbResources(t, gce, apiService, vals, nodeNames) } func TestUpdateExternalLoadBalancer(t *testing.T) { @@ -337,7 +305,8 @@ func TestUpdateExternalLoadBalancer(t *testing.T) { gce, err := fakeGCECloud((DefaultTestClusterValues())) require.NoError(t, err) - _, err = createExternalLoadBalancer(gce, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + _, err = createExternalLoadBalancer(gce, apiService, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) newNodeName := "test-node-2" @@ -345,10 +314,10 @@ func TestUpdateExternalLoadBalancer(t *testing.T) { assert.NoError(t, err) // Add the new node, then check that it is properly added to the TargetPool - err = gce.updateExternalLoadBalancer("", fakeApiService, newNodes) + err = gce.updateExternalLoadBalancer("", apiService, newNodes) assert.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + lbName := cloudprovider.GetLoadBalancerName(apiService) pool, err := gce.GetTargetPool(lbName, gce.region) require.NoError(t, err) @@ -372,7 +341,7 @@ func TestUpdateExternalLoadBalancer(t *testing.T) { // Remove the new node by calling updateExternalLoadBalancer with a list // only containing the old node, and test that the TargetPool no longer // contains the new node. - err = gce.updateExternalLoadBalancer(vals.ClusterName, fakeApiService, newNodes) + err = gce.updateExternalLoadBalancer(vals.ClusterName, apiService, newNodes) assert.NoError(t, err) pool, err = gce.GetTargetPool(lbName, gce.region) @@ -390,41 +359,14 @@ func TestEnsureExternalLoadBalancerDeleted(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - _, err = createExternalLoadBalancer(gce, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + _, err = createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) - err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, fakeApiService) + err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) assert.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) - hcName := MakeNodesHealthCheckName(vals.ClusterID) - - // Check that Firewalls are deleted for the LoadBalancer and the HealthCheck - fwNames := []string{ - MakeFirewallName(lbName), - MakeHealthCheckFirewallName(vals.ClusterID, hcName, true), - } - - for _, fwName := range fwNames { - firewall, err := gce.GetFirewall(fwName) - require.Error(t, err) - assert.Nil(t, firewall) - } - - // Check that TargetPool is deleted - pool, err := gce.GetTargetPool(lbName, gce.region) - require.Error(t, err) - assert.Nil(t, pool) - - // Check that HealthCheck is deleted - healthcheck, err := gce.GetHttpHealthCheck(hcName) - require.Error(t, err) - assert.Nil(t, healthcheck) - - // Check forwarding rule is deleted - fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region) - require.Error(t, err) - assert.Nil(t, fwdRule) + assertExternalLbResourcesDeleted(t, gce, apiService, vals, true) } func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { @@ -434,15 +376,16 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { // Enable the cloud.NetworkTiers feature gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true - fakeApiService.Annotations = map[string]string{NetworkTierAnnotationKey: "Premium"} + apiService := fakeLbApiService() + apiService.Annotations = map[string]string{NetworkTierAnnotationKey: "Premium"} // cloud.NetworkTier defaults to Premium - desiredTier, err := gce.getServiceNetworkTier(fakeApiService) + desiredTier, err := gce.getServiceNetworkTier(apiService) require.NoError(t, err) assert.Equal(t, cloud.NetworkTierPremium, desiredTier) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) - serviceName := types.NamespacedName{Namespace: fakeApiService.Namespace, Name: fakeApiService.Name} + lbName := cloudprovider.GetLoadBalancerName(apiService) + serviceName := types.NamespacedName{Namespace: apiService.Namespace, Name: apiService.Name} // create ForwardingRule and Address with the wrong tier err = createForwardingRule( @@ -452,7 +395,7 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { gce.region, "", gce.targetPoolURL(lbName), - fakeApiService.Spec.Ports, + apiService.Spec.Ports, cloud.NetworkTierStandard, ) require.NoError(t, err) @@ -466,7 +409,7 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { err = gce.ReserveAlphaRegionAddress(addressObj, gce.region) require.NoError(t, err) - _, err = createExternalLoadBalancer(gce, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NoError(t, err) // Expect forwarding rule tier to not be Standard @@ -490,9 +433,10 @@ func TestEnsureExternalLoadBalancerFailsIfInvalidNetworkTier(t *testing.T) { // Enable the cloud.NetworkTiers feature gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true - fakeApiService.Annotations = map[string]string{NetworkTierAnnotationKey: wrongTier} + apiService := fakeLbApiService() + apiService.Annotations = map[string]string{NetworkTierAnnotationKey: wrongTier} - _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, fakeApiService, nil, nodes) + _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nil, nodes) require.Error(t, err) assert.EqualError(t, err, errStrUnsupportedTier) } @@ -502,7 +446,8 @@ func TestEnsureExternalLoadBalancerFailsWithNoNodes(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, fakeApiService, nil, []*v1.Node{}) + apiService := fakeLbApiService() + _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nil, []*v1.Node{}) require.Error(t, err) assert.EqualError(t, err, errStrLbNoHosts) } @@ -511,18 +456,19 @@ func TestForwardingRuleNeedsUpdate(t *testing.T) { vals := DefaultTestClusterValues() gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - status, err := createExternalLoadBalancer(gce, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + status, err := createExternalLoadBalancer(gce, fakeLbApiService(), []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NotNil(t, status) require.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + apiService := fakeLbApiService() + lbName := cloudprovider.GetLoadBalancerName(apiService) ipAddr := status.Ingress[0].IP - lbIP := fakeApiService.Spec.LoadBalancerIP - wrongPorts := []v1.ServicePort{fakeApiService.Spec.Ports[0]} + lbIP := apiService.Spec.LoadBalancerIP + wrongPorts := []v1.ServicePort{apiService.Spec.Ports[0]} wrongPorts[0].Port = wrongPorts[0].Port + 1 - wrongProtocolPorts := []v1.ServicePort{fakeApiService.Spec.Ports[0]} + wrongProtocolPorts := []v1.ServicePort{apiService.Spec.Ports[0]} wrongProtocolPorts[0].Protocol = v1.ProtocolUDP for desc, tc := range map[string]struct { @@ -535,7 +481,7 @@ func TestForwardingRuleNeedsUpdate(t *testing.T) { }{ "When the loadBalancerIP does not equal the FwdRule IP address.": { lbIP: "1.2.3.4", - ports: fakeApiService.Spec.Ports, + ports: apiService.Spec.Ports, exists: true, needsUpdate: true, expectIpAddr: ipAddr, @@ -567,7 +513,7 @@ func TestForwardingRuleNeedsUpdate(t *testing.T) { }, "When basic workflow.": { lbIP: lbIP, - ports: fakeApiService.Spec.Ports, + ports: apiService.Spec.Ports, exists: true, needsUpdate: false, expectIpAddr: ipAddr, @@ -593,8 +539,9 @@ func TestTargetPoolNeedsRecreation(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - serviceName := fakeApiService.ObjectMeta.Name - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + apiService := fakeLbApiService() + serviceName := apiService.ObjectMeta.Name + lbName := cloudprovider.GetLoadBalancerName(apiService) nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) hostNames := nodeNames(nodes) @@ -637,14 +584,15 @@ func TestFirewallNeedsUpdate(t *testing.T) { vals := DefaultTestClusterValues() gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - status, err := createExternalLoadBalancer(gce, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + status, err := createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NotNil(t, status) require.NoError(t, err) - svcName := "/" + fakeApiService.ObjectMeta.Name + svcName := "/" + apiService.ObjectMeta.Name region := vals.Region ipAddr := status.Ingress[0].IP - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + lbName := cloudprovider.GetLoadBalancerName(apiService) ipnet, err := netsets.ParseIPNets("0.0.0.0/0") require.NoError(t, err) @@ -670,7 +618,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When response is a Non-400 HTTP error.": { lbName: lbName, ipAddr: ipAddr, - ports: fakeApiService.Spec.Ports, + ports: apiService.Spec.Ports, ipnet: ipnet, fwIPProtocol: "tcp", getHook: mock.GetFirewallsUnauthorizedErrHook, @@ -682,7 +630,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When given a wrong description.": { lbName: lbName, ipAddr: "", - ports: fakeApiService.Spec.Ports, + ports: apiService.Spec.Ports, ipnet: ipnet, fwIPProtocol: "tcp", getHook: nil, @@ -694,7 +642,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When IPProtocol doesn't match.": { lbName: lbName, ipAddr: ipAddr, - ports: fakeApiService.Spec.Ports, + ports: apiService.Spec.Ports, ipnet: ipnet, fwIPProtocol: "usps", getHook: nil, @@ -718,7 +666,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When parseIPNets returns an error.": { lbName: lbName, ipAddr: ipAddr, - ports: fakeApiService.Spec.Ports, + ports: apiService.Spec.Ports, ipnet: ipnet, fwIPProtocol: "tcp", getHook: nil, @@ -730,7 +678,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When the source ranges are not equal.": { lbName: lbName, ipAddr: ipAddr, - ports: fakeApiService.Spec.Ports, + ports: apiService.Spec.Ports, ipnet: wrongIpnet, fwIPProtocol: "tcp", getHook: nil, @@ -742,7 +690,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When basic flow without exceptions.": { lbName: lbName, ipAddr: ipAddr, - ports: fakeApiService.Spec.Ports, + ports: apiService.Spec.Ports, ipnet: ipnet, fwIPProtocol: "tcp", getHook: nil, @@ -809,10 +757,11 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) + apiService := fakeLbApiService() status, err := gce.ensureExternalLoadBalancer( vals.ClusterName, vals.ClusterID, - fakeApiService, + apiService, nil, nodes, ) @@ -824,7 +773,7 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { clusterID := vals.ClusterID ipAddr := status.Ingress[0].IP - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + lbName := cloudprovider.GetLoadBalancerName(apiService) region := vals.Region hcToCreate := makeHttpHealthCheck(MakeNodesHealthCheckName(clusterID), GetNodesHealthCheckPath(), GetNodesHealthCheckPort()) @@ -836,7 +785,7 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { pool.CreationTimestamp = tag pool, err = gce.GetTargetPool(lbName, region) require.Equal(t, tag, pool.CreationTimestamp) - err = gce.ensureTargetPoolAndHealthCheck(true, true, fakeApiService, lbName, clusterID, ipAddr, hosts, hcToCreate, hcToDelete) + err = gce.ensureTargetPoolAndHealthCheck(true, true, apiService, lbName, clusterID, ipAddr, hosts, hcToCreate, hcToDelete) assert.NoError(t, err) pool, err = gce.GetTargetPool(lbName, region) assert.NotEqual(t, pool.CreationTimestamp, tag) @@ -851,13 +800,13 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { require.NoError(t, err) manyHostNames := nodeNames(manyNodes) manyHosts, err := gce.getInstancesByNames(manyHostNames) - err = gce.ensureTargetPoolAndHealthCheck(true, true, fakeApiService, lbName, clusterID, ipAddr, manyHosts, hcToCreate, hcToDelete) + err = gce.ensureTargetPoolAndHealthCheck(true, true, apiService, lbName, clusterID, ipAddr, manyHosts, hcToCreate, hcToDelete) assert.NoError(t, err) pool, err = gce.GetTargetPool(lbName, region) assert.Equal(t, maxTargetPoolCreateInstances+1, len(pool.Instances)) - err = gce.ensureTargetPoolAndHealthCheck(true, false, fakeApiService, lbName, clusterID, ipAddr, hosts, hcToCreate, hcToDelete) + err = gce.ensureTargetPoolAndHealthCheck(true, false, apiService, lbName, clusterID, ipAddr, hosts, hcToCreate, hcToDelete) assert.NoError(t, err) pool, err = gce.GetTargetPool(lbName, region) assert.Equal(t, 1, len(pool.Instances)) @@ -897,6 +846,7 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { recorder := record.NewFakeRecorder(1024) gce.eventRecorder = recorder + apiService := fakeLbApiService() nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) hostNames := nodeNames(nodes) @@ -905,12 +855,12 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { ipnet, err := netsets.ParseIPNets("10.0.0.0/20") require.NoError(t, err) gce.createFirewall( - fakeApiService, - cloudprovider.GetLoadBalancerName(fakeApiService), + apiService, + cloudprovider.GetLoadBalancerName(apiService), gce.region, "A sad little firewall", ipnet, - fakeApiService.Spec.Ports, + apiService.Spec.Ports, hosts) require.Nil(t, err) @@ -918,12 +868,12 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { checkEvent(t, recorder, msg, true) gce.updateFirewall( - fakeApiService, - cloudprovider.GetLoadBalancerName(fakeApiService), + apiService, + cloudprovider.GetLoadBalancerName(apiService), gce.region, "A sad little firewall", ipnet, - fakeApiService.Spec.Ports, + apiService.Spec.Ports, hosts) require.Nil(t, err) @@ -936,7 +886,7 @@ func TestEnsureExternalLoadBalancerDeletedSucceedsOnXPN(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - _, err = createExternalLoadBalancer(gce, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createExternalLoadBalancer(gce, fakeLbApiService(), []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NoError(t, err) c := gce.c.(*cloud.MockGCE) @@ -947,7 +897,8 @@ func TestEnsureExternalLoadBalancerDeletedSucceedsOnXPN(t *testing.T) { recorder := record.NewFakeRecorder(1024) gce.eventRecorder = recorder - err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, fakeApiService) + apiService := fakeLbApiService() + err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) require.NoError(t, err) msg := fmt.Sprintf("%s %s %s", v1.EventTypeNormal, eventReasonManualChange, eventMsgFirewallChange) @@ -963,12 +914,12 @@ type EnsureELBParams struct { } // newEnsureELBParams is the constructor of EnsureELBParams. -func newEnsureELBParams(nodes []*v1.Node) *EnsureELBParams { +func newEnsureELBParams(nodes []*v1.Node, apiService *v1.Service) *EnsureELBParams { vals := DefaultTestClusterValues() return &EnsureELBParams{ vals.ClusterName, vals.ClusterID, - fakeApiService.DeepCopy(), + apiService, nil, nodes, } @@ -1045,7 +996,8 @@ func TestEnsureExternalLoadBalancerErrors(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) - params = newEnsureELBParams(nodes) + apiService := fakeLbApiService() + params = newEnsureELBParams(nodes, apiService) if tc.adjustParams != nil { tc.adjustParams(params) } diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go index 90cefc90190..611d3f3e5db 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go @@ -32,7 +32,7 @@ import ( "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud" ) -func createInternalLoadBalancer(gce *GCECloud, existingFwdRule *compute.ForwardingRule, nodeNames []string, clusterName, clusterID, zoneName string) (*v1.LoadBalancerStatus, error) { +func createInternalLoadBalancer(gce *GCECloud, apiService *v1.Service, existingFwdRule *compute.ForwardingRule, nodeNames []string, clusterName, clusterID, zoneName string) (*v1.LoadBalancerStatus, error) { nodes, err := createAndInsertNodes(gce, nodeNames, zoneName) if err != nil { return nil, err @@ -41,7 +41,7 @@ func createInternalLoadBalancer(gce *GCECloud, existingFwdRule *compute.Forwardi return gce.ensureInternalLoadBalancer( clusterName, clusterID, - fakeApiService, + apiService, existingFwdRule, nodes, ) @@ -54,15 +54,16 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + apiService := fakeLbApiService() + lbName := cloudprovider.GetLoadBalancerName(apiService) nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) igName := makeInstanceGroupName(vals.ClusterID) igLinks, err := gce.ensureInternalInstanceGroups(igName, nodes) require.NoError(t, err) - sharedBackend := shareBackendService(fakeApiService) - bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", fakeApiService.Spec.SessionAffinity) - err = gce.ensureInternalBackendService(bsName, "description", fakeApiService.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, "") + sharedBackend := shareBackendService(apiService) + bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) + err = gce.ensureInternalBackendService(bsName, "description", apiService.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, "") require.NoError(t, err) // Update the Internal Backend Service with a new ServiceAffinity @@ -81,15 +82,16 @@ func TestEnsureInternalBackendServiceGroups(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + apiService := fakeLbApiService() + lbName := cloudprovider.GetLoadBalancerName(apiService) nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) igName := makeInstanceGroupName(vals.ClusterID) igLinks, err := gce.ensureInternalInstanceGroups(igName, nodes) require.NoError(t, err) - sharedBackend := shareBackendService(fakeApiService) - bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", fakeApiService.Spec.SessionAffinity) - err = gce.ensureInternalBackendService(bsName, "description", fakeApiService.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, "") + sharedBackend := shareBackendService(apiService) + bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) + err = gce.ensureInternalBackendService(bsName, "description", apiService.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, "") require.NoError(t, err) // Update the BackendService with new Instances @@ -109,65 +111,16 @@ func TestEnsureInternalBackendServiceGroups(t *testing.T) { func TestEnsureInternalLoadBalancer(t *testing.T) { vals := DefaultTestClusterValues() - nodeName := "test-node-1" + nodeNames := []string{"test-node-1"} gce, err := fakeGCECloud(vals) require.NoError(t, err) - status, err := createInternalLoadBalancer(gce, nil, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + status, err := createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) - - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) - - // Check that Instance Group is created - igName := makeInstanceGroupName(vals.ClusterID) - ig, err := gce.GetInstanceGroup(igName, vals.ZoneName) - assert.NoError(t, err) - assert.Equal(t, igName, ig.Name) - - // Check that Firewalls are created for the LoadBalancer and the HealthCheck - fwNames := []string{ - lbName, - makeHealthCheckFirewallName(lbName, vals.ClusterID, true), - } - - for _, fwName := range fwNames { - firewall, err := gce.GetFirewall(fwName) - require.NoError(t, err) - assert.Equal(t, []string{nodeName}, firewall.TargetTags) - assert.NotEmpty(t, firewall.SourceRanges) - } - - // Check that HealthCheck is created - sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(fakeApiService) - hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) - healthcheck, err := gce.GetHealthCheck(hcName) - require.NoError(t, err) - assert.Equal(t, hcName, healthcheck.Name) - - // Check that BackendService exists - sharedBackend := shareBackendService(fakeApiService) - backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", fakeApiService.Spec.SessionAffinity) - backendServiceLink := gce.getBackendServiceLink(backendServiceName) - - bs, err := gce.GetRegionBackendService(backendServiceName, gce.region) - require.NoError(t, err) - assert.Equal(t, "TCP", bs.Protocol) - assert.Equal( - t, - []string{healthcheck.SelfLink}, - bs.HealthChecks, - ) - - // Check that ForwardingRule is created - fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region) - require.NoError(t, err) - assert.Equal(t, lbName, fwdRule.Name) - assert.Equal(t, "TCP", fwdRule.IPProtocol) - assert.Equal(t, backendServiceLink, fwdRule.BackendService) - // if no Subnetwork specified, defaults to the GCE NetworkURL - assert.Equal(t, gce.NetworkURL(), fwdRule.Subnetwork) + assertInternalLbResources(t, gce, apiService, vals, nodeNames) } func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { @@ -177,11 +130,12 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) + apiService := fakeLbApiService() // Create the expected resources necessary for an Internal Load Balancer - nm := types.NamespacedName{Name: fakeApiService.Name, Namespace: fakeApiService.Namespace} - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + nm := types.NamespacedName{Name: apiService.Name, Namespace: apiService.Namespace} + lbName := cloudprovider.GetLoadBalancerName(apiService) - sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(fakeApiService) + sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(apiService) hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) hcPath, hcPort := GetNodesHealthCheckPath(), GetNodesHealthCheckPort() existingHC := newInternalLBHealthCheck(hcName, nm, sharedHealthCheck, hcPath, hcPort) @@ -193,13 +147,13 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { igLinks, err := gce.ensureInternalInstanceGroups(igName, nodes) require.NoError(t, err) - sharedBackend := shareBackendService(fakeApiService) + sharedBackend := shareBackendService(apiService) bsDescription := makeBackendServiceDescription(nm, sharedBackend) - bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", fakeApiService.Spec.SessionAffinity) - err = gce.ensureInternalBackendService(bsName, bsDescription, fakeApiService.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, existingHC.SelfLink) + bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) + err = gce.ensureInternalBackendService(bsName, bsDescription, apiService.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, existingHC.SelfLink) require.NoError(t, err) - _, err = createInternalLoadBalancer(gce, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) } @@ -208,7 +162,8 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) + apiService := fakeLbApiService() + lbName := cloudprovider.GetLoadBalancerName(apiService) // Create a ForwardingRule that's missing an IP address existingFwdRule := &compute.ForwardingRule{ @@ -233,10 +188,10 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { } gce.CreateFirewall(existingFirewall) - sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(fakeApiService) + sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(apiService) hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) hcPath, hcPort := GetNodesHealthCheckPath(), GetNodesHealthCheckPort() - nm := types.NamespacedName{Name: fakeApiService.Name, Namespace: fakeApiService.Namespace} + nm := types.NamespacedName{Name: apiService.Name, Namespace: apiService.Namespace} // Create a healthcheck with an incorrect threshold existingHC := newInternalLBHealthCheck(hcName, nm, sharedHealthCheck, hcPath, hcPort) @@ -244,20 +199,20 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { gce.CreateHealthCheck(existingHC) // Create a backend Service that's missing Description and Backends - sharedBackend := shareBackendService(fakeApiService) - backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", fakeApiService.Spec.SessionAffinity) + sharedBackend := shareBackendService(apiService) + backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) existingBS := &compute.BackendService{ Name: lbName, Protocol: "TCP", HealthChecks: []string{existingHC.SelfLink}, - SessionAffinity: translateAffinityType(fakeApiService.Spec.SessionAffinity), + SessionAffinity: translateAffinityType(apiService.Spec.SessionAffinity), LoadBalancingScheme: string(cloud.SchemeInternal), } gce.CreateRegionBackendService(existingBS, gce.region) existingFwdRule.BackendService = existingBS.Name - _, err = createInternalLoadBalancer(gce, existingFwdRule, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createInternalLoadBalancer(gce, apiService, existingFwdRule, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) // Expect new resources with the correct attributes to be created @@ -284,20 +239,21 @@ func TestUpdateInternalLoadBalancerBackendServices(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - _, err = createInternalLoadBalancer(gce, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) // BackendService exists prior to updateInternalLoadBalancer call, but has // incorrect (missing) attributes. // ensureInternalBackendServiceGroups is called and creates the correct // BackendService - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) - sharedBackend := shareBackendService(fakeApiService) - backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", fakeApiService.Spec.SessionAffinity) + lbName := cloudprovider.GetLoadBalancerName(apiService) + sharedBackend := shareBackendService(apiService) + backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) existingBS := &compute.BackendService{ Name: backendServiceName, Protocol: "TCP", - SessionAffinity: translateAffinityType(fakeApiService.Spec.SessionAffinity), + SessionAffinity: translateAffinityType(apiService.Spec.SessionAffinity), LoadBalancingScheme: string(cloud.SchemeInternal), } @@ -306,7 +262,7 @@ func TestUpdateInternalLoadBalancerBackendServices(t *testing.T) { nodes, err := createAndInsertNodes(gce, []string{nodeName}, vals.ZoneName) require.NoError(t, err) - err = gce.updateInternalLoadBalancer(vals.ClusterName, vals.ClusterID, fakeApiService, nodes) + err = gce.updateInternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nodes) assert.NoError(t, err) bs, err := gce.GetRegionBackendService(backendServiceName, gce.region) @@ -334,7 +290,8 @@ func TestUpdateInternalLoadBalancerNodes(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - _, err = createInternalLoadBalancer(gce, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) // Remove the old Node and insert a new Node. @@ -342,7 +299,7 @@ func TestUpdateInternalLoadBalancerNodes(t *testing.T) { newNodes, err := createAndInsertNodes(gce, []string{newNodeName}, vals.ZoneName) require.NoError(t, err) - err = gce.updateInternalLoadBalancer(vals.ClusterName, vals.ClusterID, fakeApiService, newNodes) + err = gce.updateInternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, newNodes) assert.NoError(t, err) // Expect node 1 to be deleted and node 2 to still exist @@ -363,43 +320,14 @@ func TestEnsureInternalLoadBalancerDeleted(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - _, err = createInternalLoadBalancer(gce, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) - err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, fakeApiService) + err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) assert.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(fakeApiService) - sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(fakeApiService) - hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) - - // Check that Firewalls are deleted for the LoadBalancer and the HealthCheck - fwNames := []string{ - MakeFirewallName(lbName), - MakeHealthCheckFirewallName(vals.ClusterID, hcName, true), - } - - for _, fwName := range fwNames { - firewall, err := gce.GetFirewall(fwName) - require.Error(t, err) - assert.Nil(t, firewall) - } - - // Check that Instance Group is deleted - igName := makeInstanceGroupName(vals.ClusterID) - ig, err := gce.GetInstanceGroup(igName, vals.ZoneName) - assert.Error(t, err) - assert.Nil(t, ig) - - // Check that HealthCheck is deleted - healthcheck, err := gce.GetHealthCheck(hcName) - require.Error(t, err) - assert.Nil(t, healthcheck) - - // Check forwarding rule is deleted - fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region) - require.Error(t, err) - assert.Nil(t, fwdRule) + assertInternalLbResourcesDeleted(t, gce, apiService, vals, true) } func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { @@ -407,13 +335,15 @@ func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - _, err = createInternalLoadBalancer(gce, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + apiService := fakeLbApiService() + _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) - err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, fakeApiService) + err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) assert.NoError(t, err) // Deleting the loadbalancer and resources again should not cause an error. - err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, fakeApiService) + err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) assert.NoError(t, err) + assertInternalLbResourcesDeleted(t, gce, apiService, vals, true) } diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go index fee620e5ad8..e8d31538afc 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go @@ -23,14 +23,18 @@ package gce import ( "fmt" "net/http" - "os" "sync" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" compute "google.golang.org/api/compute/v1" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" + v1_service "k8s.io/kubernetes/pkg/api/v1/service" + "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/meta" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/mock" @@ -65,7 +69,16 @@ func DefaultTestClusterValues() TestClusterValues { } } -var fakeApiService *v1.Service +func fakeLbApiService() *v1.Service { + return &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: ""}, + Spec: v1.ServiceSpec{ + SessionAffinity: v1.ServiceAffinityClientIP, + Type: v1.ServiceTypeLoadBalancer, + Ports: []v1.ServicePort{{Protocol: v1.ProtocolTCP, Port: int32(123)}}, + }, + } +} type fakeRoundTripper struct{} @@ -100,6 +113,7 @@ func fakeGCECloud(vals TestClusterValues) (*GCECloud, error) { AlphaFeatureGate: alphaFeatureGate, nodeZones: zonesWithNodes, nodeInformerSynced: func() bool { return true }, + ClusterID: fakeClusterID(vals.ClusterID), } c := cloud.NewMockGCE(&gceProjectRouter{gce}) @@ -180,18 +194,176 @@ func createAndInsertNodes(gce *GCECloud, nodeNames []string, zoneName string) ([ return nodes, nil } -func setup() { - fakeApiService = &v1.Service{ - ObjectMeta: metav1.ObjectMeta{Name: ""}, - Spec: v1.ServiceSpec{ - SessionAffinity: v1.ServiceAffinityClientIP, - Type: v1.ServiceTypeClusterIP, - Ports: []v1.ServicePort{{Protocol: v1.ProtocolTCP, Port: int32(123)}}, - }, +// Stubs ClusterID so that ClusterID.getOrInitialize() does not require calling +// gce.Initialize() +func fakeClusterID(clusterID string) ClusterID { + return ClusterID{ + clusterID: &clusterID, + store: cache.NewStore(func(obj interface{}) (string, error) { + return "", nil + }), } } -func TestMain(m *testing.M) { - setup() - os.Exit(m.Run()) +func assertExternalLbResources(t *testing.T, gce *GCECloud, apiService *v1.Service, vals TestClusterValues, nodeNames []string) { + lbName := cloudprovider.GetLoadBalancerName(apiService) + hcName := MakeNodesHealthCheckName(vals.ClusterID) + + // Check that Firewalls are created for the LoadBalancer and the HealthCheck + fwNames := []string{ + MakeFirewallName(lbName), + MakeHealthCheckFirewallName(vals.ClusterID, hcName, true), + } + + for _, fwName := range fwNames { + firewall, err := gce.GetFirewall(fwName) + require.NoError(t, err) + assert.Equal(t, nodeNames, firewall.TargetTags) + assert.NotEmpty(t, firewall.SourceRanges) + } + + // Check that TargetPool is Created + pool, err := gce.GetTargetPool(lbName, gce.region) + require.NoError(t, err) + assert.Equal(t, lbName, pool.Name) + assert.NotEmpty(t, pool.HealthChecks) + assert.Equal(t, 1, len(pool.Instances)) + + // Check that HealthCheck is created + healthcheck, err := gce.GetHttpHealthCheck(hcName) + require.NoError(t, err) + assert.Equal(t, hcName, healthcheck.Name) + + // Check that ForwardingRule is created + fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region) + require.NoError(t, err) + assert.Equal(t, lbName, fwdRule.Name) + assert.Equal(t, "TCP", fwdRule.IPProtocol) + assert.Equal(t, "123-123", fwdRule.PortRange) +} + +func assertExternalLbResourcesDeleted(t *testing.T, gce *GCECloud, apiService *v1.Service, vals TestClusterValues, firewallsDeleted bool) { + lbName := cloudprovider.GetLoadBalancerName(apiService) + hcName := MakeNodesHealthCheckName(vals.ClusterID) + + if firewallsDeleted { + // Check that Firewalls are deleted for the LoadBalancer and the HealthCheck + fwNames := []string{ + MakeFirewallName(lbName), + MakeHealthCheckFirewallName(vals.ClusterID, hcName, true), + } + + for _, fwName := range fwNames { + firewall, err := gce.GetFirewall(fwName) + require.Error(t, err) + assert.Nil(t, firewall) + } + + // Check forwarding rule is deleted + fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region) + require.Error(t, err) + assert.Nil(t, fwdRule) + } + + // Check that TargetPool is deleted + pool, err := gce.GetTargetPool(lbName, gce.region) + require.Error(t, err) + assert.Nil(t, pool) + + // Check that HealthCheck is deleted + healthcheck, err := gce.GetHttpHealthCheck(hcName) + require.Error(t, err) + assert.Nil(t, healthcheck) + +} + +func assertInternalLbResources(t *testing.T, gce *GCECloud, apiService *v1.Service, vals TestClusterValues, nodeNames []string) { + lbName := cloudprovider.GetLoadBalancerName(apiService) + + // Check that Instance Group is created + igName := makeInstanceGroupName(vals.ClusterID) + ig, err := gce.GetInstanceGroup(igName, vals.ZoneName) + assert.NoError(t, err) + assert.Equal(t, igName, ig.Name) + + // Check that Firewalls are created for the LoadBalancer and the HealthCheck + fwNames := []string{ + lbName, + makeHealthCheckFirewallName(lbName, vals.ClusterID, true), + } + + for _, fwName := range fwNames { + firewall, err := gce.GetFirewall(fwName) + require.NoError(t, err) + assert.Equal(t, nodeNames, firewall.TargetTags) + assert.NotEmpty(t, firewall.SourceRanges) + } + + // Check that HealthCheck is created + sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(apiService) + hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) + healthcheck, err := gce.GetHealthCheck(hcName) + require.NoError(t, err) + assert.Equal(t, hcName, healthcheck.Name) + + // Check that BackendService exists + sharedBackend := shareBackendService(apiService) + backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) + backendServiceLink := gce.getBackendServiceLink(backendServiceName) + + bs, err := gce.GetRegionBackendService(backendServiceName, gce.region) + require.NoError(t, err) + assert.Equal(t, "TCP", bs.Protocol) + assert.Equal( + t, + []string{healthcheck.SelfLink}, + bs.HealthChecks, + ) + + // Check that ForwardingRule is created + fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region) + require.NoError(t, err) + assert.Equal(t, lbName, fwdRule.Name) + assert.Equal(t, "TCP", fwdRule.IPProtocol) + assert.Equal(t, backendServiceLink, fwdRule.BackendService) + // if no Subnetwork specified, defaults to the GCE NetworkURL + assert.Equal(t, gce.NetworkURL(), fwdRule.Subnetwork) +} + +func assertInternalLbResourcesDeleted(t *testing.T, gce *GCECloud, apiService *v1.Service, vals TestClusterValues, firewallsDeleted bool) { + lbName := cloudprovider.GetLoadBalancerName(apiService) + sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(apiService) + hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) + + // ensureExternalLoadBalancer and ensureInternalLoadBalancer both create + // Firewalls with the same name. + if firewallsDeleted { + // Check that Firewalls are deleted for the LoadBalancer and the HealthCheck + fwNames := []string{ + MakeFirewallName(lbName), + MakeHealthCheckFirewallName(vals.ClusterID, hcName, true), + } + + for _, fwName := range fwNames { + firewall, err := gce.GetFirewall(fwName) + require.Error(t, err) + assert.Nil(t, firewall) + } + + // Check forwarding rule is deleted + fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region) + require.Error(t, err) + assert.Nil(t, fwdRule) + } + + // Check that Instance Group is deleted + igName := makeInstanceGroupName(vals.ClusterID) + ig, err := gce.GetInstanceGroup(igName, vals.ZoneName) + assert.Error(t, err) + assert.Nil(t, ig) + + // Check that HealthCheck is deleted + healthcheck, err := gce.GetHealthCheck(hcName) + require.Error(t, err) + assert.Nil(t, healthcheck) } From f105397b88f67b7224998ab570d012b40ab3b73a Mon Sep 17 00:00:00 2001 From: Ashley Gau Date: Fri, 13 Apr 2018 14:39:28 -0700 Subject: [PATCH 2/5] tests for EnsureLoadBalancer, EnsureLoadBalancerDeleted --- .../providers/gce/gce_loadbalancer_test.go | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go new file mode 100644 index 00000000000..b4ab061ee71 --- /dev/null +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go @@ -0,0 +1,166 @@ +/* +Copyright 2017 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 gce + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetLoadBalancer(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + apiService := fakeLbApiService() + + // When a loadbalancer has not been created + status, found, err := gce.GetLoadBalancer(context.Background(), vals.ClusterName, apiService) + assert.Nil(t, status) + assert.False(t, found) + assert.Nil(t, err) + + nodeNames := []string{"test-node-1"} + nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) + require.NoError(t, err) + expectedStatus, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) + require.NoError(t, err) + + status, found, err = gce.GetLoadBalancer(context.Background(), vals.ClusterName, apiService) + assert.Equal(t, expectedStatus, status) + assert.True(t, found) + assert.Nil(t, err) +} + +func TestEnsureLoadBalancerCreatesExternalLb(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + nodeNames := []string{"test-node-1"} + nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) + require.NoError(t, err) + + apiService := fakeLbApiService() + status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) + assert.NoError(t, err) + assert.NotEmpty(t, status.Ingress) + assertExternalLbResources(t, gce, apiService, vals, nodeNames) +} + +func TestEnsureLoadBalancerCreatesInternalLb(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + nodeNames := []string{"test-node-1"} + nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) + require.NoError(t, err) + + apiService := fakeLbApiService() + annotations := make(map[string]string) + annotations[ServiceAnnotationLoadBalancerType] = string(LBTypeInternal) + apiService.Annotations = annotations + + status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) + assert.NoError(t, err) + assert.NotEmpty(t, status.Ingress) + assertInternalLbResources(t, gce, apiService, vals, nodeNames) +} + +func TestEnsureLoadBalancerDeletesExistingInternalLb(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + nodeNames := []string{"test-node-1"} + nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) + require.NoError(t, err) + + apiService := fakeLbApiService() + createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) + + status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) + assert.NoError(t, err) + assert.NotEmpty(t, status.Ingress) + + assertExternalLbResources(t, gce, apiService, vals, nodeNames) + assertInternalLbResourcesDeleted(t, gce, apiService, vals, false) +} + +func TestEnsureLoadBalancerDeletesExistingExternalLb(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + nodeNames := []string{"test-node-1"} + nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) + require.NoError(t, err) + + apiService := fakeLbApiService() + createExternalLoadBalancer(gce, apiService, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) + + annotations := make(map[string]string) + annotations[ServiceAnnotationLoadBalancerType] = string(LBTypeInternal) + apiService.Annotations = annotations + status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) + assert.NoError(t, err) + assert.NotEmpty(t, status.Ingress) + + assertInternalLbResources(t, gce, apiService, vals, nodeNames) + assertExternalLbResourcesDeleted(t, gce, apiService, vals, false) +} + +func TestEnsureLoadBalancerDeletedDeletesExternalLb(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + nodeNames := []string{"test-node-1"} + _, err = createAndInsertNodes(gce, nodeNames, vals.ZoneName) + require.NoError(t, err) + + apiService := fakeLbApiService() + createExternalLoadBalancer(gce, apiService, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) + + err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, apiService) + assert.NoError(t, err) + assertExternalLbResourcesDeleted(t, gce, apiService, vals, true) +} + +func TestEnsureLoadBalancerDeletedDeletesInternalLb(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + nodeNames := []string{"test-node-1"} + _, err = createAndInsertNodes(gce, nodeNames, vals.ZoneName) + require.NoError(t, err) + + apiService := fakeLbApiService() + annotations := make(map[string]string) + annotations[ServiceAnnotationLoadBalancerType] = string(LBTypeInternal) + apiService.Annotations = annotations + createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) + + err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, apiService) + assert.NoError(t, err) + assertInternalLbResourcesDeleted(t, gce, apiService, vals, true) +} From 74092228b22c5da5d592d5f2bafd972ea121640a Mon Sep 17 00:00:00 2001 From: Ashley Gau Date: Fri, 13 Apr 2018 14:59:14 -0700 Subject: [PATCH 3/5] have fakeLoadbalancerService take lb type as argument --- .../gce/gce_loadbalancer_external_test.go | 30 +++++++++---------- .../gce/gce_loadbalancer_internal_test.go | 18 +++++------ .../providers/gce/gce_loadbalancer_test.go | 25 +++++----------- .../gce/gce_loadbalancer_utils_test.go | 7 +++-- 4 files changed, 37 insertions(+), 43 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go index c6772780056..fb5261e92ba 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go @@ -290,7 +290,7 @@ func TestEnsureExternalLoadBalancer(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") status, err := createExternalLoadBalancer(gce, apiService, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) @@ -305,7 +305,7 @@ func TestUpdateExternalLoadBalancer(t *testing.T) { gce, err := fakeGCECloud((DefaultTestClusterValues())) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") _, err = createExternalLoadBalancer(gce, apiService, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) @@ -359,7 +359,7 @@ func TestEnsureExternalLoadBalancerDeleted(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") _, err = createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) @@ -376,7 +376,7 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { // Enable the cloud.NetworkTiers feature gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") apiService.Annotations = map[string]string{NetworkTierAnnotationKey: "Premium"} // cloud.NetworkTier defaults to Premium @@ -433,7 +433,7 @@ func TestEnsureExternalLoadBalancerFailsIfInvalidNetworkTier(t *testing.T) { // Enable the cloud.NetworkTiers feature gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") apiService.Annotations = map[string]string{NetworkTierAnnotationKey: wrongTier} _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nil, nodes) @@ -446,7 +446,7 @@ func TestEnsureExternalLoadBalancerFailsWithNoNodes(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nil, []*v1.Node{}) require.Error(t, err) assert.EqualError(t, err, errStrLbNoHosts) @@ -456,11 +456,11 @@ func TestForwardingRuleNeedsUpdate(t *testing.T) { vals := DefaultTestClusterValues() gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - status, err := createExternalLoadBalancer(gce, fakeLbApiService(), []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + status, err := createExternalLoadBalancer(gce, fakeLoadbalancerService(""), []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NotNil(t, status) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") lbName := cloudprovider.GetLoadBalancerName(apiService) ipAddr := status.Ingress[0].IP @@ -539,7 +539,7 @@ func TestTargetPoolNeedsRecreation(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") serviceName := apiService.ObjectMeta.Name lbName := cloudprovider.GetLoadBalancerName(apiService) nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) @@ -584,7 +584,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { vals := DefaultTestClusterValues() gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") status, err := createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NotNil(t, status) require.NoError(t, err) @@ -757,7 +757,7 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") status, err := gce.ensureExternalLoadBalancer( vals.ClusterName, vals.ClusterID, @@ -846,7 +846,7 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { recorder := record.NewFakeRecorder(1024) gce.eventRecorder = recorder - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) hostNames := nodeNames(nodes) @@ -886,7 +886,7 @@ func TestEnsureExternalLoadBalancerDeletedSucceedsOnXPN(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - _, err = createExternalLoadBalancer(gce, fakeLbApiService(), []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createExternalLoadBalancer(gce, fakeLoadbalancerService(""), []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NoError(t, err) c := gce.c.(*cloud.MockGCE) @@ -897,7 +897,7 @@ func TestEnsureExternalLoadBalancerDeletedSucceedsOnXPN(t *testing.T) { recorder := record.NewFakeRecorder(1024) gce.eventRecorder = recorder - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) require.NoError(t, err) @@ -996,7 +996,7 @@ func TestEnsureExternalLoadBalancerErrors(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") params = newEnsureELBParams(nodes, apiService) if tc.adjustParams != nil { tc.adjustParams(params) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go index 611d3f3e5db..1e14dbab77c 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go @@ -54,7 +54,7 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) lbName := cloudprovider.GetLoadBalancerName(apiService) nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) igName := makeInstanceGroupName(vals.ClusterID) @@ -82,7 +82,7 @@ func TestEnsureInternalBackendServiceGroups(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) lbName := cloudprovider.GetLoadBalancerName(apiService) nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) igName := makeInstanceGroupName(vals.ClusterID) @@ -116,7 +116,7 @@ func TestEnsureInternalLoadBalancer(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) status, err := createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) @@ -130,7 +130,7 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) // Create the expected resources necessary for an Internal Load Balancer nm := types.NamespacedName{Name: apiService.Name, Namespace: apiService.Namespace} lbName := cloudprovider.GetLoadBalancerName(apiService) @@ -162,7 +162,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) lbName := cloudprovider.GetLoadBalancerName(apiService) // Create a ForwardingRule that's missing an IP address @@ -239,7 +239,7 @@ func TestUpdateInternalLoadBalancerBackendServices(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) @@ -290,7 +290,7 @@ func TestUpdateInternalLoadBalancerNodes(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) @@ -320,7 +320,7 @@ func TestEnsureInternalLoadBalancerDeleted(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) @@ -335,7 +335,7 @@ func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService(string(LBTypeInternal)) _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go index b4ab061ee71..c141033c986 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go @@ -29,7 +29,7 @@ func TestGetLoadBalancer(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") // When a loadbalancer has not been created status, found, err := gce.GetLoadBalancer(context.Background(), vals.ClusterName, apiService) @@ -58,7 +58,7 @@ func TestEnsureLoadBalancerCreatesExternalLb(t *testing.T) { nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) @@ -74,11 +74,7 @@ func TestEnsureLoadBalancerCreatesInternalLb(t *testing.T) { nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) require.NoError(t, err) - apiService := fakeLbApiService() - annotations := make(map[string]string) - annotations[ServiceAnnotationLoadBalancerType] = string(LBTypeInternal) - apiService.Annotations = annotations - + apiService := fakeLoadbalancerService(string(LBTypeInternal)) status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) @@ -94,7 +90,7 @@ func TestEnsureLoadBalancerDeletesExistingInternalLb(t *testing.T) { nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) @@ -114,12 +110,10 @@ func TestEnsureLoadBalancerDeletesExistingExternalLb(t *testing.T) { nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") createExternalLoadBalancer(gce, apiService, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) - annotations := make(map[string]string) - annotations[ServiceAnnotationLoadBalancerType] = string(LBTypeInternal) - apiService.Annotations = annotations + apiService = fakeLoadbalancerService(string(LBTypeInternal)) status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) @@ -137,7 +131,7 @@ func TestEnsureLoadBalancerDeletedDeletesExternalLb(t *testing.T) { _, err = createAndInsertNodes(gce, nodeNames, vals.ZoneName) require.NoError(t, err) - apiService := fakeLbApiService() + apiService := fakeLoadbalancerService("") createExternalLoadBalancer(gce, apiService, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, apiService) @@ -154,10 +148,7 @@ func TestEnsureLoadBalancerDeletedDeletesInternalLb(t *testing.T) { _, err = createAndInsertNodes(gce, nodeNames, vals.ZoneName) require.NoError(t, err) - apiService := fakeLbApiService() - annotations := make(map[string]string) - annotations[ServiceAnnotationLoadBalancerType] = string(LBTypeInternal) - apiService.Annotations = annotations + apiService := fakeLoadbalancerService(string(LBTypeInternal)) createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, apiService) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go index e8d31538afc..d34ee3ad52e 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go @@ -69,9 +69,12 @@ func DefaultTestClusterValues() TestClusterValues { } } -func fakeLbApiService() *v1.Service { +func fakeLoadbalancerService(lbType string) *v1.Service { return &v1.Service{ - ObjectMeta: metav1.ObjectMeta{Name: ""}, + ObjectMeta: metav1.ObjectMeta{ + Name: "", + Annotations: map[string]string{ServiceAnnotationLoadBalancerType: lbType}, + }, Spec: v1.ServiceSpec{ SessionAffinity: v1.ServiceAffinityClientIP, Type: v1.ServiceTypeLoadBalancer, From 023a477c6348fb4ef7a7967823578717c9d12e44 Mon Sep 17 00:00:00 2001 From: Ashley Gau Date: Fri, 13 Apr 2018 15:31:27 -0700 Subject: [PATCH 4/5] Add comments, t.Parallel() --- .../providers/gce/gce_loadbalancer_test.go | 14 ++++++++++++++ .../providers/gce/gce_loadbalancer_utils_test.go | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go index c141033c986..7d7224892c1 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_test.go @@ -25,6 +25,8 @@ import ( ) func TestGetLoadBalancer(t *testing.T) { + t.Parallel() + vals := DefaultTestClusterValues() gce, err := fakeGCECloud(vals) require.NoError(t, err) @@ -50,6 +52,8 @@ func TestGetLoadBalancer(t *testing.T) { } func TestEnsureLoadBalancerCreatesExternalLb(t *testing.T) { + t.Parallel() + vals := DefaultTestClusterValues() gce, err := fakeGCECloud(vals) require.NoError(t, err) @@ -66,6 +70,8 @@ func TestEnsureLoadBalancerCreatesExternalLb(t *testing.T) { } func TestEnsureLoadBalancerCreatesInternalLb(t *testing.T) { + t.Parallel() + vals := DefaultTestClusterValues() gce, err := fakeGCECloud(vals) require.NoError(t, err) @@ -82,6 +88,8 @@ func TestEnsureLoadBalancerCreatesInternalLb(t *testing.T) { } func TestEnsureLoadBalancerDeletesExistingInternalLb(t *testing.T) { + t.Parallel() + vals := DefaultTestClusterValues() gce, err := fakeGCECloud(vals) require.NoError(t, err) @@ -102,6 +110,8 @@ func TestEnsureLoadBalancerDeletesExistingInternalLb(t *testing.T) { } func TestEnsureLoadBalancerDeletesExistingExternalLb(t *testing.T) { + t.Parallel() + vals := DefaultTestClusterValues() gce, err := fakeGCECloud(vals) require.NoError(t, err) @@ -123,6 +133,8 @@ func TestEnsureLoadBalancerDeletesExistingExternalLb(t *testing.T) { } func TestEnsureLoadBalancerDeletedDeletesExternalLb(t *testing.T) { + t.Parallel() + vals := DefaultTestClusterValues() gce, err := fakeGCECloud(vals) require.NoError(t, err) @@ -140,6 +152,8 @@ func TestEnsureLoadBalancerDeletedDeletesExternalLb(t *testing.T) { } func TestEnsureLoadBalancerDeletedDeletesInternalLb(t *testing.T) { + t.Parallel() + vals := DefaultTestClusterValues() gce, err := fakeGCECloud(vals) require.NoError(t, err) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go index d34ee3ad52e..366d347e7b6 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go @@ -214,7 +214,7 @@ func assertExternalLbResources(t *testing.T, gce *GCECloud, apiService *v1.Servi // Check that Firewalls are created for the LoadBalancer and the HealthCheck fwNames := []string{ - MakeFirewallName(lbName), + MakeFirewallName(lbName), // Firewalls for external LBs are prefixed with k8s-fw- MakeHealthCheckFirewallName(vals.ClusterID, hcName, true), } @@ -291,7 +291,7 @@ func assertInternalLbResources(t *testing.T, gce *GCECloud, apiService *v1.Servi // Check that Firewalls are created for the LoadBalancer and the HealthCheck fwNames := []string{ - lbName, + lbName, // Firewalls for internal LBs are named the same name as the loadbalancer. makeHealthCheckFirewallName(lbName, vals.ClusterID, true), } From 6ff2afefbb22d893c3ec2346638083df4856bed9 Mon Sep 17 00:00:00 2001 From: Ashley Gau Date: Fri, 13 Apr 2018 15:34:08 -0700 Subject: [PATCH 5/5] update-bazel --- pkg/cloudprovider/providers/gce/BUILD | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cloudprovider/providers/gce/BUILD b/pkg/cloudprovider/providers/gce/BUILD index fea76b3aeca..86b2c9e6c67 100644 --- a/pkg/cloudprovider/providers/gce/BUILD +++ b/pkg/cloudprovider/providers/gce/BUILD @@ -102,6 +102,7 @@ go_test( "gce_healthchecks_test.go", "gce_loadbalancer_external_test.go", "gce_loadbalancer_internal_test.go", + "gce_loadbalancer_test.go", "gce_loadbalancer_utils_test.go", "gce_test.go", "gce_util_test.go", @@ -127,6 +128,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//vendor/k8s.io/client-go/tools/cache:go_default_library", "//vendor/k8s.io/client-go/tools/record:go_default_library", ], )