Allow non-subsetting ILBs to update when the feature is enabled.

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go

@@ -274,7 +274,9 @@ func (g *Cloud) clearPreviousInternalResources(svc *v1.Service, loadBalancerName
 // updateInternalLoadBalancer is called when the list of nodes has changed. Therefore, only the instance groups
 // and possibly the backend service need to be updated.
 func (g *Cloud) updateInternalLoadBalancer(clusterName, clusterID string, svc *v1.Service, nodes []*v1.Node) error {
-	if g.AlphaFeatureGate.Enabled(AlphaFeatureILBSubsets) {
+	if g.AlphaFeatureGate.Enabled(AlphaFeatureILBSubsets) && !hasFinalizer(svc, ILBFinalizerV1) {
+		g.eventRecorder.Eventf(svc, v1.EventTypeNormal, "SkippingUpdateInternalLoadBalancer",
+			"Skipped updateInternalLoadBalancer as service does not contain '%s' finalizer.", ILBFinalizerV1)
 		return cloudprovider.ImplementedElsewhere
 	}
 	g.sharedResourceLock.Lock()

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go

@@ -1111,6 +1111,60 @@ func TestEnsureInternalLoadBalancerDeletedSubsetting(t *testing.T) {
 	assertInternalLbResourcesDeleted(t, gce, svc, vals, true)
 }
 
+// TestEnsureInternalLoadBalancerUpdateSubsetting verifies that updates of existing ILB instance groups
+// continue to work, even if ILBSubsets feature is enabled.
+func TestEnsureInternalLoadBalancerUpdateSubsetting(t *testing.T) {
+	t.Parallel()
+
+	vals := DefaultTestClusterValues()
+	gce, err := fakeGCECloud(vals)
+	assert.NoError(t, err)
+	recorder := record.NewFakeRecorder(1024)
+	gce.eventRecorder = recorder
+
+	nodeNames := []string{"test-node-1"}
+	svc := fakeLoadbalancerService(string(LBTypeInternal))
+	svc, err = gce.client.CoreV1().Services(svc.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
+	assert.NoError(t, err)
+	status, err := createInternalLoadBalancer(gce, svc, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName)
+
+	assert.NoError(t, err)
+	assert.NotEmpty(t, status.Ingress)
+	svc, err = gce.client.CoreV1().Services(svc.Namespace).Get(context.TODO(), svc.Name, metav1.GetOptions{})
+	assert.NoError(t, err)
+	if !hasFinalizer(svc, ILBFinalizerV1) {
+		t.Errorf("Expected finalizer '%s' not found in Finalizer list - %v", ILBFinalizerV1, svc.Finalizers)
+	}
+	// Enable FeatureGate after service has been created.
+	gce.AlphaFeatureGate = NewAlphaFeatureGate([]string{AlphaFeatureILBSubsets})
+	// mock scenario where user adds more nodes, this should be updated in the ILB.
+	nodeNames = []string{"test-node-1", "test-node-2"}
+	nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName)
+	assert.NoError(t, err)
+	err = gce.UpdateLoadBalancer(context.Background(), vals.ClusterName, svc, nodes)
+	assert.NoError(t, err)
+	// Ensure that the backend service/Instance group has both nodes.
+	igName := makeInstanceGroupName(vals.ClusterID)
+	instances, err := gce.ListInstancesInInstanceGroup(igName, vals.ZoneName, allInstances)
+	assert.NoError(t, err)
+	var instanceNames []string
+	for _, inst := range instances {
+		resourceID, err := cloud.ParseResourceURL(inst.Instance)
+		if err != nil || resourceID == nil || resourceID.Key == nil {
+			t.Errorf("Failed to parse instance url - %q, error - %v", inst.Instance, err)
+			continue
+		}
+		instanceNames = append(instanceNames, resourceID.Key.Name)
+	}
+	if !equalStringSets(instanceNames, nodeNames) {
+		t.Errorf("Got instances - %v, want %v", instanceNames, nodeNames)
+	}
+	// Invoked when service is deleted.
+	err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, svc)
+	assert.NoError(t, err)
+	assertInternalLbResourcesDeleted(t, gce, svc, vals, true)
+}
+
 func TestEnsureInternalLoadBalancerGlobalAccess(t *testing.T) {
 	t.Parallel()