Merge pull request #307 from Jont828/reconcile-strategy

✨ Add `ReconcileStrategy` field to support continuous reconciliation or installing the Helm chart once

Author: k8s-ci-robot

Makefile

@@ -99,6 +99,10 @@ get_go_version = $(shell go list -f "{{.Version}}" -m $1)
 # Binaries.
 #
 # Note: Need to use abspath so we can invoke these from subdirectories
+
+# curl retries
+CURL_RETRIES=3
+
 KUSTOMIZE_VER := v5.3.0
 KUSTOMIZE_BIN := kustomize
 KUSTOMIZE := $(abspath $(TOOLS_BIN_DIR)/$(KUSTOMIZE_BIN)-$(KUSTOMIZE_VER))
@@ -190,6 +194,10 @@ TILT_PREPARE := $(abspath $(TOOLS_BIN_DIR)/$(TILT_PREPARE_BIN))
 GOLANGCI_LINT_BIN := golangci-lint
 GOLANGCI_LINT := $(abspath $(TOOLS_BIN_DIR)/$(GOLANGCI_LINT_BIN))
 
+HELM_VER := v3.14.4
+HELM_BIN := helm
+HELM := $(TOOLS_BIN_DIR)/$(HELM_BIN)-$(HELM_VER)
+
 # Define Docker related variables. Releases should modify and double check these vars.
 REGISTRY ?= gcr.io/$(shell gcloud config get-value project)
 PROD_REGISTRY ?= registry.k8s.io/cluster-api-helm
@@ -394,6 +402,9 @@ ARTIFACTS ?= ${ROOT_DIR}/_artifacts
 
 KUBEBUILDER_ASSETS ?= $(shell $(SETUP_ENVTEST) use --use-env -p path $(KUBEBUILDER_ENVTEST_KUBERNETES_VERSION))
 
+.PHONY: install-tools # populate hack/tools/bin
+install-tools: $(ENVSUBST) $(KUSTOMIZE) $(HELM) $(GINKGO) # Should kubectl be added here?
+
 .PHONY: test
 test: $(SETUP_ENVTEST) ## Run unit and integration tests
 	KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test ./... $(TEST_ARGS)
@@ -428,7 +439,7 @@ test-e2e-local: ## Run `docker-build` rule then run e2e tests.
 	test-e2e-run
 
 .PHONY: test-e2e-run-skip-manifest
-test-e2e-run-skip-manifest: $(GINKGO) $(ENVSUBST) generate-e2e-templates ## Run the end-to-end tests without updating the manifest.
+test-e2e-run-skip-manifest: $(GINKGO) $(ENVSUBST) generate-e2e-templates install-tools ## Run the end-to-end tests without updating the manifest.
 	$(ENVSUBST) < $(E2E_CONF_FILE) > $(E2E_CONF_FILE_ENVSUBST) && \
 	$(GINKGO) -v --trace -poll-progress-after=$(GINKGO_POLL_PROGRESS_AFTER) \
 		-poll-progress-interval=$(GINKGO_POLL_PROGRESS_INTERVAL) --tags=e2e --focus="$(GINKGO_FOCUS)" \
@@ -729,6 +740,9 @@ $(GOLANGCI_LINT_BIN): $(GOLANGCI_LINT) ## Build a local copy of golangci-lint
 .PHONY: $(GINKGO_BIN)
 $(GINKGO_BIN): $(GINKGO) ## Build a local copy of ginkgo
 
+.PHONY: $(HELM_BIN)
+$(HELM_BIN): $(HELM) ## Build a local copy of Helm
+
 $(CONTROLLER_GEN): # Build controller-gen from tools folder.
 	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(CONTROLLER_GEN_PKG) $(CONTROLLER_GEN_BIN) $(CONTROLLER_GEN_VER)
 
@@ -787,3 +801,12 @@ $(GOLANGCI_LINT): .github/workflows/golangci-lint.yml # Download golangci-lint u
 
 $(GINKGO): # Build ginkgo from tools folder.
 	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(GINKGO_PKG) $(GINKGO_BIN) $(GINGKO_VER)
