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

Feature Request: Support translating patch rules to template in ClusterClass controller #10556

Closed
Pangzhihong opened this issue May 4, 2024 · 13 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.

Comments

@Pangzhihong
Copy link

What would you like to be added (User Story)?

Support translating patch rules to template in ClusterClass controller

Detailed Description

The current clusterclass function in the cluster-api can apply a cluster's patch rule to a template, which was an internal method at the time. We have a scenario where we need to use this patch function, and we want to provide an ability to escape the content of the patch rule to the corresponding template.

Anything else you would like to add?

No response

Label(s) to be applied

/kind feature
One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 4, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

CAPI contributors will take a look as soon as possible, apply one of the triage/* labels and provide further guidance.

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.

@fabriziopandini
Copy link
Member

/priority awaiting-more-evidence

Could you kindly provide more detail about what do you mean with "The current clusterclass function in the cluster-api" and what you want to achieve.

As a general note, please be aware that generally we don't want to expose CAPI controllers code as a library / public functions; there are only two exception:

  • the main reconcile function is alway spublic, so folks can eventually bin pack controllers
  • we expose carefully few functions for the sake of making it easier to implement E2E tests.

@k8s-ci-robot k8s-ci-robot added the priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. label May 6, 2024
@Pangzhihong
Copy link
Author

Pangzhihong commented May 6, 2024

/priority awaiting-more-evidence

Could you kindly provide more detail about what do you mean with "The current clusterclass function in the cluster-api" and what you want to achieve.

As a general note, please be aware that generally we don't want to expose CAPI controllers code as a library / public functions; there are only two exception:

  • the main reconcile function is alway spublic, so folks can eventually bin pack controllers
  • we expose carefully few functions for the sake of making it easier to implement E2E tests.

Thank you for your reply!
We want to use this method(applyPatchesToRequest)to implement the patch capability of the template (instead of relying on CAPI controllers),currently, this method is not public:

func applyPatchesToRequest(ctx context.Context, req *runtimehooksv1.GeneratePatchesRequest, resp *runtimehooksv1.GeneratePatchesResponse) error {}

@sbueringer
Copy link
Member

sbueringer commented May 6, 2024

Sorry but we're not going to make arbitrary internal functions public. Feel free to copy the code if it's useful to you. It's just not sustainable for us to maintain something like this.

@Pangzhihong
Copy link
Author

Sorry but we're not going to make arbitrary internal functions public. Feel free to copy the code if it's useful to you. It's just not sustainable for us to maintain something like this.

Can you provide relevant controllers to parse the clusterclass results into various templates and provide them to the Portal or Web to demonstrate the correctness of the patch rule?

@sbueringer
Copy link
Member

sbueringer commented May 7, 2024

No sorry. We can't add additional controllers to CAPI for this use case

@Pangzhihong
Copy link
Author

No sorry. We can't add additional controllers to CAPI for this use case

Thanks for the reply, but I'm wondering why,does it affect the definition of the CAPI architecture? If we want to implement such requirements, can we only copy the code?

@sbueringer
Copy link
Member

We are usually not adding controllers to CAPI to provide a backend for UIs that folks are building on top of CAPI.

The goal of our controllers is to reconcile objects within Kubernetes clusters.

@sbueringer
Copy link
Member

@fabriziopandini Maybe you have a better way to express it, but this seems entirely out of scope for the project

@fabriziopandini
Copy link
Member

fabriziopandini commented May 7, 2024

if we want to implement such requirements, can we only copy the code?

Yes, the project uses Apache 2 licence, so you are free to copy the code.

@fabriziopandini Maybe you have a better way to express it, but this seems entirely out of scope for the project

The reason is explained in https://cluster-api.sigs.k8s.io/user/manifesto#the-complexity-budget
TL;DR As a project we must focus on what is core to the project/important for the majority of the users, and do it well. Transforming CAPI controllers into a library for others to re-use bit and pieces is not core to the project, and it is the reason why we moved all the controllers to internal packages back in the 1.0 times (with the only allowed exceptions I have already documented above)

Also, PTAL #10138 who document previous attempt to solve the same problem and why we are now dropping this effort (because there was no interest to implement it up to a good state)

Happy to discuss this at the office hours if it can help, but I agree with @sbueringer to not proceed with this feature request

@Pangzhihong
Copy link
Author

if we want to implement such requirements, can we only copy the code?

Yes, the project uses Apache 2 licence, so you are free to copy the code.

@fabriziopandini Maybe you have a better way to express it, but this seems entirely out of scope for the project

The reason is explained in https://cluster-api.sigs.k8s.io/user/manifesto#the-complexity-budget TL;DR As a project we must focus on what is core to the project/important for the majority of the users, and do it well. Transforming CAPI controllers into a library for others to re-use bit and pieces is not core to the project, and it is the reason why we moved all the controllers to internal packages back in the 1.0 times (with the only allowed exceptions I have already documented above)

Also, PTAL #10138 who document previous attempt to solve the same problem and why we are now dropping this effort (because there was no interest to implement it up to a good state)

Happy to discuss this at the office hours if it can help, but I agree with @sbueringer to not proceed with this feature request

Thanks, what do you guys suggest, folk to a personal repository and then copy it and use it or is there a better suggestion?

@fabriziopandini
Copy link
Member

I think there is no perfect answers, it boils up to use the process you are comfortable with.

Since this is not going to be implemented in Cluster API
/close

@k8s-ci-robot
Copy link
Contributor

@fabriziopandini: Closing this issue.

In response to this:

I think there is no perfect answers, it boils up to use the process you are comfortable with.

Since this is not going to be implemented in Cluster API
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.
Projects
None yet
Development

No branches or pull requests

4 participants