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
WIP - Add support for OpenShift cluster provider #421
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>
[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 |
Hi @adetalhouet. Thanks for your PR. I'm waiting for a nephio-project member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
My only question here is whether this should instead be part of a separate, OpenShift set of controllers. If you look, for example, at K8s, they started with kube controller manager that embedded cloud-specific things, just like this. But eventually they realized maintaining that was hard - you end up with a binary with many, many, many dependencies, and those dependencies are updated on different schedules for different providers. To fix this, they spent literally years splitting all that code out into separate, per cloud-provider controller managers. My guess is we will find ourselves in a similar situation, where there are various controllers we need for each environment; those can all be bundled up into a vendor-specific controller manager where the user can run that controller-manager if they want to work with that environment. I would rather we start with the right set up for our code base, rather than a major refactor later on. So, I would suggest:
In the openshift environment (either for the mgmt cluster or even just for workload clusters), users would enable the openshift-controller-manager. WDYT? I have a couple GCP controllers (well, one that I use, but there are others) that are part of Porch right now. Ultimately I want to do the same with those. |
/ok-to-test |
val, ok := secret.Labels["hive.openshift.io/secret-type"] | ||
if ok && val == kubeconfig { | ||
return &openshift.OpenShift{Client: r.Client, Secret: secret}, true | ||
} | ||
} |
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 another approach here rather than hard coding the Openshift specific call? Perhaps a way of configuring this so that the label and the function to call is read from config/metadata would be a better approach.
Can we get a status update on this PR @adetalhouet ? |
What kind of update would you like? I don't have the time to test this between now and mid-Feb. So if this is a gating factor, then discard this PR for R2. I tested this back then when I submitted the PR. I'll have some time after mid-Feb. |
Hi @adetalhouet , I was just trying to get some movement on the outstanding PRs really. Support for alternative cluster provisioning is definitely needed so maybe we mark as WIP and revisit in R-3 |
OpenShift cluster provisioning works without this. This PR is simply to use the bootstrap sequence defined in Nephio, with the ultimate goal to install configsync and being able to then sync anything from there. Given the tight timeline for R2 and the actual bandwidth I have, I doubt I can revisit this. I'll mark as WIP and put in draft for now. |
Thank you sir :) |
The following PR nephio-project/nephio-example-packages#86 adds the proper RBAC to the nephio bootstrap controller