-
Notifications
You must be signed in to change notification settings - Fork 51
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 Submariner OVN Documentation #699
Conversation
🤖 Created branch: z_pr699/astoycos/update-ovn-k-resources |
/ovn |
/hold until OVN related submariner PR's have merged |
✅ Deploy Preview for elated-bell-2913d9 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
47bd5cf
to
03d57ae
Compare
@astoycos as you probably noticed, we don't use Prow in this org so you have to manually add labels, etc. To mark a PR as draft, we typically use https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request as documented here. |
Yep no worries! |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks outstanding, major fixes/improvements. Thank you @astoycos!
Are we ready to undraft/review/merge?
|
||
![Submariner with OVNKubernetes architecture](/images/ovn-kubernetes/submariner-on-ovn-v2.svg) | ||
![Submariner with OVNKubernetes architecture](/images/ovn-kubernetes/ovn-submariner-architecture.svg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice diagram with explanation @astoycos, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@astoycos In the attached diagram, On OVN Submariner router, it is mentioned that "OVN Nat rules are applied here to ensure traffic is SNAT'ed to the globalnetIP when Globalnet is enabled".
Since this will be done when we support OVN with Globalnet and is not there currently, I'm not sure if we should capture this in the diagram. Basically, it might give an impression that Globalnet is supported even with OVN by looking into the diagram. What you do say?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@dfarrell07 This should be pretty much ready to go, I just wonder if we should wait until submariner-io/submariner#1689? Although there isn't any major architectural changes, I may need to edit/augment these docs based on reviews |
@vthapar this one should be ready to get merged now, can you please take it? |
The following diagram illustrates the OVN Kubernetes handler architecture where the | ||
blue elements represent the OVN Kubernetes native network elements and the yellow | ||
elements are introduced by Submariner. | ||
The following diagram illustrates the required Submariner OVN architecture overlayed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please copy the original image to the following PPT [*] and add the following comment which will help us to make further changes in future if required.
<!-- Image Source: https://docs.google.com/presentation/d/180CtHZnr9PP5Rh98VEmkQz3ovc5AGXG9wosoHMLhgaY/edit -->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
2271d60
to
f572960
Compare
This commit updates the documents involving the submariner + OVN-K integration, and also re-does the architecture diagram. Signed-off-by: Andrew Stoycos <astoycos@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @astoycos
f572960
to
949d8f1
Compare
🤖 Closed branches: [z_pr699/astoycos/update-ovn-k-resources] |
Thanks @astoycos |
This commit updates the documents involving the
submariner + OVN-K integration, and also re-does
the architecture diagram.
Signed-off-by: Andrew Stoycos astoycos@redhat.com