Do not skip externalLB update if some nodes are not found.

Log a warning instead and continue with the update. This is useful in cases where
the number of nodes is changing due to autoscaling or updgrades. It is possible
that the nodes picked by service controller don't all exist when gce layer lists
them. Update should still succeed with the nodes in the input that are valid.
This will still return an error if 0 nodes were found, when a non-zero input was passed in.

same

Author: prameshj

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

@@ -561,7 +561,11 @@ func (g *Cloud) getInstancesByNames(names []string) ([]*gceInstance, error) {
 		return nil, err
 	}
 	if len(foundInstances) != len(names) {
-		return nil, cloudprovider.InstanceNotFound
+		if len(foundInstances) == 0 {
+			// return error so the TargetPool nodecount does not drop to 0 unexpectedly.
+			return nil, cloudprovider.InstanceNotFound
+		}
+		klog.Warningf("getFoundInstanceByNames - input instances %d, found %d. Continuing LoadBalancer Update", len(names), len(foundInstances))
 	}
 	return foundInstances, nil
 }

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

@@ -372,6 +372,24 @@ func TestUpdateExternalLoadBalancer(t *testing.T) {
 		[]string{fmt.Sprintf("/zones/%s/instances/%s", vals.ZoneName, nodeName)},
 		pool.Instances,
 	)
+
+	anotherNewNodeName := "test-node-3"
+	newNodes, err = createAndInsertNodes(gce, []string{nodeName, newNodeName, anotherNewNodeName}, vals.ZoneName)
+	assert.NoError(t, err)
+
+	// delete one of the existing nodes, but include it in the list
+	err = gce.DeleteInstance(gce.ProjectID(), vals.ZoneName, nodeName)
+	require.NoError(t, err)
+
+	// The update should ignore the reference to non-existent node "test-node-1", but update target pool with rest of the valid nodes.
+	err = gce.updateExternalLoadBalancer(vals.ClusterName, svc, newNodes)
+	assert.NoError(t, err)
+
+	pool, err = gce.GetTargetPool(lbName, gce.region)
+	require.NoError(t, err)
+
+	namePrefix := fmt.Sprintf("/zones/%s/instances/", vals.ZoneName)
+	assert.ElementsMatch(t, pool.Instances, []string{namePrefix + newNodeName, namePrefix + anotherNewNodeName})
 }
 
 func TestEnsureExternalLoadBalancerDeleted(t *testing.T) {