Merge pull request #91392 from prameshj/getlb

Check for GCE finalizer in GetLoadBalancer.

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

@@ -111,6 +111,11 @@ func (g *Cloud) GetLoadBalancer(ctx context.Context, clusterName string, svc *v1
 
 		return status, true, nil
 	}
+	// Checking for finalizer is more accurate because controller restart could happen in the middle of resource
+	// deletion. So even though forwarding rule was deleted, cleanup might not have been complete.
+	if hasFinalizer(svc, ILBFinalizerV1) {
+		return &v1.LoadBalancerStatus{}, true, nil
+	}
 	return nil, false, ignoreNotFound(err)
 }
 

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

@@ -62,6 +62,9 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
 			"Skipped ensureInternalLoadBalancer as service contains '%s' finalizer.", ILBFinalizerV2)
 		return nil, cloudprovider.ImplementedElsewhere
 	}
+
+	loadBalancerName := g.GetLoadBalancerName(context.TODO(), clusterName, svc)
+	klog.V(2).Infof("ensureInternalLoadBalancer(%v): Attaching %q finalizer", loadBalancerName, ILBFinalizerV1)
 	if err := addFinalizer(svc, g.client.CoreV1(), ILBFinalizerV1); err != nil {
 		klog.Errorf("Failed to attach finalizer '%s' on service %s/%s - %v", ILBFinalizerV1, svc.Namespace, svc.Name, err)
 		return nil, err
@@ -85,7 +88,6 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
 		}
 	}
 
-	loadBalancerName := g.GetLoadBalancerName(context.TODO(), clusterName, svc)
 	sharedBackend := shareBackendService(svc)
 	backendServiceName := makeBackendServiceName(loadBalancerName, clusterID, sharedBackend, scheme, protocol, svc.Spec.SessionAffinity)
 	backendServiceLink := g.getBackendServiceLink(backendServiceName)
@@ -317,10 +319,12 @@ func (g *Cloud) ensureInternalLoadBalancerDeleted(clusterName, clusterID string,
 
 	// Try deleting instance groups - expect ResourceInuse error if needed by other LBs
 	igName := makeInstanceGroupName(clusterID)
+	klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): Attempting delete of instanceGroup %v", loadBalancerName, igName)
 	if err := g.ensureInternalInstanceGroupsDeleted(igName); err != nil && !isInUsedByError(err) {
 		return err
 	}
 
+	klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): Removing %q finalizer", loadBalancerName, ILBFinalizerV1)
 	if err := removeFinalizer(svc, g.client.CoreV1(), ILBFinalizerV1); err != nil {
 		klog.Errorf("Failed to remove finalizer '%s' on service %s/%s - %v", ILBFinalizerV1, svc.Namespace, svc.Name, err)
 		return err

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

@@ -1590,5 +1590,51 @@ func TestEnsureLoadBalancerSkipped(t *testing.T) {
 	// No loadbalancer resources will be created due to the ILB Feature Gate
 	assert.Empty(t, status)
 	assertInternalLbResourcesDeleted(t, gce, svc, vals, true)
-
+}
+
+// TestEnsureLoadBalancerPartialDelete simulates a partial delete and checks whether deletion completes after a second
+// attempt.
+func TestEnsureLoadBalancerPartialDelete(t *testing.T) {
+	t.Parallel()
+
+	vals := DefaultTestClusterValues()
+	nodeNames := []string{"test-node-1"}
+
+	gce, err := fakeGCECloud(vals)
+	require.NoError(t, err)
+
+	svc := fakeLoadbalancerService(string(LBTypeInternal))
+	svc, err = gce.client.CoreV1().Services(svc.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
+	require.NoError(t, err)
+	status, err := createInternalLoadBalancer(gce, svc, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName)
+	require.NoError(t, err)
+	assert.NotEmpty(t, status.Ingress)
+	assertInternalLbResources(t, gce, svc, vals, nodeNames)
+	svc, err = gce.client.CoreV1().Services(svc.Namespace).Get(context.TODO(), svc.Name, metav1.GetOptions{})
+	require.NoError(t, err)
+	if !hasFinalizer(svc, ILBFinalizerV1) {
+		t.Errorf("Expected finalizer '%s' not found in Finalizer list - %v", ILBFinalizerV1, svc.Finalizers)
+	}
+	// Delete the forwarding rule to simulate controller getting shut down on partial cleanup
+	lbName := gce.GetLoadBalancerName(context.TODO(), "", svc)
+	err = gce.DeleteRegionForwardingRule(lbName, gce.region)
+	require.NoError(t, err)
+	// Check output of GetLoadBalancer
+	_, exists, err := gce.GetLoadBalancer(context.TODO(), vals.ClusterName, svc)
+	require.NoError(t, err)
+	assert.True(t, exists)
+	// call EnsureDeleted again
+	err = gce.EnsureLoadBalancerDeleted(context.TODO(), vals.ClusterName, svc)
+	require.NoError(t, err)
+	// Make sure all resources are gone
+	assertInternalLbResourcesDeleted(t, gce, svc, vals, true)
+	// Ensure that the finalizer has been deleted
+	svc, err = gce.client.CoreV1().Services(svc.Namespace).Get(context.TODO(), svc.Name, metav1.GetOptions{})
+	require.NoError(t, err)
+	if hasFinalizer(svc, ILBFinalizerV1) {
+		t.Errorf("Finalizer '%s' not deleted from service - %v", ILBFinalizerV1, svc.Finalizers)
+	}
+	_, exists, err = gce.GetLoadBalancer(context.TODO(), vals.ClusterName, svc)
+	require.NoError(t, err)
+	assert.False(t, exists)
 }