OCPBUGS-79536: Removing AWS security group OVNDB ports as they are no longer used#10441
Conversation
|
Hi @ssonigra. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughRemoved the CNI security-group ingress rule for the OVN database named "ovndb" that allowed TCP traffic on ports 6441–6442 from the cluster asset generation logic. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/retitle OCPBUGS-79536: aws: updating security group OVNDB ports to 6641-6642 Let's see what tests say... but why hasn't anything failed yet if the port is incorrect... 🤔 |
|
@ssonigra: This pull request references Jira Issue OCPBUGS-79536, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@tthvo: This pull request references Jira Issue OCPBUGS-79536, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
|
hello @tthvo the issue here is with what port is displayed in the AWS console , the backend code is correct its just issue with whats displayed on the AWS console under security gorup. |
| FromPort: 6641, | ||
| ToPort: 6642, |
There was a problem hiding this comment.
hello @tthvo the issue here is with what port is displayed in the AWS console , the backend code is correct its just issue with whats displayed on the AWS console under security gorup.
@ssonigra Indeed. Though, the PR also changed the value for FromPort and ToPort, which determines which port range is open for this specific rule.
In other words, this configures the security group to now allow 6641-6642 while denying the previous 6441-6442 ports.
There was a problem hiding this comment.
The e2e jobs here indicates this change is fine... This leads me to wonder if these ports are ever used at all 🤔
There was a problem hiding this comment.
This leads me to wonder if these ports are ever used at all 🤔
I just did an install with a patch that does not include a rule to allow either 6441-6442 or 6641-6642 at all; and the install completed successfully. GCP and Azure codes don't seem to include these ports either... 🤔
|
Hello those port are used by OVNkubernetes so those are needed to make sure there is connectivity between masters and workers.here is the upstream documentaion https://docs.ovn.org/en/latest/howto/firewalld.html |
Yes, definitely... I do agree with your concern. However, this PR raises more questions than just a typo:
That brought me to the question here: Do |
|
Hello Thuan, agree with you, I am testing something on my clusters, maybe we dont need the port itself , if thats the case I will raise a separate PR to remove the entry itself so the SG is not created at all... |
|
/retest-required |
|
/retest |
|
/test e2e-aws-ovn-single-node |
Yea, the e2e results seem to indicate the ingress rule for |
The installation tests did not use these ports either. |
These ports are used by different ovn-db transport methods: @ssonigra For the bug, let's move it to eng team and get them review what change is needed. |
|
Hello marty, tthov the commit has been squashed and its merged into a single commit , please review and let me know if we are good to merge it.. |
|
Commits have been squashed as requested. Ready for re-review @marty-power |
|
@ssonigra: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
tthvo
left a comment
There was a problem hiding this comment.
/lgtm
/approve
Thanks! I cross-ref with aws-upi-template, which also does not include the 6441-6442 port either. After this merges, we also need to make sure openshift-docs does not mention this port anymore👍
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: marty-power, tthvo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/verified by CI (see above discussions too) |
|
@tthvo: This PR has been marked as verified by DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
b248fa6
into
openshift:main
|
@ssonigra: Jira Issue Verification Checks: Jira Issue OCPBUGS-79536 Jira Issue OCPBUGS-79536 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/cherry-pick release-4.22 |
|
@ssonigra: only openshift org members may request cherry picks. If you are already part of the org, make sure to change your membership to public. Otherwise you can still do the cherry-pick manually. DetailsIn 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-sigs/prow repository. |
|
/cherry-pick release-4.22 |
|
@ssonigra: only openshift org members may request cherry picks. If you are already part of the org, make sure to change your membership to public. Otherwise you can still do the cherry-pick manually. DetailsIn 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-sigs/prow repository. |
|
/cherry-pick release-4.21 |
|
@ssonigra: only openshift org members may request cherry picks. If you are already part of the org, make sure to change your membership to public. Otherwise you can still do the cherry-pick manually. DetailsIn 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-sigs/prow repository. |
|
/cherry-pick release-4.22 |
|
/cherry-pick release-4.21 |
|
/cherry-pick release-4.20 |
|
@ssonigra: new pull request created: #10667 DetailsIn 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-sigs/prow repository. |
|
@ssonigra: new pull request created: #10668 DetailsIn 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-sigs/prow repository. |
|
@ssonigra: new pull request created: #10669 DetailsIn 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-sigs/prow repository. |
The security group created in AWS using IPI installation was having ports mentioned as 6441 & 6442 , as these ports are no longer needed because they are not used so removing it from the installer
Making the changes as per the Bug https://redhat.atlassian.net/browse/OCPBUGS-79536