Merge pull request #61877 from mikedanese/depeid

Automatic merge from submit-queue (batch tested with PRs 62481, 62643, 61877, 62515). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

remove deprecated ExternalID

This field has been deprecated since 1.1. After we remove it we can remove "self delete" from the node's permission set.

@kubernetes/api-reviewers 
@kubernetes/sig-auth-pr-reviews 

fixes https://github.com/kubernetes/kubernetes/issues/61966
part of https://github.com/kubernetes/community/pull/911

```release-note
Kubelets will no longer set `externalID` in their node spec.
```

api/openapi-spec/swagger.json

@@ -77367,7 +77367,7 @@
       "$ref": "#/definitions/io.k8s.api.core.v1.NodeConfigSource"
      },
      "externalID": {
-      "description": "External ID of the node assigned by some machine database (e.g. a cloud provider). Deprecated.",
+      "description": "Deprecated. Not all kubelets will set this field. Remove field after 1.13. see: https://issues.k8s.io/61966",
       "type": "string"
      },
      "podCIDR": {

api/swagger-spec/v1.json

@@ -18663,10 +18663,6 @@
       "type": "string",
       "description": "PodCIDR represents the pod IP range assigned to the node."
      },
-     "externalID": {
-      "type": "string",
-      "description": "External ID of the node assigned by some machine database (e.g. a cloud provider). Deprecated."
-     },
      "providerID": {
       "type": "string",
       "description": "ID of the node assigned by the cloud provider in the format: \u003cProviderName\u003e://\u003cProviderSpecificNodeID\u003e"
@@ -18685,6 +18681,10 @@
      "configSource": {
       "$ref": "v1.NodeConfigSource",
       "description": "If specified, the source to get node configuration from The DynamicKubeletConfig feature gate must be enabled for the Kubelet to use this field"
+     },
+     "externalID": {
+      "type": "string",
+      "description": "Deprecated. Not all kubelets will set this field. Remove field after 1.13. see: https://issues.k8s.io/61966"
      }
     }
    },

cmd/kubeadm/app/util/apiclient/init_dryrun.go

@@ -133,9 +133,6 @@ func (idr *InitDryRunGetter) handleGetNode(action core.GetAction) (bool, runtime
 				"kubernetes.io/hostname": idr.masterName,
 			},
 		},
-		Spec: v1.NodeSpec{
-			ExternalID: idr.masterName,
-		},
 	}, nil
 }
 

cmd/kubeadm/app/util/apiclient/init_dryrun_test.go

@@ -46,7 +46,7 @@ func TestHandleGetAction(t *testing.T) {
 		{
 			action:             core.NewRootGetAction(schema.GroupVersionResource{Version: "v1", Resource: "nodes"}, masterName),
 			expectedHandled:    true,
-			expectedObjectJSON: []byte(`{"metadata":{"name":"master-foo","creationTimestamp":null,"labels":{"kubernetes.io/hostname":"master-foo"}},"spec":{"externalID":"master-foo"},"status":{"daemonEndpoints":{"kubeletEndpoint":{"Port":0}},"nodeInfo":{"machineID":"","systemUUID":"","bootID":"","kernelVersion":"","osImage":"","containerRuntimeVersion":"","kubeletVersion":"","kubeProxyVersion":"","operatingSystem":"","architecture":""}}}`),
+			expectedObjectJSON: []byte(`{"metadata":{"name":"master-foo","creationTimestamp":null,"labels":{"kubernetes.io/hostname":"master-foo"}},"spec":{},"status":{"daemonEndpoints":{"kubeletEndpoint":{"Port":0}},"nodeInfo":{"machineID":"","systemUUID":"","bootID":"","kernelVersion":"","osImage":"","containerRuntimeVersion":"","kubeletVersion":"","kubeProxyVersion":"","operatingSystem":"","architecture":""}}}`),
 			expectedErr:        false,
 		},
 		{

docs/api-reference/v1/definitions.html

@@ -11385,13 +11385,6 @@ Examples:<br>
 <td class="tableblock halign-left valign-top"></td>
 </tr>
 <tr>
-<td class="tableblock halign-left valign-top"><p class="tableblock">externalID</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">External ID of the node assigned by some machine database (e.g. a cloud provider). Deprecated.</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">string</p></td>
-<td class="tableblock halign-left valign-top"></td>
-</tr>
-<tr>
 <td class="tableblock halign-left valign-top"><p class="tableblock">providerID</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">ID of the node assigned by the cloud provider in the format: &lt;ProviderName&gt;://&lt;ProviderSpecificNodeID&gt;</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
@@ -11419,6 +11412,13 @@ Examples:<br>
 <td class="tableblock halign-left valign-top"><p class="tableblock"><a href="#_v1_nodeconfigsource">v1.NodeConfigSource</a></p></td>
 <td class="tableblock halign-left valign-top"></td>
 </tr>
+<tr>
+<td class="tableblock halign-left valign-top"><p class="tableblock">externalID</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">Deprecated. Not all kubelets will set this field. Remove field after 1.13. see: <a href="https://issues.k8s.io/61966">https://issues.k8s.io/61966</a></p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">string</p></td>
+<td class="tableblock halign-left valign-top"></td>
+</tr>
 </tbody>
 </table>
 

pkg/apis/core/fuzzer/fuzzer.go

@@ -469,10 +469,6 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 				ss.SessionAffinityConfig = nil
 			}
 		},
-		func(n *core.Node, c fuzz.Continue) {
-			c.FuzzNoCustom(n)
-			n.Spec.ExternalID = "external"
-		},
 		func(s *core.NodeStatus, c fuzz.Continue) {
 			c.FuzzNoCustom(s)
 			s.Allocatable = s.Capacity

pkg/apis/core/types.go

@@ -3228,10 +3228,6 @@ type NodeSpec struct {
 	// +optional
 	PodCIDR string
 
-	// External ID of the node assigned by some machine database (e.g. a cloud provider)
-	// +optional
-	ExternalID string
-
 	// ID of the node assigned by the cloud provider
 	// Note: format is "<ProviderName>://<ProviderSpecificNodeID>"
 	// +optional
@@ -3249,6 +3245,11 @@ type NodeSpec struct {
 	// The DynamicKubeletConfig feature gate must be enabled for the Kubelet to use this field
 	// +optional
 	ConfigSource *NodeConfigSource
+
+	// Deprecated. Not all kubelets will set this field. Remove field after 1.13.
+	// see: https://issues.k8s.io/61966
+	// +optional
+	DoNotUse_ExternalID string
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/apis/core/v1/defaults.go

@@ -296,11 +296,6 @@ func SetDefaults_NamespaceStatus(obj *v1.NamespaceStatus) {
 		obj.Phase = v1.NamespaceActive
 	}
 }
-func SetDefaults_Node(obj *v1.Node) {
-	if obj.Spec.ExternalID == "" {
-		obj.Spec.ExternalID = obj.Name
-	}
-}
 func SetDefaults_NodeStatus(obj *v1.NodeStatus) {
 	if obj.Allocatable == nil && obj.Capacity != nil {
 		obj.Allocatable = make(v1.ResourceList, len(obj.Capacity))

pkg/apis/core/v1/defaults_test.go

@@ -1033,20 +1033,6 @@ func TestSetDefaultPodSpecHostNetwork(t *testing.T) {
 	}
 }
 
-func TestSetDefaultNodeExternalID(t *testing.T) {
-	name := "node0"
-	n := &v1.Node{}
-	n.Name = name
-	obj2 := roundTrip(t, runtime.Object(n))
-	n2 := obj2.(*v1.Node)
-	if n2.Spec.ExternalID != name {
-		t.Errorf("Expected default External ID: %s, got: %s", name, n2.Spec.ExternalID)
-	}
-	if n2.Spec.ProviderID != "" {
-		t.Errorf("Expected empty default Cloud Provider ID, got: %s", n2.Spec.ProviderID)
-	}
-}
-
 func TestSetDefaultNodeStatusAllocatable(t *testing.T) {
 	capacity := v1.ResourceList{
 		v1.ResourceCPU:    resource.MustParse("1000m"),

pkg/apis/core/v1/zz_generated.conversion.go

@@ -2757,11 +2757,11 @@ func Convert_core_NodeSelectorTerm_To_v1_NodeSelectorTerm(in *core.NodeSelectorT
 
 func autoConvert_v1_NodeSpec_To_core_NodeSpec(in *v1.NodeSpec, out *core.NodeSpec, s conversion.Scope) error {
 	out.PodCIDR = in.PodCIDR
-	out.ExternalID = in.ExternalID
 	out.ProviderID = in.ProviderID
 	out.Unschedulable = in.Unschedulable
 	out.Taints = *(*[]core.Taint)(unsafe.Pointer(&in.Taints))
 	out.ConfigSource = (*core.NodeConfigSource)(unsafe.Pointer(in.ConfigSource))
+	out.DoNotUse_ExternalID = in.DoNotUse_ExternalID
 	return nil
 }
 
@@ -2772,11 +2772,11 @@ func Convert_v1_NodeSpec_To_core_NodeSpec(in *v1.NodeSpec, out *core.NodeSpec, s
 
 func autoConvert_core_NodeSpec_To_v1_NodeSpec(in *core.NodeSpec, out *v1.NodeSpec, s conversion.Scope) error {
 	out.PodCIDR = in.PodCIDR
-	out.ExternalID = in.ExternalID
 	out.ProviderID = in.ProviderID
 	out.Unschedulable = in.Unschedulable
 	out.Taints = *(*[]v1.Taint)(unsafe.Pointer(&in.Taints))
 	out.ConfigSource = (*v1.NodeConfigSource)(unsafe.Pointer(in.ConfigSource))
+	out.DoNotUse_ExternalID = in.DoNotUse_ExternalID
 	return nil
 }
 

pkg/apis/core/v1/zz_generated.defaults.go

@@ -115,7 +115,6 @@ func SetObjectDefaults_NamespaceList(in *v1.NamespaceList) {
 }
 
 func SetObjectDefaults_Node(in *v1.Node) {
-	SetDefaults_Node(in)
 	SetDefaults_NodeStatus(&in.Status)
 	SetDefaults_ResourceList(&in.Status.Capacity)
 	SetDefaults_ResourceList(&in.Status.Allocatable)

pkg/apis/core/validation/validation.go

@@ -3973,11 +3973,6 @@ func ValidateNode(node *core.Node) field.ErrorList {
 	// That said, if specified, we need to ensure they are valid.
 	allErrs = append(allErrs, ValidateNodeResources(node)...)
 
-	// external ID is required.
-	if len(node.Spec.ExternalID) == 0 {
-		allErrs = append(allErrs, field.Required(field.NewPath("spec", "externalID"), ""))
-	}
-
 	// Only allow Node.Spec.ConfigSource to be set if the DynamicKubeletConfig feature gate is enabled
 	if node.Spec.ConfigSource != nil && !utilfeature.DefaultFeatureGate.Enabled(features.DynamicKubeletConfig) {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "configSource"), "configSource may only be set if the DynamicKubeletConfig feature gate is enabled)"))

pkg/apis/core/validation/validation_test.go

@@ -9364,9 +9364,6 @@ func TestValidateNode(t *testing.T) {
 					core.ResourceName("hugepages-1Gi"):     resource.MustParse("0"),
 				},
 			},
-			Spec: core.NodeSpec{
-				ExternalID: "external",
-			},
 		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -9381,9 +9378,6 @@ func TestValidateNode(t *testing.T) {
 					core.ResourceName(core.ResourceMemory): resource.MustParse("0"),
 				},
 			},
-			Spec: core.NodeSpec{
-				ExternalID: "external",
-			},
 		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -9399,7 +9393,6 @@ func TestValidateNode(t *testing.T) {
 				},
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
 				// Add a valid taint to a node
 				Taints: []core.Taint{{Key: "GPU", Value: "true", Effect: "NoSchedule"}},
 			},
@@ -9437,9 +9430,6 @@ func TestValidateNode(t *testing.T) {
 					core.ResourceName(core.ResourceMemory): resource.MustParse("0"),
 				},
 			},
-			Spec: core.NodeSpec{
-				ExternalID: "external",
-			},
 		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -9455,8 +9445,7 @@ func TestValidateNode(t *testing.T) {
 				},
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
+				PodCIDR: "192.168.0.0/16",
-				PodCIDR:    "192.168.0.0/16",
 			},
 		},
 	}
