-
Notifications
You must be signed in to change notification settings - Fork 521
CORENET-6429: OVN-Kubernetes support for EVPN #1862
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
base: master
Are you sure you want to change the base?
Conversation
|
@jcaamano: This pull request references CORENET-6429 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.21.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
maiqueb
left a comment
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.
Some initial thoughts.
Tks for this enhancement.
| This enhancement describes how OVN-Kubernetes support for EVPN integrates in | ||
| OpenShift Container Platform (OCP). The feature allows exposing primary Cluster | ||
| User Defined Networks (P-CUDNs) externally via a VPN to other entities either | ||
| inside, or outside the cluster; using BGP and EVPN as a common and native |
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.
| inside, or outside the cluster; using BGP and EVPN as a common and native | |
| inside, or outside the cluster; using BGP and EVPN is a common and native |
I think you have a typo here.
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.
not really, it read fine to me as it was, but I will rephrase to avoid this
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.
No need to, if this is what you originally intended, it is fine to keep as is currently is.
| The EVPN feature is only supported when `routingViaHost` is set to false, also | ||
| known as local gateway mode. CNO will perform no validation, OVN-Kubernetes will | ||
| be in charge of reporting invalid configurations in this regard. |
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.
would you elaborate a bit on how will it report invalid configurations ? events (which object) / logs / ...
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.
I will add a link to the specific section on the upstream enhancement for this
|
|
||
| ### API Extensions | ||
|
|
||
| There are no changes required to OCP specific APIs. |
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.
but we want the API team to provide feedback on the ovn-k / frr-k8s APIs that are added / updated, right ?
Where are they supposed to provide feedback ? In the original ovn-k enhancement ?
I think we should try to capture all the required API changes in this document, to get an holistic view on how to configure the feature in a single place.
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.
Where are they supposed to provide feedback ? In the original ovn-k enhancement ?
Yup, ...
I think we should try to capture all the required API changes in this document,
... we are not designing the API (or the OVN-K functionality for that matter) in the context of this enhancement. OVN-K is just another downstream component for us and this enhancement is about integrating what it provides with OCP.
to get an holistic view on how to configure the feature in a single place.
I think this comment applies to the workflow section. I can certainly replace my links with literal copy pastes from the upstream enhancement workflow once that is done but the point is that it is reasonable to think that there is no interaction of that workflow with other OCP components for which any additional design is needed (otherwise I would have included something else); and I don't want to mislead people into thinking the upstream workflow is something we are discussing here nor me having to constantly keep the section updated as it things change upstream anyway. I will try to clarify this better, and once the upstream enhancement is done I will re-evaluate the contents of the workflow section.
I would say reviewers are required to be aware and follow through on the referenced components to some degree and there should be some middle ground so that we don't duplicate content and effort, while accommodating some parallelization upstream vs downstream. We don't want to get it as far as this constituting a user guide.
But this is one of the first enhancements for which we do this type of split so I am sure we will have more opinions on this, let's see what other people think.
|
|
||
| ### Risks and Mitigations | ||
|
|
||
| N/A |
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.
I'd say the FRR-k8s v10 delivery is a risk; each option you've presented will add/require a different amount of work ...
For instance, if we end up having to do our own build of FRR v10 for RHEL 9 and cross tag it for OCP, we'll have extra work on our plate.
Is that considered a risk ? Is there any way to mitigate it ?
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.
It didn't occur to me to add it as a risk just because it requires effort and time as this enhancement is not estimating that. I guess you think it might take an unreasonable amount of effort with respect to your typical enhancement size. I can see how one of the options can be seen in that way, I will add more detail and think again if we should have a risk for it.
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.
Sure thing, makes total sense from my perspective.
|
|
||
| ## Alternatives (Not Implemented) | ||
|
|
||
| N/A |
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.
There are alternatives ...
I think we should list other options, like integrating with secondary networks via openperouter, and indicate why that option was discarded.
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.
Well, the thing is that in the context of this enhancement we did not consider, evaluate and discard that alternative so I am not sure what to add. This enhancement is just about integrating what OVN-K provides as a downstream component. I will try to add something while making it clear it was not an alternative for this enhancement.
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.
I understand that perspective, but, IMHO, the enhancement shouldn't be about "integrating what OVN-K provides", but rather present to the customer a way to achieve the use cases listed in the .
One alternative, the one it seems we as an org are favoring, is "integrating what OVN-K provides".
| * Enable the EVPN feature gate. | ||
| * Enable the internal BGP fabric. | ||
|
|
||
| The OTE testing framework will be used for the implementation combined with use |
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.
what is OTE ? can we get some info on that - or pointers to that info ?
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.
@maiqueb OTE is Openshift Test Extension, that will help enable u/s e2e tests to be able to run d/s, https://issues.redhat.com/browse/CORENET-4510
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, I will expand the term.
| title: OVN-Kubernetes support for EVPN | ||
| authors: | ||
| - @jcaamano | ||
| reviewers: |
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.
I'd appreciate if we had someone from our org (OpenShift Virt) represented here.
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.
I will add people as reviews happen. I will add you since you reviewed.
| QE testing should include testing upgrades from a cluster already making use of | ||
| the EVPN feature. | ||
|
|
||
| QE testing will need to emulate an BGP/EVPN fabric. While any kind of custom adn |
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.
| QE testing will need to emulate an BGP/EVPN fabric. While any kind of custom adn | |
| QE testing will need to emulate an BGP/EVPN fabric. While any kind of custom and |
maybe a typo ? I don't know, but it's confusing.
| * Request RHEL to package FRR v10 for RHEL 9. | ||
| * If RHEL 10 is available, build a FRR-k8s image against a RHEL10 stream. | ||
| * Do our own build of FRR v10 for RHEL 9 and cross tag it for OCP as we do with | ||
| OVN. This will require to assess and cover testing gaps vs the official RHEL |
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.
do you mean that we need to validate that FRR‑k8s v10 with RHEL9 ourselves?
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.
I have been told one of the ways this could happen is making FRR available through the FDP project, just as we have it for OVN and libreswan.
| QE testing should include testing upgrades from a cluster already making use of | ||
| the EVPN feature. | ||
|
|
||
| QE testing will need to emulate an BGP/EVPN fabric. While any kind of custom adn |
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.
Is there typo here in this line? what is "custom adn"?
c62a1c9 to
97be2a3
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
254eef6 to
ba82194
Compare
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
No description provided.