kubernetes-sigs / cluster-api-addon-provider-helm

CAAPH uses Helm charts to manage the installation and lifecycle of Cluster API add-ons.
Apache License 2.0
98 stars 26 forks source link

Support one-shot mode #65

Open tommasopozzetti opened 1 year ago

tommasopozzetti commented 1 year ago

User Story

As a user/operator I would like to be able to use this provider to bootstrap addons on a workload cluster, but then allow the workload cluster to take over their management. This mode is currently supported for CRS and I think there are use cases for mirroring it here.

Detailed Description

Add a field to the HelmChartProxy to request a "one-shot" mode for the helm release. If this is turned on, the provider requeues the helm chart installation until it gets a successful install on the workload cluster and then forgets about the helm release. After a successful install, there should be no more reconciliation, drift-detection, deletions or anything happening on that helm release irrespectively of what happens on the management cluster or on the workload cluster.

The use case for this is bootstrapping components that are crucial for successful initialization of a cluster (CNI, cloud-controller, some workload-cluster package manager itself, etc...) but then delegating the subsequent management of such releases to the workload cluster itself (which might have its own package manager or its own lifecycle manager for helm releases).

/kind feature

dtzar commented 1 year ago

Today CAAPH is a "one-shot" operation by default - no need for a special switch. The use case you describe I believe should be the primary focus of CAAPH.

I do think there can be more resiliency to make sure that the "one-time" operation for install is successful such as #16

Are you hoping for something else with CAAPH to "handoff" to something else for management?
Effectively, I don't see a feature here for CAAPH. I believe whatever else you decide to use for helm lifecycle management (e.g. GitOps) would be outside of scope of CAAPH.

tommasopozzetti commented 1 year ago

@dtzar thanks for the info and details!

I might have gotten confused then. Based on the following line in the docs:

Remove the label nginxIngressChart: enabled from the workload cluster. On the next reconciliation, the HelmChartProxy will notice that the workload cluster no longer matches the clusterSelector and will delete the HelmReleaseProxy associated with the Cluster and uninstall the chart.

and the comment in the issue you linked #16:

However, I think if the Helm install command succeeds it will requeue every 10 minutes (I believe) to check if the Helm release spec differs from the HelmChartProxy spec

I was under the impression that if something else were to then apply changes to the helm release on the workload cluster after initialization (future upgrades, deletions....etc), CAAPH would try to revert/reinstall it. Similarly, if on the management cluster something changes in the spec of the HelmChartProxy or the label is removed, it will remove the corresponding helm release.

Are any of the above the current behavior?

If so, then what my feature request would be some way to set an ignoreChanges of sorts, where after initial successful installation (as per recorded in the HelmReleaseProxy), it will stop monitoring for drift and it will never try to upgrade, reinstall or delete the workload cluster's helm release instance, irrespectively of whether the HelmChartProxy is updated/changed, whether the label on it no longer matches the cluster, or whether the workload cluster's helm release spec drift due to upgrades/deletions.

dtzar commented 1 year ago

@tommasopozzetti - you're right I have an incorrect knowledge of how it works. Certainly, we need some documentation update to explain the behavior in better detail. 🙂

Would it work for you if we did implement retryCount and set it to a value of 0? e.g. retryCount: 0

tommasopozzetti commented 1 year ago