@@ -9479,9 +9468,6 @@ func TestValidateNode(t *testing.T) {
 					core.ResourceName(core.ResourceMemory): resource.MustParse("10G"),
 				},
 			},
-			Spec: core.NodeSpec{
-				ExternalID: "external",
-			},
 		},
 		"invalid-labels": {
 			ObjectMeta: metav1.ObjectMeta{
@@ -9494,28 +9480,12 @@ func TestValidateNode(t *testing.T) {
 					core.ResourceName(core.ResourceMemory): resource.MustParse("10G"),
 				},
 			},
-			Spec: core.NodeSpec{
-				ExternalID: "external",
-			},
-		},
-		"missing-external-id": {
-			ObjectMeta: metav1.ObjectMeta{
-				Name:   "abc-123",
-				Labels: validSelector,
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName(core.ResourceCPU):    resource.MustParse("10"),
-					core.ResourceName(core.ResourceMemory): resource.MustParse("10G"),
-				},
-			},
 		},
 		"missing-taint-key": {
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "dedicated-node1",
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
 				// Add a taint with an empty key to a node
 				Taints: []core.Taint{{Key: "", Value: "special-user-1", Effect: "NoSchedule"}},
 			},
@@ -9525,7 +9495,6 @@ func TestValidateNode(t *testing.T) {
 				Name: "dedicated-node1",
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
 				// Add a taint with an invalid  key to a node
 				Taints: []core.Taint{{Key: "NoUppercaseOrSpecialCharsLike=Equals", Value: "special-user-1", Effect: "NoSchedule"}},
 			},
@@ -9544,7 +9513,6 @@ func TestValidateNode(t *testing.T) {
 				},
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
 				// Add a taint with a bad value to a node
 				Taints: []core.Taint{{Key: "dedicated", Value: "some\\bad\\value", Effect: "NoSchedule"}},
 			},
@@ -9563,7 +9531,6 @@ func TestValidateNode(t *testing.T) {
 				},
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
 				// Add a taint with an empty effect to a node
 				Taints: []core.Taint{{Key: "dedicated", Value: "special-user-3", Effect: ""}},
 			},
@@ -9582,7 +9549,6 @@ func TestValidateNode(t *testing.T) {
 				},
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
 				// Add a taint with NoExecute effect to a node
 				Taints: []core.Taint{{Key: "dedicated", Value: "special-user-3", Effect: "NoScheduleNoAdmit"}},
 			},
@@ -9592,7 +9558,6 @@ func TestValidateNode(t *testing.T) {
 				Name: "dedicated-node1",
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
 				// Add two taints to the node with the same key and effect; should be rejected.
 				Taints: []core.Taint{
 					{Key: "dedicated", Value: "special-user-1", Effect: "NoSchedule"},
@@ -9622,9 +9587,6 @@ func TestValidateNode(t *testing.T) {
 					core.ResourceName(core.ResourceMemory): resource.MustParse("0"),
 				},
 			},
-			Spec: core.NodeSpec{
-				ExternalID: "external",
-			},
 		},
 		"invalid-podController": {
 			ObjectMeta: metav1.ObjectMeta{
@@ -9657,9 +9619,6 @@ func TestValidateNode(t *testing.T) {
 					core.ResourceName(core.ResourceMemory): resource.MustParse("0"),
 				},
 			},
-			Spec: core.NodeSpec{
-				ExternalID: "external",
-			},
 		},
 		"multiple-pre-allocated-hugepages": {
 			ObjectMeta: metav1.ObjectMeta{
@@ -9678,9 +9637,6 @@ func TestValidateNode(t *testing.T) {
 					core.ResourceName("hugepages-1Gi"):     resource.MustParse("10Gi"),
 				},
 			},
-			Spec: core.NodeSpec{
-				ExternalID: "external",
-			},
 		},
 		"invalid-pod-cidr": {
 			ObjectMeta: metav1.ObjectMeta{
@@ -9696,8 +9652,7 @@ func TestValidateNode(t *testing.T) {
 				},
 			},
 			Spec: core.NodeSpec{
-				ExternalID: "external",
+				PodCIDR: "192.168.0.0",
-				PodCIDR:    "192.168.0.0",
 			},
 		},
 	}

