kubelet: reading cloudinfo from cadvisor #21373

enoodle · 2016-02-17T10:14:14Z

When no --cloud-provider flag is given, try to use data from cadvisor to
determine the ProviderID

googlebot · 2016-02-17T10:14:17Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

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 your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

k8s-bot · 2016-02-17T10:15:13Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

k8s-bot · 2016-02-17T10:16:12Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

enoodle · 2016-02-17T10:16:51Z

This depends on google/cadvisor#1109

enoodle · 2016-02-17T10:41:52Z

@deads2k I could use your help with this patch. It is small but depends on another patch of cAdvisor.

deads2k · 2016-02-17T13:42:48Z

ok to test

deads2k · 2016-02-17T13:44:48Z

@googlebot @enoodle should be covered under the Red Hat CLA.

deads2k · 2016-02-17T13:53:53Z

pkg/kubelet/kubelet.go

@@ -2786,6 +2786,17 @@ func (kl *Kubelet) setNodeAddress(node *api.Node) error {
 	return nil
 }

+func (kl *Kubelet) updateCloudProviderFromMachineInfo(
+	node *api.Node,


no need for these newlines.

@enoodle ^^

eparis · 2016-02-17T13:54:47Z

@deads2k you need to let myself/clayton/andy know. I added @enoodle to the CLA group.
@googlebot he signed it!

eparis · 2016-02-17T13:55:24Z

ok to test

deads2k · 2016-02-17T13:55:54Z

pkg/kubelet/kubelet.go

+	if info.CloudProvider != cadvisorapi.UnkownProvider &&
+		info.CloudProvider != cadvisorapi.Baremetal {
+		node.Spec.ProviderID = strings.ToLower(string(info.CloudProvider)) +
+			":////" + string(info.InstanceID)


My lack of cadvisor knowledge shows here. Is this guaranteed to have a value? Setting it to ://// seems less than ideal.

Sorry, I probably should have documented it.
If info.CloudProvider will always have a non empty value (it has a set of defined values in info/v1/machine.go. If it is not UnkownProvider (typo in code, will fix here) nor Baremetal then it is the name of a real cloud service. Same thing goes for info.InstanceID , it will be receive its value from a function that is supposed to return "Unknown" if there was an error while retrieving the value.
":////" is to conform with kubernetes's cloudproviders (pkg/cloudprovider/providers/*) that will write to ProviderID if cloud-provider flag is given to kubelet. The most verbose one write in the form of "cloudprovider://project/availability_zone/instance_name". Here I dont have project name or availability zone so I return them empty.

@enoodle Can you add some comments explaining this?

googlebot · 2016-02-18T11:10:12Z

CLAs look good, thanks!

k8s-bot · 2016-02-23T21:15:14Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

deads2k · 2016-02-24T21:22:21Z

@enoodle you'll need to bring in the new bump so this can pass tests, right?

vishh · 2016-02-24T21:43:07Z

FYI: A new bump has to be a cherrypick on
google/cadvisor/tree/release-0.21. We want to restrict/audit changes to
cadvisor since we are stabilizing kube for v1.2 release.

On Wed, Feb 24, 2016 at 1:22 PM, David Eads notifications@github.com
wrote:

@enoodle https://github.com/enoodle you'll need to bring in the new
bump so this can pass tests, right?

—
Reply to this email directly or view it on GitHub
#21373 (comment)
.

enoodle · 2016-02-25T13:21:31Z

@vishh @deads2k I am not 100% clear on the process of cherrypick to bump this. Should I make a PR in cadvisor to cherrypick google/cadvisor#1109 to google/cadvisor/tree/release-0.21 and then add a commit here that bumps kubernetes's cadvisor t 0.21 ?

deads2k · 2016-02-25T13:49:25Z

Should I make a PR in cadvisor to cherrypick google/cadvisor#1109 to google/cadvisor/tree/release-0.21 and then add a commit here that bumps kubernetes's cadvisor t 0.21 ?

Based on how we do it for other godeps, yes. The one wrinkle is that you may need a new cadvisor tag cut to get the comment section in godeps.json to look clean. Ask in your cherry-pick pull.

enoodle · 2016-02-28T09:26:57Z

I ended up waiting on 0.22 :)

ixdy · 2016-03-03T05:45:31Z

ok to test

I really don't know what's wrong here.

enoodle · 2016-03-03T06:00:20Z

@ixdy could it be something in the patch? Can you take a look at the diff and see if something pops up? It's only 16 lines. Thanks.

deads2k · 2016-03-03T13:07:45Z

@vishh it's a small patch that adds some significant utility. Can you review for 1.2?

vishh · 2016-03-03T18:34:37Z

pkg/kubelet/kubelet.go

@@ -1089,6 +1089,11 @@ func (kl *Kubelet) initialNodeStatus() (*api.Node, error) {
 		}
 	} else {
 		node.Spec.ExternalID = kl.hostname
+		// If no cloud provider is defined - use the one detected by cadvisor
+		info, err := kl.GetCachedMachineInfo()


FYI @roberthbailey: cAdvisor is already using the local metadata server to access GCE specific information. Relevant logic here.

vishh · 2016-03-03T18:43:37Z

What is the meta issue that this PR is trying to solve? I see that kubelet can now autodetect cloud provider. What other features will this PR enable?

enoodle · 2016-03-04T08:38:00Z

@vishh Mainly this one feature. This will also detect the instance name with the cloud provider allowing users to run specific code on the cloud provider platform regarding this specific instance.

bgrant0607 · 2016-03-04T14:58:17Z

Punting out of 1.2

enoodle · 2016-05-19T09:34:59Z

@vishh Can we take another look at this? It will be nice to automatically be able to know on which cloud we are running and also its currently the only way to work identify Azure as they don't have any cloud provider. This will allow for easier interaction with cross-cloud deployments that use Azure.

vishh · 2016-05-19T19:09:56Z

@enoodle Can you address the existing comments? This PR is good to go once you fix them.

enoodle · 2016-05-20T06:13:41Z

@vishh Thanks, I updated the PR.

When no --cloud-provider flag is given, try to use data from cadvisor to determine the current cloud provider.

enoodle · 2016-05-26T12:03:02Z

@vishh ping. sorry for the nagging, just don't want this to be forgotten :)

vishh · 2016-05-27T21:34:12Z

@enoodle LGTM

k8s-bot · 2016-05-27T22:28:33Z

GCE e2e build/test passed for commit 7fb82d5.

googlebot added the cla: no label Feb 17, 2016

k8s-github-robot assigned eparis Feb 17, 2016

k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 17, 2016

enoodle changed the title ~~kubelet: reading cloudinfo from cadvisor~~ [WIP] kubelet: reading cloudinfo from cadvisor Feb 17, 2016

deads2k reviewed Feb 17, 2016
View reviewed changes

deads2k mentioned this pull request Feb 17, 2016

cloudinfo: Adding AWS, Azure support and InstanceID google/cadvisor#1109

Merged

enoodle force-pushed the read_cadvisor_cloudinfo_in_kubelet branch from 8206a53 to 90d478e Compare February 18, 2016 11:10

googlebot added cla: yes and removed cla: no labels Feb 18, 2016

bgrant0607 assigned vishh and unassigned eparis Feb 18, 2016

enoodle changed the title ~~[WIP] kubelet: reading cloudinfo from cadvisor~~ kubelet: reading cloudinfo from cadvisor Mar 1, 2016

k8s-github-robot added the needs-ok-to-merge label Mar 3, 2016

deads2k added this to the v1.2-candidate milestone Mar 3, 2016

vishh reviewed Mar 3, 2016
View reviewed changes

bgrant0607 removed this from the v1.2-candidate milestone Mar 4, 2016

deads2k mentioned this pull request Mar 15, 2016

Finalize Kube items 3.2 openshift/origin#6766

Closed

85 tasks

k8s-github-robot added the release-note-label-needed label Mar 31, 2016

vishh added the ok-to-merge label May 19, 2016

k8s-github-robot removed the needs-ok-to-merge label May 19, 2016

enoodle force-pushed the read_cadvisor_cloudinfo_in_kubelet branch from 915143c to 5ad12d9 Compare May 20, 2016 06:12

kubelet: reading cloudinfo from cadvisor

7fb82d5

When no --cloud-provider flag is given, try to use data from cadvisor to determine the current cloud provider.

enoodle force-pushed the read_cadvisor_cloudinfo_in_kubelet branch from 5ad12d9 to 7fb82d5 Compare May 22, 2016 15:42

vishh added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels May 27, 2016

vishh added this to the v1.3 milestone May 27, 2016

a-robinson merged commit 91f8c78 into kubernetes:master May 27, 2016

mfojtik mentioned this pull request May 31, 2016

Rebase openshift/origin#8856

Merged

23 tasks

yujuhong mentioned this pull request Jun 29, 2016

In 1.3, kubelet cloud-provider setting is not respected when --cloud-provider="" #28231

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubelet: reading cloudinfo from cadvisor #21373

kubelet: reading cloudinfo from cadvisor #21373

enoodle commented Feb 17, 2016

googlebot commented Feb 17, 2016

k8s-bot commented Feb 17, 2016

k8s-bot commented Feb 17, 2016

enoodle commented Feb 17, 2016

enoodle commented Feb 17, 2016

deads2k commented Feb 17, 2016

deads2k commented Feb 17, 2016

deads2k Feb 17, 2016

vishh May 19, 2016

eparis commented Feb 17, 2016

eparis commented Feb 17, 2016

deads2k Feb 17, 2016

enoodle Feb 17, 2016

vishh May 19, 2016

googlebot commented Feb 18, 2016

k8s-bot commented Feb 23, 2016

deads2k commented Feb 24, 2016

vishh commented Feb 24, 2016

enoodle commented Feb 25, 2016

deads2k commented Feb 25, 2016

enoodle commented Feb 28, 2016

ixdy commented Mar 3, 2016

enoodle commented Mar 3, 2016

deads2k commented Mar 3, 2016

vishh Mar 3, 2016

vishh commented Mar 3, 2016

enoodle commented Mar 4, 2016

bgrant0607 commented Mar 4, 2016

enoodle commented May 19, 2016

vishh commented May 19, 2016

enoodle commented May 20, 2016

enoodle commented May 26, 2016

vishh commented May 27, 2016

k8s-bot commented May 27, 2016

kubelet: reading cloudinfo from cadvisor #21373

kubelet: reading cloudinfo from cadvisor #21373

Conversation

enoodle commented Feb 17, 2016

googlebot commented Feb 17, 2016

k8s-bot commented Feb 17, 2016

k8s-bot commented Feb 17, 2016

enoodle commented Feb 17, 2016

enoodle commented Feb 17, 2016

deads2k commented Feb 17, 2016

deads2k commented Feb 17, 2016

deads2k Feb 17, 2016

Choose a reason for hiding this comment

vishh May 19, 2016

Choose a reason for hiding this comment

eparis commented Feb 17, 2016

eparis commented Feb 17, 2016

deads2k Feb 17, 2016

Choose a reason for hiding this comment

enoodle Feb 17, 2016

Choose a reason for hiding this comment

vishh May 19, 2016

Choose a reason for hiding this comment

googlebot commented Feb 18, 2016

k8s-bot commented Feb 23, 2016

deads2k commented Feb 24, 2016

vishh commented Feb 24, 2016

enoodle commented Feb 25, 2016

deads2k commented Feb 25, 2016

enoodle commented Feb 28, 2016

ixdy commented Mar 3, 2016

enoodle commented Mar 3, 2016

deads2k commented Mar 3, 2016

vishh Mar 3, 2016

Choose a reason for hiding this comment

vishh commented Mar 3, 2016

enoodle commented Mar 4, 2016

bgrant0607 commented Mar 4, 2016

enoodle commented May 19, 2016

vishh commented May 19, 2016

enoodle commented May 20, 2016

enoodle commented May 26, 2016

vishh commented May 27, 2016

k8s-bot commented May 27, 2016