aws: allow GENEVE (6081) and OVN database ports (6641 & 6642) #1563
Conversation
openshift-ci-robot
added
the
size/M
|
There's discussion in #1218. Consensus seemed to be leaning toward "CNO should manage this itself, somehow", with the "somehow" being not really at all figured out. |
Can we #1218 and figure this out? I feel like I'm personally unqualified to review "network type x needs ports y" pull requests, and would be much happier handing security-group information off to the network operator so it could set up whatever it needs directly. |
|
/lgtm |
|
/retest |
|
@danwinship @squeed @pecameron updated to address danw's issue with SBDB ports on nodes. |
openshift-ci-robot
added
the
do-not-merge/hold
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
So, we've decided that we definitely want to move port opening to the CNO, but it won't happen in time for 4.2. These ports are for OVN, which is tech-preview. We can move the ovndb ports to the 9000-9999 space, but the GENEVE port can't be changed. So, we'd like to merge this PR (with the db ports removed) for 4.2, but remove it as soon as the CNO gains this capability. |
Really? It's one thing to move around metrics and health checks and things like that that don't necessarily have canonical port numbers, but for "real" services, keeping the traffic on the "right" ports seems nice to me in terms of debuggability, etc. (Eg, tools like tcpdump will recognize certain protocols when they are on the right port, but not when they are on other ports.) |
|
CLosing this one in favor or #1905 |
|
/reopen |
|
@danwinship: Reopened this PR. In response to this:
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. |
|
/lgtm |
|
/hold cancel |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
@abhinavdahiya as per our conversation, this is the way you'd like to go forward. |
|
/retest |
1) Allow GENEVE (6081) between all nodes (masters & workers) 2) Allow OVN databases (6641 & 6642) between all masters 3) Allow OVN databases (6641 & 6642) between masters and workers, but not between workers themselves
|
@abhinavdahiya PTAL thanks! |
|
@jstuever We should update the GCP list be match this PR. /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, danwinship, dcbw, pecameron, squeed 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 |
openshift-ci-robot
added
the
approved
|
/retest |
|
@dcbw: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
@squeed @pecameron @danwinship @JacobTanenbaum @rcarrillocruz
[question: should these be enabled by default even if the install isn't using ovn-kubernetes? Or, how do SG rules get selectively added based on specific network plugin requirements? eg when running ovn-kubernetes we don't need VXLAN between nodes, and when running openshift-sdn we don't need GENEVE...]