pkg/cloudprovider/providers/azure/azure_vmss.go

@@ -161,7 +161,7 @@ func (ss *scaleSet) GetInstanceIDByNodeName(name string) (string, error) {
 // GetNodeNameByProviderID gets the node name by provider ID.
 func (ss *scaleSet) GetNodeNameByProviderID(providerID string) (types.NodeName, error) {
 	// NodeName is not part of providerID for vmss instances.
-	scaleSetName, err := extractScaleSetNameByExternalID(providerID)
+	scaleSetName, err := extractScaleSetNameByProviderID(providerID)
 	if err != nil {
 		glog.V(4).Infof("Can not extract scale set name from providerID (%s), assuming it is mananaged by availability set: %v", providerID, err)
 		return ss.availabilitySet.GetNodeNameByProviderID(providerID)
@@ -295,9 +295,9 @@ func getScaleSetVMInstanceID(machineName string) (string, error) {
 	return fmt.Sprintf("%d", instanceID), nil
 }
 
-// extractScaleSetNameByExternalID extracts the scaleset name by node's externalID.
+// extractScaleSetNameByProviderID extracts the scaleset name by node's ProviderID.
-func extractScaleSetNameByExternalID(externalID string) (string, error) {
+func extractScaleSetNameByProviderID(providerID string) (string, error) {
-	matches := scaleSetNameRE.FindStringSubmatch(externalID)
+	matches := scaleSetNameRE.FindStringSubmatch(providerID)
 	if len(matches) != 2 {
 		return "", ErrorNotVmssInstance
 	}
@@ -612,7 +612,7 @@ func (ss *scaleSet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, back
 	// Construct instanceIDs from nodes.
 	instanceIDs := []string{}
 	for _, curNode := range nodes {
-		curScaleSetName, err := extractScaleSetNameByExternalID(curNode.Spec.ExternalID)
+		curScaleSetName, err := extractScaleSetNameByProviderID(curNode.Spec.ProviderID)
 		if err != nil {
 			glog.V(4).Infof("Node %q is not belonging to any scale sets, omitting it", curNode.Name)
 			continue
@@ -622,9 +622,9 @@ func (ss *scaleSet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, back
 			continue
 		}
 
-		instanceID, err := getLastSegment(curNode.Spec.ExternalID)
+		instanceID, err := getLastSegment(curNode.Spec.ProviderID)
 		if err != nil {
-			glog.Errorf("Failed to get last segment from %q: %v", curNode.Spec.ExternalID, err)
+			glog.Errorf("Failed to get last segment from %q: %v", curNode.Spec.ProviderID, err)
 			return err
 		}
 

pkg/cloudprovider/providers/gce/gce_instances.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	"strconv"
 	"strings"
 	"time"
 
@@ -155,27 +154,6 @@ func (gce *GCECloud) InstanceTypeByProviderID(ctx context.Context, providerID st
 	return instance.Type, nil
 }
 
-// ExternalID returns the cloud provider ID of the node with the specified NodeName (deprecated).
-func (gce *GCECloud) ExternalID(ctx context.Context, nodeName types.NodeName) (string, error) {
-	instanceName := mapNodeNameToInstanceName(nodeName)
-	if gce.useMetadataServer {
-		// Use metadata, if possible, to fetch ID. See issue #12000
-		if gce.isCurrentInstance(instanceName) {
-			externalInstanceID, err := getCurrentExternalIDViaMetadata()
-			if err == nil {
-				return externalInstanceID, nil
-			}
-		}
-	}
-
-	// Fallback to GCE API call if metadata server fails to retrieve ID
-	inst, err := gce.getInstanceByName(instanceName)
-	if err != nil {
-		return "", err
-	}
-	return strconv.FormatUint(inst.ID, 10), nil
-}
-
 // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running.
 // If false is returned with no error, the instance will be immediately deleted by the cloud controller manager.
 func (gce *GCECloud) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) {
@@ -516,14 +494,6 @@ func getInstanceIDViaMetadata() (string, error) {
 	return parts[0], nil
 }
 
-func getCurrentExternalIDViaMetadata() (string, error) {
-	externalID, err := metadata.Get("instance/id")
-	if err != nil {
-		return "", fmt.Errorf("couldn't get external ID: %v", err)
-	}
-	return externalID, nil
-}
-
 func getCurrentMachineTypeViaMetadata() (string, error) {
 	mType, err := metadata.Get("instance/machine-type")
 	if err != nil {

pkg/controller/cloud/node_controller.go

@@ -152,7 +152,7 @@ func (cnc *CloudNodeController) updateNodeAddress(node *v1.Node, instances cloud
 		return
 	}
 	// Node that isn't present according to the cloud provider shouldn't have its address updated
-	exists, err := ensureNodeExistsByProviderIDOrExternalID(instances, node)
+	exists, err := ensureNodeExistsByProviderIDOrInstanceID(instances, node)
 	if err != nil {
 		// Continue to update node address when not sure the node is not exists
 		glog.Errorf("%v", err)
@@ -245,7 +245,7 @@ func (cnc *CloudNodeController) MonitorNode() {
 			if currentReadyCondition.Status != v1.ConditionTrue {
 				// Check with the cloud provider to see if the node still exists. If it
 				// doesn't, delete the node immediately.
-				exists, err := ensureNodeExistsByProviderIDOrExternalID(instances, node)
+				exists, err := ensureNodeExistsByProviderIDOrInstanceID(instances, node)
 				if err != nil {
 					glog.Errorf("Error getting data for node %s from cloud: %v", node.Name, err)
 					continue
@@ -412,13 +412,13 @@ func excludeTaintFromList(taints []v1.Taint, toExclude v1.Taint) []v1.Taint {
 	return newTaints
 }
 
-// ensureNodeExistsByProviderIDOrExternalID first checks if the instance exists by the provider id and then by calling external id with node name
+// ensureNodeExistsByProviderIDOrInstanceID first checks if the instance exists by the provider id and then by calling instance id with node name
-func ensureNodeExistsByProviderIDOrExternalID(instances cloudprovider.Instances, node *v1.Node) (bool, error) {
+func ensureNodeExistsByProviderIDOrInstanceID(instances cloudprovider.Instances, node *v1.Node) (bool, error) {
 	exists, err := instances.InstanceExistsByProviderID(context.TODO(), node.Spec.ProviderID)
 	if err != nil {
 		providerIDErr := err
 		_, err = instances.InstanceID(context.TODO(), types.NodeName(node.Name))
-		//<anupn> Changing the check as InstanceID does not return error
+		//TODO(anupn): Changing the check as InstanceID does not return error
 		if err == nil {
 			return false, nil
 		}

pkg/controller/cloud/node_controller_test.go

@@ -128,7 +128,7 @@ func TestEnsureNodeExistsByProviderIDOrNodeName(t *testing.T) {
 			}
 
 			instances, _ := fc.Instances()
-			exists, err := ensureNodeExistsByProviderIDOrExternalID(instances, tc.node)
+			exists, err := ensureNodeExistsByProviderIDOrInstanceID(instances, tc.node)
 			assert.NoError(t, err)
 			assert.EqualValues(t, tc.expectedCalls, fc.Calls,
 				"expected cloud provider methods `%v` to be called but `%v` was called ",

pkg/controller/nodelifecycle/node_lifecycle_controller_test.go

@@ -1543,9 +1543,6 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) {
 								v1.ResourceName(v1.ResourceMemory): resource.MustParse("10G"),
 							},
 						},
-						Spec: v1.NodeSpec{
-							ExternalID: "node0",
-						},
 					},
 				},
 				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
@@ -1620,9 +1617,6 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) {
 							v1.ResourceName(v1.ResourceMemory): resource.MustParse("10G"),
 						},
 					},
-					Spec: v1.NodeSpec{
-						ExternalID: "node0",
-					},
 				},
 			},
 		},
@@ -1651,9 +1645,6 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) {
 								v1.ResourceName(v1.ResourceMemory): resource.MustParse("10G"),
 							},
 						},
-						Spec: v1.NodeSpec{
-							ExternalID: "node0",
-						},
 					},
 				},
 				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
@@ -1755,9 +1746,6 @@ func TestMonitorNodeStatusMarkPodsNotReady(t *testing.T) {
 								v1.ResourceName(v1.ResourceMemory): resource.MustParse("10G"),
 							},
 						},
-						Spec: v1.NodeSpec{
-							ExternalID: "node0",
-						},
 					},
 				},
 				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
@@ -1789,9 +1777,6 @@ func TestMonitorNodeStatusMarkPodsNotReady(t *testing.T) {
 								v1.ResourceName(v1.ResourceMemory): resource.MustParse("10G"),
 							},
 						},
-						Spec: v1.NodeSpec{
-							ExternalID: "node0",
-						},
 					},
 				},
 				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
