diff --git a/api/v2beta2/annotations.go b/api/v2beta2/annotations.go new file mode 100644 index 000000000..bcf4664be --- /dev/null +++ b/api/v2beta2/annotations.go @@ -0,0 +1,84 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v2beta2 + +import "github.com/fluxcd/pkg/apis/meta" + +const ( + // ForceRequestAnnotation is the annotation used for triggering a one-off forced + // Helm release, even when there are no new changes in the HelmRelease. + // The value is interpreted as a token, and must equal the value of + // meta.ReconcileRequestAnnotation in order to trigger a release. + ForceRequestAnnotation string = "reconcile.fluxcd.io/forceAt" + + // ResetRequestAnnotation is the annotation used for resetting the failure counts + // of a HelmRelease, so that it can be retried again. + // The value is interpreted as a token, and must equal the value of + // meta.ReconcileRequestAnnotation in order to reset the failure counts. + ResetRequestAnnotation string = "reconcile.fluxcd.io/resetAt" +) + +// ShouldHandleResetRequest returns true if the HelmRelease has a reset request +// annotation, and the value of the annotation matches the value of the +// meta.ReconcileRequestAnnotation annotation. +// +// To ensure that the reset request is handled only once, the value of +// HelmReleaseStatus.LastHandledResetAt is updated to match the value of the +// reset request annotation (even if the reset request is not handled because +// the value of the meta.ReconcileRequestAnnotation annotation does not match). +func ShouldHandleResetRequest(obj *HelmRelease) bool { + return handleRequest(obj, ResetRequestAnnotation, &obj.Status.LastHandledResetAt) +} + +// ShouldHandleForceRequest returns true if the HelmRelease has a force request +// annotation, and the value of the annotation matches the value of the +// meta.ReconcileRequestAnnotation annotation. +// +// To ensure that the force request is handled only once, the value of +// HelmReleaseStatus.LastHandledForceAt is updated to match the value of the +// force request annotation (even if the force request is not handled because +// the value of the meta.ReconcileRequestAnnotation annotation does not match). +func ShouldHandleForceRequest(obj *HelmRelease) bool { + return handleRequest(obj, ForceRequestAnnotation, &obj.Status.LastHandledForceAt) +} + +// handleRequest returns true if the HelmRelease has a request annotation, and +// the value of the annotation matches the value of the meta.ReconcileRequestAnnotation +// annotation. +// +// The lastHandled argument is used to ensure that the request is handled only +// once, and is updated to match the value of the request annotation (even if +// the request is not handled because the value of the meta.ReconcileRequestAnnotation +// annotation does not match). +func handleRequest(obj *HelmRelease, annotation string, lastHandled *string) bool { + requestAt, requestOk := obj.GetAnnotations()[annotation] + reconcileAt, reconcileOk := meta.ReconcileAnnotationValue(obj.GetAnnotations()) + + var lastHandledRequest string + if requestOk { + lastHandledRequest = *lastHandled + *lastHandled = requestAt + } + + if requestOk && reconcileOk && requestAt == reconcileAt { + lastHandledReconcile := obj.Status.GetLastHandledReconcileRequest() + if lastHandledReconcile != reconcileAt && lastHandledRequest != requestAt { + return true + } + } + return false +} diff --git a/api/v2beta2/annotations_test.go b/api/v2beta2/annotations_test.go new file mode 100644 index 000000000..44b593005 --- /dev/null +++ b/api/v2beta2/annotations_test.go @@ -0,0 +1,165 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v2beta2 + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/fluxcd/pkg/apis/meta" +) + +func TestShouldHandleResetRequest(t *testing.T) { + t.Run("should handle reset request", func(t *testing.T) { + obj := &HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "b", + ResetRequestAnnotation: "b", + }, + }, + Status: HelmReleaseStatus{ + LastHandledResetAt: "a", + ReconcileRequestStatus: meta.ReconcileRequestStatus{ + LastHandledReconcileAt: "a", + }, + }, + } + + if !ShouldHandleResetRequest(obj) { + t.Error("ShouldHandleResetRequest() = false") + } + + if obj.Status.LastHandledResetAt != "b" { + t.Error("ShouldHandleResetRequest did not update LastHandledResetAt") + } + }) +} + +func TestShouldHandleForceRequest(t *testing.T) { + t.Run("should handle force request", func(t *testing.T) { + obj := &HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "b", + ForceRequestAnnotation: "b", + }, + }, + Status: HelmReleaseStatus{ + LastHandledForceAt: "a", + ReconcileRequestStatus: meta.ReconcileRequestStatus{ + LastHandledReconcileAt: "a", + }, + }, + } + + if !ShouldHandleForceRequest(obj) { + t.Error("ShouldHandleForceRequest() = false") + } + + if obj.Status.LastHandledForceAt != "b" { + t.Error("ShouldHandleForceRequest did not update LastHandledForceAt") + } + }) +} + +func Test_handleRequest(t *testing.T) { + const requestAnnotation = "requestAnnotation" + + tests := []struct { + name string + annotations map[string]string + lastHandledReconcile string + lastHandledRequest string + want bool + expectLastHandledRequest string + }{ + { + name: "valid request and reconcile annotations", + annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "b", + requestAnnotation: "b", + }, + want: true, + expectLastHandledRequest: "b", + }, + { + name: "mismatched annotations", + annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "b", + requestAnnotation: "c", + }, + want: false, + expectLastHandledRequest: "c", + }, + { + name: "reconcile matches previous request", + annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "b", + requestAnnotation: "b", + }, + lastHandledReconcile: "a", + lastHandledRequest: "b", + want: false, + expectLastHandledRequest: "b", + }, + { + name: "request matches previous reconcile", + annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "b", + requestAnnotation: "b", + }, + lastHandledReconcile: "b", + lastHandledRequest: "a", + want: false, + expectLastHandledRequest: "b", + }, + { + name: "missing annotations", + annotations: map[string]string{}, + lastHandledRequest: "a", + want: false, + expectLastHandledRequest: "a", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := &HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: tt.annotations, + }, + Status: HelmReleaseStatus{ + ReconcileRequestStatus: meta.ReconcileRequestStatus{ + LastHandledReconcileAt: tt.lastHandledReconcile, + }, + }, + } + + lastHandled := tt.lastHandledRequest + result := handleRequest(obj, requestAnnotation, &lastHandled) + + if result != tt.want { + t.Errorf("handleRequest() = %v, want %v", result, tt.want) + } + if lastHandled != tt.expectLastHandledRequest { + t.Errorf("lastHandledRequest = %v, want %v", lastHandled, tt.expectLastHandledRequest) + } + }) + } +} diff --git a/api/v2beta2/helmrelease_types.go b/api/v2beta2/helmrelease_types.go index 19d49937f..56cf9709d 100644 --- a/api/v2beta2/helmrelease_types.go +++ b/api/v2beta2/helmrelease_types.go @@ -1009,6 +1009,16 @@ type HelmReleaseStatus struct { // +optional LastAttemptedConfigDigest string `json:"lastAttemptedConfigDigest,omitempty"` + // LastHandledForceAt holds the value of the most recent force request + // value, so a change of the annotation value can be detected. + // +optional + LastHandledForceAt string `json:"lastHandledForceAt,omitempty"` + + // LastHandledResetAt holds the value of the most recent reset request + // value, so a change of the annotation value can be detected. + // +optional + LastHandledResetAt string `json:"lastHandledResetAt,omitempty"` + meta.ReconcileRequestStatus `json:",inline"` } diff --git a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml index 24ae9cc4e..fd970a64c 100644 --- a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml +++ b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml @@ -2126,11 +2126,21 @@ spec: the values of the last reconciliation attempt. Deprecated: Use LastAttemptedConfigDigest instead.' type: string + lastHandledForceAt: + description: LastHandledForceAt holds the value of the most recent + force request value, so a change of the annotation value can be + detected. + type: string lastHandledReconcileAt: description: LastHandledReconcileAt holds the value of the most recent reconcile request value, so a change of the annotation value can be detected. type: string + lastHandledResetAt: + description: LastHandledResetAt holds the value of the most recent + reset request value, so a change of the annotation value can be + detected. + type: string lastReleaseRevision: description: 'LastReleaseRevision is the revision of the last successful Helm release. Deprecated: Use History instead.' diff --git a/docs/api/v2beta2/helm.md b/docs/api/v2beta2/helm.md index 49216b3a8..3a62206c3 100644 --- a/docs/api/v2beta2/helm.md +++ b/docs/api/v2beta2/helm.md @@ -1528,6 +1528,32 @@ string +lastHandledForceAt
+ +string + + + +(Optional) +

LastHandledForceAt holds the value of the most recent force request +value, so a change of the annotation value can be detected.

+ + + + +lastHandledResetAt
+ +string + + + +(Optional) +

LastHandledResetAt holds the value of the most recent reset request +value, so a change of the annotation value can be detected.

+ + + + ReconcileRequestStatus
diff --git a/internal/action/reset.go b/internal/action/reset.go index da0caf69f..a293b7386 100644 --- a/internal/action/reset.go +++ b/internal/action/reset.go @@ -29,6 +29,7 @@ const ( differentGenerationReason = "generation differs from last attempt" differentRevisionReason = "chart version differs from last attempt" differentValuesReason = "values differ from last attempt" + resetRequestedReason = "reset requested through annotation" ) // MustResetFailures returns a reason and true if the HelmRelease's status @@ -38,6 +39,12 @@ const ( // For example, a change in generation, chart version, or values. // If no change is detected, an empty string is returned along with false. func MustResetFailures(obj *v2.HelmRelease, chart *chart.Metadata, values chartutil.Values) (string, bool) { + // Always check if a reset is requested. + // This is done first, so that the HelmReleaseStatus.LastHandledResetAt + // field is updated even if the reset request is not handled due to other + // diverging data. + resetRequested := v2.ShouldHandleResetRequest(obj) + switch { case obj.Status.LastAttemptedGeneration != obj.Generation: return differentGenerationReason, true @@ -53,5 +60,10 @@ func MustResetFailures(obj *v2.HelmRelease, chart *chart.Metadata, values chartu return differentValuesReason, true } } + + if resetRequested { + return resetRequestedReason, true + } + return "", false } diff --git a/internal/action/reset_test.go b/internal/action/reset_test.go index 465df0e81..cd9f6fad6 100644 --- a/internal/action/reset_test.go +++ b/internal/action/reset_test.go @@ -24,6 +24,8 @@ import ( "helm.sh/helm/v3/pkg/chartutil" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/fluxcd/pkg/apis/meta" + v2 "github.com/fluxcd/helm-controller/api/v2beta2" ) @@ -108,6 +110,31 @@ func TestMustResetFailures(t *testing.T) { want: true, wantReason: differentValuesReason, }, + { + name: "on reset request through annotation", + obj: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + Annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "a", + v2.ResetRequestAnnotation: "a", + }, + }, + Status: v2.HelmReleaseStatus{ + LastAttemptedGeneration: 1, + LastAttemptedRevision: "1.0.0", + LastAttemptedConfigDigest: "sha256:1dabc4e3cbbd6a0818bd460f3a6c9855bfe95d506c74726bc0f2edb0aecb1f4e", + }, + }, + chart: &chart.Metadata{ + Version: "1.0.0", + }, + values: chartutil.Values{ + "foo": "bar", + }, + want: true, + wantReason: resetRequestedReason, + }, { name: "without change no reset", obj: &v2.HelmRelease{ diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index 1c3c8f6da..d10911653 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -154,6 +154,10 @@ func (r *HelmReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Always attempt to patch the object after each reconciliation. defer func() { + if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok { + obj.Status.SetLastHandledReconcileRequest(v) + } + patchOpts := []patch.Option{ patch.WithFieldOwner(r.FieldManager), patch.WithOwnedConditions{Conditions: intreconcile.OwnedConditions}, diff --git a/internal/reconcile/atomic_release.go b/internal/reconcile/atomic_release.go index 87190f6ad..1e6a9e8af 100644 --- a/internal/reconcile/atomic_release.go +++ b/internal/reconcile/atomic_release.go @@ -175,7 +175,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error { } return fmt.Errorf("atomic release canceled: %w", ctx.Err()) default: - // Determine the next action to run based on the current state. + // Determine the current state of the Helm release. log.V(logger.DebugLevel).Info("determining current state of Helm release") state, err := DetermineReleaseState(ctx, r.configFactory, req) if err != nil { @@ -273,6 +273,13 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error { func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state ReleaseState) (ActionReconciler, error) { log := ctrl.LoggerFrom(ctx) + // Determine whether we may need to force a release action. + // We do this before determining the next action to run, as otherwise we may + // end up running a Helm upgrade (due to e.g. ReleaseStatusUnmanaged) and + // then forcing an upgrade (due to the release now being in + // ReleaseStatusInSync with a yet unhandled force request). + forceRequested := v2.ShouldHandleForceRequest(req.Object) + switch state.Status { case ReleaseStatusInSync: log.Info("release in-sync with desired state") @@ -291,6 +298,11 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state // field, but should be removed in a future release. req.Object.Status.LastAppliedRevision = req.Object.Status.History.Latest().ChartVersion + if forceRequested { + log.Info(msgWithReason("forcing upgrade for in-sync release", "force requested through annotation")) + return NewUpgrade(r.configFactory, r.eventRecorder), nil + } + return nil, nil case ReleaseStatusLocked: log.Info(msgWithReason("release locked", state.Reason)) @@ -299,6 +311,11 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state log.Info(msgWithReason("release not installed", state.Reason)) if req.Object.GetInstall().GetRemediation().RetriesExhausted(req.Object) { + if forceRequested { + log.Info(msgWithReason("forcing install while out of retries", "force requested through annotation")) + return NewInstall(r.configFactory, r.eventRecorder), nil + } + return nil, fmt.Errorf("%w: cannot install release", ErrExceededMaxRetries) } @@ -314,6 +331,11 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state log.Info(msgWithReason("release out-of-sync with desired state", state.Reason)) if req.Object.GetUpgrade().GetRemediation().RetriesExhausted(req.Object) { + if forceRequested { + log.Info(msgWithReason("forcing upgrade while out of retries", "force requested through annotation")) + return NewUpgrade(r.configFactory, r.eventRecorder), nil + } + return nil, fmt.Errorf("%w: cannot upgrade release", ErrExceededMaxRetries) } @@ -367,6 +389,13 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state return NewUpgrade(r.configFactory, r.eventRecorder), nil } + // If the force annotation is set, we can attempt to upgrade the release + // without any further checks. + if forceRequested { + log.Info(msgWithReason("forcing upgrade for failed release", "force requested through annotation")) + return NewUpgrade(r.configFactory, r.eventRecorder), nil + } + // We have exhausted the number of retries for the remediation // strategy. if remediation.RetriesExhausted(req.Object) && !remediation.MustRemediateLastFailure() { diff --git a/internal/reconcile/atomic_release_test.go b/internal/reconcile/atomic_release_test.go index d932fb65f..d2050e425 100644 --- a/internal/reconcile/atomic_release_test.go +++ b/internal/reconcile/atomic_release_test.go @@ -1015,14 +1015,15 @@ func TestAtomicRelease_Reconcile_Scenarios(t *testing.T) { func TestAtomicRelease_actionForState(t *testing.T) { tests := []struct { - name string - releases []*helmrelease.Release - spec func(spec *v2.HelmReleaseSpec) - status func(releases []*helmrelease.Release) v2.HelmReleaseStatus - state ReleaseState - want ActionReconciler - wantEvent *corev1.Event - wantErr error + name string + releases []*helmrelease.Release + annotations map[string]string + spec func(spec *v2.HelmReleaseSpec) + status func(releases []*helmrelease.Release) v2.HelmReleaseStatus + state ReleaseState + want ActionReconciler + wantEvent *corev1.Event + wantErr error }{ { name: "in-sync release does not trigger any action", @@ -1036,6 +1037,22 @@ func TestAtomicRelease_actionForState(t *testing.T) { state: ReleaseState{Status: ReleaseStatusInSync}, want: nil, }, + { + name: "in-sync release with force annotation triggers upgrade action", + state: ReleaseState{Status: ReleaseStatusInSync}, + annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "force", + v2.ForceRequestAnnotation: "force", + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + {Version: 1}, + }, + } + }, + want: &Upgrade{}, + }, { name: "locked release triggers unlock action", state: ReleaseState{Status: ReleaseStatusLocked}, @@ -1046,6 +1063,20 @@ func TestAtomicRelease_actionForState(t *testing.T) { state: ReleaseState{Status: ReleaseStatusAbsent}, want: &Install{}, }, + { + name: "absent release without remaining retries and force annotation triggers install", + annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "force", + v2.ForceRequestAnnotation: "force", + }, + state: ReleaseState{Status: ReleaseStatusAbsent}, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + InstallFailures: 1, + } + }, + want: &Install{}, + }, { name: "absent release without remaining retries returns error", state: ReleaseState{Status: ReleaseStatusAbsent}, @@ -1181,6 +1212,22 @@ func TestAtomicRelease_actionForState(t *testing.T) { }, want: &Upgrade{}, }, + { + name: "out-of-sync release with no remaining retries and force annotation triggers upgrade", + state: ReleaseState{ + Status: ReleaseStatusOutOfSync, + }, + annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "force", + v2.ForceRequestAnnotation: "force", + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + UpgradeFailures: 1, + } + }, + want: &Upgrade{}, + }, { name: "out-of-sync release with no remaining retries returns error", state: ReleaseState{ @@ -1220,6 +1267,21 @@ func TestAtomicRelease_actionForState(t *testing.T) { }, want: &Upgrade{}, }, + { + name: "failed release with exhausted retries and force annotation triggers upgrade", + state: ReleaseState{Status: ReleaseStatusFailed}, + annotations: map[string]string{ + meta.ReconcileRequestAnnotation: "force", + v2.ForceRequestAnnotation: "force", + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + LastAttemptedReleaseAction: v2.ReleaseActionUpgrade, + UpgradeFailures: 1, + } + }, + want: &Upgrade{}, + }, { name: "failed release with exhausted retries returns error", state: ReleaseState{Status: ReleaseStatusFailed}, @@ -1370,6 +1432,9 @@ func TestAtomicRelease_actionForState(t *testing.T) { g := NewWithT(t) obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: tt.annotations, + }, Spec: v2.HelmReleaseSpec{ ReleaseName: mockReleaseName, TargetNamespace: mockReleaseNamespace,