Skip to content
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

Draft: Add an unofficial and unsupported Kluctl based deployment/distribution to /contrib #2665

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

codablock
Copy link
Contributor

@codablock codablock commented Apr 2, 2024

This adds a Kluctl based deployment to the contrib folder. It reuses and references the original Kustomize manifests as much as possible, while introducing kluctl specific overlays when needed to add templating to those.

I had a short discussion with @juliusvonkohout about this being part of manifests/contrib and we agreed that I should create a PoC and bring this to discussion. My plan is to also present this in the community and manifests meetings.

I'd suggest to read the README.md of this PR for more details about motivation and reasoning. Please note that this README.md is originally from my out-of-tree version here (this repo is not outdated as I moved all work into manifests/contrib), which means it might be worded in a way that doesn't sound like it's part of manifests/contrib...I will change wording in the future.

This is all still WIP and really just at the point where discussion can begin. Long-term, I believe that this is a viable form of a distribution that could be offered alongside plain/classical Kustomize deployments. I understand that the Kustomize based deployment is not meant for end-users and I hope that the Kluctl based version (which builds on top of the Kustomize manifests) can fill this gap.

I have also copied two e2e tests (pipeline_kind_kluctl_test.yaml and pipeline_m2m_kind_kluctl_test.yaml) to also showcase how this can be used in testing. @diegolovison This is what I mentioned in Slack a few days ago.

Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: codablock
Once this PR has been reviewed and has the lgtm label, please assign juliusvonkohout for approval. For more information see the Kubernetes Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codablock
Copy link
Contributor Author

Important, if you're reviewing this before the release of v2.24.0 of Kluctl, you will need to download the devel release instead of following the installation instructions.

@diegolovison
Copy link
Contributor

From the README.md

leading to a lot of complicated and partially manual steps required to install

I think the opposite, you just need to have docker, kind and run while ! kustomize build example | kubectl apply -f -; do echo "Retrying to apply resources"; sleep 10; done

This project might even turn out to be a viable distribution of Kubeflow

I was going to say that this PR looks like a distribution of Kubeflow but I saw the same phrase in the README.md.


Maybe this PR should be a new repository and add your distribution at https://www.kubeflow.org/docs/started/installing-kubeflow/#packaged-distributions-of-kubeflow ?

@diegolovison
Copy link
Contributor

I saw that your PR contains also potential fixes.. What do you think about having it in a different PR?
In this case, while the Kluctl discussion is happening the fixes can be applied in the project.

@codablock
Copy link
Contributor Author

I think the opposite, you just need to have docker, kind and run while ! kustomize build example | kubectl apply -f -; do echo "Retrying to apply resources"; sleep 10; done

@diegolovison My statement also included things like choosing which kustomize deployments/overlays to enable and which not, basically building your own kustomization.yaml from the example. The while+kustomize loop is another topic that I assume has much potential for discussion on how optimal it is :)

I was going to say that this PR looks like a distribution of Kubeflow but I saw the same phrase in the README.md.

Yes, I'd like to make this part of the discussion. I'm fine with both approaches and already have a good setup in my head how I'd integrate the upstream manifests in case I'd go with a dedicated repo.

If this becomes part of manifests/contrib, it will somewhat become an official distribution or at least endorsed by the Kubeflow maintainers. If it moves into its own distribution, it would "just" be another distribution. In that case, I'd also be happy to see some official support (e.g by being listed in the distro list) as I feel that this type of distribution fits well into the overall project.

I saw that your PR contains also potential fixes.. What do you think about having it in a different PR?
In this case, while the Kluctl discussion is happening the fixes can be applied in the project.

Yes, this was the plan. I will do this in the next days.

@juliusvonkohout
Copy link
Member

/contrib is no official endorsement. It is incubating stuff.

@juliusvonkohout
Copy link
Member

@codablock i am rerunning the tests now. Please check the second and third attempts.

We also need to confine everything to /contrib for now.

@codablock
Copy link
Contributor Author

I've rebased the PR on master and force-pushed.

I extracted a few fixes of this PR into the upstream repos:

  1. fix(manifests): Move metacontroller to the top in kustmization.yaml pipelines#10669. I added sortOptions: legacy into the kluctl deployment for now until this fix is synced back into manifests.
  2. Reorder oauth2-proxy manifests so that namespace is first #2668
  3. Also use hashFromEnv for oauth2-proxy overlay #2669
  4. Split istio-external-auth into two components #2670

I'll rebase this PR again after these got merged.

Besides from the above extraced PRs, this PR does not touch anything outside of contrib/kluctl anymore. I refactored to have all its overlays inside contrib from now on. One thing unfortunately has to stay in apps: The .templateignore to force kluctl to never try to template upstream manifests.

@codablock codablock force-pushed the contrib-kluctl branch 2 times, most recently from 392a13b to 791dd97 Compare April 15, 2024 09:48
This deployment is a 100% replication of example/kustomization.yaml.
Configurability will follow in future commits.

Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
Signed-off-by: Alexander Block <ablock84@gmail.com>
@codablock
Copy link
Contributor Author

I've rebased on master and force-pushed. For some reason my kluctl based e2e workflows are not being run.

@juliusvonkohout
Copy link
Member

Are you sure that the paths you list are changed in this PR?

@juliusvonkohout
Copy link
Member

juliusvonkohout commented May 2, 2024

cc @kubeflow/kubeflow-steering-committee for input on having multiple also unofficial/unsupported deployment options.

@juliusvonkohout juliusvonkohout changed the title Draft: Add Kluctl based deployment/distribution to contrib Draft: Add an unofficial and unsupported Kluctl based deployment/distribution to /contrib May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants