diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go index 8db1b8ec136..f8f190e8b69 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go +++ b/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() diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go index d8e9a91b43f..4744ff60dac 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go +++ b/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()