+
+$(HELM): ## Put helm into tools folder.
+	mkdir -p $(TOOLS_BIN_DIR)
+	rm -f "$(TOOLS_BIN_DIR)/$(HELM_BIN)*"
+	curl --retry $(CURL_RETRIES) -fsSL -o $(TOOLS_BIN_DIR)/get_helm.sh https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3
+	chmod 700 $(TOOLS_BIN_DIR)/get_helm.sh
+	USE_SUDO=false HELM_INSTALL_DIR=$(TOOLS_BIN_DIR) DESIRED_VERSION=$(HELM_VER) BINARY_NAME=$(HELM_BIN)-$(HELM_VER) $(TOOLS_BIN_DIR)/get_helm.sh
+	ln -sf $(HELM) $(TOOLS_BIN_DIR)/$(HELM_BIN)
+	rm -f $(TOOLS_BIN_DIR)/get_helm.sh

api/v1alpha1/helmchartproxy_types.go

@@ -22,13 +22,25 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
+// ReconcileStrategy is a string representation of the reconciliation strategy of a HelmChartProxy.
+type ReconcileStrategy string
+
 const (
 	// HelmChartProxyFinalizer is the finalizer used by the HelmChartProxy controller to cleanup add-on resources when
 	// a HelmChartProxy is being deleted.
 	HelmChartProxyFinalizer = "helmchartproxy.addons.cluster.x-k8s.io"
 
 	// DefaultOCIKey is the default file name of the OCI secret key.
 	DefaultOCIKey = "config.json"
+
+	// ReconcileStrategyContinuous is the default reconciliation strategy for HelmChartProxy. It will attempt to install the Helm
+	// chart on a selected Cluster, update the Helm release to match the current HelmChartProxy spec, and delete the Helm release
+	// if the Cluster no longer selected.
+	ReconcileStrategyContinuous ReconcileStrategy = "Continuous"
+
+	// ReconcileStrategyInstallOnce attempts to install the Helm chart for a HelmChartProxy on a selected Cluster, and once
+	// it is installed, it will not attempt to update or delete the Helm release on the Cluster again.
+	ReconcileStrategyInstallOnce ReconcileStrategy = "InstallOnce"
 )
 
 // HelmChartProxySpec defines the desired state of HelmChartProxy.
