Merge pull request #9910 from justinsb/instanceid_contract

pkg/cloudprovider/aws/aws.go

@@ -588,13 +588,17 @@ func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) {
 }
 
 // ExternalID returns the cloud provider ID of the specified instance (deprecated).
+// Note that if the instance does not exist or is no longer running, we must return ("", cloudprovider.InstanceNotFound)
 func (aws *AWSCloud) ExternalID(name string) (string, error) {
-	// TODO: Do we need to verify it exists, or can we just return name
-	inst, err := aws.getInstanceById(name)
+	// We must verify that the instance still exists
+	instance, err := aws.getInstanceById(name)
 	if err != nil {
 		return "", err
 	}
-	return orEmpty(inst.InstanceID), nil
+	if instance == nil || !isAlive(instance) {
+		return "", cloudprovider.InstanceNotFound
+	}
+	return orEmpty(instance.InstanceID), nil
 }
 
 // InstanceID returns the cloud provider ID of the specified instance.

pkg/cloudprovider/cloud.go

@@ -103,6 +103,7 @@ type Instances interface {
 	// ExternalID returns the cloud provider ID of the specified instance (deprecated).
 	ExternalID(name string) (string, error)
 	// InstanceID returns the cloud provider ID of the specified instance.
+	// Note that if the instance does not exist or is no longer running, we must return ("", cloudprovider.InstanceNotFound)
 	InstanceID(name string) (string, error)
 	// List lists instances that match 'filter' which is a regular expression which must match the entire instance name (fqdn)
 	List(filter string) ([]string, error)