From 6d535f1ada39acfd3ca041c0c78c45e58898788c Mon Sep 17 00:00:00 2001 From: Pavithra Ramesh Date: Wed, 14 Oct 2020 06:42:02 -0700 Subject: [PATCH] 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 --- .../gce/gce_instances.go | 6 +++++- .../gce/gce_loadbalancer_external_test.go | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go index 14859e68fcd..b0286f1fc58 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go +++ b/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 } diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go index 18f604c62fd..18cf3dc66a8 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go +++ b/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) {