@dtzar I think that would prevent the desired behavior of retrying until the first successful install. For example if upon initial install something were to fail due to some component not yet being available (like the CRD case mentioned in #16), I think it’s reasonable to have a retryCount such that it can retry several times until succeeding. After success, though, it should then stop monitoring.

Also, the retry count would still allow the behavior of deleting the helm release if the management cluster’s HelmChartProxy is deleted or if the label is removed, which is also something I think that should be ignored if this is enabled.

What are your thoughts on a ignoreChanges Boolean field defaulting to false that, if set, simply skips any reconciliation operation if the HelmReleaseProxy has a status of deployed?

Jont828 commented 1 year ago

@tommasopozzetti Yeah so as of now it's not designed to ignore or hand off an existing Helm release. Just so I understand, do you essentially want to have the cluster selector and template logic in this project without the rest?

tommasopozzetti commented 1 year ago

@Jont828 Here is my use case:

I create a workload cluster using cluster-api. This cluster will manage all its content through gitops. It will have a gitops controller of some sort running on it and it will keep itself in sync. The issue is the bootstrap. Right now cluster-api only allows creation of a cluster but components like the CNI and possibly the gitops controller of the cluster itself cannot be bootstrapped so there would need to be an initial manual operation to bootstrap those components. The current cluster-api official solution to this is ClusterResourceSets which are sets of k8s manifests that can get applied automatically at cluster creation by the management cluster. ClusterResourceSets support two modes of operation: ApplyOnce or Reconcile. As the names suggest, the former tries to apply these manifests until it finally succeeds to cleanly apply them all once (recording such success in a ClusterResourceSetBinding resource) and then forgets this ever was a thing. It doesn't matter whether the resource gets changed on the workload cluster or whether the CRS gets changed on the management cluster. That application on that cluster has been recorded as successful so it will never be done again. The latter instead monitors the resources for drift so that if anything is changed in the management cluster's CRS, it gets reapplied to the workload's cluster to keep the two in sync (effectively implementing a GitOps controller of sorts).

This system works very well and supports my use case with the use of the ApplyOnce paradigm as I can initially apply the CNI and the GitOps controller and then the GitOps controller on the workload cluster will take over management of all resources (CNI and GitOps controller itself included). The caveat is that CRS only supports plain k8s manifests, so if trying to install helm releases the only way to make use of it is to template out the helm release and use the raw manifests.

This project seems to fill that gap by providing a very similar concept but for helm-releases, although it seems it only currently support the equivalent of the Reconcile mode. My feature request is to implement the ApplyOnce mode equivalent where it runs a helm install on the workload cluster, it continues to retry it until a success and then after the success is recorded in the HelmReleaseProxy, it never tries to upgrade or delete such helm release ever again, no matter what happens on the management cluster.

Let me know if this scenario makes sense as use case and thanks for looking into this!

Jont828 commented 1 year ago

Gotcha thanks for explaining. The Cluster API Add-on Orchestration was designed specifically to fill the "Reconcile" gap that CRS was missing. At the time, CRS only had the ApplyOnce capability and we wanted to integrate add-ons to fit the CAPI convention of declarative specs. So I don't think adding an ApplyOnce option would really make sense with the design we're trying to move towards. That being said, I'm open to the idea if there's enough interest in the community for it. I also think the idea of "opting out" an existing release from further reconciliation is interesting and could potentially solve your problem.

I'll tag @dtzar, @CecileRobertMichon, and @fabriziopandini for additional thoughts too.

jackfrancis commented 1 year ago

@tommasopozzetti Why would you prefer to manually helm upgrade <helm release> <updated details> from your gitops flow rather than update the helm chart proxy resource via a kubectl apply patch (or there are a bunch of other different ways to instrument updates to k8s resources)?

What is the advantage to using a helm chart proxy resource once to install, but then to update the helm chart (or the underlying resources that helm creates) manually after that?

CecileRobertMichon commented 1 year ago

@tommasopozzetti have you checked if the paused (cluster.x-k8s.io/paused) annotation would allow you to achieve this (by adding it to the HelmChartProxy when you're ready for the workload cluster to take over control)

dtzar commented 1 year ago

@tommasopozzetti - your use case makes sense to me. I'd be curious to see (if you're open to it) how you do the GitOps bootstrapping with this addon.

The ClusterResourceSet is still experimental and hasn't been touched since 2020. I haven't proposed this yet, but my rough thoughts are if we could have this helm chart addon replace ClusterResourceSet. I realize there is some feature gap between the two, but it certainly is confusing IMO having both with overlapping functionalities.

dtzar commented 1 year ago

@tommasopozzetti Why would you prefer to manually helm upgrade <helm release> <updated details> from your gitops flow rather than update the helm chart proxy resource via a kubectl apply patch (or there are a bunch of other different ways to instrument updates to k8s resources)?

What is the advantage to using a helm chart proxy resource once to install, but then to update the helm chart (or the underlying resources that helm creates) manually after that?

The big reason is it doesn't scale well. Once you bootstrap with GitOps, you then have a much more powerful toolset than what CAPI* provides (by design) where everything else lives and trying to track maintaining this helm chart addon for reconciliation is now an out-of-band thing to care for.

tommasopozzetti commented 1 year ago

@dtzar hit the nail in the head of exactly why I would rather manage it this way. It boils down to scalability and consistency. Assuming we already have a GitOps workflow for managing components deployed to a Kubernetes cluster from the cluster itself, it would be awkward to maintain just a couple of components (namely the CNI and the GitOps controller itself) in a different (less feature rich by design) way. We are however forced to bootstrap them through a different system since clearly the GitOps controller cannot initially automatically install itself.

I think that cluster-api's focus should be mostly on what is needed to provide an interface between what already exists in the community and cluster creation. There are already several battle tested GitOps controllers in the community, such as ArgoCD or Flux, and I think that re-implementing one as a cluster-api add-on is likely to always fall short (in terms of features and customizability) of these more mature projects. Providing, however, a link between a new cluster being created by CAPI and these projects is a gap that CRS can partially fill today and this project has the potential to fill as well by adding the much needed ability to bootstrap helm releases.

Jont828 commented 1 year ago

I think that makes sense to me. @tommasopozzetti, do you think the pause annotation that @CecileRobertMichon mentioned would be sufficient to stop reconciliation for your use case?

tuxtof commented 1 year ago

I'm fully aligned with @tommasopozzetti we also need a possibility to choose between a ApplyOnce or Reconcile for a specific HelmChartProxy resource

Our use case is that a CAPI based automation deploy the workload cluster and CNI (via CAAPH) but once deployed we want the admin of workload cluster was able to tune the CNI settings by changing CNI helm value

we can also add a SmartReconcile approach where CAAPH will use the helm upgrade --reuse-values logic

Jont828 commented 1 year ago

@tuxtof @tommasopozzetti Sounds good, let's look into it then. Would either of you like to pick up the issue? If not I can start on it once I have some time.

manoj-nutanix commented 1 year ago

I'm fully aligned with @tommasopozzetti we also need a possibility to choose between a ApplyOnce or Reconcile for a specific HelmChartProxy resource

Our use case is that a CAPI based automation deploy the workload cluster and CNI (via CAAPH) but once deployed we want the admin of workload cluster was able to tune the CNI settings by changing CNI helm value

we can also add a SmartReconcile approach where CAAPH will use the helm upgrade --reuse-values logic

We should explore possibility of adding ApplyOnce or Reconcile as a part of annotation, it would be easier for users to annotate/de-annotate HCP CRs later. We shall not need SmartReconcile as it's option provided to upgrade operation, we can add it as part of resolution to issue where users can control behaviour of upgrade operation with options like reuse-values, reset-values, etc.

Jont828 commented 1 year ago

I think just adding ApplyOnce alongside Reconcile would be a good place to start. It looks like in CRS set this field in the spec to specify the strategy. From a UX perspective, I think adding ApplyOnce vs Reconcile as a spec field would make it pretty front and center to the user as part of the core design. Whereas adding it as an annotation would make it seem like more of a side feature since it wouldn't be as obvious to the user that this exists. WDYT?

manoj-nutanix commented 1 year ago

Sure, makes sense. If we want to make this feature extensible, have strict schema checks, adding it in spec would be better.

Jont828 commented 1 year ago

Sounds good. Would any of you like to work on this issue? If not, I can pick it up when I wrap up my existing PRs for the next CAPI release cycle.

manoj-nutanix commented 1 year ago

@Jont828 Later, if I get some bandwidth, I'll update you. If not, please feel free to proceed.

haiwu commented 7 months ago

Would there be any movement for this one?

Jont828 commented 7 months ago

I spoke with @haiwu and he said that he no longer needs this for his use case. @nikParasyr @tommasopozzetti do you still need this feature?

tommasopozzetti commented 7 months ago

@Jont828 we have gotten around our immediate needs with a different implementation so I don’t have an immediate use case for this anymore. That being said I do still think the above discussion is valid and this would be a very useful feature to add to fill a gap that is currently impossible to fill with other open source tools.

Jont828 commented 6 months ago

Fair enough, if anyone else needs this, I can try to take a look now that things are quieter after the CAPI release. Otherwise, I can leave it on the backlog.

k8s-triage-robot commented 3 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

dtzar commented 3 months ago

/remove-lifecycle stale

I think this is still a valid ask. IMO as discussed a bit above, I think we should do a feature gap analysis to CRS and then have CAAPH replace CRS at least for the places where installs happen on many clusters. At a quick glance, CAAPH doesn't fulfill the requirements for sharing secrets/config-maps across clusters and that would be out of scope for CAAPH.

fad3t commented 2 months ago

I'm interested by this feature as well, with the exact same use case (minimal cluster bootstrap, then ArgoCD takes over). We're using ClusterResourceSets for now but they come with limitations (e.g. large manifests don't fit the 1M object limit).

groundnuty commented 2 months ago

I'm also interested, I've been following this issue for a year, and now that we actually need this option (having integrated CAPI/CAAPH) I was hoping it's already done.

Jellyfrog commented 4 weeks ago

+1. We have the same use case as already mentioned, minimal bootstraping and then other tools takes over.