When using host network, the pod IP is always empty in downward API

[yifan-gu]

Turns our that when running a container in host network, its podStatus *kubecontainer.PodStatus that is passed to dockertools.SyncPod() will always contain empty IP, as a result, the PodIP environment becomes empty.

Maybe we want to use the apiPodStatus instead of podStatus in this case to determine the pod IP ? Since the apiPodStatus has the valid PodIP, which is populated by kubelet when the container runs in host network.

Or just populate the podIP with hostIP in dockertools.syncPod when the container runs on host network.

Ref #22666

cc @aaronlevy @kubernetes/sig-node @pmorie

[Random-Liu]

Why not just set podStatus.IP the same with apiPodStatus.IP before SyncPod? Ideally, we want to remove apiPodStatus from SyncPod() in the future if possible, it may be better to get it from podStatus.

@yujuhong WDYT?

[yujuhong]

Why not just set the apiPodStatus.IP to podStatus.IP before SyncPod?

Do you mean "set the podStatus.IP to apiPodStatus.IP before SyncPod?"

IMO, we should call the block to determine the IP only once. Why don't we populate set the kubecontainer.PodStatus.IP before calling generateAPIPodStatus?

[Random-Liu]

IMO, we should call the block to determine the IP only once. Why don't we populate set the kubecontainer.PodStatus.IP before calling generateAPIPodStatus?

We also use generateAPIPodStatus here. However, because we don't support host network bandwidth limit, it is fine to leave the IP unset here.

However, I think it would be good to have IP always correctly set in generateAPIPodStatus.

[pmorie]

I must admit, I have been thinking about this in the background since @yifan-gu created this issue, and the discussion hasn't added up for me. I finally sat down and spent some time getting my numerous fixes for podIP via downward API back into my brain, and now the problem seems very obvious to me.

So, after some thought, I believe that for the docker runtime, the problem is that GetPodStatus, which is responsible for generating the kubecontainer.PodStatus which is used to supply the pod IP, doesn't check to see whether the host network is being used and return the correct IP address if so. However, generateAPIPodStatus does, which is why the obvious fix seems to be to use the value from this method. I think the right thing to do instead is to change the following call stack to correctly determine the IP when the host network is being used.

DockerManager.GetPodStatus ->
  DockerManager.inspectContainer ->
    DockerManager.determineContainerIP

@Random-Liu @yifan-gu @yujuhong

[yifan-gu]

@pmorie Today GetPodStatus doesn't pass the pod spec, so it cannot know if the pod is running on host. This could be solved if we serialize the pod spec into docker labels.

BTW, I think we have a similar issue here for rkt.

[Random-Liu]

@pmorie As @yifan-gu said, GetPodStatus doesn't have full pod spec today. And I don't think docker manager has knowledge about the host ip now.

[pmorie]

@Random-Liu @yifan-gu I think we should think about adding the plbuming. I have not had a good experience storingthis information in multiple places :)

[yujuhong]

The code was written so that it'd work in the absence of the pod specs (which we don't always have).

@Random-Liu @yifan-gu I think we should think about adding the plbuming. I have not had a good experience storingthis information in multiple places :)

I agree with you, but adding the dependency today without some form of checkpointing (e.g., container annotations) of the pod specs may be problematic now. We can revisit this after we store pod specs in the annotations. What do you think? :)

[pmorie]

I'm cool with revisiting @yujuhong but if we do that we should drop this to p2

[yifan-gu]

Reopen the issue to capture #24633 (comment)

[xiang90]

@yifan-gu Is this fixed?

[bassam]

hi, also interested in knowing if this is fixed.

[yifan-gu]

@bassam @xiang90 I don't think anyone is tacking on this one right now since people are mostly working on the new CRI implementation for different runtimes. cc @Random-Liu @yujuhong

[bassam]

@yifan-gu thanks. it also looks like there might be a specific issue with rkt here. Looking at https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/rkt/rkt.go#L804 and
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/rkt/rkt.go#L1304, it looks like rkt does not handle the case of status.PodIP when using the host network.

[yifan-gu]

@bassam Thanks for the heads up. I will take a look to see if we can quickly work around this.

[fejta-bot]

Issues go stale after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle rotten
/remove-lifecycle stale

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close