@@ -2208,9 +2193,6 @@ func TestNodeEventGeneration(t *testing.T) {
 					UID:               "1234567890",
 					CreationTimestamp: metav1.Date(2015, 8, 10, 0, 0, 0, 0, time.UTC),
 				},
-				Spec: v1.NodeSpec{
-					ExternalID: "node0",
-				},
 				Status: v1.NodeStatus{
 					Conditions: []v1.NodeCondition{
 						{

pkg/controller/testutil/test_utils.go

@@ -421,9 +421,6 @@ func NewFakeRecorder() *FakeRecorder {
 func NewNode(name string) *v1.Node {
 	return &v1.Node{
 		ObjectMeta: metav1.ObjectMeta{Name: name},
-		Spec: v1.NodeSpec{
-			ExternalID: name,
-		},
 		Status: v1.NodeStatus{
 			Capacity: v1.ResourceList{
 				v1.ResourceName(v1.ResourceCPU):    resource.MustParse("10"),

pkg/controller/volume/attachdetach/testing/testvolumespec.go

@@ -153,7 +153,6 @@ func CreateTestClient() *fake.Clientset {
 						},
 					},
 				},
-				Spec: v1.NodeSpec{ExternalID: string(nodeName)},
 			}
 			obj.Items = append(obj.Items, node)
 		}

pkg/kubectl/cmd/drain_test.go

@@ -70,9 +70,6 @@ func TestMain(m *testing.M) {
 			Name:              "node",
 			CreationTimestamp: metav1.Time{Time: time.Now()},
 		},
-		Spec: corev1.NodeSpec{
-			ExternalID: "node",
-		},
 		Status: corev1.NodeStatus{},
 	}
 

pkg/kubectl/cmd/resource/get_test.go

@@ -867,9 +867,6 @@ func TestGetMultipleTypeObjectsWithDirectReference(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "foo",
 		},
-		Spec: api.NodeSpec{
-			ExternalID: "ext",
-		},
 	}
 
 	tf := cmdtesting.NewTestFactory()

pkg/kubectl/cmd/taint_test.go

@@ -45,8 +45,7 @@ func generateNodeAndTaintedNode(oldTaints []v1.Taint, newTaints []v1.Taint) (*v1
 			CreationTimestamp: metav1.Time{Time: time.Now()},
 		},
 		Spec: v1.NodeSpec{
-			ExternalID: "node-name",
+			Taints: oldTaints,
-			Taints:     oldTaints,
 		},
 		Status: v1.NodeStatus{},
 	}

pkg/kubectl/resource/builder_test.go

@@ -1067,7 +1067,7 @@ func TestSingleItemImpliedObjectNoExtension(t *testing.T) {
 }
 
 func TestSingleItemImpliedRootScopedObject(t *testing.T) {
-	node := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "test"}, Spec: v1.NodeSpec{ExternalID: "test"}}
+	node := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "test"}}
 	r := streamTestObject(node)
 	infos, err := newDefaultBuilder().
 		NamespaceParam("test").DefaultNamespace().