@@ -64,6 +76,14 @@ type HelmChartProxySpec struct {
 	// +optional
 	ValuesTemplate string `json:"valuesTemplate,omitempty"`
 
+	// ReconcileStrategy indicates whether a Helm chart should be continuously installed, updated, and uninstalled on selected Clusters,
+	// or if it should be reconciled until it is successfully installed on selected Clusters and not otherwise updated or uninstalled.
+	// If not specified, the default behavior will be to reconcile continuously. This field is immutable.
+	// Possible values are `Continuous`, `InstallOnce`, or unset.
+	// +kubebuilder:validation:Enum="";InstallOnce;Continuous;
+	// +optional
+	ReconcileStrategy string `json:"reconcileStrategy,omitempty"`
+
 	// Options represents CLI flags passed to Helm operations (i.e. install, upgrade, delete) and
 	// include options such as wait, skipCRDs, timeout, waitForJobs, etc.
 	// +optional

api/v1alpha1/helmchartproxy_webhook.go

@@ -21,8 +21,10 @@ import (
 	"net/url"
 	"time"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -70,30 +72,50 @@ func (p *HelmChartProxy) Default() {
 var _ webhook.Validator = &HelmChartProxy{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (r *HelmChartProxy) ValidateCreate() (admission.Warnings, error) {
-	helmchartproxylog.Info("validate create", "name", r.Name)
+func (p *HelmChartProxy) ValidateCreate() (admission.Warnings, error) {
+	helmchartproxylog.Info("validate create", "name", p.Name)
 
-	if err := isUrlValid(r.Spec.RepoURL); err != nil {
+	if err := isUrlValid(p.Spec.RepoURL); err != nil {
 		return nil, err
 	}
 
 	return nil, nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (r *HelmChartProxy) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	helmchartproxylog.Info("validate update", "name", r.Name)
+func (p *HelmChartProxy) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) {
+	helmchartproxylog.Info("validate update", "name", p.Name)
 
-	if err := isUrlValid(r.Spec.RepoURL); err != nil {
-		return nil, err
+	var allErrs field.ErrorList
+	old, ok := oldRaw.(*HelmChartProxy)
+	if !ok {
+		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a HelmChartProxy but got a %T", old))
+	}
+
+	if err := isUrlValid(p.Spec.RepoURL); err != nil {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "RepoURL"),
+				p.Spec.ReleaseNamespace, err.Error()),
+		)
+	}
+
+	if p.Spec.ReconcileStrategy != old.Spec.ReconcileStrategy {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "ReconcileStrategy"),
+				p.Spec.ReconcileStrategy, "field is immutable"),
+		)
+	}
+
+	if len(allErrs) > 0 {
+		return nil, apierrors.NewInvalid(GroupVersion.WithKind("HelmChartProxy").GroupKind(), p.Name, allErrs)
 	}
 
 	return nil, nil
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (r *HelmChartProxy) ValidateDelete() (admission.Warnings, error) {
-	helmchartproxylog.Info("validate delete", "name", r.Name)
+func (p *HelmChartProxy) ValidateDelete() (admission.Warnings, error) {
+	helmchartproxylog.Info("validate delete", "name", p.Name)
 
 	// TODO(user): fill in your validation logic upon object deletion.
 	return nil, nil

api/v1alpha1/helmreleaseproxy_types.go

@@ -32,6 +32,10 @@ const (
 
 	// IsReleaseNameGeneratedAnnotation is the annotation signifying the Helm release name is auto-generated.
 	IsReleaseNameGeneratedAnnotation = "helmreleaseproxy.addons.cluster.x-k8s.io/is-release-name-generated"
+
+	// ReleaseSuccessfullyInstalledAnnotation is the annotation signifying the Helm release has been successfully installed at least once.
+	// This is used to determine if the HelmReleaseProxy is in a ready state for the InstallOnce strategy.
+	ReleaseSuccessfullyInstalledAnnotation = "helmreleaseproxy.addons.cluster.x-k8s.io/release-successfully-installed"
 )
 
 // HelmReleaseProxySpec defines the desired state of HelmReleaseProxy.
@@ -66,6 +70,14 @@ type HelmReleaseProxySpec struct {
 	// +optional
 	Values string `json:"values,omitempty"`
 
+	// ReconcileStrategy indicates whether a Helm chart should be continuously installed, updated, and uninstalled on the Cluster,
+	// or if it should be reconciled until it is successfully installed on the Cluster and not otherwise updated or uninstalled.
+	// If not specified, the default behavior will be to reconcile continuously. This field is immutable.
+	// Possible values are `Continuous`, `InstallOnce`, or unset.
+	// +kubebuilder:validation:Enum="";InstallOnce;Continuous;
+	// +optional
+	ReconcileStrategy string `json:"reconcileStrategy,omitempty"`
+
 	// Options represents the helm setting options which can be used to control behaviour of helm operations(Install, Upgrade, Delete, etc)
 	// via options like wait, skipCrds, timeout, waitForJobs, etc.
 	// +optional

api/v1alpha1/helmreleaseproxy_webhook.go

@@ -59,48 +59,55 @@ func (p *HelmReleaseProxy) Default() {
 var _ webhook.Validator = &HelmReleaseProxy{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (r *HelmReleaseProxy) ValidateCreate() (admission.Warnings, error) {
-	helmreleaseproxylog.Info("validate create", "name", r.Name)
+func (p *HelmReleaseProxy) ValidateCreate() (admission.Warnings, error) {
+	helmreleaseproxylog.Info("validate create", "name", p.Name)
 
 	// TODO(user): fill in your validation logic upon object creation.
 	return nil, nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (r *HelmReleaseProxy) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) {
-	helmreleaseproxylog.Info("validate update", "name", r.Name)
+func (p *HelmReleaseProxy) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) {
+	helmreleaseproxylog.Info("validate update", "name", p.Name)
 
 	var allErrs field.ErrorList
 	old, ok := oldRaw.(*HelmReleaseProxy)
 	if !ok {
 		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a HelmReleaseProxy but got a %T", old))
 	}
 
-	if !reflect.DeepEqual(r.Spec.RepoURL, old.Spec.RepoURL) {
+	if !reflect.DeepEqual(p.Spec.RepoURL, old.Spec.RepoURL) {
 		allErrs = append(allErrs,
 			field.Invalid(field.NewPath("spec", "RepoURL"),
-				r.Spec.RepoURL, "field is immutable"),
+				p.Spec.RepoURL, "field is immutable"),
 		)
 	}
 
-	if !reflect.DeepEqual(r.Spec.ChartName, old.Spec.ChartName) {
+	if !reflect.DeepEqual(p.Spec.ChartName, old.Spec.ChartName) {
 		allErrs = append(allErrs,
 			field.Invalid(field.NewPath("spec", "ChartName"),
-				r.Spec.ChartName, "field is immutable"),
+				p.Spec.ChartName, "field is immutable"),
 		)
 	}
 
-	if !reflect.DeepEqual(r.Spec.ReleaseNamespace, old.Spec.ReleaseNamespace) {
+	if !reflect.DeepEqual(p.Spec.ReleaseNamespace, old.Spec.ReleaseNamespace) {
 		allErrs = append(allErrs,
 			field.Invalid(field.NewPath("spec", "ReleaseNamespace"),
-				r.Spec.ReleaseNamespace, "field is immutable"),
+				p.Spec.ReleaseNamespace, "field is immutable"),
+		)
+	}
+
+	if p.Spec.ReconcileStrategy != old.Spec.ReconcileStrategy {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "ReconcileStrategy"),
+				p.Spec.ReconcileStrategy, "field is immutable"),
 		)
 	}
 
 	// TODO: add webhook for ReleaseName. Currently it's being set if the release name is generated.
 
 	if len(allErrs) > 0 {
-		return nil, apierrors.NewInvalid(GroupVersion.WithKind("HelmReleaseProxy").GroupKind(), r.Name, allErrs)
+		return nil, apierrors.NewInvalid(GroupVersion.WithKind("HelmReleaseProxy").GroupKind(), p.Name, allErrs)
 	}
 
 	return nil, nil

config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml

@@ -259,6 +259,17 @@ spec:
                       after a Helm install/upgrade has been performed.
                     type: boolean
                 type: object
+              reconcileStrategy:
+                description: |-
+                  ReconcileStrategy indicates whether a Helm chart should be continuously installed, updated, and uninstalled on selected Clusters,
+                  or if it should be reconciled until it is successfully installed on selected Clusters and not otherwise updated or uninstalled.
+                  If not specified, the default behavior will be to reconcile continuously. This field is immutable.
+                  Possible values are `Continuous`, `InstallOnce`, or unset.
+                enum:
+                - ""
+                - InstallOnce
+                - Continuous
+                type: string
               releaseName:
                 description: ReleaseName is the release name of the installed Helm
                   chart. If it is not specified, a name will be generated.

config/crd/bases/addons.cluster.x-k8s.io_helmreleaseproxies.yaml

@@ -266,6 +266,17 @@ spec:
                       after a Helm install/upgrade has been performed.
                     type: boolean
                 type: object
+              reconcileStrategy:
+                description: |-
+                  ReconcileStrategy indicates whether a Helm chart should be continuously installed, updated, and uninstalled on the Cluster,
+                  or if it should be reconciled until it is successfully installed on the Cluster and not otherwise updated or uninstalled.
+                  If not specified, the default behavior will be to reconcile continuously. This field is immutable.
+                  Possible values are `Continuous`, `InstallOnce`, or unset.
+                enum:
+                - ""
+                - InstallOnce
+                - Continuous
+                type: string
               releaseName:
                 description: ReleaseName is the release name of the installed Helm
                   chart. If it is not specified, a name will be generated.

config/manager/manager.yaml

@@ -33,6 +33,7 @@ spec:
           - --leader-elect
           - "--diagnostics-address=${CAAPH_DIAGNOSTICS_ADDRESS:=:8443}"
           - "--insecure-diagnostics=${CAAPH_INSECURE_DIAGNOSTICS:=false}"
+          - "--sync-period=${CAAPH_SYNC_PERIOD:=10m}"
           - "--v=2"
           image: controller:latest
           imagePullPolicy: Always

controllers/helmchartproxy/helmchartproxy_controller.go

@@ -187,11 +187,14 @@ func (r *HelmChartProxyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChartProxy *addonsv1alpha1.HelmChartProxy, clusters []clusterv1.Cluster, helmReleaseProxies []addonsv1alpha1.HelmReleaseProxy) error {
 	log := ctrl.LoggerFrom(ctx)
 
-	log.V(2).Info("Starting reconcileNormal for chart proxy", "name", helmChartProxy.Name)
+	log.V(2).Info("Starting reconcileNormal for chart proxy", "name", helmChartProxy.Name, "strategy", helmChartProxy.Spec.ReconcileStrategy)
 
-	err := r.deleteOrphanedHelmReleaseProxies(ctx, helmChartProxy, clusters, helmReleaseProxies)
-	if err != nil {
-		return err
+	// If Reconcile strategy is not InstallOnce, delete orphaned HelmReleaseProxies
+	if !(helmChartProxy.Spec.ReconcileStrategy == string(addonsv1alpha1.ReconcileStrategyInstallOnce)) {
+		err := r.deleteOrphanedHelmReleaseProxies(ctx, helmChartProxy, clusters, helmReleaseProxies)
+		if err != nil {
+			return err
+		}
 	}
 
 	for _, cluster := range clusters {

controllers/helmchartproxy/helmchartproxy_controller_phases.go

@@ -70,22 +70,29 @@ func (r *HelmChartProxyReconciler) reconcileForCluster(ctx context.Context, helm
 		// TODO: Should we set a condition here?
 		return errors.Wrapf(err, "failed to get HelmReleaseProxy for cluster %s", cluster.Name)
 	}
-	// log.V(2).Info("Found existing HelmReleaseProxy", "cluster", cluster.Name, "release", existingHelmReleaseProxy.Name)
 
-	if existingHelmReleaseProxy != nil && shouldReinstallHelmRelease(ctx, existingHelmReleaseProxy, helmChartProxy) {
-		log.V(2).Info("Reinstalling Helm release by deleting and creating HelmReleaseProxy", "helmReleaseProxy", existingHelmReleaseProxy.Name)
-		if err := r.deleteHelmReleaseProxy(ctx, existingHelmReleaseProxy); err != nil {
-			conditions.MarkFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition, addonsv1alpha1.HelmReleaseProxyDeletionFailedReason, clusterv1.ConditionSeverityError, "%s", err.Error())
+	if helmChartProxy.Spec.ReconcileStrategy == string(addonsv1alpha1.ReconcileStrategyInstallOnce) {
+		if internal.HasHelmReleaseBeenSuccessfullyInstalled(existingHelmReleaseProxy) {
+			log.V(2).Info("HelmReleaseProxy has been installed on InstallOnce mode, nothing to do", "helmReleaseProxy", existingHelmReleaseProxy.Name, "cluster", cluster.Name)
 
-			return err
+			return nil
 		}
+	} else { // ReconcileStrategy == `Continuous` or unset
+		if existingHelmReleaseProxy != nil && shouldReinstallHelmRelease(ctx, existingHelmReleaseProxy, helmChartProxy) {
+			log.V(2).Info("Reinstalling Helm release by deleting and creating HelmReleaseProxy", "helmReleaseProxy", existingHelmReleaseProxy.Name)
+			if err := r.deleteHelmReleaseProxy(ctx, existingHelmReleaseProxy); err != nil {
+				conditions.MarkFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition, addonsv1alpha1.HelmReleaseProxyDeletionFailedReason, clusterv1.ConditionSeverityError, "%s", err.Error())
 
-		// TODO: Add a check on requeue to make sure that the HelmReleaseProxy isn't still deleting
-		log.V(2).Info("Successfully deleted HelmReleaseProxy on cluster, returning to requeue for reconcile", "cluster", cluster.Name)
-		conditions.MarkFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition, addonsv1alpha1.HelmReleaseProxyReinstallingReason, clusterv1.ConditionSeverityInfo, "HelmReleaseProxy on cluster '%s' successfully deleted, preparing to reinstall", cluster.Name)
+				return err
+			}
 
-		return nil // Try returning early so it will requeue
-		// TODO: should we continue in the loop or just requeue?
+			// TODO: Add a check on requeue to make sure that the HelmReleaseProxy isn't still deleting
+			log.V(2).Info("Successfully deleted HelmReleaseProxy on cluster, returning to requeue for reconcile", "cluster", cluster.Name)
+			conditions.MarkFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition, addonsv1alpha1.HelmReleaseProxyReinstallingReason, clusterv1.ConditionSeverityInfo, "HelmReleaseProxy on cluster '%s' successfully deleted, preparing to reinstall", cluster.Name)
+
+			return nil // Try returning early so it will requeue
+			// TODO: should we continue in the loop or just requeue?
+		}
 	}
 
 	values, err := internal.ParseValues(ctx, r.Client, helmChartProxy.Spec, &cluster)
@@ -219,6 +226,7 @@ func constructHelmReleaseProxy(existing *addonsv1alpha1.HelmReleaseProxy, helmCh
 		}
 	}
 
+	helmReleaseProxy.Spec.ReconcileStrategy = helmChartProxy.Spec.ReconcileStrategy
 	helmReleaseProxy.Spec.Version = helmChartProxy.Spec.Version
 	helmReleaseProxy.Spec.Values = parsedValues
 	helmReleaseProxy.Spec.Options = helmChartProxy.Spec.Options