api
api copied to clipboard
openshift
SDN-5086: Add API and feature gates for OVN-K BGP support
Add API & feature gate to control the deployment of components that provide additional routing capabilities. MetalLB operator and OVN-K route advertisements feature will rely on this API.
Add API & feature gate to enable OVN-K route advertisements.
ref: https://github.com/openshift/enhancements/pull/1636/files
Hello @jcaamano! Some important instructions when contributing to openshift/api: API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.
![openshift-ci[bot] avatar](https://avatars.githubusercontent.com/in/91280?v=4 "Published by a pub.dev verified publisher"){kind=small}
/hold
While I would like relevant feedback, this may still not be set on its final form.
The option proposed with two flags is clean but it may appear evident that the extra flag is just there to hook into the MetalLB operator workflow, or from the other perspective, to allow MetalLB operator to hook into the CNO workflow. The guist is that both ovn-kubernetes BGP and MetalLB operator would need the same stuff deployed and we don't want to have it duplicated in the cluster.
Other options:
- Single flag and MetalLB operator users will have to tolerate an ovn-kubernetes rollout that they didn't have before. Users may consider the temporary control plane downtime a breaking change.
- Single flag and ovn-kubernetes will have the functionality enabled always (listening for CRD events) even if not planned to be used. Seems like waste of resources and not the best alignment with our 36 month lifecycle plans either.
- Single flag and a some other sort of CNO hidden API (unpublished config map) for MetalLB operator to leverage. Not great not terrible.
- Single flag and MetalLB operator deploys its own FRR-k8s deployment if the flag is not set, or uses the CNO FRR-k8s deployment if flag is set. Complex.
PTAL @JoelSpeed @trozet @fedepaol
@jcaamano: This pull request references SDN-5086 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 story to target the "4.17.0" version, but no target version was set.
In response to this:
Add API and feature gate for OVN-K BGP support
Add API to control the deployment of components that provide advanced routing capabilities. MetalLB operator and OVN-K route advertisements feature will rely on this API.
Add flag to enable OVN-K route advertisements feature.
ref: https://github.com/openshift/enhancements/pull/1636/files
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.
This last update goes for AdditionalRoutingCapabilities for the top most attribute & feature gate name.
Also splits RouteAdvertisements attribute to a separate feature gate, since there is a chance they both graduate at different times.
@JoelSpeed PTAL
@jcaamano: This pull request references SDN-5086 which is a valid jira issue.
In response to this:
Add API & feature gate to control the deployment of components that provide additional routing capabilities. MetalLB operator and OVN-K route advertisements feature will rely on this API.
Add API & feature gate to enable OVN-K route advertisements.
ref: https://github.com/openshift/enhancements/pull/1636/files
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.
My continuing niggle with this API is that we are telling users to enable a capability, that is in fact, a project name, and not a technical feature.
What is it exactly that FRR is providing to the end user, and is it possible we could name the capability after the end user visible functionality, rather than this project?
I guess this is the way to see it in the current proposal:
We are providing additional routing capabilities through different providers; what those specific capabilities are and how they are configured depend on the provider type (a provider does not necessarily represent a unique specific capability, one provider can provide many capabilities; and specific capabilities are not exclusive to one provider): at this time we are not offering any configuration to enable/disable specific capabilities through this API, the user (or other components) will need to deal directly with the provider's API; and just selecting a provider in this API will not make any functional change in the cluster other than deploying whatever is necessary for the provider.
The only provider available right now will be FRR-k8s which aims to provide all FRR capabilities.
I guess we could have taken a completely different route, which is add an API to enable specific features, completely abstracting the user for the fact that we are using FRR-k8s, FRR or whatever. That has its pros but also cons, specifically:
- Makes user life easier but takes choice out from him
- Forces us to replicate/abstract, and keep up with, the FRR-K8s API in our own API, which is something we are not ready to do
@JoelSpeed I dont really see a valid argument that we should not allow values for a specific technology or project. Let's take newtorkType in the API for example:
https://github.com/openshift/api/blob/84047ef4a2ce54dc7f879b1382690079081128f1/config/v1/types_network.go#L58
A user could provide values as OpenShiftSDN or OVNKubernetes. These are both 2 different projects/technologies.
You can think of FRR as its own networking stack that has a bunch of features like @jcaamano mentioned. Just like if you use OVNKubernetes you get a lot of features that OpenShiftSDN doesn't have. OVNKubernetes is limited in its overall feature set, and for advanced networking use cases we need the features in FRR. The two technologies will complement each other.
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema
In response to this:
/override ci/prow/verify-crd-schema
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.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: jcaamano, JoelSpeed, trozet
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [JoelSpeed]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/retest-required
Remaining retests: 0 against base HEAD 6b4a57ec20b0bfc77cb3909da4f1a9234d498571 and 2 for PR HEAD fd4baa4540dfd4e10158bccb545680ce81cd382e in total
@jcaamano: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| ci/prow/e2e-gcp | fd4baa4540dfd4e10158bccb545680ce81cd382e | link | false | /test e2e-gcp |
Full PR test history. Your PR dashboard.
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. I understand the commands that are listed here.
[ART PR BUILD NOTIFIER]
Distgit: ose-cluster-config-api This PR has been included in build ose-cluster-config-api-container-v4.18.0-202408071012.p0.g3cab566.assembly.stream.el9. All builds following this will include this PR.