pkg/kubelet/kubelet_node_status.go

@@ -123,34 +123,21 @@ func (kl *Kubelet) tryRegisterWithAPIServer(node *v1.Node) bool {
 		return false
 	}
 
-	if existingNode.Spec.ExternalID == node.Spec.ExternalID {
+	glog.Infof("Node %s was previously registered", kl.nodeName)
-		glog.Infof("Node %s was previously registered", kl.nodeName)
 
-		// Edge case: the node was previously registered; reconcile
+	// Edge case: the node was previously registered; reconcile
-		// the value of the controller-managed attach-detach
+	// the value of the controller-managed attach-detach
-		// annotation.
+	// annotation.
-		requiresUpdate := kl.reconcileCMADAnnotationWithExistingNode(node, existingNode)
+	requiresUpdate := kl.reconcileCMADAnnotationWithExistingNode(node, existingNode)
-		requiresUpdate = kl.updateDefaultLabels(node, existingNode) || requiresUpdate
+	requiresUpdate = kl.updateDefaultLabels(node, existingNode) || requiresUpdate
-		if requiresUpdate {
+	if requiresUpdate {
-			if _, _, err := nodeutil.PatchNodeStatus(kl.kubeClient.CoreV1(), types.NodeName(kl.nodeName), originalNode, existingNode); err != nil {
+		if _, _, err := nodeutil.PatchNodeStatus(kl.kubeClient.CoreV1(), types.NodeName(kl.nodeName), originalNode, existingNode); err != nil {
-				glog.Errorf("Unable to reconcile node %q with API server: error updating node: %v", kl.nodeName, err)
+			glog.Errorf("Unable to reconcile node %q with API server: error updating node: %v", kl.nodeName, err)
-				return false
+			return false
-			}
 		}
-
-		return true
 	}
 
-	glog.Errorf("Previously node %q had externalID %q; now it is %q; will delete and recreate.",
+	return true
-		kl.nodeName, node.Spec.ExternalID, existingNode.Spec.ExternalID,
-	)
-	if err := kl.kubeClient.CoreV1().Nodes().Delete(node.Name, nil); err != nil {
-		glog.Errorf("Unable to register node %q with API server: error deleting old node: %v", kl.nodeName, err)
-	} else {
-		glog.Infof("Deleted old node object %q", kl.nodeName)
-	}
-
-	return false
 }
 
 // updateDefaultLabels will set the default labels on the node
@@ -300,18 +287,10 @@ func (kl *Kubelet) initialNode() (*v1.Node, error) {
 			return nil, fmt.Errorf("failed to get instances from cloud provider")
 		}
 
-		// TODO(roberthbailey): Can we do this without having credentials to talk
-		// to the cloud provider?
-		// ExternalID is deprecated, so ProviderID is retrieved using InstanceID
-		externalID, err := instances.InstanceID(context.TODO(), kl.nodeName)
-		if err != nil {
-			return nil, fmt.Errorf("failed to get external ID from cloud provider: %v", err)
-		}
-		node.Spec.ExternalID = externalID
-
 		// TODO: We can't assume that the node has credentials to talk to the
 		// cloudprovider from arbitrary nodes. At most, we should talk to a
 		// local metadata server here.
+		var err error
 		if node.Spec.ProviderID == "" {
 			node.Spec.ProviderID, err = cloudprovider.GetInstanceProviderID(context.TODO(), kl.cloud, kl.nodeName)
 			if err != nil {
@@ -343,9 +322,8 @@ func (kl *Kubelet) initialNode() (*v1.Node, error) {
 				node.ObjectMeta.Labels[kubeletapis.LabelZoneRegion] = zone.Region
 			}
 		}
-	} else {
-		node.Spec.ExternalID = kl.hostname
 	}
+
 	kl.setNodeStatus(node)
 
 	return node, nil

pkg/kubelet/kubelet_node_status_test.go

@@ -912,7 +912,6 @@ func TestRegisterWithApiServer(t *testing.T) {
 					kubeletapis.LabelArch:     goruntime.GOARCH,
 				},
 			},
-			Spec: v1.NodeSpec{ExternalID: testKubeletHostname},
 		}, nil
 	})
 	kubeClient.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {
@@ -966,7 +965,7 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 		ErrStatus: metav1.Status{Reason: metav1.StatusReasonConflict},
 	}
 
