Merge pull request #13289 from olemarkus/no-node-deletion

Only delete node object on GCE

pkg/instancegroups/instancegroups.go

@@ -409,9 +409,8 @@ func (c *RollingUpdateCluster) drainTerminateAndWait(u *cloudinstances.CloudInst
 		}
 	}
 
-	// We unregister the node before deleting it; if the replacement comes up with the same name it would otherwise still be cordoned
-	// (It often seems like GCE tries to re-use names)
-	if !isBastion && !c.CloudOnly {
+	// GCE often re-uses names, so we delete the node object to prevent the new instance from using the cordoned Node object
+	if c.Cluster.Spec.GetCloudProvider() == api.CloudProviderGCE && !isBastion && !c.CloudOnly {
 		if u.Node == nil {
 			klog.Warningf("no kubernetes Node associated with %s, skipping node deletion", instanceID)
 		} else {

pkg/instancegroups/rollingupdate_test.go

@@ -222,17 +222,14 @@ func TestRollingUpdateAllNeedUpdate(t *testing.T) {
 		case testingclient.PatchAction:
 			if string(a.GetPatch()) == cordonPatch {
 				assertCordon(t, a)
-				assert.Equal(t, "", cordoned, "at most one node cordoned at a time")
 				assert.True(t, tainted[a.GetName()], "node", a.GetName(), "tainted")
 				cordoned = a.GetName()
 			} else if string(a.GetPatch()) == excludeLBPatch {
 				assertExclude(t, a)
-				assert.Equal(t, "", excluded, "at most one node excluded at a time")
 				assert.True(t, tainted[a.GetName()], "node", a.GetName(), "tainted")
 				excluded = a.GetName()
 			} else {
 				assertTaint(t, a)
-				assert.Equal(t, "", cordoned, "not tainting while node cordoned")
 				assert.False(t, tainted[a.GetName()], "node", a.GetName(), "already tainted")
 				tainted[a.GetName()] = true
 			}
@@ -811,11 +808,9 @@ func TestRollingUpdateTaintAllButOneNeedUpdate(t *testing.T) {
 		case testingclient.PatchAction:
 			if string(a.GetPatch()) == cordonPatch {
 				assertCordon(t, a)
-				assert.Equal(t, "", cordoned, "at most one node cordoned at a time")
 				cordoned = a.GetName()
 			} else if string(a.GetPatch()) == excludeLBPatch {
 				assertExclude(t, a)
-				assert.Equal(t, "", excluded, "at most one node excluded at a time")
 				excluded = a.GetName()
 			} else {
 				assertTaint(t, a)
@@ -863,12 +858,10 @@ func TestRollingUpdateMaxSurgeIgnoredForMaster(t *testing.T) {
 		case testingclient.PatchAction:
 			if string(a.GetPatch()) == cordonPatch {
 				assertCordon(t, a)
-				assert.Equal(t, "", cordoned, "at most one node cordoned at a time")
 				assert.True(t, tainted[a.GetName()], "node", a.GetName(), "tainted")
 				cordoned = a.GetName()
 			} else if string(a.GetPatch()) == excludeLBPatch {
 				assertExclude(t, a)
-				assert.Equal(t, "", excluded, "at most one node excluded at a time")
 				assert.True(t, tainted[a.GetName()], "node", a.GetName(), "tainted")
 				excluded = a.GetName()
 			} else {