From 1a166766489022366e52346e93d3b6bcbe15d803 Mon Sep 17 00:00:00 2001 From: Ilya Drey Date: Thu, 24 Sep 2026 10:08:02 +0300 Subject: [PATCH] feat(api): let a release set the helm action timeout HelmApplication and HelmClusterAddon take spec.timeout, shaped like the flux HelmRelease field and bounded to above zero and at most 2h, and pass it to the internal HelmRelease, including while the release is being deleted so a stuck uninstall can be given more time. Signed-off-by: Ilya Drey --- api/v1alpha1/helm_application.go | 8 +++ api/v1alpha1/helm_cluster_addon.go | 8 +++ api/v1alpha1/zz_generated.deepcopy.go | 10 ++++ crds/doc-ru-helmapplications.yaml | 3 + crds/doc-ru-helmclusteraddons.yaml | 3 + crds/helmapplications.yaml | 11 ++++ crds/helmclusteraddons.yaml | 11 ++++ .../internal/adapter/addon_release.go | 1 + .../internal/adapter/application_release.go | 1 + .../internal/services/release_service.go | 2 + .../internal/services/release_service_test.go | 57 +++++++++++++++++++ .../internal/source/release.go | 2 + 12 files changed, 117 insertions(+) diff --git a/api/v1alpha1/helm_application.go b/api/v1alpha1/helm_application.go index efc5d678..d6e930fa 100644 --- a/api/v1alpha1/helm_application.go +++ b/api/v1alpha1/helm_application.go @@ -180,6 +180,14 @@ type HelmApplicationSpec struct { // +kubebuilder:validation:Enum="";NoResourceReconciliation // +optional Maintenance string `json:"maintenance,omitempty"` + // Timeout is the time to wait for any individual Kubernetes operation (like Jobs + // for hooks) during the performance of any Helm action. Defaults to 5m. + // +kubebuilder:validation:Type=string + // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ms|s|m|h))+$" + // +kubebuilder:validation:XValidation:rule="duration(self) > duration('0s')",message="timeout must be greater than zero" + // +kubebuilder:validation:XValidation:rule="duration(self) <= duration('2h')",message="timeout must not exceed 2h" + // +optional + Timeout *metav1.Duration `json:"timeout,omitempty"` } // The XValidation rule below states the relationship between the two reference diff --git a/api/v1alpha1/helm_cluster_addon.go b/api/v1alpha1/helm_cluster_addon.go index e0ea11ec..561dec7b 100644 --- a/api/v1alpha1/helm_cluster_addon.go +++ b/api/v1alpha1/helm_cluster_addon.go @@ -153,6 +153,14 @@ type HelmClusterAddonSpec struct { // +kubebuilder:validation:Enum="";NoResourceReconciliation // +optional Maintenance string `json:"maintenance,omitempty"` + // Timeout is the time to wait for any individual Kubernetes operation (like Jobs + // for hooks) during the performance of any Helm action. Defaults to 5m. + // +kubebuilder:validation:Type=string + // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ms|s|m|h))+$" + // +kubebuilder:validation:XValidation:rule="duration(self) > duration('0s')",message="timeout must be greater than zero" + // +kubebuilder:validation:XValidation:rule="duration(self) <= duration('2h')",message="timeout must not exceed 2h" + // +optional + Timeout *metav1.Duration `json:"timeout,omitempty"` } type HelmClusterAddonChartRef struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 4a24d4ad..12e3daaf 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -294,6 +294,11 @@ func (in *HelmApplicationSpec) DeepCopyInto(out *HelmApplicationSpec) { *out = new(apiextensionsv1.JSON) (*in).DeepCopyInto(*out) } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(v1.Duration) + **out = **in + } return } @@ -567,6 +572,11 @@ func (in *HelmClusterAddonSpec) DeepCopyInto(out *HelmClusterAddonSpec) { *out = new(apiextensionsv1.JSON) (*in).DeepCopyInto(*out) } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(v1.Duration) + **out = **in + } return } diff --git a/crds/doc-ru-helmapplications.yaml b/crds/doc-ru-helmapplications.yaml index 308b5986..6795908b 100644 --- a/crds/doc-ru-helmapplications.yaml +++ b/crds/doc-ru-helmapplications.yaml @@ -31,6 +31,9 @@ spec: При значении `NoResourceReconciliation` контроллер прекращает обновление управляемых ресурсов, что позволяет выполнять ручное вмешательство или обслуживание без перезаписи изменений оператором. При пустом значении (`""`) используется стандартное согласование. + timeout: + description: | + Время ожидания каждой отдельной операции Kubernetes (например, Job для хуков) при выполнении любого действия Helm. По умолчанию — `5m`. Должно быть больше нуля и не может превышать `2h`. values: description: Пользовательские значения для релиза HelmApplication. status: diff --git a/crds/doc-ru-helmclusteraddons.yaml b/crds/doc-ru-helmclusteraddons.yaml index e9ef696e..dbe1236d 100644 --- a/crds/doc-ru-helmclusteraddons.yaml +++ b/crds/doc-ru-helmclusteraddons.yaml @@ -30,6 +30,9 @@ spec: При пустом значении (`""`) используется стандартное согласование. namespace: description: Пространство имён для развёртывания релиза аддона. + timeout: + description: | + Время ожидания каждой отдельной операции Kubernetes (например, Job для хуков) при выполнении любого действия Helm. По умолчанию — `5m`. Должно быть больше нуля и не может превышать `2h`. values: description: Пользовательские значения для релиза HelmClusterAddon. status: diff --git a/crds/helmapplications.yaml b/crds/helmapplications.yaml index 923bd1d5..c185dc58 100644 --- a/crds/helmapplications.yaml +++ b/crds/helmapplications.yaml @@ -120,6 +120,17 @@ spec: - "" - NoResourceReconciliation type: string + timeout: + description: |- + Timeout is the time to wait for any individual Kubernetes operation (like Jobs + for hooks) during the performance of any Helm action. Defaults to 5m. + pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$ + type: string + x-kubernetes-validations: + - message: timeout must be greater than zero + rule: duration(self) > duration('0s') + - message: timeout must not exceed 2h + rule: duration(self) <= duration('2h') values: description: Values holds the values for this HelmApplication release. x-kubernetes-preserve-unknown-fields: true diff --git a/crds/helmclusteraddons.yaml b/crds/helmclusteraddons.yaml index 6a31b1e8..c9146f57 100644 --- a/crds/helmclusteraddons.yaml +++ b/crds/helmclusteraddons.yaml @@ -94,6 +94,17 @@ spec: maxLength: 63 minLength: 3 type: string + timeout: + description: |- + Timeout is the time to wait for any individual Kubernetes operation (like Jobs + for hooks) during the performance of any Helm action. Defaults to 5m. + pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$ + type: string + x-kubernetes-validations: + - message: timeout must be greater than zero + rule: duration(self) > duration('0s') + - message: timeout must not exceed 2h + rule: duration(self) <= duration('2h') values: description: Values holds the values for this HelmClusterAddon release. x-kubernetes-preserve-unknown-fields: true diff --git a/images/operator-helm-controller/internal/adapter/addon_release.go b/images/operator-helm-controller/internal/adapter/addon_release.go index 174ac9bf..2bcb9518 100644 --- a/images/operator-helm-controller/internal/adapter/addon_release.go +++ b/images/operator-helm-controller/internal/adapter/addon_release.go @@ -70,6 +70,7 @@ func (r *AddonRelease) ChartRef() source.ChartRef { func (r *AddonRelease) TargetNamespace() string { return r.obj.Spec.Namespace } func (r *AddonRelease) Values() *apiextensionsv1.JSON { return r.obj.Spec.Values } +func (r *AddonRelease) Timeout() *metav1.Duration { return r.obj.Spec.Timeout } func (r *AddonRelease) MaintenanceActivated() bool { return r.obj.MaintenanceModeActivated() } func (r *AddonRelease) MaintenanceEnabled() bool { return r.obj.MaintenanceModeEnabled() } func (r *AddonRelease) ForceReconcileRequired() bool { return r.obj.ForceReconcileRequired() } diff --git a/images/operator-helm-controller/internal/adapter/application_release.go b/images/operator-helm-controller/internal/adapter/application_release.go index 15ba220d..3a39c236 100644 --- a/images/operator-helm-controller/internal/adapter/application_release.go +++ b/images/operator-helm-controller/internal/adapter/application_release.go @@ -82,6 +82,7 @@ func (r *ApplicationRelease) ChartRef() source.ChartRef { func (r *ApplicationRelease) TargetNamespace() string { return r.obj.Namespace } func (r *ApplicationRelease) Values() *apiextensionsv1.JSON { return r.obj.Spec.Values } +func (r *ApplicationRelease) Timeout() *metav1.Duration { return r.obj.Spec.Timeout } func (r *ApplicationRelease) MaintenanceActivated() bool { return r.obj.MaintenanceModeActivated() } func (r *ApplicationRelease) MaintenanceEnabled() bool { return r.obj.MaintenanceModeEnabled() } diff --git a/images/operator-helm-controller/internal/services/release_service.go b/images/operator-helm-controller/internal/services/release_service.go index cf9e2349..7eea7689 100644 --- a/images/operator-helm-controller/internal/services/release_service.go +++ b/images/operator-helm-controller/internal/services/release_service.go @@ -130,6 +130,7 @@ func (s *ReleaseService) SyncReleaseSpec(ctx context.Context, rel source.Release release.Spec.TargetNamespace = rel.TargetNamespace() release.Spec.Values = rel.Values() + release.Spec.Timeout = rel.Timeout() release.Spec.Suspend = rel.MaintenanceActivated() setReconcileRequestAnnotations(release) @@ -162,6 +163,7 @@ func applyHelmReleaseSpec(rel source.Release, existing *helmv2.HelmRelease, sour existing.Spec.ReleaseName = rel.ReleaseName() existing.Spec.TargetNamespace = rel.TargetNamespace() existing.Spec.Values = rel.Values() + existing.Spec.Timeout = rel.Timeout() existing.Spec.Suspend = rel.MaintenanceActivated() diff --git a/images/operator-helm-controller/internal/services/release_service_test.go b/images/operator-helm-controller/internal/services/release_service_test.go index 2bbb1936..dc368483 100644 --- a/images/operator-helm-controller/internal/services/release_service_test.go +++ b/images/operator-helm-controller/internal/services/release_service_test.go @@ -19,6 +19,7 @@ package services import ( "context" "testing" + "time" helmv2 "github.com/fluxcd/helm-controller/api/v2" sourcev1 "github.com/fluxcd/source-controller/api/v1" @@ -127,6 +128,62 @@ func TestEnsureHelmReleaseKeepsForeignLabels(t *testing.T) { } } +// TestEnsureHelmReleaseCarriesTheTimeout pins that spec.timeout reaches the +// HelmRelease for both families, and that removing it from the spec removes it +// from the HelmRelease too, so helm-controller falls back to its own default +// instead of keeping the last value it was given. +func TestEnsureHelmReleaseCarriesTheTimeout(t *testing.T) { + timeout := &metav1.Duration{Duration: 15 * time.Minute} + + withTimeout := func(d *metav1.Duration) []source.Release { + app := testApplication() + app.Spec.Timeout = d + addon := testAddon() + addon.Spec.Timeout = d + + return []source.Release{adapter.NewApplicationRelease(app), adapter.NewAddonRelease(addon)} + } + + for i, rel := range withTimeout(timeout) { + t.Run(rel.Kind(), func(t *testing.T) { + service, c := newReleaseService(t) + + release := ensureRelease(t, service, c, rel) + if release.Spec.Timeout == nil || *release.Spec.Timeout != *timeout { + t.Fatalf("timeout = %v, want %v", release.Spec.Timeout, timeout) + } + + release = ensureRelease(t, service, c, withTimeout(nil)[i]) + if release.Spec.Timeout != nil { + t.Fatalf("timeout = %v, want it unset once the spec drops it", release.Spec.Timeout) + } + }) + } +} + +// TestSyncReleaseSpecCarriesTheTimeout pins that a timeout changed while the +// release is being deleted reaches the HelmRelease: it bounds the uninstall, and +// raising it is how a stuck uninstall is let through. +func TestSyncReleaseSpecCarriesTheTimeout(t *testing.T) { + app := testApplication() + rel := adapter.NewApplicationRelease(app) + service, c := newReleaseService(t) + release := ensureRelease(t, service, c, rel) + + app.Spec.Timeout = &metav1.Duration{Duration: 20 * time.Minute} + if err := service.SyncReleaseSpec(context.Background(), rel, release); err != nil { + t.Fatalf("SyncReleaseSpec returned %v", err) + } + + synced := &helmv2.HelmRelease{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(release), synced); err != nil { + t.Fatalf("getting helm release: %v", err) + } + if synced.Spec.Timeout == nil || *synced.Spec.Timeout != *app.Spec.Timeout { + t.Fatalf("timeout = %v, want %v", synced.Spec.Timeout, app.Spec.Timeout) + } +} + // TestIsDesiredChartDeployedSurvivesASourceKindFlip pins that a revision carrying no // digest is read as "not the desired chart" rather than indexed into. A release // installed from a registry keeps an OCI digest in its history; when the repository diff --git a/images/operator-helm-controller/internal/source/release.go b/images/operator-helm-controller/internal/source/release.go index b5519693..808f5a3e 100644 --- a/images/operator-helm-controller/internal/source/release.go +++ b/images/operator-helm-controller/internal/source/release.go @@ -69,6 +69,8 @@ type Release interface { // TargetNamespace is where the release is deployed. TargetNamespace() string Values() *apiextensionsv1.JSON + // Timeout is nil when the spec leaves it to helm-controller's default. + Timeout() *metav1.Duration MaintenanceActivated() bool MaintenanceEnabled() bool ForceReconcileRequired() bool