-	newNode := func(cmad bool, externalID string) *v1.Node {
+	newNode := func(cmad bool) *v1.Node {
 		node := &v1.Node{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{
@@ -975,9 +974,6 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 					kubeletapis.LabelArch:     goruntime.GOARCH,
 				},
 			},
-			Spec: v1.NodeSpec{
-				ExternalID: externalID,
-			},
 		}
 
 		if cmad {
@@ -1010,17 +1006,17 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 		},
 		{
 			name:            "success case - existing node - no change in CMAD",
-			newNode:         newNode(true, "a"),
+			newNode:         newNode(true),
 			createError:     alreadyExists,
-			existingNode:    newNode(true, "a"),
+			existingNode:    newNode(true),
 			expectedResult:  true,
 			expectedActions: 2,
 		},
 		{
 			name:            "success case - existing node - CMAD disabled",
-			newNode:         newNode(false, "a"),
+			newNode:         newNode(false),
 			createError:     alreadyExists,
-			existingNode:    newNode(true, "a"),
+			existingNode:    newNode(true),
 			expectedResult:  true,
 			expectedActions: 3,
 			testSavedNode:   true,
@@ -1029,33 +1025,25 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 		},
 		{
 			name:            "success case - existing node - CMAD enabled",
-			newNode:         newNode(true, "a"),
+			newNode:         newNode(true),
 			createError:     alreadyExists,
-			existingNode:    newNode(false, "a"),
+			existingNode:    newNode(false),
 			expectedResult:  true,
 			expectedActions: 3,
 			testSavedNode:   true,
 			savedNodeIndex:  2,
 			savedNodeCMAD:   true,
 		},
-		{
-			name:            "success case - external ID changed",
-			newNode:         newNode(false, "b"),
-			createError:     alreadyExists,
-			existingNode:    newNode(false, "a"),
-			expectedResult:  false,
-			expectedActions: 3,
-		},
 		{
 			name:            "create failed",
-			newNode:         newNode(false, "b"),
+			newNode:         newNode(false),
 			createError:     conflict,
 			expectedResult:  false,
 			expectedActions: 1,
 		},
 		{
 			name:            "get existing node failed",
-			newNode:         newNode(false, "a"),
+			newNode:         newNode(false),
 			createError:     alreadyExists,
 			getError:        conflict,
 			expectedResult:  false,
@@ -1063,22 +1051,13 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 		},
 		{
 			name:            "update existing node failed",
-			newNode:         newNode(false, "a"),
+			newNode:         newNode(false),
 			createError:     alreadyExists,
-			existingNode:    newNode(true, "a"),
+			existingNode:    newNode(true),
 			patchError:      conflict,
 			expectedResult:  false,
 			expectedActions: 3,
 		},
-		{
-			name:            "delete existing node failed",
-			newNode:         newNode(false, "b"),
-			createError:     alreadyExists,
-			existingNode:    newNode(false, "a"),
-			deleteError:     conflict,
-			expectedResult:  false,
-			expectedActions: 3,
-		},
 	}
 
 	notImplemented := func(action core.Action) (bool, runtime.Object, error) {

pkg/kubelet/kubelet_volumes_test.go

@@ -283,7 +283,6 @@ func TestVolumeAttachAndMountControllerEnabled(t *testing.T) {
 							DevicePath: "fake/path",
 						},
 					}},
-				Spec: v1.NodeSpec{ExternalID: testKubeletHostname},
 			}, nil
 		})
 	kubeClient.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {
@@ -350,7 +349,6 @@ func TestVolumeUnmountAndDetachControllerEnabled(t *testing.T) {
 							DevicePath: "fake/path",
 						},
 					}},
-				Spec: v1.NodeSpec{ExternalID: testKubeletHostname},
 			}, nil
 		})
 	kubeClient.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {

pkg/kubelet/volumemanager/reconciler/reconciler_test.go

@@ -1033,7 +1033,6 @@ func createTestClient() *fake.Clientset {
 							DevicePath: "fake/path",
 						},
 					}},
-				Spec: v1.NodeSpec{ExternalID: string(nodeName)},
 			}, nil
 		})
 	fakeClient.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {

pkg/kubelet/volumemanager/volume_manager_test.go

@@ -250,7 +250,6 @@ func createObjects() (*v1.Node, *v1.Pod, *v1.PersistentVolume, *v1.PersistentVol
 					DevicePath: "fake/path",
 				},
 			}},
