Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update network doc for ip-per-pod reqs #12182

Merged

Conversation

thockin
Copy link
Member

@thockin thockin commented Jan 11, 2019

As per SIG-Network discussion, we can relax the requirement for all
nodes to be able to reach all pods on those platforms that do not
support hostNetwork.

The current thinking on Windows support is that hostNetwork is not a
requirement for Window users. As such, satisfying the old requirement
is very difficult and has no practical benefit.

This DOES NOT change the requirements for platforms that support
hostNetwork (e.g. Linux) nor does it make hostNetwork optional for Linux
nodes.

This PR also removes most comparisons to Docker, as they are stale and
no longer germane, and cleans up some of the text.

@dineshgovindasamy

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 11, 2019
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Jan 11, 2019
@dineshgovindasamy
Copy link

Thanks @thockin

Looks good from my perspective.

@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Jan 11, 2019

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 55fe4e2

https://deploy-preview-12182--kubernetes-io-master-staging.netlify.com

how to find each other, etc. Rather than deal with this, Kubernetes takes a
different approach.
Kubernetes is all about sharing machines between applications. Typically,
sharing machines requires ensuring that two applications on the same do not try
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a word missing? i.e. "on the same..."? machine?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned up a bit

@bowei
Copy link
Member

bowei commented Jan 12, 2019

added text about host port support lgtm

network (e.g. Linux supports this):

* pods in the host network of any node can communicate with all pods on all
nodes without NAT

What this means in practice is that you can not just take two computers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to remove "editorial" style text?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

* all nodes can communicate with all containers (and vice-versa) without NAT
* the IP that a container sees itself as is the same IP that others see it as
* pods on any node can communicate with all pods on all nodes without NAT
* agents on a node can communicate with all pods on that node

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth to spend a one liner to define what we consider an agent? I did not see it mentioned in any other part of the doc. Maybe we can link to https://kubernetes.io/docs/concepts/overview/components/#kubelet like in the section above?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should clarify what they are. "infrastructure agents" or something, which include networking, logging, health checking, networking; eg anything required to get and keep the cluster functional

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will change to "agents on a node (e.g. system daemons, kubelet) can communicate with all pods on that node"

@dcbw
Copy link
Member

dcbw commented Jan 14, 2019

@danwinship

In addition to these, for those platforms that support `Pods` running in the host
network (e.g. Linux supports this):

* pods in the host network of any node can communicate with all pods on all
Copy link
Member

@dcbw dcbw Jan 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thockin hmm this is new; was this something that just came up or that should have been clarified from the start?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, this was always intended, and was implied by the first rule (all pods <-> all pods). Otherwise some pods are not able to reach other pods.

I think it is in e2e, too.

As much as I dislike it, some apps run hostNetwork for performance reasons. Removing this constraint would potentially break them.

insert dynamic port numbers into configuration blocks, services have to know
how to find each other, etc. Rather than deal with this, Kubernetes takes a
different approach.
Kubernetes is all about sharing machines between applications. Typically,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like even the rewritten version of this is still kind of explaining Kubernetes to former docker users who have been in a coma since 2015. :) No one these days is going to come into Kubernetes expecting to have to coordinate port numbers between unrelated apps, so it's just confusing to spend so much time explaining why we don't force them to.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, I get asked ALL THE TIME why we don't support dynamic port mapping.

* agents on a node can communicate with all pods on that node
* the IP that a pod sees itself as is the same IP that others see it as

In addition to these, for those platforms that support `Pods` running in the host
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't "in addition"; we already required that "pods on any node can communicate with all pods on all nodes without NAT", so if host network pods exist, then they are already required to be able to communicate with all pods.

Maybe just something like "Note that for platforms that support Pods running in the host network, these requirements apply to those Pods as well."

