From 8aeb77f9c84a0369a03cb1454c3317d97a56e4b4 Mon Sep 17 00:00:00 2001 From: yankaiz Date: Fri, 30 Mar 2018 11:45:29 -0700 Subject: [PATCH] Add unit tests for gce loadbalancer internal. Increase coverage for gce_loadbalancer_internal from 76.7% to 91.0%. --- .../providers/gce/cloud/mock/mock.go | 78 ++- .../gce/gce_loadbalancer_external_test.go | 157 +++--- .../gce/gce_loadbalancer_internal_test.go | 472 ++++++++++++++++-- .../gce/gce_loadbalancer_utils_test.go | 27 + 4 files changed, 589 insertions(+), 145 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/cloud/mock/mock.go b/pkg/cloudprovider/providers/gce/cloud/mock/mock.go index 082e4891947..f0e87cf0ce7 100644 --- a/pkg/cloudprovider/providers/gce/cloud/mock/mock.go +++ b/pkg/cloudprovider/providers/gce/cloud/mock/mock.go @@ -39,6 +39,13 @@ import ( "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/meta" ) +var ( + // InUseError is a shared variable with error code StatusBadRequest for error verification. + InUseError = &googleapi.Error{Code: http.StatusBadRequest, Message: "It's being used by god."} + // InternalServerError is shared variable with error code StatusInternalServerError for error verification. + InternalServerError = &googleapi.Error{Code: http.StatusInternalServerError} +) + // gceObject is an abstraction of all GCE API object in go client type gceObject interface { MarshalJSON() ([]byte, error) @@ -451,32 +458,32 @@ func GetFirewallsUnauthorizedErrHook(ctx context.Context, key *meta.Key, m *clou // GetTargetPoolInternalErrHook mocks getting target pool. It returns a internal server error. func GetTargetPoolInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockTargetPools) (bool, *ga.TargetPool, error) { - return true, nil, &googleapi.Error{Code: http.StatusInternalServerError} + return true, nil, InternalServerError } // GetForwardingRulesInternalErrHook mocks getting forwarding rules and returns an internal server error. func GetForwardingRulesInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockForwardingRules) (bool, *ga.ForwardingRule, error) { - return true, nil, &googleapi.Error{Code: http.StatusInternalServerError} + return true, nil, InternalServerError } // GetAddressesInternalErrHook mocks getting network address and returns an internal server error. func GetAddressesInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockAddresses) (bool, *ga.Address, error) { - return true, nil, &googleapi.Error{Code: http.StatusInternalServerError} + return true, nil, InternalServerError } // GetHTTPHealthChecksInternalErrHook mocks getting http health check and returns an internal server error. func GetHTTPHealthChecksInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockHttpHealthChecks) (bool, *ga.HttpHealthCheck, error) { - return true, nil, &googleapi.Error{Code: http.StatusInternalServerError} + return true, nil, InternalServerError } // InsertTargetPoolsInternalErrHook mocks getting target pool and returns an internal server error. func InsertTargetPoolsInternalErrHook(ctx context.Context, key *meta.Key, obj *ga.TargetPool, m *cloud.MockTargetPools) (bool, error) { - return true, &googleapi.Error{Code: http.StatusInternalServerError} + return true, InternalServerError } // InsertForwardingRulesInternalErrHook mocks getting forwarding rule and returns an internal server error. func InsertForwardingRulesInternalErrHook(ctx context.Context, key *meta.Key, obj *ga.ForwardingRule, m *cloud.MockForwardingRules) (bool, error) { - return true, &googleapi.Error{Code: http.StatusInternalServerError} + return true, InternalServerError } // DeleteAddressesNotFoundErrHook mocks deleting network address and returns a not found error. @@ -484,7 +491,62 @@ func DeleteAddressesNotFoundErrHook(ctx context.Context, key *meta.Key, m *cloud return true, &googleapi.Error{Code: http.StatusNotFound} } -// DeleteAddressesInternalErrHook mocks delete address and returns an internal server error. +// DeleteAddressesInternalErrHook mocks deleting address and returns an internal server error. func DeleteAddressesInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockAddresses) (bool, error) { - return true, &googleapi.Error{Code: http.StatusInternalServerError} + return true, InternalServerError +} + +// GetRegionBackendServicesErrHook mocks getting region backend service and returns an internal server error. +func GetRegionBackendServicesErrHook(ctx context.Context, key *meta.Key, m *cloud.MockRegionBackendServices) (bool, *ga.BackendService, error) { + return true, nil, InternalServerError +} + +// UpdateRegionBackendServicesErrHook mocks updating a reegion backend service and returns an internal server error. +func UpdateRegionBackendServicesErrHook(ctx context.Context, key *meta.Key, svc *ga.BackendService, m *cloud.MockRegionBackendServices) error { + return InternalServerError +} + +// DeleteRegionBackendServicesErrHook mocks deleting region backend service and returns an internal server error. +func DeleteRegionBackendServicesErrHook(ctx context.Context, key *meta.Key, m *cloud.MockRegionBackendServices) (bool, error) { + return true, InternalServerError +} + +// DeleteRegionBackendServicesInUseErrHook mocks deleting region backend service and returns an InUseError. +func DeleteRegionBackendServicesInUseErrHook(ctx context.Context, key *meta.Key, m *cloud.MockRegionBackendServices) (bool, error) { + return true, InUseError +} + +// GetInstanceGroupInternalErrHook mocks getting instance group and returns an internal server error. +func GetInstanceGroupInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockInstanceGroups) (bool, *ga.InstanceGroup, error) { + return true, nil, InternalServerError +} + +// GetHealthChecksInternalErrHook mocks getting health check and returns an internal server erorr. +func GetHealthChecksInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockHealthChecks) (bool, *ga.HealthCheck, error) { + return true, nil, InternalServerError +} + +// DeleteHealthChecksInternalErrHook mocks deleting health check and returns an internal server error. +func DeleteHealthChecksInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockHealthChecks) (bool, error) { + return true, InternalServerError +} + +// DeleteHealthChecksInuseErrHook mocks deleting health check and returns an in use error. +func DeleteHealthChecksInuseErrHook(ctx context.Context, key *meta.Key, m *cloud.MockHealthChecks) (bool, error) { + return true, InUseError +} + +// DeleteForwardingRuleErrHook mocks deleting forwarding rule and returns an internal server error. +func DeleteForwardingRuleErrHook(ctx context.Context, key *meta.Key, m *cloud.MockForwardingRules) (bool, error) { + return true, InternalServerError +} + +// ListZonesInternalErrHook mocks listing zone and returns an internal server error. +func ListZonesInternalErrHook(ctx context.Context, fl *filter.F, m *cloud.MockZones) (bool, []*ga.Zone, error) { + return true, []*ga.Zone{}, InternalServerError +} + +// DeleteInstanceGroupInternalErrHook mocks deleting instance group and returns an internal server error. +func DeleteInstanceGroupInternalErrHook(ctx context.Context, key *meta.Key, m *cloud.MockInstanceGroups) (bool, error) { + return true, InternalServerError } diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go index a131011bbc1..ac53fa01582 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go @@ -21,7 +21,6 @@ import ( "fmt" "strings" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -278,7 +277,7 @@ func TestDeleteAddressWithWrongTier(t *testing.T) { } } -func createExternalLoadBalancer(gce *GCECloud, apiService *v1.Service, nodeNames []string, clusterName, clusterID, zoneName string) (*v1.LoadBalancerStatus, error) { +func createExternalLoadBalancer(gce *GCECloud, svc *v1.Service, nodeNames []string, clusterName, clusterID, zoneName string) (*v1.LoadBalancerStatus, error) { nodes, err := createAndInsertNodes(gce, nodeNames, zoneName) if err != nil { return nil, err @@ -287,7 +286,7 @@ func createExternalLoadBalancer(gce *GCECloud, apiService *v1.Service, nodeNames return gce.ensureExternalLoadBalancer( clusterName, clusterID, - apiService, + svc, nil, nodes, ) @@ -302,12 +301,12 @@ func TestEnsureExternalLoadBalancer(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLoadbalancerService("") - status, err := createExternalLoadBalancer(gce, apiService, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) + svc := fakeLoadbalancerService("") + status, err := createExternalLoadBalancer(gce, svc, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) - assertExternalLbResources(t, gce, apiService, vals, nodeNames) + assertExternalLbResources(t, gce, svc, vals, nodeNames) } func TestUpdateExternalLoadBalancer(t *testing.T) { @@ -319,8 +318,8 @@ func TestUpdateExternalLoadBalancer(t *testing.T) { gce, err := fakeGCECloud((DefaultTestClusterValues())) require.NoError(t, err) - apiService := fakeLoadbalancerService("") - _, err = createExternalLoadBalancer(gce, apiService, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + svc := fakeLoadbalancerService("") + _, err = createExternalLoadBalancer(gce, svc, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) newNodeName := "test-node-2" @@ -328,10 +327,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("", apiService, newNodes) + err = gce.updateExternalLoadBalancer("", svc, newNodes) assert.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(apiService) + lbName := cloudprovider.GetLoadBalancerName(svc) pool, err := gce.GetTargetPool(lbName, gce.region) require.NoError(t, err) @@ -355,7 +354,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, apiService, newNodes) + err = gce.updateExternalLoadBalancer(vals.ClusterName, svc, newNodes) assert.NoError(t, err) pool, err = gce.GetTargetPool(lbName, gce.region) @@ -375,14 +374,14 @@ func TestEnsureExternalLoadBalancerDeleted(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLoadbalancerService("") - _, err = createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + svc := fakeLoadbalancerService("") + _, err = createExternalLoadBalancer(gce, svc, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) - err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) + err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, svc) assert.NoError(t, err) - assertExternalLbResourcesDeleted(t, gce, apiService, vals, true) + assertExternalLbResourcesDeleted(t, gce, svc, vals, true) } func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { @@ -394,16 +393,16 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { // Enable the cloud.NetworkTiers feature gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true - apiService := fakeLoadbalancerService("") - apiService.Annotations = map[string]string{NetworkTierAnnotationKey: "Premium"} + svc := fakeLoadbalancerService("") + svc.Annotations = map[string]string{NetworkTierAnnotationKey: "Premium"} // cloud.NetworkTier defaults to Premium - desiredTier, err := gce.getServiceNetworkTier(apiService) + desiredTier, err := gce.getServiceNetworkTier(svc) require.NoError(t, err) assert.Equal(t, cloud.NetworkTierPremium, desiredTier) - lbName := cloudprovider.GetLoadBalancerName(apiService) - serviceName := types.NamespacedName{Namespace: apiService.Namespace, Name: apiService.Name} + lbName := cloudprovider.GetLoadBalancerName(svc) + serviceName := types.NamespacedName{Namespace: svc.Namespace, Name: svc.Name} // create ForwardingRule and Address with the wrong tier err = createForwardingRule( @@ -413,7 +412,7 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { gce.region, "", gce.targetPoolURL(lbName), - apiService.Spec.Ports, + svc.Spec.Ports, cloud.NetworkTierStandard, ) require.NoError(t, err) @@ -427,7 +426,7 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { err = gce.ReserveAlphaRegionAddress(addressObj, gce.region) require.NoError(t, err) - _, err = createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createExternalLoadBalancer(gce, svc, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NoError(t, err) // Expect forwarding rule tier to not be Standard @@ -453,10 +452,10 @@ func TestEnsureExternalLoadBalancerFailsIfInvalidNetworkTier(t *testing.T) { // Enable the cloud.NetworkTiers feature gce.AlphaFeatureGate.features[AlphaFeatureNetworkTiers] = true - apiService := fakeLoadbalancerService("") - apiService.Annotations = map[string]string{NetworkTierAnnotationKey: wrongTier} + svc := fakeLoadbalancerService("") + svc.Annotations = map[string]string{NetworkTierAnnotationKey: wrongTier} - _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nil, nodes) + _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, svc, nil, nodes) require.Error(t, err) assert.EqualError(t, err, errStrUnsupportedTier) } @@ -468,8 +467,8 @@ func TestEnsureExternalLoadBalancerFailsWithNoNodes(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - apiService := fakeLoadbalancerService("") - _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nil, []*v1.Node{}) + svc := fakeLoadbalancerService("") + _, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, svc, nil, []*v1.Node{}) require.Error(t, err) assert.EqualError(t, err, errStrLbNoHosts) } @@ -484,15 +483,15 @@ func TestForwardingRuleNeedsUpdate(t *testing.T) { require.NotNil(t, status) require.NoError(t, err) - apiService := fakeLoadbalancerService("") - lbName := cloudprovider.GetLoadBalancerName(apiService) + svc := fakeLoadbalancerService("") + lbName := cloudprovider.GetLoadBalancerName(svc) ipAddr := status.Ingress[0].IP - lbIP := apiService.Spec.LoadBalancerIP - wrongPorts := []v1.ServicePort{apiService.Spec.Ports[0]} + lbIP := svc.Spec.LoadBalancerIP + wrongPorts := []v1.ServicePort{svc.Spec.Ports[0]} wrongPorts[0].Port = wrongPorts[0].Port + 1 - wrongProtocolPorts := []v1.ServicePort{apiService.Spec.Ports[0]} + wrongProtocolPorts := []v1.ServicePort{svc.Spec.Ports[0]} wrongProtocolPorts[0].Protocol = v1.ProtocolUDP for desc, tc := range map[string]struct { @@ -505,7 +504,7 @@ func TestForwardingRuleNeedsUpdate(t *testing.T) { }{ "When the loadBalancerIP does not equal the FwdRule IP address.": { lbIP: "1.2.3.4", - ports: apiService.Spec.Ports, + ports: svc.Spec.Ports, exists: true, needsUpdate: true, expectIpAddr: ipAddr, @@ -537,7 +536,7 @@ func TestForwardingRuleNeedsUpdate(t *testing.T) { }, "When basic workflow.": { lbIP: lbIP, - ports: apiService.Spec.Ports, + ports: svc.Spec.Ports, exists: true, needsUpdate: false, expectIpAddr: ipAddr, @@ -565,9 +564,9 @@ func TestTargetPoolNeedsRecreation(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - apiService := fakeLoadbalancerService("") - serviceName := apiService.ObjectMeta.Name - lbName := cloudprovider.GetLoadBalancerName(apiService) + svc := fakeLoadbalancerService("") + serviceName := svc.ObjectMeta.Name + lbName := cloudprovider.GetLoadBalancerName(svc) nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) hostNames := nodeNames(nodes) @@ -612,15 +611,15 @@ func TestFirewallNeedsUpdate(t *testing.T) { vals := DefaultTestClusterValues() gce, err := fakeGCECloud(DefaultTestClusterValues()) require.NoError(t, err) - apiService := fakeLoadbalancerService("") - status, err := createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + svc := fakeLoadbalancerService("") + status, err := createExternalLoadBalancer(gce, svc, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) require.NotNil(t, status) require.NoError(t, err) - svcName := "/" + apiService.ObjectMeta.Name + svcName := "/" + svc.ObjectMeta.Name region := vals.Region ipAddr := status.Ingress[0].IP - lbName := cloudprovider.GetLoadBalancerName(apiService) + lbName := cloudprovider.GetLoadBalancerName(svc) ipnet, err := netsets.ParseIPNets("0.0.0.0/0") require.NoError(t, err) @@ -646,7 +645,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When response is a Non-400 HTTP error.": { lbName: lbName, ipAddr: ipAddr, - ports: apiService.Spec.Ports, + ports: svc.Spec.Ports, ipnet: ipnet, fwIPProtocol: "tcp", getHook: mock.GetFirewallsUnauthorizedErrHook, @@ -658,7 +657,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When given a wrong description.": { lbName: lbName, ipAddr: "", - ports: apiService.Spec.Ports, + ports: svc.Spec.Ports, ipnet: ipnet, fwIPProtocol: "tcp", getHook: nil, @@ -670,7 +669,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When IPProtocol doesn't match.": { lbName: lbName, ipAddr: ipAddr, - ports: apiService.Spec.Ports, + ports: svc.Spec.Ports, ipnet: ipnet, fwIPProtocol: "usps", getHook: nil, @@ -694,7 +693,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When parseIPNets returns an error.": { lbName: lbName, ipAddr: ipAddr, - ports: apiService.Spec.Ports, + ports: svc.Spec.Ports, ipnet: ipnet, fwIPProtocol: "tcp", getHook: nil, @@ -706,7 +705,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When the source ranges are not equal.": { lbName: lbName, ipAddr: ipAddr, - ports: apiService.Spec.Ports, + ports: svc.Spec.Ports, ipnet: wrongIpnet, fwIPProtocol: "tcp", getHook: nil, @@ -718,7 +717,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { "When basic flow without exceptions.": { lbName: lbName, ipAddr: ipAddr, - ports: apiService.Spec.Ports, + ports: svc.Spec.Ports, ipnet: ipnet, fwIPProtocol: "tcp", getHook: nil, @@ -789,11 +788,11 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) - apiService := fakeLoadbalancerService("") + svc := fakeLoadbalancerService("") status, err := gce.ensureExternalLoadBalancer( vals.ClusterName, vals.ClusterID, - apiService, + svc, nil, nodes, ) @@ -805,7 +804,7 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { clusterID := vals.ClusterID ipAddr := status.Ingress[0].IP - lbName := cloudprovider.GetLoadBalancerName(apiService) + lbName := cloudprovider.GetLoadBalancerName(svc) region := vals.Region hcToCreate := makeHttpHealthCheck(MakeNodesHealthCheckName(clusterID), GetNodesHealthCheckPath(), GetNodesHealthCheckPort()) @@ -817,7 +816,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, apiService, lbName, clusterID, ipAddr, hosts, hcToCreate, hcToDelete) + err = gce.ensureTargetPoolAndHealthCheck(true, true, svc, lbName, clusterID, ipAddr, hosts, hcToCreate, hcToDelete) assert.NoError(t, err) pool, err = gce.GetTargetPool(lbName, region) assert.NotEqual(t, pool.CreationTimestamp, tag) @@ -832,38 +831,18 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { require.NoError(t, err) manyHostNames := nodeNames(manyNodes) manyHosts, err := gce.getInstancesByNames(manyHostNames) - err = gce.ensureTargetPoolAndHealthCheck(true, true, apiService, lbName, clusterID, ipAddr, manyHosts, hcToCreate, hcToDelete) + err = gce.ensureTargetPoolAndHealthCheck(true, true, svc, 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, apiService, lbName, clusterID, ipAddr, hosts, hcToCreate, hcToDelete) + err = gce.ensureTargetPoolAndHealthCheck(true, false, svc, lbName, clusterID, ipAddr, hosts, hcToCreate, hcToDelete) assert.NoError(t, err) pool, err = gce.GetTargetPool(lbName, region) assert.Equal(t, 1, len(pool.Instances)) } -func checkEvent(t *testing.T, recorder *record.FakeRecorder, expected string, shouldMatch bool) bool { - select { - case received := <-recorder.Events: - if strings.HasPrefix(received, expected) != shouldMatch { - t.Errorf(received) - if shouldMatch { - t.Errorf("Should receive message \"%v\" but got \"%v\".", expected, received) - } else { - t.Errorf("Unexpected event \"%v\".", received) - } - } - return false - case <-time.After(2 * time.Second): - if shouldMatch { - t.Errorf("Should receive message \"%v\" but got timed out.", expected) - } - return true - } -} - func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { t.Parallel() @@ -880,7 +859,7 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { recorder := record.NewFakeRecorder(1024) gce.eventRecorder = recorder - apiService := fakeLoadbalancerService("") + svc := fakeLoadbalancerService("") nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) hostNames := nodeNames(nodes) @@ -889,12 +868,12 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { ipnet, err := netsets.ParseIPNets("10.0.0.0/20") require.NoError(t, err) gce.createFirewall( - apiService, - cloudprovider.GetLoadBalancerName(apiService), + svc, + cloudprovider.GetLoadBalancerName(svc), gce.region, "A sad little firewall", ipnet, - apiService.Spec.Ports, + svc.Spec.Ports, hosts) require.Nil(t, err) @@ -902,12 +881,12 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { checkEvent(t, recorder, msg, true) gce.updateFirewall( - apiService, - cloudprovider.GetLoadBalancerName(apiService), + svc, + cloudprovider.GetLoadBalancerName(svc), gce.region, "A sad little firewall", ipnet, - apiService.Spec.Ports, + svc.Spec.Ports, hosts) require.Nil(t, err) @@ -933,8 +912,8 @@ func TestEnsureExternalLoadBalancerDeletedSucceedsOnXPN(t *testing.T) { recorder := record.NewFakeRecorder(1024) gce.eventRecorder = recorder - apiService := fakeLoadbalancerService("") - err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) + svc := fakeLoadbalancerService("") + err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, svc) require.NoError(t, err) msg := fmt.Sprintf("%s %s %s", v1.EventTypeNormal, eventReasonManualChange, eventMsgFirewallChange) @@ -944,18 +923,18 @@ func TestEnsureExternalLoadBalancerDeletedSucceedsOnXPN(t *testing.T) { type EnsureELBParams struct { clusterName string clusterID string - apiService *v1.Service + service *v1.Service existingFwdRule *compute.ForwardingRule nodes []*v1.Node } // newEnsureELBParams is the constructor of EnsureELBParams. -func newEnsureELBParams(nodes []*v1.Node, apiService *v1.Service) *EnsureELBParams { +func newEnsureELBParams(nodes []*v1.Node, svc *v1.Service) *EnsureELBParams { vals := DefaultTestClusterValues() return &EnsureELBParams{ vals.ClusterName, vals.ClusterID, - apiService, + svc, nil, nodes, } @@ -996,7 +975,7 @@ func TestEnsureExternalLoadBalancerErrors(t *testing.T) { }, "Bad load balancer source range provided": { adjustParams: func(params *EnsureELBParams) { - params.apiService.Spec.LoadBalancerSourceRanges = []string{"BadSourceRange"} + params.service.Spec.LoadBalancerSourceRanges = []string{"BadSourceRange"} }, }, "Get firewall failed": { @@ -1034,8 +1013,8 @@ func TestEnsureExternalLoadBalancerErrors(t *testing.T) { gce, err := fakeGCECloud(DefaultTestClusterValues()) nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) - apiService := fakeLoadbalancerService("") - params = newEnsureELBParams(nodes, apiService) + svc := fakeLoadbalancerService("") + params = newEnsureELBParams(nodes, svc) if tc.adjustParams != nil { tc.adjustParams(params) } @@ -1045,7 +1024,7 @@ func TestEnsureExternalLoadBalancerErrors(t *testing.T) { status, err := gce.ensureExternalLoadBalancer( params.clusterName, params.clusterID, - params.apiService, + params.service, params.existingFwdRule, params.nodes, ) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go index 227f53ba051..62bd15cc6f9 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go @@ -27,12 +27,14 @@ import ( compute "google.golang.org/api/compute/v1" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" 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/mock" ) -func createInternalLoadBalancer(gce *GCECloud, apiService *v1.Service, existingFwdRule *compute.ForwardingRule, nodeNames []string, clusterName, clusterID, zoneName string) (*v1.LoadBalancerStatus, error) { +func createInternalLoadBalancer(gce *GCECloud, svc *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 +43,7 @@ func createInternalLoadBalancer(gce *GCECloud, apiService *v1.Service, existingF return gce.ensureInternalLoadBalancer( clusterName, clusterID, - apiService, + svc, existingFwdRule, nodes, ) @@ -56,16 +58,16 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLoadbalancerService(string(LBTypeInternal)) - lbName := cloudprovider.GetLoadBalancerName(apiService) + svc := fakeLoadbalancerService(string(LBTypeInternal)) + lbName := cloudprovider.GetLoadBalancerName(svc) nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) igName := makeInstanceGroupName(vals.ClusterID) igLinks, err := gce.ensureInternalInstanceGroups(igName, nodes) require.NoError(t, err) - 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, "") + sharedBackend := shareBackendService(svc) + bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity) + err = gce.ensureInternalBackendService(bsName, "description", svc.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, "") require.NoError(t, err) // Update the Internal Backend Service with a new ServiceAffinity @@ -77,6 +79,68 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) { assert.Equal(t, bs.SessionAffinity, strings.ToUpper(string(v1.ServiceAffinityNone))) } +func TestEnsureInternalBackendServiceGroups(t *testing.T) { + for desc, tc := range map[string]struct { + mockModifier func(*cloud.MockGCE) + }{ + "Basic workflow": {}, + "GetRegionBackendService failed": { + mockModifier: func(c *cloud.MockGCE) { + c.MockRegionBackendServices.GetHook = mock.GetRegionBackendServicesErrHook + }, + }, + "UpdateRegionBackendServices failed": { + mockModifier: func(c *cloud.MockGCE) { + c.MockRegionBackendServices.UpdateHook = mock.UpdateRegionBackendServicesErrHook + }, + }, + } { + t.Run(desc, func(t *testing.T) { + t.Parallel() + + vals := DefaultTestClusterValues() + nodeNames := []string{"test-node-1"} + + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + svc := fakeLoadbalancerService(string(LBTypeInternal)) + lbName := cloudprovider.GetLoadBalancerName(svc) + nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) + igName := makeInstanceGroupName(vals.ClusterID) + igLinks, err := gce.ensureInternalInstanceGroups(igName, nodes) + require.NoError(t, err) + + sharedBackend := shareBackendService(svc) + bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity) + + err = gce.ensureInternalBackendService(bsName, "description", svc.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, "") + require.NoError(t, err) + + // Update the BackendService with new Instances + if tc.mockModifier != nil { + tc.mockModifier(gce.c.(*cloud.MockGCE)) + } + newNodeNames := []string{"new-test-node-1", "new-test-node-2"} + err = gce.ensureInternalBackendServiceGroups(bsName, newNodeNames) + if tc.mockModifier != nil { + assert.Error(t, err) + return + } + assert.NoError(t, err) + + bs, err := gce.GetRegionBackendService(bsName, gce.region) + assert.NoError(t, err) + + // Check that the instances are updated + newNodes, err := createAndInsertNodes(gce, newNodeNames, vals.ZoneName) + newIgLinks, err := gce.ensureInternalInstanceGroups(igName, newNodes) + backends := backendsFromGroupLinks(newIgLinks) + assert.Equal(t, bs.Backends, backends) + }) + } +} + func TestEnsureInternalLoadBalancer(t *testing.T) { t.Parallel() @@ -86,11 +150,11 @@ func TestEnsureInternalLoadBalancer(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLoadbalancerService(string(LBTypeInternal)) - status, err := createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) + svc := fakeLoadbalancerService(string(LBTypeInternal)) + status, err := createInternalLoadBalancer(gce, svc, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) - assertInternalLbResources(t, gce, apiService, vals, nodeNames) + assertInternalLbResources(t, gce, svc, vals, nodeNames) } func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { @@ -101,13 +165,13 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) + svc := fakeLoadbalancerService(string(LBTypeInternal)) - 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) + nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} + lbName := cloudprovider.GetLoadBalancerName(svc) - sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(apiService) + sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(svc) hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) hcPath, hcPort := GetNodesHealthCheckPath(), GetNodesHealthCheckPort() existingHC := newInternalLBHealthCheck(hcName, nm, sharedHealthCheck, hcPath, hcPort) @@ -119,13 +183,13 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { igLinks, err := gce.ensureInternalInstanceGroups(igName, nodes) require.NoError(t, err) - sharedBackend := shareBackendService(apiService) + sharedBackend := shareBackendService(svc) bsDescription := makeBackendServiceDescription(nm, sharedBackend) - 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) + bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity) + err = gce.ensureInternalBackendService(bsName, bsDescription, svc.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, existingHC.SelfLink) require.NoError(t, err) - _, err = createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createInternalLoadBalancer(gce, svc, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) } @@ -136,8 +200,8 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLoadbalancerService(string(LBTypeInternal)) - lbName := cloudprovider.GetLoadBalancerName(apiService) + svc := fakeLoadbalancerService(string(LBTypeInternal)) + lbName := cloudprovider.GetLoadBalancerName(svc) // Create a ForwardingRule that's missing an IP address existingFwdRule := &compute.ForwardingRule{ @@ -162,10 +226,10 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { } gce.CreateFirewall(existingFirewall) - sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(apiService) + sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(svc) hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) hcPath, hcPort := GetNodesHealthCheckPath(), GetNodesHealthCheckPort() - nm := types.NamespacedName{Name: apiService.Name, Namespace: apiService.Namespace} + nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} // Create a healthcheck with an incorrect threshold existingHC := newInternalLBHealthCheck(hcName, nm, sharedHealthCheck, hcPath, hcPort) @@ -173,20 +237,20 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { gce.CreateHealthCheck(existingHC) // Create a backend Service that's missing Description and Backends - sharedBackend := shareBackendService(apiService) - backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) + sharedBackend := shareBackendService(svc) + backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity) existingBS := &compute.BackendService{ Name: lbName, Protocol: "TCP", HealthChecks: []string{existingHC.SelfLink}, - SessionAffinity: translateAffinityType(apiService.Spec.SessionAffinity), + SessionAffinity: translateAffinityType(svc.Spec.SessionAffinity), LoadBalancingScheme: string(cloud.SchemeInternal), } gce.CreateRegionBackendService(existingBS, gce.region) existingFwdRule.BackendService = existingBS.Name - _, err = createInternalLoadBalancer(gce, apiService, existingFwdRule, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createInternalLoadBalancer(gce, svc, 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 @@ -215,21 +279,21 @@ func TestUpdateInternalLoadBalancerBackendServices(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLoadbalancerService(string(LBTypeInternal)) - _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + svc := fakeLoadbalancerService(string(LBTypeInternal)) + _, err = createInternalLoadBalancer(gce, svc, 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(apiService) - sharedBackend := shareBackendService(apiService) - backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) + lbName := cloudprovider.GetLoadBalancerName(svc) + sharedBackend := shareBackendService(svc) + backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity) existingBS := &compute.BackendService{ Name: backendServiceName, Protocol: "TCP", - SessionAffinity: translateAffinityType(apiService.Spec.SessionAffinity), + SessionAffinity: translateAffinityType(svc.Spec.SessionAffinity), LoadBalancingScheme: string(cloud.SchemeInternal), } @@ -238,7 +302,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, apiService, nodes) + err = gce.updateInternalLoadBalancer(vals.ClusterName, vals.ClusterID, svc, nodes) assert.NoError(t, err) bs, err := gce.GetRegionBackendService(backendServiceName, gce.region) @@ -269,11 +333,11 @@ func TestUpdateInternalLoadBalancerNodes(t *testing.T) { require.NoError(t, err) node1Name := []string{"test-node-1"} - apiService := fakeLoadbalancerService(string(LBTypeInternal)) + svc := fakeLoadbalancerService(string(LBTypeInternal)) nodes, err := createAndInsertNodes(gce, node1Name, vals.ZoneName) require.NoError(t, err) - _, err = gce.ensureInternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nil, nodes) + _, err = gce.ensureInternalLoadBalancer(vals.ClusterName, vals.ClusterID, svc, nil, nodes) assert.NoError(t, err) // Replace the node in initial zone; add new node in a new zone. @@ -284,12 +348,12 @@ func TestUpdateInternalLoadBalancerNodes(t *testing.T) { require.NoError(t, err) nodes = append(newNodesZoneA, newNodesZoneB...) - err = gce.updateInternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nodes) + err = gce.updateInternalLoadBalancer(vals.ClusterName, vals.ClusterID, svc, nodes) assert.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(apiService) - sharedBackend := shareBackendService(apiService) - backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity) + lbName := cloudprovider.GetLoadBalancerName(svc) + sharedBackend := shareBackendService(svc) + backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity) bs, err := gce.GetRegionBackendService(backendServiceName, gce.region) require.NoError(t, err) assert.Equal(t, 2, len(bs.Backends), "Want two backends referencing two instances groups") @@ -334,14 +398,14 @@ func TestEnsureInternalLoadBalancerDeleted(t *testing.T) { gce, err := fakeGCECloud(vals) require.NoError(t, err) - apiService := fakeLoadbalancerService(string(LBTypeInternal)) - _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + svc := fakeLoadbalancerService(string(LBTypeInternal)) + _, err = createInternalLoadBalancer(gce, svc, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) - err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) + err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, svc) assert.NoError(t, err) - assertInternalLbResourcesDeleted(t, gce, apiService, vals, true) + assertInternalLbResourcesDeleted(t, gce, svc, vals, true) } func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { @@ -350,16 +414,328 @@ func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { vals := DefaultTestClusterValues() gce, err := fakeGCECloud(vals) require.NoError(t, err) + svc := fakeLoadbalancerService(string(LBTypeInternal)) - apiService := fakeLoadbalancerService(string(LBTypeInternal)) - _, err = createInternalLoadBalancer(gce, apiService, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + _, err = createInternalLoadBalancer(gce, svc, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) assert.NoError(t, err) - err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) + err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, svc) assert.NoError(t, err) // Deleting the loadbalancer and resources again should not cause an error. - err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService) + err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, svc) assert.NoError(t, err) - assertInternalLbResourcesDeleted(t, gce, apiService, vals, true) + assertInternalLbResourcesDeleted(t, gce, svc, vals, true) +} + +func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) { + vals := DefaultTestClusterValues() + nodeName := "test-node-1" + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + healthCheckNodePort := int32(10101) + svc := fakeLoadbalancerService(string(LBTypeInternal)) + svc.Spec.HealthCheckNodePort = healthCheckNodePort + svc.Spec.Type = v1.ServiceTypeLoadBalancer + svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal + + status, err := createInternalLoadBalancer(gce, svc, nil, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + assert.NoError(t, err) + assert.NotEmpty(t, status.Ingress) + + loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + hc, err := gce.GetHealthCheck(loadBalancerName) + assert.NoError(t, err) + assert.NotNil(t, hc) + assert.Equal(t, int64(healthCheckNodePort), hc.HttpHealthCheck.Port) +} + +func TestClearPreviousInternalResources(t *testing.T) { + // Configure testing environment. + vals := DefaultTestClusterValues() + svc := fakeLoadbalancerService(string(LBTypeInternal)) + loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} + gce, err := fakeGCECloud(vals) + c := gce.c.(*cloud.MockGCE) + require.NoError(t, err) + + hc_1, err := gce.ensureInternalHealthCheck("hc_1", nm, false, "healthz", 12345) + require.NoError(t, err) + + hc_2, err := gce.ensureInternalHealthCheck("hc_2", nm, false, "healthz", 12346) + require.NoError(t, err) + + err = gce.ensureInternalBackendService(svc.ObjectMeta.Name, "", svc.Spec.SessionAffinity, cloud.SchemeInternal, v1.ProtocolTCP, []string{}, "") + require.NoError(t, err) + backendSvc, err := gce.GetRegionBackendService(svc.ObjectMeta.Name, gce.region) + backendSvc.HealthChecks = []string{hc_1.SelfLink, hc_2.SelfLink} + + c.MockRegionBackendServices.DeleteHook = mock.DeleteRegionBackendServicesErrHook + c.MockHealthChecks.DeleteHook = mock.DeleteHealthChecksInternalErrHook + gce.clearPreviousInternalResources(svc, loadBalancerName, backendSvc, "expectedBSName", "expectedHCName") + + backendSvc, err = gce.GetRegionBackendService(svc.ObjectMeta.Name, gce.region) + assert.NoError(t, err) + assert.NotNil(t, backendSvc, "BackendService should not be deleted when api is mocked out.") + hc_1, err = gce.GetHealthCheck("hc_1") + assert.NoError(t, err) + assert.NotNil(t, hc_1, "HealthCheck should not be deleted when there are more than one healthcheck attached.") + hc_2, err = gce.GetHealthCheck("hc_2") + assert.NoError(t, err) + assert.NotNil(t, hc_2, "HealthCheck should not be deleted when there are more than one healthcheck attached.") + + c.MockRegionBackendServices.DeleteHook = mock.DeleteRegionBackendServicesInUseErrHook + backendSvc.HealthChecks = []string{hc_1.SelfLink} + gce.clearPreviousInternalResources(svc, loadBalancerName, backendSvc, "expectedBSName", "expectedHCName") + + hc_1, err = gce.GetHealthCheck("hc_1") + assert.NoError(t, err) + assert.NotNil(t, hc_1, "HealthCheck should not be deleted when api is mocked out.") + + c.MockHealthChecks.DeleteHook = mock.DeleteHealthChecksInuseErrHook + gce.clearPreviousInternalResources(svc, loadBalancerName, backendSvc, "expectedBSName", "expectedHCName") + + hc_1, err = gce.GetHealthCheck("hc_1") + assert.NoError(t, err) + assert.NotNil(t, hc_1, "HealthCheck should not be deleted when api is mocked out.") + + c.MockRegionBackendServices.DeleteHook = nil + c.MockHealthChecks.DeleteHook = nil + gce.clearPreviousInternalResources(svc, loadBalancerName, backendSvc, "expectedBSName", "expectedHCName") + + backendSvc, err = gce.GetRegionBackendService(svc.ObjectMeta.Name, gce.region) + assert.Error(t, err) + assert.Nil(t, backendSvc, "BackendService should be deleted.") + hc_1, err = gce.GetHealthCheck("hc_1") + assert.Error(t, err) + assert.Nil(t, hc_1, "HealthCheck should be deleted.") +} + +func TestEnsureInternalFirewallSucceedsOnXPN(t *testing.T) { + gce, err := fakeGCECloud(DefaultTestClusterValues()) + require.NoError(t, err) + vals := DefaultTestClusterValues() + svc := fakeLoadbalancerService(string(LBTypeInternal)) + fwName := cloudprovider.GetLoadBalancerName(svc) + + c := gce.c.(*cloud.MockGCE) + c.MockFirewalls.InsertHook = mock.InsertFirewallsUnauthorizedErrHook + c.MockFirewalls.UpdateHook = mock.UpdateFirewallsUnauthorizedErrHook + gce.onXPN = true + require.True(t, gce.OnXPN()) + + recorder := record.NewFakeRecorder(1024) + gce.eventRecorder = recorder + + nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) + require.NoError(t, err) + sourceRange := []string{"10.0.0.0/20"} + gce.ensureInternalFirewall( + svc, + fwName, + "A sad little firewall", + sourceRange, + []string{"123"}, + v1.ProtocolTCP, + nodes) + require.Nil(t, err, "Should success when XPN is on.") + + checkEvent(t, recorder, FilewallChangeMsg, true) + + // Create a firewall. + c.MockFirewalls.InsertHook = nil + c.MockFirewalls.UpdateHook = nil + gce.onXPN = false + + gce.ensureInternalFirewall( + svc, + fwName, + "A sad little firewall", + sourceRange, + []string{"123"}, + v1.ProtocolTCP, + nodes) + require.Nil(t, err) + existingFirewall, err := gce.GetFirewall(fwName) + require.Nil(t, err) + require.NotNil(t, existingFirewall) + + gce.onXPN = true + c.MockFirewalls.InsertHook = mock.InsertFirewallsUnauthorizedErrHook + c.MockFirewalls.UpdateHook = mock.UpdateFirewallsUnauthorizedErrHook + + // Try to update the firewall just created. + gce.ensureInternalFirewall( + svc, + fwName, + "A happy little firewall", + sourceRange, + []string{"123"}, + v1.ProtocolTCP, + nodes) + require.Nil(t, err, "Should success when XPN is on.") + + checkEvent(t, recorder, FilewallChangeMsg, true) +} + +func TestEnsureLoadBalancerDeletedSucceedsOnXPN(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + c := gce.c.(*cloud.MockGCE) + recorder := record.NewFakeRecorder(1024) + gce.eventRecorder = recorder + require.NoError(t, err) + + _, err = createInternalLoadBalancer(gce, fakeLoadbalancerService(string(LBTypeInternal)), nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + assert.NoError(t, err) + + c.MockFirewalls.DeleteHook = mock.DeleteFirewallsUnauthorizedErrHook + gce.onXPN = true + + err = gce.ensureInternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, fakeLoadbalancerService(string(LBTypeInternal))) + assert.NoError(t, err) + checkEvent(t, recorder, FilewallChangeMsg, true) +} + +func TestEnsureInternalInstanceGroupsDeleted(t *testing.T) { + vals := DefaultTestClusterValues() + gce, err := fakeGCECloud(vals) + c := gce.c.(*cloud.MockGCE) + recorder := record.NewFakeRecorder(1024) + gce.eventRecorder = recorder + require.NoError(t, err) + + igName := makeInstanceGroupName(vals.ClusterID) + + svc := fakeLoadbalancerService(string(LBTypeInternal)) + _, err = createInternalLoadBalancer(gce, svc, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + assert.NoError(t, err) + + c.MockZones.ListHook = mock.ListZonesInternalErrHook + + err = gce.ensureInternalLoadBalancerDeleted(igName, vals.ClusterID, svc) + assert.Error(t, err, mock.InternalServerError) + ig, err := gce.GetInstanceGroup(igName, vals.ZoneName) + assert.NoError(t, err) + assert.NotNil(t, ig) + + c.MockZones.ListHook = nil + c.MockInstanceGroups.DeleteHook = mock.DeleteInstanceGroupInternalErrHook + + err = gce.ensureInternalInstanceGroupsDeleted(igName) + assert.Error(t, err, mock.InternalServerError) + ig, err = gce.GetInstanceGroup(igName, vals.ZoneName) + assert.NoError(t, err) + assert.NotNil(t, ig) + + c.MockInstanceGroups.DeleteHook = nil + err = gce.ensureInternalInstanceGroupsDeleted(igName) + assert.NoError(t, err) + ig, err = gce.GetInstanceGroup(igName, vals.ZoneName) + assert.Error(t, err) + assert.Nil(t, ig) +} + +type EnsureILBParams struct { + clusterName string + clusterID string + service *v1.Service + existingFwdRule *compute.ForwardingRule + nodes []*v1.Node +} + +// newEnsureILBParams is the constructor of EnsureILBParams. +func newEnsureILBParams(nodes []*v1.Node) *EnsureILBParams { + vals := DefaultTestClusterValues() + return &EnsureILBParams{ + vals.ClusterName, + vals.ClusterID, + fakeLoadbalancerService(string(LBTypeInternal)), + nil, + nodes, + } +} + +// TestEnsureInternalLoadBalancerErrors tests the function +// ensureInternalLoadBalancer, making sure the system won't panic when +// exceptions raised by gce. +func TestEnsureInternalLoadBalancerErrors(t *testing.T) { + vals := DefaultTestClusterValues() + var params *EnsureILBParams + + for desc, tc := range map[string]struct { + adjustParams func(*EnsureILBParams) + injectMock func(*cloud.MockGCE) + }{ + "Create internal instance groups failed": { + injectMock: func(c *cloud.MockGCE) { + c.MockInstanceGroups.GetHook = mock.GetInstanceGroupInternalErrHook + }, + }, + "Invalid existing forwarding rules given": { + adjustParams: func(params *EnsureILBParams) { + params.existingFwdRule = &compute.ForwardingRule{BackendService: "badBackendService"} + }, + injectMock: func(c *cloud.MockGCE) { + c.MockRegionBackendServices.GetHook = mock.GetRegionBackendServicesErrHook + }, + }, + "EnsureInternalBackendService failed": { + injectMock: func(c *cloud.MockGCE) { + c.MockRegionBackendServices.GetHook = mock.GetRegionBackendServicesErrHook + }, + }, + "Create internal health check failed": { + injectMock: func(c *cloud.MockGCE) { + c.MockHealthChecks.GetHook = mock.GetHealthChecksInternalErrHook + }, + }, + "Create firewall failed": { + injectMock: func(c *cloud.MockGCE) { + c.MockFirewalls.InsertHook = mock.InsertFirewallsUnauthorizedErrHook + }, + }, + "Create region forwarding rule failed": { + injectMock: func(c *cloud.MockGCE) { + c.MockForwardingRules.InsertHook = mock.InsertForwardingRulesInternalErrHook + }, + }, + "Get region forwarding rule failed": { + injectMock: func(c *cloud.MockGCE) { + c.MockForwardingRules.GetHook = mock.GetForwardingRulesInternalErrHook + }, + }, + "Delete region forwarding rule failed": { + adjustParams: func(params *EnsureILBParams) { + params.existingFwdRule = &compute.ForwardingRule{BackendService: "badBackendService"} + }, + injectMock: func(c *cloud.MockGCE) { + c.MockForwardingRules.DeleteHook = mock.DeleteForwardingRuleErrHook + }, + }, + } { + t.Run(desc, func(t *testing.T) { + gce, err := fakeGCECloud(DefaultTestClusterValues()) + nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) + require.NoError(t, err) + params = newEnsureILBParams(nodes) + if tc.adjustParams != nil { + tc.adjustParams(params) + } + if tc.injectMock != nil { + tc.injectMock(gce.c.(*cloud.MockGCE)) + } + status, err := gce.ensureInternalLoadBalancer( + params.clusterName, + params.clusterID, + params.service, + params.existingFwdRule, + params.nodes, + ) + assert.Error(t, err, "Should return an error when "+desc) + assert.Nil(t, status, "Should not return a status when "+desc) + }) + } } diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go index aabe6fd5be2..7a5dbc715c9 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go @@ -23,8 +23,10 @@ package gce import ( "fmt" "net/http" + "strings" "sync" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -33,6 +35,7 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" + "k8s.io/client-go/tools/record" v1_service "k8s.io/kubernetes/pkg/api/v1/service" "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud" @@ -85,6 +88,10 @@ func fakeLoadbalancerService(lbType string) *v1.Service { } } +var ( + FilewallChangeMsg = fmt.Sprintf("%s %s %s", v1.EventTypeNormal, eventReasonManualChange, eventMsgFirewallChange) +) + type fakeRoundTripper struct{} func (*fakeRoundTripper) RoundTrip(*http.Request) (*http.Response, error) { @@ -374,3 +381,23 @@ func assertInternalLbResourcesDeleted(t *testing.T, gce *GCECloud, apiService *v require.Error(t, err) assert.Nil(t, healthcheck) } + +func checkEvent(t *testing.T, recorder *record.FakeRecorder, expected string, shouldMatch bool) bool { + select { + case received := <-recorder.Events: + if strings.HasPrefix(received, expected) != shouldMatch { + t.Errorf(received) + if shouldMatch { + t.Errorf("Should receive message \"%v\" but got \"%v\".", expected, received) + } else { + t.Errorf("Unexpected event \"%v\".", received) + } + } + return false + case <-time.After(2 * time.Second): + if shouldMatch { + t.Errorf("Should receive message \"%v\" but got timed out.", expected) + } + return true + } +}