-		Spec: v1.NodeSpec{ExternalID: testHostname},
 	}
 	pod := &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{

pkg/printers/internalversion/describe.go

@@ -2710,9 +2710,6 @@ func describeNode(node *api.Node, nodeNonTerminatedPodsList *api.PodList, events
 		if len(node.Spec.PodCIDR) > 0 {
 			w.Write(LEVEL_0, "PodCIDR:\t%s\n", node.Spec.PodCIDR)
 		}
-		if len(node.Spec.ExternalID) > 0 {
-			w.Write(LEVEL_0, "ExternalID:\t%s\n", node.Spec.ExternalID)
-		}
 		if len(node.Spec.ProviderID) > 0 {
 			w.Write(LEVEL_0, "ProviderID:\t%s\n", node.Spec.ProviderID)
 		}

pkg/registry/core/node/storage/storage_test.go

@@ -55,9 +55,6 @@ func validNewNode() *api.Node {
 				"name": "foo",
 			},
 		},
-		Spec: api.NodeSpec{
-			ExternalID: "external",
-		},
 		Status: api.NodeStatus{
 			Capacity: api.ResourceList{
 				api.ResourceName(api.ResourceCPU):    resource.MustParse("10"),

pkg/volume/cinder/attacher_test.go

@@ -712,10 +712,6 @@ func (instances *instances) NodeAddressesByProviderID(ctx context.Context, provi
 	return []v1.NodeAddress{}, errors.New("Not implemented")
 }
 
-func (instances *instances) ExternalID(ctx context.Context, name types.NodeName) (string, error) {
-	return "", errors.New("Not implemented")
-}
-
 func (instances *instances) InstanceID(ctx context.Context, name types.NodeName) (string, error) {
 	return instances.instanceID, nil
 }

staging/src/k8s.io/api/core/v1/generated.proto

@@ -1887,11 +1887,6 @@ message NodeSpec {
   // +optional
   optional string podCIDR = 1;
 
-  // External ID of the node assigned by some machine database (e.g. a cloud provider).
-  // Deprecated.
-  // +optional
-  optional string externalID = 2;
-
   // ID of the node assigned by the cloud provider in the format: <ProviderName>://<ProviderSpecificNodeID>
   // +optional
   optional string providerID = 3;
@@ -1909,6 +1904,11 @@ message NodeSpec {
   // The DynamicKubeletConfig feature gate must be enabled for the Kubelet to use this field
   // +optional
   optional NodeConfigSource configSource = 6;
+
+  // Deprecated. Not all kubelets will set this field. Remove field after 1.13.
+  // see: https://issues.k8s.io/61966
+  // +optional
+  optional string externalID = 2;
 }
 
 // NodeStatus is information about the current status of a node.

staging/src/k8s.io/api/core/v1/types.go

@@ -3612,10 +3612,6 @@ type NodeSpec struct {
 	// PodCIDR represents the pod IP range assigned to the node.
 	// +optional
 	PodCIDR string `json:"podCIDR,omitempty" protobuf:"bytes,1,opt,name=podCIDR"`
-	// External ID of the node assigned by some machine database (e.g. a cloud provider).
-	// Deprecated.
-	// +optional
-	ExternalID string `json:"externalID,omitempty" protobuf:"bytes,2,opt,name=externalID"`
 	// ID of the node assigned by the cloud provider in the format: <ProviderName>://<ProviderSpecificNodeID>
 	// +optional
 	ProviderID string `json:"providerID,omitempty" protobuf:"bytes,3,opt,name=providerID"`
@@ -3630,6 +3626,11 @@ type NodeSpec struct {
 	// The DynamicKubeletConfig feature gate must be enabled for the Kubelet to use this field
 	// +optional
 	ConfigSource *NodeConfigSource `json:"configSource,omitempty" protobuf:"bytes,6,opt,name=configSource"`
+
+	// Deprecated. Not all kubelets will set this field. Remove field after 1.13.
+	// see: https://issues.k8s.io/61966
+	// +optional
+	DoNotUse_ExternalID string `json:"externalID,omitempty" protobuf:"bytes,2,opt,name=externalID"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go

@@ -1045,11 +1045,11 @@ func (NodeSelectorTerm) SwaggerDoc() map[string]string {
 var map_NodeSpec = map[string]string{
 	"":              "NodeSpec describes the attributes that a node is created with.",
 	"podCIDR":       "PodCIDR represents the pod IP range assigned to the node.",
-	"externalID":    "External ID of the node assigned by some machine database (e.g. a cloud provider). Deprecated.",
 	"providerID":    "ID of the node assigned by the cloud provider in the format: <ProviderName>://<ProviderSpecificNodeID>",
 	"unschedulable": "Unschedulable controls node schedulability of new pods. By default, node is schedulable. More info: https://kubernetes.io/docs/concepts/nodes/node/#manual-node-administration",
 	"taints":        "If specified, the node's taints.",
 	"configSource":  "If specified, the source to get node configuration from The DynamicKubeletConfig feature gate must be enabled for the Kubelet to use this field",
+	"externalID":    "Deprecated. Not all kubelets will set this field. Remove field after 1.13. see: https://issues.k8s.io/61966",
 }
 
 func (NodeSpec) SwaggerDoc() map[string]string {

test/e2e/instrumentation/logging/utils/misc.go

@@ -26,7 +26,7 @@ func GetNodeIds(cs clientset.Interface) []string {
 	nodes := framework.GetReadySchedulableNodesOrDie(cs)
 	nodeIds := []string{}
 	for _, n := range nodes.Items {
-		nodeIds = append(nodeIds, n.Spec.ExternalID)
+		nodeIds = append(nodeIds, n.Name)
 	}
 	return nodeIds
 }

test/integration/framework/perf_utils.go

@@ -56,10 +56,6 @@ func (p *IntegrationTestNodePreparer) PrepareNodes() error {
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: p.nodeNamePrefix,
 		},
-		Spec: v1.NodeSpec{
-			// TODO: investigate why this is needed.
-			ExternalID: "foo",
-		},
 		Status: v1.NodeStatus{
 			Capacity: v1.ResourceList{
 				v1.ResourcePods:   *resource.NewQuantity(110, resource.DecimalSI),

test/integration/ipamperf/util.go

@@ -39,9 +39,6 @@ var (
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "sample-node-",
 		},
-		Spec: v1.NodeSpec{
-			ExternalID: "foo",
-		},
 		Status: v1.NodeStatus{
 			Capacity: v1.ResourceList{
 				v1.ResourcePods:   *resource.NewQuantity(110, resource.DecimalSI),

test/integration/scheduler_perf/scheduler_test.go

@@ -50,10 +50,6 @@ var (
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "sample-node-",
 		},
-		Spec: v1.NodeSpec{
-			// TODO: investigate why this is needed.
-			ExternalID: "foo",
-		},
 		Status: v1.NodeStatus{
 			Capacity: v1.ResourceList{
 				v1.ResourcePods:   *resource.NewQuantity(110, resource.DecimalSI),