(Except see my other comment about hostNetwork pods.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reworded. Good point.

How this is implemented is a detail of the particular container runtime in use.

It is possible to request ports on the `Node` itself which forward to your `Pod`
(called host-ports), but this is reduced to a very niche operation. How that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop "reduced to" since that's part of the docker-vs-kubernetes comparison, which is now mostly gone

(also, I feel like the original "host ports" is better than "host-ports"?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

* the IP that a container sees itself as is the same IP that others see it as
* pods on any node can communicate with all pods on all nodes without NAT
* agents on a node can communicate with all pods on that node
* the IP that a pod sees itself as is the same IP that others see it as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... what if a pod has multiple IPs? In particular, what about the case of a hostNetwork pod on a node with multiple IPs?

In openshift-sdn, every node has an IP address in the cluster network CIDR in addition to its external/primary/"eth0" IP. A hostNetwork pod would list the node's primary IP as its .spec.podIP, but when it communicates with other non-hostNetwork pods, the source IP would be the node's cluster network IP, not the primary IP. Is that legit?

(If so, we probably want to treat the hostNetwork case separately here, since I think we definitely want to say that for non-hostNetwork pods, the source address of pod-to-pod traffic will always be the pod's .spec.podIP.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need this line? It's somewhat redundant with the "no NAT" line. I propose to remove it.

As per SIG-Network discussion, we can relax the requirement for all
nodes to be able to reach all pods on those platforms that do not
support hostNetwork.

The current thinking on Windows support is that hostNetwork is not a
requirement for Window users.  As such, satisfying the old requirement
is very difficult and has no practical benefit.

This DOES NOT change the requirements for platforms that support
hostNetwork (e.g. Linux) nor does it make hostNetwork optional for Linux
nodes.

This PR also removes most comparisons to Docker, as they are stale and
no longer germane, and cleans up some of the text.
@thockin thockin force-pushed the relax-network-reqs-nodes-to-pods branch from 471e507 to 55fe4e2 Compare February 8, 2019 00:49
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 8, 2019
Copy link
Member Author

@thockin thockin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that this was still open.

Pushed a new commit fixing comments.

insert dynamic port numbers into configuration blocks, services have to know
how to find each other, etc. Rather than deal with this, Kubernetes takes a
different approach.
Kubernetes is all about sharing machines between applications. Typically,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, I get asked ALL THE TIME why we don't support dynamic port mapping.

how to find each other, etc. Rather than deal with this, Kubernetes takes a
different approach.
Kubernetes is all about sharing machines between applications. Typically,
sharing machines requires ensuring that two applications on the same do not try
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned up a bit

* all nodes can communicate with all containers (and vice-versa) without NAT
* the IP that a container sees itself as is the same IP that others see it as
* pods on any node can communicate with all pods on all nodes without NAT
* agents on a node can communicate with all pods on that node
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will change to "agents on a node (e.g. system daemons, kubelet) can communicate with all pods on that node"

* the IP that a container sees itself as is the same IP that others see it as
* pods on any node can communicate with all pods on all nodes without NAT
* agents on a node can communicate with all pods on that node
* the IP that a pod sees itself as is the same IP that others see it as
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need this line? It's somewhat redundant with the "no NAT" line. I propose to remove it.

* agents on a node can communicate with all pods on that node
* the IP that a pod sees itself as is the same IP that others see it as

In addition to these, for those platforms that support `Pods` running in the host
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reworded. Good point.

In addition to these, for those platforms that support `Pods` running in the host
network (e.g. Linux supports this):

* pods in the host network of any node can communicate with all pods on all
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, this was always intended, and was implied by the first rule (all pods <-> all pods). Otherwise some pods are not able to reach other pods.

I think it is in e2e, too.

As much as I dislike it, some apps run hostNetwork for performance reasons. Removing this constraint would potentially break them.

network (e.g. Linux supports this):

* pods in the host network of any node can communicate with all pods on all
nodes without NAT

What this means in practice is that you can not just take two computers
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

How this is implemented is a detail of the particular container runtime in use.

It is possible to request ports on the `Node` itself which forward to your `Pod`
(called host-ports), but this is reduced to a very niche operation. How that
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@danwinship
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 8, 2019
@fcrisciani
Copy link

/lgtm

@k8s-ci-robot
Copy link
Contributor

@fcrisciani: changing LGTM is restricted to assignees, and only kubernetes/website repo collaborators may be assigned issues.

In response to this:

/lgtm

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.

@thockin
Copy link
Member Author

thockin commented Feb 11, 2019

@chenopis

@Rajakavitha1
Copy link
Contributor

@chenopis Could you please take a look at the updated content.

@zparnold
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zparnold

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 18, 2019
@k8s-ci-robot k8s-ci-robot merged commit 971d7af into kubernetes:master Feb 18, 2019
kwiesmueller pushed a commit to kwiesmueller/website that referenced this pull request Feb 28, 2019
* Update network doc for ip-per-pod reqs

As per SIG-Network discussion, we can relax the requirement for all
nodes to be able to reach all pods on those platforms that do not
support hostNetwork.

The current thinking on Windows support is that hostNetwork is not a
requirement for Window users.  As such, satisfying the old requirement
is very difficult and has no practical benefit.

This DOES NOT change the requirements for platforms that support
hostNetwork (e.g. Linux) nor does it make hostNetwork optional for Linux
nodes.

This PR also removes most comparisons to Docker, as they are stale and
no longer germane, and cleans up some of the text.

* Feedback on contents
Kubernetes imposes the following fundamental requirements on any networking
implementation (barring any intentional network segmentation policies):

* all containers can communicate with all other containers without NAT
* all nodes can communicate with all containers (and vice-versa) without NAT
* the IP that a container sees itself as is the same IP that others see it as

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

krmayankk pushed a commit to krmayankk/kubernetes.github.io that referenced this pull request Mar 11, 2019
* Update network doc for ip-per-pod reqs

As per SIG-Network discussion, we can relax the requirement for all
nodes to be able to reach all pods on those platforms that do not
support hostNetwork.

The current thinking on Windows support is that hostNetwork is not a
requirement for Window users.  As such, satisfying the old requirement
is very difficult and has no practical benefit.

This DOES NOT change the requirements for platforms that support
hostNetwork (e.g. Linux) nor does it make hostNetwork optional for Linux
nodes.

This PR also removes most comparisons to Docker, as they are stale and
no longer germane, and cleans up some of the text.

* Feedback on contents
yagonobre pushed a commit to yagonobre/website that referenced this pull request Mar 14, 2019
* Update network doc for ip-per-pod reqs

As per SIG-Network discussion, we can relax the requirement for all
nodes to be able to reach all pods on those platforms that do not
support hostNetwork.

The current thinking on Windows support is that hostNetwork is not a
requirement for Window users.  As such, satisfying the old requirement
is very difficult and has no practical benefit.

This DOES NOT change the requirements for platforms that support
hostNetwork (e.g. Linux) nor does it make hostNetwork optional for Linux
nodes.

This PR also removes most comparisons to Docker, as they are stale and
no longer germane, and cleans up some of the text.

* Feedback on contents
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.