Support status.hostIP in downward API #42717

andrewsykim · 2017-03-08T08:40:04Z

What this PR does / why we need it:
Exposes pod's hostIP (node IP) via downward API.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged):
fixes #24657

Special notes for your reviewer:
Not sure if there's more documentation that's needed, please point me in the right direction and I will add some :)

k8s-ci-robot · 2017-03-08T08:40:05Z

Hi @andrewsykim. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-reviewable · 2017-03-08T08:40:10Z

This change is

andrewsykim · 2017-03-13T20:46:56Z

pkg/fieldpath/fieldpath.go

@@ -41,11 +41,6 @@ func FormatMap(m map[string]string) (fmtStr string) {
 // ExtractFieldPathAsString extracts the field from the given object
 // and returns it as a string.  The object must be a pointer to an
 // API type.
-//
-// Currently, this API is limited to supporting the fieldpaths:


Seemed out of date

thockin · 2017-03-13T21:41:11Z

@k8s-bot ok to test

andrewsykim · 2017-03-14T19:44:35Z

@thockin seems like HOST_IP="" in the e2e test 🤔. Any ideas why?

thockin · 2017-03-15T04:09:01Z

Possible that there's a race between kubelet starting the pod and writing back the status?

andrewsykim · 2017-03-16T04:34:37Z

At a quick glance of the kubelet code it seems like the PodIP update should happen on the same UpdateStatus call as HostIP, PodIP is defined in the e2e tests so race condition seems unlikely, you definitely know the code better than me so correct me if I'm wrong please :).

Do you know if kubelet for E2E tests run on standalone mode (seems unlikely that it would)? It seems like the kubelet skips setting HostIP in PodStatus in standalone mode. https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/kubelet_pods.go#L1175-L1185

andrewsykim · 2017-03-16T04:37:30Z

^ I'll temporarily skip the check to verify

smarterclayton · 2017-03-16T04:38:21Z

This is probably similar to the original problems we had with podIP, but I'm not positive that hostIP is set until after the first status is calculated.

…

On Thu, Mar 16, 2017 at 12:34 AM, Andrew Sy Kim ***@***.***> wrote: At a quick glance of the kubelet code it seems like the PodIP update should happen on the same UpdateStatus call as the HostIP, the PodIP is defined in the e2e tests so race condition seems unlikely, you definitely know the code better than me so correct me if I'm wrong please :). Do you know if kubelet for E2E tests run on standalone mode? It seems like the kubelet skips setting HostIP in PodStatus in standalone mode. https://github.com/kubernetes/kubernetes/blob/master/pkg/ kubelet/kubelet_pods.go#L1175-L1185 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#42717 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_pydvSR8r2LWPLYHFcvdhzPUQsKIEks5rmLvlgaJpZM4MWgj2> .

andrewsykim · 2017-03-16T05:52:56Z

@smarterclayton this seems relevant to what you're referring to.

andrewsykim · 2017-03-16T05:57:40Z

Oh God, what rabbit hole have I gotten myself into 🤦‍♂️ 😛

smarterclayton · 2017-03-16T23:33:06Z

Yes, @pmorie can warn you of the infinite abyss yawning under your very feet.

…

andrewsykim · 2017-03-17T21:04:14Z

I dug into the code a bit more and have a better idea of what to do. If we stick with the way podIP is passed into kubelet's runtime it'll avoid any big refactoring at the cost of adding more cruft to the kubelet code (mostly passing in hostIP along with podIP). If we're OK with this I can have the kubelet code updated soon. Let me know what you think @smarterclayton @thockin @pmorie

yujuhong

With the new commit, the PR looks much cleaner.

yujuhong · 2017-03-31T00:45:05Z

pkg/kubelet/kubelet_pods.go

@@ -617,6 +617,13 @@ func (kl *Kubelet) podFieldSelectorRuntimeValue(fs *v1.ObjectFieldSelector, pod
 		return pod.Spec.NodeName, nil
 	case "spec.serviceAccountName":
 		return pod.Spec.ServiceAccountName, nil
+	case "status.hostIP":
+		hostIP, err := kl.GetHostIP()


Use kl.getHostIPAnyWay() to make sure we can always get the host IP, even if kubelet hasn't registered yet.

yujuhong · 2017-03-31T00:45:15Z

pkg/kubelet/kubelet_pods.go

+		if err != nil {
+			return "", err
+		}
+


nit: remove the extra line.

yujuhong · 2017-04-03T21:48:40Z

/lgtm

k8s-github-robot · 2017-04-03T21:50:12Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: andrewsykim, yujuhong
We suggest the following additional approver: @bgrant0607

Needs approval from an approver in each of these OWNERS Files:

OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-04-03T22:48:12Z

Automatic merge from submit-queue

andrewsykim · 2017-04-04T01:26:25Z

Any chance we can get this cherry picked into the next 1.6 release?

yujuhong · 2017-04-04T16:29:15Z

@andrewsykim, supporting hostIP in the downward API is a feature. We usually cherry-pick bug fixes.

deitch · 2017-04-20T16:49:53Z

So what is the target release version for this?

yujuhong · 2017-04-27T23:28:20Z

So what is the target release version for this?

1.7

Reintroduce auto-registration flow on VDSC container. Get the external engine address fetching the CN field on the TLS cert got connecting the engine on its service address. Get the node address using kubernetes Environment Variables since status.hostIP will be available within downward volume API only with kubernetes 1.7 (see kubernetes/kubernetes#42717 ). Since the registration command it's running in a systemd unit we have to get its env directly from /proc/1/environ Change-Id: I4ff7b7999d089aa581b44bf031f62f10fbbaa3cb Signed-off-by: Simone Tiraboschi <[email protected]>

timothysc · 2017-10-04T21:04:24Z

test/e2e/common/downward_api.go

@@ -74,10 +74,20 @@ var _ = framework.KubeDescribe("Downward API", func() {
 					},
 				},
 			},
+			{
+				Name: "HOST_IP",


You added a new field to an existing check without a version block check.

ping @andrewsykim , seems you left a bug to fix here :)

@timothysc @resouer Thanks for bringing this to my attention :), will try to fix it this week. Is this blocking someones PR from getting in?

Seems odd that this would come up as a problem after 6 months

I don't think this is a blocker

PR to fix in 1.8.1 is here - #53673

Thanks Tim, it seems we also version blocked pod IP e2e test for > v1.8 but it was added way back, was that intentional?

@andrewsykim I don't think we want to version block pod IP e2e test. I send PR #53772 to split the test case :)

@timothysc

Automatic merge from submit-queue (batch tested with PRs 53507, 53772, 52903, 53543). 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>. Split downward API e2e test case for pod/host IP into two **What this PR does / why we need it**: Split the test case in order to avoid version block pod IP e2e test. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # ref: #42717 (comment) **Special notes for your reviewer**: /cc @timothysc @andrewsykim

ralgara · 2018-10-16T20:43:58Z

Hi -- any word on this? Just tested status.hostIP and spec.nodeName on v1.10. Both return blank. Need either of these for a service to register with Eureka for existing external clients (not on k8s). E.g.

...
env:
  - name: EUREKA_SVC_HOST
    valueFrom:
      fieldRef:
        fieldPath: status.hostIP

Thanks.

#24657
#42717

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 8, 2017

andrewsykim changed the title ~~support hostIP in downward API~~ Support hostIP in downward API Mar 8, 2017

k8s-github-robot assigned liggitt Mar 8, 2017

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Mar 8, 2017

andrewsykim changed the title ~~Support hostIP in downward API~~ Support status.hostIP in downward API Mar 8, 2017

andrewsykim mentioned this pull request Mar 8, 2017

how can get the hostIP in a pod #24657

Closed

liggitt assigned thockin and unassigned liggitt Mar 8, 2017

andrewsykim commented Mar 13, 2017

View reviewed changes

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Mar 14, 2017

andrewsykim force-pushed the support-host-ip-downward-api branch from 5570bdc to 22d72ab Compare March 14, 2017 02:24

andrewsykim force-pushed the support-host-ip-downward-api branch from 2b282cc to 22d72ab Compare March 16, 2017 05:54

yujuhong reviewed Mar 31, 2017

View reviewed changes

use kl.getHostIPAnyWay() to get host ip even if node is not registered

a626534

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 3, 2017

k8s-github-robot merged commit e28cb42 into kubernetes:master Apr 3, 2017

andyxning mentioned this pull request Apr 5, 2017

Add dead simple k8s configs linkerd/linkerd-examples#32

Merged

krousey mentioned this pull request Apr 5, 2017

kubeadm release-1.6 branch e2e failing a Conformance test kubernetes/kubeadm#227

Closed

derekchiang mentioned this pull request Apr 21, 2017

Get rid of data transfer indirections by having the client directly talk to the storage backend pachyderm/pachyderm#223

Closed

JoshRagem mentioned this pull request Jun 11, 2017

resolve agent ip on init WhoopInc/dogstatsde#27

Closed

hkaj mentioned this pull request Jul 6, 2017

[kube] Allow to run agent on the node DataDog/integrations-core#508

Merged

seletskiy mentioned this pull request Aug 29, 2017

[core] unable to start dd-agent in Kubernetes cluster as described in installation instructions DataDog/dd-agent#3496

Closed

cliffburdick mentioned this pull request Sep 5, 2017

Update downward-api-volume-expose-pod-information.md kubernetes/website#5302

Merged

timothysc reviewed Oct 4, 2017

View reviewed changes

klingerf mentioned this pull request Oct 10, 2017

Start using status.hostIP in k8s daemonsets examples linkerd/linkerd-examples#184

Open

xiangpengzhao mentioned this pull request Oct 12, 2017

Split downward API e2e test case for pod/host IP into two #53772

Merged

liggitt mentioned this pull request Dec 15, 2017

inconsistency field exposure - spec.nodeName and status.hostIP #53362

Closed

jonmoter mentioned this pull request Mar 15, 2018

Kubernetes Daemonset manifest does not work on clusters < 1.7 DataDog/datadog-agent#1465

Closed

liggitt mentioned this pull request May 23, 2018

downwardapi volumes FieldPath bug fix #64186

Closed

yuchengwu mentioned this pull request May 14, 2019

document pod downwardAPI fieldRef & resourceFiledRef values kubernetes/website#14321

Merged

andrewsykim mentioned this pull request Sep 1, 2020

add myself to sig-node-reviewers #94401

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support status.hostIP in downward API #42717

Support status.hostIP in downward API #42717

andrewsykim commented Mar 8, 2017

k8s-ci-robot commented Mar 8, 2017

k8s-reviewable commented Mar 8, 2017

andrewsykim Mar 13, 2017

thockin commented Mar 13, 2017

andrewsykim commented Mar 14, 2017

thockin commented Mar 15, 2017

andrewsykim commented Mar 16, 2017 •

edited

Loading

andrewsykim commented Mar 16, 2017

smarterclayton commented Mar 16, 2017 via email

andrewsykim commented Mar 16, 2017

andrewsykim commented Mar 16, 2017 •

edited

Loading

smarterclayton commented Mar 16, 2017 via email

andrewsykim commented Mar 17, 2017

yujuhong left a comment

yujuhong Mar 31, 2017

yujuhong Mar 31, 2017

yujuhong commented Apr 3, 2017

k8s-github-robot commented Apr 3, 2017

k8s-github-robot commented Apr 3, 2017

andrewsykim commented Apr 4, 2017

yujuhong commented Apr 4, 2017

deitch commented Apr 20, 2017

yujuhong commented Apr 27, 2017

timothysc Oct 4, 2017

resouer Oct 10, 2017

andrewsykim Oct 10, 2017

andrewsykim Oct 10, 2017

resouer Oct 10, 2017

timothysc Oct 10, 2017

andrewsykim Oct 11, 2017

xiangpengzhao Oct 12, 2017

ralgara commented Oct 16, 2018

Support status.hostIP in downward API #42717

Support status.hostIP in downward API #42717

Conversation

andrewsykim commented Mar 8, 2017

k8s-ci-robot commented Mar 8, 2017

k8s-reviewable commented Mar 8, 2017

Choose a reason for hiding this comment

thockin commented Mar 13, 2017

andrewsykim commented Mar 14, 2017

thockin commented Mar 15, 2017

andrewsykim commented Mar 16, 2017 • edited Loading

andrewsykim commented Mar 16, 2017

smarterclayton commented Mar 16, 2017 via email

andrewsykim commented Mar 16, 2017

andrewsykim commented Mar 16, 2017 • edited Loading

smarterclayton commented Mar 16, 2017 via email

andrewsykim commented Mar 17, 2017

yujuhong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yujuhong commented Apr 3, 2017

k8s-github-robot commented Apr 3, 2017

k8s-github-robot commented Apr 3, 2017

andrewsykim commented Apr 4, 2017

yujuhong commented Apr 4, 2017

deitch commented Apr 20, 2017

yujuhong commented Apr 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ralgara commented Oct 16, 2018

andrewsykim commented Mar 16, 2017 •

edited

Loading

andrewsykim commented Mar 16, 2017 •

edited

Loading