Skip to content

BZ#1726773 Enhance 4.x required ports tables #20811

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

Merged
merged 3 commits into from
Apr 22, 2020

Conversation

codyhoag
Copy link
Contributor

@codyhoag codyhoag commented Apr 1, 2020

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 1, 2020
@openshift-docs-preview-bot

The preview will be available shortly at:

@codyhoag
Copy link
Contributor Author

codyhoag commented Apr 8, 2020

@sdodson can you confirm the accuracy of the additional table I added for the All machines to control plane table? Also, can you verify this applies to OCP 4.1-4.4?

From what I've gathered, this change aligns the docs with how we'd like to present our port requirements to the public for 4.x. I'll wrap up any additional edits for providing this info for other cloud platforms in another PR. Thanks!

@codyhoag codyhoag changed the title [WIP] BZ#1726773 Enhance 4.x required ports tables BZ#1726773 Enhance 4.x required ports tables Apr 8, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 8, 2020
@codyhoag codyhoag added this to the Next Release milestone Apr 8, 2020
@sdodson
Copy link
Member

sdodson commented Apr 15, 2020

/lgtm
Thank you @codyhoag

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 15, 2020
@codyhoag
Copy link
Contributor Author

@gpei PTAL?

@gpei
Copy link

gpei commented Apr 16, 2020

According to the Ingress rules set in upi-on-aws doc https://github.com/openshift/installer/blob/5e0bb6753f13da91ffa83eee2e5a99f411b6f5af/upi/aws/cloudformation/03_cluster_security.yaml
I could see something different in the detailed Protocol&Port rules:

Ingress for masters from the VPC:

  • ICMP
  • TCP/22
  • TCP/6443
  • TCP/22623

Ingress for workers from the VPC:

  • ICMP
  • TCP/22

Ingress for masters from masters:

  • TCP/2379-2380 "etcd"

Ingress for masters from masters and workers:

  • UDP/4789 "Vxlan packets"
  • UDP/6081 "Geneve packets"
  • TCP/9000-9999 "Internal cluster communication"
  • TCP/10250-10259 "Kubernetes kubelet, scheduler and controller manager"
  • TCP/30000-32767 "Kubernetes ingress services"

Ingress for workers from masters and workers:

  • UDP/4789 "Vxlan packets"
  • UDP/6081 "Geneve packets"
  • TCP/9000-9999 "Internal cluster communication"
  • TCP/10250-10259 "Kubernetes kubelet, scheduler and controller manager"
  • TCP/30000-32767 "Kubernetes ingress services"

@sdodson
Copy link
Member

sdodson commented Apr 16, 2020

@gpei Can you pinpoint what you're saying is different?

  • ICMP
  • TCP/22
  • TCP/10250-10259 vs TCP/10249-10259

I guess we can add ICMP to the all machines to all machines portion.
The TCP/22 is not strictly required of the platform but is if you intend to SSH to hosts, I would expect admins to decide if that's necessary for their use or not and wouldn't document that.
We should fix the last item.

Anything else that I missed?

@gpei
Copy link

gpei commented Apr 16, 2020

@gpei Can you pinpoint what you're saying is different?

  • ICMP
  • TCP/22
  • TCP/10250-10259 vs TCP/10249-10259

I guess we can add ICMP to the all machines to all machines portion.
The TCP/22 is not strictly required of the platform but is if you intend to SSH to hosts, I would expect admins to decide if that's necessary for their use or not and wouldn't document that.

+1 , TCP/22 should not be a strictly requirement.

Besides these 3 items, the other differences I noticed:

  • TCP/22623 should be opened on masters for MCS.
  • TCP/2379-2380 should be control plane to control plane.
  • 9000-9999 ports with only TCP configured in the AWS cloudformation template.
  • 30000-32767 ports with TCP configured in the AWS cloudformation template.
    I'm also not very clear about the port usage in the last two items, just noticed they're configured like that in the template.

@sdodson
Copy link
Member

sdodson commented Apr 16, 2020

TCP/22623 should be opened on masters for MCS.

That's covered in the load balancer section, while it doesn't explicitly say that the firewall needs to be open I would assume that if the load balancer is used to connect to the control plane hosts on that port it need not be explicitly stated.

* 30000-32767 ports with TCP configured in the AWS cloudformation template.
  I'm also not very clear about the port usage in the last two items, just noticed they're configured like that in the template.

This is the range used by NodePort and should be TCP and UDP on all hosts. @codyhoag lets fix that too.

9000-9999 ports with only TCP configured in the AWS cloudformation template.

That's out of sync with IPI terraform code, we'll fix the cloud formations template, docs are correct.

@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2020
@codyhoag
Copy link
Contributor Author

@sdodson two questions:

  1. What about the last difference in BZ#1726773 Enhance 4.x required ports tables #20811 (comment)?

    TCP/2379-2380 should be control plane to control plane.

    Should I move those ports to a new table for control plane to control plane?

  2. What should I include as the description for ICMP (port 0)?

@codyhoag
Copy link
Contributor Author

@sdodson when you have a moment, can you answer the two questions in my previous comment? Thanks!

@sdodson
Copy link
Member

sdodson commented Apr 21, 2020

@codyhoag
1 -- is correct as it is already, all hosts to control plane, this appears to be potentially a descrepancy between the reference that @gpei used versus what's defined in the IPI installation here https://github.com/openshift/installer/blob/master/data/data/aws/vpc/sg-master.tf#L226-L234

2 -- ICMP doesn't have a port so put "N/A" and description of "Network reachability tests" or something to that effect would be fine. "ICMP" is also fine for a description, networking people should know what it means.

@gpei
Copy link

gpei commented Apr 22, 2020

Thanks to the confirmation from @sdodson , lgtm now.

@codyhoag codyhoag added the peer-review-needed Signifies that the peer review team needs to review this PR label Apr 22, 2020
@lamek
Copy link

lamek commented Apr 22, 2020

LGTM.

@lamek lamek added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Apr 22, 2020
@codyhoag codyhoag merged commit ab13e74 into openshift:master Apr 22, 2020
@codyhoag
Copy link
Contributor Author

/cherrypick enterprise-4.4

@openshift-cherrypick-robot

@codyhoag: new pull request created: #21382

In response to this:

/cherrypick enterprise-4.4

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.

@codyhoag
Copy link
Contributor Author

/cherrypick enterprise-4.3

@openshift-cherrypick-robot

@codyhoag: new pull request created: #21383

In response to this:

/cherrypick enterprise-4.3

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.

@codyhoag
Copy link
Contributor Author

/cherrypick enterprise-4.2

@openshift-cherrypick-robot

@codyhoag: new pull request created: #21384

In response to this:

/cherrypick enterprise-4.2

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.

@codyhoag
Copy link
Contributor Author

/cherrypick enterprise-4.1

@openshift-cherrypick-robot

@codyhoag: #20811 failed to apply on top of branch "enterprise-4.1":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	modules/installation-network-user-infra.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/installation-network-user-infra.adoc
CONFLICT (content): Merge conflict in modules/installation-network-user-infra.adoc
Patch failed at 0001 Adjust port tables

In response to this:

/cherrypick enterprise-4.1

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.

@codyhoag
Copy link
Contributor Author

The manual 4.1 PR was sent in #21392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants