Add unit tests for gce loadbalancer internal.

Increase coverage for gce_loadbalancer_internal from 76.7% to 91.0%.

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
 }

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("")
+	svc := fakeLoadbalancerService("")
-	status, err := createExternalLoadBalancer(gce, apiService, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName)
+	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("")
+	svc := fakeLoadbalancerService("")
-	_, err = createExternalLoadBalancer(gce, apiService, []string{nodeName}, vals.ClusterName, vals.ClusterID, vals.ZoneName)
+	_, 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("")
+	svc := fakeLoadbalancerService("")
-	_, 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)
 	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("")
+	svc := fakeLoadbalancerService("")
-	apiService.Annotations = map[string]string{NetworkTierAnnotationKey: "Premium"}
+	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)
+	lbName := cloudprovider.GetLoadBalancerName(svc)
-	serviceName := types.NamespacedName{Namespace: apiService.Namespace, Name: apiService.Name}
+	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("")
+	svc := fakeLoadbalancerService("")
-	apiService.Annotations = map[string]string{NetworkTierAnnotationKey: wrongTier}
+	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("")
+	svc := fakeLoadbalancerService("")
-	_, err = gce.ensureExternalLoadBalancer(vals.ClusterName, vals.ClusterID, apiService, nil, []*v1.Node{})
+	_, 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("")
+	svc := fakeLoadbalancerService("")
-	lbName := cloudprovider.GetLoadBalancerName(apiService)
+	lbName := cloudprovider.GetLoadBalancerName(svc)
 	ipAddr := status.Ingress[0].IP
 
-	lbIP := apiService.Spec.LoadBalancerIP
+	lbIP := svc.Spec.LoadBalancerIP
-	wrongPorts := []v1.ServicePort{apiService.Spec.Ports[0]}
+	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("")
+	svc := fakeLoadbalancerService("")
-	serviceName := apiService.ObjectMeta.Name
+	serviceName := svc.ObjectMeta.Name
-	lbName := cloudprovider.GetLoadBalancerName(apiService)
+	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("")
+	svc := fakeLoadbalancerService("")
-	status, err := createExternalLoadBalancer(gce, apiService, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName)
+	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,
+		svc,
-		cloudprovider.GetLoadBalancerName(apiService),
+		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,
+		svc,
-		cloudprovider.GetLoadBalancerName(apiService),
+		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("")
+	svc := fakeLoadbalancerService("")
-	err = gce.ensureExternalLoadBalancerDeleted(vals.ClusterName, vals.ClusterID, apiService)
+	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("")
+			svc := fakeLoadbalancerService("")
-			params = newEnsureELBParams(nodes, apiService)
+			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,
 			)

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))
+	svc := fakeLoadbalancerService(string(LBTypeInternal))
-	lbName := cloudprovider.GetLoadBalancerName(apiService)
+	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)
+	sharedBackend := shareBackendService(svc)
-	bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity)
+	bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity)
-	err = gce.ensureInternalBackendService(bsName, "description", apiService.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, "")
+	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))
+	svc := fakeLoadbalancerService(string(LBTypeInternal))
-	status, err := createInternalLoadBalancer(gce, apiService, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName)
+	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}
+	nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace}
-	lbName := cloudprovider.GetLoadBalancerName(apiService)
+	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)
+	bsName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity)
-	err = gce.ensureInternalBackendService(bsName, bsDescription, apiService.Spec.SessionAffinity, cloud.SchemeInternal, "TCP", igLinks, existingHC.SelfLink)
+	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))
+	svc := fakeLoadbalancerService(string(LBTypeInternal))
-	lbName := cloudprovider.GetLoadBalancerName(apiService)
+	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)
+	sharedBackend := shareBackendService(svc)
-	backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity)
+	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))
+	svc := 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)
 
 	// BackendService exists prior to updateInternalLoadBalancer call, but has
 	// incorrect (missing) attributes.
 	// ensureInternalBackendServiceGroups is called and creates the correct
 	// BackendService
-	lbName := cloudprovider.GetLoadBalancerName(apiService)
+	lbName := cloudprovider.GetLoadBalancerName(svc)
-	sharedBackend := shareBackendService(apiService)
+	sharedBackend := shareBackendService(svc)
-	backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity)
+	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)
+	lbName := cloudprovider.GetLoadBalancerName(svc)
-	sharedBackend := shareBackendService(apiService)
+	sharedBackend := shareBackendService(svc)
-	backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", apiService.Spec.SessionAffinity)
+	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))
+	svc := 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)
 
-	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, svc, nil, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName)
-	_, 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, 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)
+		})
+	}
 }

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
+	}
+}