@bgrant0607 for API thoughts.

This seems reasonable to me, but I'd prefer to land it with implementations for AWS and GCE (if not OpenStack and Vagrant)

Re. API:

1. This is the Node API. "Instance" is cloud jargon. As @brendandburns pointed out, we run on bare metal, too, so I don't want to couple the API too closely to cloud providers. I'd be happier with externalID, which would better match the description.
2. Status information should be reconstructable. We can rediscover container IDs from Docker. Do we expect all externalIDs to be similarly discoverable? It seems like that might not be true, in general, esp. for bare metal. Given that the external ID will be provided when the node is created and shouldn't change so long as the node exists, I think it makes more sense to be in the NodeSpec.

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

@bgrant0607 I implemented all the cloud providers but I guess I'll need other people's help to test them. I could verify just the ovirt one.

I'm punting at Brendan - I am out the second half of this week and crazy slammed the first half.

I'll test on vagrant after #4468 merges.

@bgrant0607 I'd prefer to keep ExternalID in Status vs Spec because I assume that a node can change ExternalID (e.g. vm cloned / decommissioned, etc.).

@simon3z A node shouldn't be able to change ExternalID. That would be a new node object. That said, Spec is updatable. Status must be 100% automatically reconstructable from other sources, and shouldn't be provided by the cluster administrator. Does ExternalID meet that requirement?

@bgrant0607 I moved it to Spec

API LGTM. Thanks.

What happens for providers for which this is not implemented?

FYI, maintainers are here:
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/MAINTAINERS.md
https://github.com/GoogleCloudPlatform/kubernetes/tree/master/docs/getting-started-guides

@derekwaynecarr for Vagrant
@doublerr for Rackspace
@justinsb for AWS
@jeffmendoza for Azure

@bgrant0607 if it's not implemented the error message (if any) is logged and the externalId won't be reported. Returning an empty string (and nil error) would be OK for providers that don't intend to expose the id.

So it looks like we're heading for a pattern where we add lots of methods to the cloud provider, where each method extracts one piece of information. My inclination would have been a pattern where we have a "GetInfo" call, which just gathers all the information. This seems like it would be more efficient (particularly as we start to collect more and more information) and simpler.

The AWS code looks fine, though I haven't tested it.

I think I generally agree with @justinsb on the GetInfo call, but the whole cloud provider stuff needs a refactor anyway, so I'm not going to block the PR on that. Merging.

if it's not implemented the error message (if any) is logged and the externalId won't be reported.
Returning an empty string (and nil error) would be OK for providers that don't intend to expose the id.

Sounds good.

@brendandburns if you agree with the GetInfo call refactoring I can start to work on that tomorrow (my final goal is to add another information and it seems that having separate class is not sustainable anymore). Thanks!

@brendandburns can you check my last question? do you have an answer for that?