diff --git a/api/v2beta2/helmrelease_types.go b/api/v2beta2/helmrelease_types.go index e39709c26..936a9101a 100644 --- a/api/v2beta2/helmrelease_types.go +++ b/api/v2beta2/helmrelease_types.go @@ -18,6 +18,7 @@ package v2beta2 import ( "encoding/json" + "fmt" "strings" "time" @@ -138,6 +139,8 @@ type HelmReleaseSpec struct { // The name of the Kubernetes service account to impersonate // when reconciling this HelmRelease. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 // +optional ServiceAccountName string `json:"serviceAccountName,omitempty"` @@ -190,50 +193,6 @@ type HelmReleaseSpec struct { PostRenderers []PostRenderer `json:"postRenderers,omitempty"` } -// GetInstall returns the configuration for Helm install actions for the -// HelmRelease. -func (in HelmReleaseSpec) GetInstall() Install { - if in.Install == nil { - return Install{} - } - return *in.Install -} - -// GetUpgrade returns the configuration for Helm upgrade actions for this -// HelmRelease. -func (in HelmReleaseSpec) GetUpgrade() Upgrade { - if in.Upgrade == nil { - return Upgrade{} - } - return *in.Upgrade -} - -// GetTest returns the configuration for Helm test actions for this HelmRelease. -func (in HelmReleaseSpec) GetTest() Test { - if in.Test == nil { - return Test{} - } - return *in.Test -} - -// GetRollback returns the configuration for Helm rollback actions for this -// HelmRelease. -func (in HelmReleaseSpec) GetRollback() Rollback { - if in.Rollback == nil { - return Rollback{} - } - return *in.Rollback -} - -// GetUninstall returns the configuration for Helm uninstall actions for this -// HelmRelease. -func (in HelmReleaseSpec) GetUninstall() Uninstall { - if in.Uninstall == nil { - return Uninstall{} - } - return *in.Uninstall -} - // HelmChartTemplate defines the template from which the controller will // generate a v1beta2.HelmChart object in the same namespace as the referenced // v1.Source. @@ -268,6 +227,8 @@ type HelmChartTemplateObjectMeta struct { // generate a v1beta2.HelmChartSpec object. type HelmChartTemplateSpec struct { // The name or path the Helm chart is available at in the SourceRef. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=2048 // +required Chart string `json:"chart"` @@ -353,13 +314,6 @@ type HelmChartTemplateVerification struct { SecretRef *meta.LocalObjectReference `json:"secretRef,omitempty"` } -// DeploymentAction defines a consistent interface for Install and Upgrade. -// +kubebuilder:object:generate=false -type DeploymentAction interface { - GetDescription() string - GetRemediation() Remediation -} - // Remediation defines a consistent interface for InstallRemediation and // UpgradeRemediation. // +kubebuilder:object:generate=false @@ -368,9 +322,9 @@ type Remediation interface { MustIgnoreTestFailures(bool) bool MustRemediateLastFailure() bool GetStrategy() RemediationStrategy - GetFailureCount(hr HelmRelease) int64 + GetFailureCount(hr *HelmRelease) int64 IncrementFailureCount(hr *HelmRelease) - RetriesExhausted(hr HelmRelease) bool + RetriesExhausted(hr *HelmRelease) bool } // Install holds the configuration for Helm install actions performed for this @@ -459,11 +413,6 @@ func (in Install) GetTimeout(defaultTimeout metav1.Duration) metav1.Duration { return *in.Timeout } -// GetDescription returns a description for the Helm install action. -func (in Install) GetDescription() string { - return "install" -} - // GetRemediation returns the configured Remediation for the Helm install action. func (in Install) GetRemediation() Remediation { if in.Remediation == nil { @@ -522,7 +471,7 @@ func (in InstallRemediation) GetStrategy() RemediationStrategy { } // GetFailureCount gets the failure count. -func (in InstallRemediation) GetFailureCount(hr HelmRelease) int64 { +func (in InstallRemediation) GetFailureCount(hr *HelmRelease) int64 { return hr.Status.InstallFailures } @@ -532,7 +481,7 @@ func (in InstallRemediation) IncrementFailureCount(hr *HelmRelease) { } // RetriesExhausted returns true if there are no remaining retries. -func (in InstallRemediation) RetriesExhausted(hr HelmRelease) bool { +func (in InstallRemediation) RetriesExhausted(hr *HelmRelease) bool { return in.Retries >= 0 && in.GetFailureCount(hr) > int64(in.Retries) } @@ -631,11 +580,6 @@ func (in Upgrade) GetTimeout(defaultTimeout metav1.Duration) metav1.Duration { return *in.Timeout } -// GetDescription returns a description for the Helm upgrade action. -func (in Upgrade) GetDescription() string { - return "upgrade" -} - // GetRemediation returns the configured Remediation for the Helm upgrade // action. func (in Upgrade) GetRemediation() Remediation { @@ -703,7 +647,7 @@ func (in UpgradeRemediation) GetStrategy() RemediationStrategy { } // GetFailureCount gets the failure count. -func (in UpgradeRemediation) GetFailureCount(hr HelmRelease) int64 { +func (in UpgradeRemediation) GetFailureCount(hr *HelmRelease) int64 { return hr.Status.UpgradeFailures } @@ -713,7 +657,7 @@ func (in UpgradeRemediation) IncrementFailureCount(hr *HelmRelease) { } // RetriesExhausted returns true if there are no remaining retries. -func (in UpgradeRemediation) RetriesExhausted(hr HelmRelease) bool { +func (in UpgradeRemediation) RetriesExhausted(hr *HelmRelease) bool { return in.Retries >= 0 && in.GetFailureCount(hr) > int64(in.Retries) } @@ -764,9 +708,12 @@ func (in Test) GetTimeout(defaultTimeout metav1.Duration) metav1.Duration { return *in.Timeout } -// Filters holds the configuration for individual Helm test filters. +// Filter holds the configuration for individual Helm test filters. type Filter struct { // Name is the name of the test. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +required Name string `json:"name"` // Exclude is specifies whether the named test should be excluded. // +optional @@ -854,6 +801,13 @@ type Uninstall struct { // a Helm uninstall is performed. // +optional DisableWait bool `json:"disableWait,omitempty"` + + // DeletionPropagation specifies the deletion propagation policy when + // a Helm uninstall is performed. + // +kubebuilder:default=background + // +kubebuilder:validation:Enum=background;foreground;orphan + // +optional + DeletionPropagation *string `json:"deletionPropagation,omitempty"` } // GetTimeout returns the configured timeout for the Helm uninstall action, or @@ -865,6 +819,15 @@ func (in Uninstall) GetTimeout(defaultTimeout metav1.Duration) metav1.Duration { return *in.Timeout } +// GetDeletionPropagation returns the configured deletion propagation policy +// for the Helm uninstall action, or 'background'. +func (in Uninstall) GetDeletionPropagation() string { + if in.DeletionPropagation == nil { + return "background" + } + return *in.DeletionPropagation +} + // HelmReleaseInfo holds the status information for a Helm release as performed // by the controller. type HelmReleaseInfo struct { @@ -908,7 +871,50 @@ type HelmReleaseInfo struct { // TestHooks is the list of test hooks for the release as observed to be // run by the controller. // +optional - TestHooks map[string]*HelmReleaseTestHook `json:"testHooks,omitempty"` + TestHooks *map[string]*HelmReleaseTestHook `json:"testHooks,omitempty"` +} + +// FullReleaseName returns the full name of the release in the format +// of '/. +func (in *HelmReleaseInfo) FullReleaseName() string { + return fmt.Sprintf("%s/%s.%d", in.Namespace, in.Name, in.Version) +} + +// VersionedChartName returns the full name of the chart in the format of +// '@'. +func (in *HelmReleaseInfo) VersionedChartName() string { + return fmt.Sprintf("%s@%s", in.ChartName, in.ChartVersion) +} + +// HasBeenTested returns true if TestHooks is not nil. This includes an empty +// map, which indicates the chart has no tests. +func (in *HelmReleaseInfo) HasBeenTested() bool { + return in != nil && in.TestHooks != nil +} + +// GetTestHooks returns the TestHooks for the release if not nil. +func (in *HelmReleaseInfo) GetTestHooks() map[string]*HelmReleaseTestHook { + if in == nil { + return nil + } + return *in.TestHooks +} + +// HasTestInPhase returns true if any of the TestHooks is in the given phase. +func (in *HelmReleaseInfo) HasTestInPhase(phase string) bool { + if in != nil { + for _, h := range in.GetTestHooks() { + if h.Phase == phase { + return true + } + } + } + return false +} + +// SetTestHooks sets the TestHooks for the release. +func (in *HelmReleaseInfo) SetTestHooks(hooks map[string]*HelmReleaseTestHook) { + in.TestHooks = &hooks } // HelmReleaseTestHook holds the status information for a test hook as observed @@ -940,6 +946,14 @@ type HelmReleaseStatus struct { // +optional HelmChart string `json:"helmChart,omitempty"` + // StorageNamespace is the namespace of the Helm release storage for the + // Current release. + // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:Optional + // +optional + StorageNamespace string `json:"storageNamespace,omitempty"` + // Current holds the latest observed HelmReleaseInfo for the current // release. // +optional @@ -1014,6 +1028,84 @@ type HelmRelease struct { Status HelmReleaseStatus `json:"status,omitempty"` } +// GetInstall returns the configuration for Helm install actions for the +// HelmRelease. +func (in *HelmRelease) GetInstall() Install { + if in.Spec.Install == nil { + return Install{} + } + return *in.Spec.Install +} + +// GetUpgrade returns the configuration for Helm upgrade actions for this +// HelmRelease. +func (in *HelmRelease) GetUpgrade() Upgrade { + if in.Spec.Upgrade == nil { + return Upgrade{} + } + return *in.Spec.Upgrade +} + +// GetTest returns the configuration for Helm test actions for this HelmRelease. +func (in *HelmRelease) GetTest() Test { + if in.Spec.Test == nil { + return Test{} + } + return *in.Spec.Test +} + +// GetRollback returns the configuration for Helm rollback actions for this +// HelmRelease. +func (in *HelmRelease) GetRollback() Rollback { + if in.Spec.Rollback == nil { + return Rollback{} + } + return *in.Spec.Rollback +} + +// GetUninstall returns the configuration for Helm uninstall actions for this +// HelmRelease. +func (in *HelmRelease) GetUninstall() Uninstall { + if in.Spec.Uninstall == nil { + return Uninstall{} + } + return *in.Spec.Uninstall +} + +// GetCurrent returns HelmReleaseStatus.Current. +func (in HelmRelease) GetCurrent() *HelmReleaseInfo { + if in.HasCurrent() { + return in.Status.Current + } + return nil +} + +// HasCurrent returns true if the HelmRelease has a HelmReleaseStatus.Current. +func (in HelmRelease) HasCurrent() bool { + return in.Status.Current != nil +} + +// GetPrevious returns HelmReleaseStatus.Previous. +func (in HelmRelease) GetPrevious() *HelmReleaseInfo { + if in.HasPrevious() { + return in.Status.Previous + } + return nil +} + +// HasPrevious returns true if the HelmRelease has a HelmReleaseStatus.Previous. +func (in HelmRelease) HasPrevious() bool { + return in.Status.Previous != nil +} + +// GetActiveRemediation returns the active Remediation for the HelmRelease. +func (in HelmRelease) GetActiveRemediation() Remediation { + if in.Status.Previous != nil { + return in.GetUpgrade().GetRemediation() + } + return in.GetInstall().GetRemediation() +} + // GetRequeueAfter returns the duration after which the HelmRelease // must be reconciled again. func (in HelmRelease) GetRequeueAfter() time.Duration { diff --git a/api/v2beta2/zz_generated.deepcopy.go b/api/v2beta2/zz_generated.deepcopy.go index 05b3614b1..31282f767 100644 --- a/api/v2beta2/zz_generated.deepcopy.go +++ b/api/v2beta2/zz_generated.deepcopy.go @@ -195,17 +195,21 @@ func (in *HelmReleaseInfo) DeepCopyInto(out *HelmReleaseInfo) { in.Deleted.DeepCopyInto(&out.Deleted) if in.TestHooks != nil { in, out := &in.TestHooks, &out.TestHooks - *out = make(map[string]*HelmReleaseTestHook, len(*in)) - for key, val := range *in { - var outVal *HelmReleaseTestHook - if val == nil { - (*out)[key] = nil - } else { - in, out := &val, &outVal - *out = new(HelmReleaseTestHook) - (*in).DeepCopyInto(*out) + *out = new(map[string]*HelmReleaseTestHook) + if **in != nil { + in, out := *in, *out + *out = make(map[string]*HelmReleaseTestHook, len(*in)) + for key, val := range *in { + var outVal *HelmReleaseTestHook + if val == nil { + (*out)[key] = nil + } else { + in, out := &val, &outVal + *out = new(HelmReleaseTestHook) + (*in).DeepCopyInto(*out) + } + (*out)[key] = outVal } - (*out)[key] = outVal } } } @@ -554,6 +558,11 @@ func (in *Uninstall) DeepCopyInto(out *Uninstall) { *out = new(metav1.Duration) **out = **in } + if in.DeletionPropagation != nil { + in, out := &in.DeletionPropagation, &out.DeletionPropagation + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Uninstall. diff --git a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml index 3792414f1..15d3f8399 100644 --- a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml +++ b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml @@ -982,6 +982,8 @@ spec: chart: description: The name or path the Helm chart is available at in the SourceRef. + maxLength: 2048 + minLength: 1 type: string interval: description: Interval at which to check the v1.Source for @@ -1480,6 +1482,8 @@ spec: serviceAccountName: description: The name of the Kubernetes service account to impersonate when reconciling this HelmRelease. + maxLength: 253 + minLength: 1 type: string storageNamespace: description: StorageNamespace used for the Helm storage. Defaults @@ -1510,8 +1514,8 @@ spec: description: Filters is a list of tests to run or exclude from running. items: - description: Filters holds the configuration for individual - Helm test filters. + description: Filter holds the configuration for individual Helm + test filters. properties: exclude: description: Exclude is specifies whether the named test @@ -1519,6 +1523,8 @@ spec: type: boolean name: description: Name is the name of the test. + maxLength: 253 + minLength: 1 type: string required: - name @@ -1547,6 +1553,15 @@ spec: description: Uninstall holds the configuration for Helm uninstall actions for this HelmRelease. properties: + deletionPropagation: + default: background + description: DeletionPropagation specifies the deletion propagation + policy when a Helm uninstall is performed. + enum: + - background + - foreground + - orphan + type: string disableHooks: description: DisableHooks prevents hooks from running during the Helm rollback action. @@ -1972,6 +1987,12 @@ spec: - status - version type: object + storageNamespace: + description: StorageNamespace is the namespace of the Helm release + storage for the Current release. + maxLength: 63 + minLength: 1 + type: string upgradeFailures: description: UpgradeFailures is the upgrade failure count against the latest desired state. It is reset after a successful reconciliation. diff --git a/internal/action/install.go b/internal/action/install.go index 96fe49a1f..035cd3ab5 100644 --- a/internal/action/install.go +++ b/internal/action/install.go @@ -28,6 +28,7 @@ import ( v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/features" "github.com/fluxcd/helm-controller/internal/postrender" + "github.com/fluxcd/helm-controller/internal/release" ) // InstallOption can be used to modify Helm's action.Install after the instructions @@ -50,7 +51,7 @@ func Install(ctx context.Context, config *helmaction.Configuration, obj *v2.Helm chrt *helmchart.Chart, vals helmchartutil.Values, opts ...InstallOption) (*helmrelease.Release, error) { install := newInstall(config, obj, opts) - policy, err := crdPolicyOrDefault(obj.Spec.GetInstall().CRDs) + policy, err := crdPolicyOrDefault(obj.GetInstall().CRDs) if err != nil { return nil, err } @@ -64,19 +65,19 @@ func Install(ctx context.Context, config *helmaction.Configuration, obj *v2.Helm func newInstall(config *helmaction.Configuration, obj *v2.HelmRelease, opts []InstallOption) *helmaction.Install { install := helmaction.NewInstall(config) - install.ReleaseName = obj.GetReleaseName() + install.ReleaseName = release.ShortenName(obj.GetReleaseName()) install.Namespace = obj.GetReleaseNamespace() - install.Timeout = obj.Spec.GetInstall().GetTimeout(obj.GetTimeout()).Duration - install.Wait = !obj.Spec.GetInstall().DisableWait - install.WaitForJobs = !obj.Spec.GetInstall().DisableWaitForJobs - install.DisableHooks = obj.Spec.GetInstall().DisableHooks - install.DisableOpenAPIValidation = obj.Spec.GetInstall().DisableOpenAPIValidation - install.Replace = obj.Spec.GetInstall().Replace + install.Timeout = obj.GetInstall().GetTimeout(obj.GetTimeout()).Duration + install.Wait = !obj.GetInstall().DisableWait + install.WaitForJobs = !obj.GetInstall().DisableWaitForJobs + install.DisableHooks = obj.GetInstall().DisableHooks + install.DisableOpenAPIValidation = obj.GetInstall().DisableOpenAPIValidation + install.Replace = obj.GetInstall().Replace install.Devel = true install.SkipCRDs = true if obj.Spec.TargetNamespace != "" { - install.CreateNamespace = obj.Spec.GetInstall().CreateNamespace + install.CreateNamespace = obj.GetInstall().CreateNamespace } // If the user opted-in to allow DNS lookups, enable it. diff --git a/internal/action/log.go b/internal/action/log.go index 2a2b35bf8..4a7fcd6eb 100644 --- a/internal/action/log.go +++ b/internal/action/log.go @@ -21,6 +21,7 @@ import ( "fmt" "strings" "sync" + "time" "github.com/go-logr/logr" helmaction "helm.sh/helm/v3/pkg/action" @@ -29,6 +30,9 @@ import ( // DefaultLogBufferSize is the default size of the LogBuffer. const DefaultLogBufferSize = 5 +// nowTS can be used to stub out time.Now() in tests. +var nowTS = time.Now + // NewDebugLog returns an action.DebugLog that logs to the given logr.Logger. func NewDebugLog(log logr.Logger) helmaction.DebugLog { return func(format string, v ...interface{}) { @@ -43,6 +47,35 @@ type LogBuffer struct { buffer *ring.Ring } +// logLine is a log message with a timestamp. +type logLine struct { + ts time.Time + lastTS time.Time + msg string + count int64 +} + +// String returns the log line as a string, in the format of: +// ': '. But only if the message is not empty. +func (l *logLine) String() string { + if l == nil || l.msg == "" { + return "" + } + + msg := fmt.Sprintf("%s %s", l.ts.Format(time.RFC3339Nano), l.msg) + if c := l.count; c > 0 { + msg += fmt.Sprintf("\n%s %s", l.lastTS.Format(time.RFC3339Nano), l.msg) + } + if c := l.count - 1; c > 0 { + var dup = "line" + if c > 1 { + dup += "s" + } + msg += fmt.Sprintf(" (%d duplicate %s omitted)", c, dup) + } + return msg +} + // NewLogBuffer creates a new LogBuffer with the given log function // and a buffer of the given size. If size <= 0, it defaults to // DefaultLogBufferSize. @@ -64,8 +97,17 @@ func (l *LogBuffer) Log(format string, v ...interface{}) { // Filter out duplicate log lines, this happens for example when // Helm is waiting on workloads to become ready. msg := fmt.Sprintf(format, v...) - if prev := l.buffer.Prev(); prev.Value != msg { - l.buffer.Value = msg + prev, ok := l.buffer.Prev().Value.(*logLine) + if ok && prev.msg == msg { + prev.count++ + prev.lastTS = nowTS().UTC() + l.buffer.Prev().Value = prev + } + if !ok || prev.msg != msg { + l.buffer.Value = &logLine{ + ts: nowTS().UTC(), + msg: msg, + } l.buffer = l.buffer.Next() } @@ -73,6 +115,23 @@ func (l *LogBuffer) Log(format string, v ...interface{}) { l.log(format, v...) } +// Len returns the count of non-empty values in the buffer. +func (l *LogBuffer) Len() (count int) { + l.mu.RLock() + l.buffer.Do(func(s interface{}) { + if s == nil { + return + } + ll, ok := s.(*logLine) + if !ok || ll.String() == "" { + return + } + count++ + }) + l.mu.RUnlock() + return +} + // Reset clears the buffer. func (l *LogBuffer) Reset() { l.mu.Lock() @@ -88,7 +147,13 @@ func (l *LogBuffer) String() string { if s == nil { return } - str += s.(string) + "\n" + ll, ok := s.(*logLine) + if !ok { + return + } + if msg := ll.String(); msg != "" { + str += msg + "\n" + } }) l.mu.RUnlock() return strings.TrimSpace(str) diff --git a/internal/action/log_test.go b/internal/action/log_test.go index c4c29cae6..5e01f1802 100644 --- a/internal/action/log_test.go +++ b/internal/action/log_test.go @@ -17,12 +17,16 @@ limitations under the License. package action import ( + "fmt" "testing" + "time" "github.com/go-logr/logr" ) func TestLogBuffer_Log(t *testing.T) { + nowTS = stubNowTS + tests := []struct { name string size int @@ -30,7 +34,7 @@ func TestLogBuffer_Log(t *testing.T) { wantCount int want string }{ - {name: "log", size: 2, fill: []string{"a", "b", "c"}, wantCount: 3, want: "b\nc"}, + {name: "log", size: 2, fill: []string{"a", "b", "c"}, wantCount: 3, want: fmt.Sprintf("%[1]s b\n%[1]s c", stubNowTS().Format(time.RFC3339Nano))}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -51,6 +55,30 @@ func TestLogBuffer_Log(t *testing.T) { } } +func TestLogBuffer_Len(t *testing.T) { + tests := []struct { + name string + size int + fill []string + want int + }{ + {name: "empty buffer", fill: []string{}, want: 0}, + {name: "filled buffer", size: 2, fill: []string{"a", "b"}, want: 2}, + {name: "half full buffer", size: 4, fill: []string{"a", "b"}, want: 2}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + l := NewLogBuffer(NewDebugLog(logr.Discard()), tt.size) + for _, v := range tt.fill { + l.Log("%s", v) + } + if got := l.Len(); got != tt.want { + t.Errorf("String() = %v, want %v", got, tt.want) + } + }) + } +} + func TestLogBuffer_Reset(t *testing.T) { bufferSize := 10 l := NewLogBuffer(NewDebugLog(logr.Discard()), bufferSize) @@ -78,6 +106,8 @@ func TestLogBuffer_Reset(t *testing.T) { } func TestLogBuffer_String(t *testing.T) { + nowTS = stubNowTS + tests := []struct { name string size int @@ -85,8 +115,11 @@ func TestLogBuffer_String(t *testing.T) { want string }{ {name: "empty buffer", fill: []string{}, want: ""}, - {name: "filled buffer", size: 2, fill: []string{"a", "b", "c"}, want: "b\nc"}, - {name: "duplicate buffer items", fill: []string{"b", "b", "b"}, want: "b"}, + {name: "filled buffer", size: 2, fill: []string{"a", "b", "c"}, want: fmt.Sprintf("%[1]s b\n%[1]s c", stubNowTS().Format(time.RFC3339Nano))}, + {name: "duplicate buffer items", fill: []string{"b", "b"}, want: fmt.Sprintf("%[1]s b\n%[1]s b", stubNowTS().Format(time.RFC3339Nano))}, + {name: "duplicate buffer items", fill: []string{"b", "b", "b"}, want: fmt.Sprintf("%[1]s b\n%[1]s b (1 duplicate line omitted)", stubNowTS().Format(time.RFC3339Nano))}, + {name: "duplicate buffer items", fill: []string{"b", "b", "b", "b"}, want: fmt.Sprintf("%[1]s b\n%[1]s b (2 duplicate lines omitted)", stubNowTS().Format(time.RFC3339Nano))}, + {name: "duplicate buffer items", fill: []string{"a", "b", "b", "b", "c", "c"}, want: fmt.Sprintf("%[1]s a\n%[1]s b\n%[1]s b (1 duplicate line omitted)\n%[1]s c\n%[1]s c", stubNowTS().Format(time.RFC3339Nano))}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -100,3 +133,8 @@ func TestLogBuffer_String(t *testing.T) { }) } } + +// stubNowTS returns a fixed time for testing purposes. +func stubNowTS() time.Time { + return time.Date(2016, 2, 18, 12, 24, 5, 12345600, time.UTC) +} diff --git a/internal/action/rollback.go b/internal/action/rollback.go index 0af54bd3c..3985597d4 100644 --- a/internal/action/rollback.go +++ b/internal/action/rollback.go @@ -27,33 +27,44 @@ import ( // example useful to enable the dry-run setting as a CLI. type RollbackOption func(*helmaction.Rollback) -// Rollback runs the Helm rollback action with the provided config, using the -// v2beta2.HelmReleaseSpec of the given object to determine the target release -// and rollback configuration. +// RollbackToVersion returns a RollbackOption which sets the version to +// roll back to. +func RollbackToVersion(version int) RollbackOption { + return func(rollback *helmaction.Rollback) { + rollback.Version = version + } +} + +// RollbackDryRun returns a RollbackOption which enables the dry-run setting. +func RollbackDryRun() RollbackOption { + return func(rollback *helmaction.Rollback) { + rollback.DryRun = true + } +} + +// Rollback runs the Helm rollback action with the provided config. Targeting +// a specific release or enabling dry-run is possible by providing +// RollbackToVersion and/or RollbackDryRun as options. // // It does not determine if there is a desire to perform the action, this is // expected to be done by the caller. In addition, it does not take note of the // action result. The caller is expected to listen to this using a // storage.ObserveFunc, which provides superior access to Helm storage writes. -func Rollback(config *helmaction.Configuration, obj *v2.HelmRelease, opts ...RollbackOption) error { +func Rollback(config *helmaction.Configuration, obj *v2.HelmRelease, releaseName string, opts ...RollbackOption) error { rollback := newRollback(config, obj, opts) - return rollback.Run(obj.GetReleaseName()) + return rollback.Run(releaseName) } func newRollback(config *helmaction.Configuration, obj *v2.HelmRelease, opts []RollbackOption) *helmaction.Rollback { rollback := helmaction.NewRollback(config) - rollback.Timeout = obj.Spec.GetRollback().GetTimeout(obj.GetTimeout()).Duration - rollback.Wait = !obj.Spec.GetRollback().DisableWait - rollback.WaitForJobs = !obj.Spec.GetRollback().DisableWaitForJobs - rollback.DisableHooks = obj.Spec.GetRollback().DisableHooks - rollback.Force = obj.Spec.GetRollback().Force - rollback.Recreate = obj.Spec.GetRollback().Recreate - rollback.CleanupOnFail = obj.Spec.GetRollback().CleanupOnFail - - if prev := obj.Status.Previous; prev != nil && prev.Name == obj.GetReleaseName() && prev.Namespace == obj.GetReleaseNamespace() { - rollback.Version = prev.Version - } + rollback.Timeout = obj.GetRollback().GetTimeout(obj.GetTimeout()).Duration + rollback.Wait = !obj.GetRollback().DisableWait + rollback.WaitForJobs = !obj.GetRollback().DisableWaitForJobs + rollback.DisableHooks = obj.GetRollback().DisableHooks + rollback.Force = obj.GetRollback().Force + rollback.Recreate = obj.GetRollback().Recreate + rollback.CleanupOnFail = obj.GetRollback().CleanupOnFail for _, opt := range opts { opt(rollback) diff --git a/internal/action/rollback_test.go b/internal/action/rollback_test.go index 34d880bd0..adb66fd5b 100644 --- a/internal/action/rollback_test.go +++ b/internal/action/rollback_test.go @@ -51,7 +51,7 @@ func Test_newRollback(t *testing.T) { g.Expect(got.Force).To(Equal(obj.Spec.Rollback.Force)) }) - t.Run("rollback with previous", func(t *testing.T) { + t.Run("rollback to version", func(t *testing.T) { g := NewWithT(t) obj := &v2.HelmRelease{ @@ -59,40 +59,12 @@ func Test_newRollback(t *testing.T) { Name: "rollback", Namespace: "rollback-ns", }, - Status: v2.HelmReleaseStatus{ - Previous: &v2.HelmReleaseInfo{ - Name: "rollback", - Namespace: "rollback-ns", - Version: 3, - }, - }, - } - - got := newRollback(&helmaction.Configuration{}, obj, nil) - g.Expect(got).ToNot(BeNil()) - g.Expect(got.Version).To(Equal(obj.Status.Previous.Version)) - }) - - t.Run("rollback with stale previous", func(t *testing.T) { - g := NewWithT(t) - - obj := &v2.HelmRelease{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rollback", - Namespace: "rollback-ns", - }, - Status: v2.HelmReleaseStatus{ - Previous: &v2.HelmReleaseInfo{ - Name: "rollback", - Namespace: "other-ns", - Version: 3, - }, - }, } - got := newRollback(&helmaction.Configuration{}, obj, nil) + toVersion := 3 + got := newRollback(&helmaction.Configuration{}, obj, []RollbackOption{RollbackToVersion(toVersion)}) g.Expect(got).ToNot(BeNil()) - g.Expect(got.Version).To(BeZero()) + g.Expect(got.Version).To(Equal(toVersion)) }) t.Run("timeout fallback", func(t *testing.T) { diff --git a/internal/action/test.go b/internal/action/test.go index f3c5ffd2f..04a4e509d 100644 --- a/internal/action/test.go +++ b/internal/action/test.go @@ -47,11 +47,11 @@ func newTest(config *helmaction.Configuration, obj *v2.HelmRelease, opts []TestO test := helmaction.NewReleaseTesting(config) test.Namespace = obj.GetReleaseNamespace() - test.Timeout = obj.Spec.GetTest().GetTimeout(obj.GetTimeout()).Duration + test.Timeout = obj.GetTest().GetTimeout(obj.GetTimeout()).Duration filters := make(map[string][]string) - for _, f := range obj.Spec.GetTest().GetFilters() { + for _, f := range obj.GetTest().GetFilters() { name := "name" if f.Exclude { diff --git a/internal/action/uninstall.go b/internal/action/uninstall.go index bfbafd2d9..75fe6126d 100644 --- a/internal/action/uninstall.go +++ b/internal/action/uninstall.go @@ -28,7 +28,7 @@ import ( // UninstallOption can be used to modify Helm's action.Uninstall after the // instructions from the v2beta2.HelmRelease have been applied. This is for // example useful to enable the dry-run setting as a CLI. -type UninstallOption func(*helmaction.Uninstall) +type UninstallOption func(cfg *helmaction.Uninstall) // Uninstall runs the Helm uninstall action with the provided config, using the // v2beta2.HelmReleaseSpec of the given object to determine the target release @@ -38,18 +38,19 @@ type UninstallOption func(*helmaction.Uninstall) // expected to be done by the caller. In addition, it does not take note of the // action result. The caller is expected to listen to this using a // storage.ObserveFunc, which provides superior access to Helm storage writes. -func Uninstall(_ context.Context, config *helmaction.Configuration, obj *v2.HelmRelease, opts ...UninstallOption) (*helmrelease.UninstallReleaseResponse, error) { +func Uninstall(_ context.Context, config *helmaction.Configuration, obj *v2.HelmRelease, releaseName string, opts ...UninstallOption) (*helmrelease.UninstallReleaseResponse, error) { uninstall := newUninstall(config, obj, opts) - return uninstall.Run(obj.GetReleaseName()) + return uninstall.Run(releaseName) } func newUninstall(config *helmaction.Configuration, obj *v2.HelmRelease, opts []UninstallOption) *helmaction.Uninstall { uninstall := helmaction.NewUninstall(config) - uninstall.Timeout = obj.Spec.GetUninstall().GetTimeout(obj.GetTimeout()).Duration - uninstall.DisableHooks = obj.Spec.GetUninstall().DisableHooks - uninstall.KeepHistory = obj.Spec.GetUninstall().KeepHistory - uninstall.Wait = !obj.Spec.GetUninstall().DisableWait + uninstall.Timeout = obj.GetUninstall().GetTimeout(obj.GetTimeout()).Duration + uninstall.DisableHooks = obj.GetUninstall().DisableHooks + uninstall.KeepHistory = obj.GetUninstall().KeepHistory + uninstall.Wait = !obj.GetUninstall().DisableWait + uninstall.DeletionPropagation = obj.GetUninstall().GetDeletionPropagation() for _, opt := range opts { opt(uninstall) diff --git a/internal/action/upgrade.go b/internal/action/upgrade.go index 9901542f5..f18e50a26 100644 --- a/internal/action/upgrade.go +++ b/internal/action/upgrade.go @@ -28,6 +28,7 @@ import ( v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/features" "github.com/fluxcd/helm-controller/internal/postrender" + "github.com/fluxcd/helm-controller/internal/release" ) // UpgradeOption can be used to modify Helm's action.Upgrade after the instructions @@ -50,7 +51,7 @@ func Upgrade(ctx context.Context, config *helmaction.Configuration, obj *v2.Helm vals helmchartutil.Values, opts ...UpgradeOption) (*helmrelease.Release, error) { upgrade := newUpgrade(config, obj, opts) - policy, err := crdPolicyOrDefault(obj.Spec.GetInstall().CRDs) + policy, err := crdPolicyOrDefault(obj.GetInstall().CRDs) if err != nil { return nil, err } @@ -58,23 +59,22 @@ func Upgrade(ctx context.Context, config *helmaction.Configuration, obj *v2.Helm return nil, fmt.Errorf("failed to apply CustomResourceDefinitions: %w", err) } - return upgrade.RunWithContext(ctx, obj.GetReleaseName(), chrt, vals.AsMap()) + return upgrade.RunWithContext(ctx, release.ShortenName(obj.GetReleaseName()), chrt, vals.AsMap()) } func newUpgrade(config *helmaction.Configuration, obj *v2.HelmRelease, opts []UpgradeOption) *helmaction.Upgrade { upgrade := helmaction.NewUpgrade(config) - upgrade.Namespace = obj.GetReleaseNamespace() - upgrade.ResetValues = !obj.Spec.GetUpgrade().PreserveValues - upgrade.ReuseValues = obj.Spec.GetUpgrade().PreserveValues + upgrade.ResetValues = !obj.GetUpgrade().PreserveValues + upgrade.ReuseValues = obj.GetUpgrade().PreserveValues upgrade.MaxHistory = obj.GetMaxHistory() - upgrade.Timeout = obj.Spec.GetUpgrade().GetTimeout(obj.GetTimeout()).Duration - upgrade.Wait = !obj.Spec.GetUpgrade().DisableWait - upgrade.WaitForJobs = !obj.Spec.GetUpgrade().DisableWaitForJobs - upgrade.DisableHooks = obj.Spec.GetUpgrade().DisableHooks - upgrade.DisableOpenAPIValidation = obj.Spec.GetUpgrade().DisableOpenAPIValidation - upgrade.Force = obj.Spec.GetUpgrade().Force - upgrade.CleanupOnFail = obj.Spec.GetUpgrade().CleanupOnFail + upgrade.Timeout = obj.GetUpgrade().GetTimeout(obj.GetTimeout()).Duration + upgrade.Wait = !obj.GetUpgrade().DisableWait + upgrade.WaitForJobs = !obj.GetUpgrade().DisableWaitForJobs + upgrade.DisableHooks = obj.GetUpgrade().DisableHooks + upgrade.DisableOpenAPIValidation = obj.GetUpgrade().DisableOpenAPIValidation + upgrade.Force = obj.GetUpgrade().Force + upgrade.CleanupOnFail = obj.GetUpgrade().CleanupOnFail upgrade.Devel = true // If the user opted-in to allow DNS lookups, enable it. diff --git a/internal/action/verify.go b/internal/action/verify.go index f7040bc0e..02f1e980c 100644 --- a/internal/action/verify.go +++ b/internal/action/verify.go @@ -33,57 +33,126 @@ import ( ) var ( - ErrReleaseDisappeared = errors.New("observed release disappeared from storage") + ErrReleaseDisappeared = errors.New("release disappeared from storage") ErrReleaseNotFound = errors.New("no release found") - ErrReleaseNotObserved = errors.New("release not observed to be made by reconciler") + ErrReleaseNotObserved = errors.New("release not observed to be made for object") ErrReleaseDigest = errors.New("release digest verification error") ErrChartChanged = errors.New("release chart changed") ErrConfigDigest = errors.New("release config digest verification error") ) -// VerifyStorage verifies that the last release in the Helm storage matches the -// Current state of the given HelmRelease. It returns the release, or an error -// of type ErrReleaseDisappeared, ErrReleaseNotFound, ErrReleaseNotObserved, or -// ErrReleaseDigest. -func VerifyStorage(config *helmaction.Configuration, obj *v2.HelmRelease) (*helmrelease.Release, error) { - curRel := obj.Status.Current - rls, err := config.Releases.Last(obj.GetReleaseName()) +// ReleaseTargetChanged returns true if the given release and/or chart +// name have been mutated in such a way that it no longer has the same release +// target as the Status.Current, by comparing the (storage) namespace, and +// release and chart names. This can be used to e.g. trigger a garbage +// collection of the old release before installing the new one. +func ReleaseTargetChanged(obj *v2.HelmRelease, chartName string) bool { + cur := obj.GetCurrent() + switch { + case obj.Status.StorageNamespace == "", cur == nil: + return false + case obj.GetStorageNamespace() != obj.Status.StorageNamespace: + return true + case obj.GetReleaseNamespace() != cur.Namespace: + return true + case release.ShortenName(obj.GetReleaseName()) != cur.Name: + return true + case chartName != cur.ChartName: + return true + default: + return false + } +} + +// IsInstalled returns true if there is any release in the Helm storage with the +// given name. It returns any error other than driver.ErrReleaseNotFound. +func IsInstalled(config *helmaction.Configuration, releaseName string) (bool, error) { + _, err := config.Releases.Last(release.ShortenName(releaseName)) + if err != nil { + if errors.Is(err, helmdriver.ErrReleaseNotFound) { + return false, nil + } + return false, err + } + return true, nil +} + +// VerifyReleaseInfo verifies the data of the given v2beta2.HelmReleaseInfo +// matches the release object in the Helm storage. It returns the verified +// release, or an error of type ErrReleaseNotFound, ErrReleaseDisappeared, +// ErrReleaseDigest or ErrReleaseNotObserved indicating the reason for the +// verification failure. +func VerifyReleaseInfo(config *helmaction.Configuration, info *v2.HelmReleaseInfo) (rls *helmrelease.Release, err error) { + if info == nil { + return nil, ErrReleaseNotFound + } + + rls, err = config.Releases.Get(info.Name, info.Version) if err != nil { if errors.Is(err, helmdriver.ErrReleaseNotFound) { - if curRel != nil && curRel.Name == obj.GetReleaseName() && curRel.Namespace == obj.GetReleaseNamespace() { - return nil, ErrReleaseDisappeared - } - return nil, ErrReleaseNotFound + return nil, ErrReleaseDisappeared } return nil, err } - if curRel == nil { - return rls, ErrReleaseNotObserved + + if err = VerifyReleaseObject(info, rls); err != nil { + return nil, err } + return rls, nil +} - relDig, err := digest.Parse(obj.Status.Current.Digest) +// VerifyLastStorageItem verifies the data of the given v2beta2.HelmReleaseInfo +// matches the last release object in the Helm storage. It returns the verified +// release, or an error of type ErrReleaseNotFound, ErrReleaseDisappeared, +// ErrReleaseDigest or ErrReleaseNotObserved indicating the reason for the +// verification failure. +func VerifyLastStorageItem(config *helmaction.Configuration, info *v2.HelmReleaseInfo) (rls *helmrelease.Release, err error) { + if info == nil { + return nil, ErrReleaseNotFound + } + + rls, err = config.Releases.Last(info.Name) if err != nil { - return rls, ErrReleaseDigest + if errors.Is(err, helmdriver.ErrReleaseNotFound) { + return nil, ErrReleaseDisappeared + } + return nil, err + } + + if err = VerifyReleaseObject(info, rls); err != nil { + return nil, err + } + return rls, nil +} + +// VerifyReleaseObject verifies the data of the given v2beta2.HelmReleaseInfo +// matches the given Helm release object. It returns an error of type +// ErrReleaseDigest or ErrReleaseNotObserved indicating the reason for the +// verification failure, or nil. +func VerifyReleaseObject(info *v2.HelmReleaseInfo, rls *helmrelease.Release) error { + relDig, err := digest.Parse(info.Digest) + if err != nil { + return ErrReleaseDigest } verifier := relDig.Verifier() obs := release.ObserveRelease(rls) - if err := obs.Encode(verifier); err != nil { + if err = obs.Encode(verifier); err != nil { // We are expected to be able to encode valid JSON, error out without a // typed error assuming malfunction to signal to e.g. retry. - return nil, err + return err } if !verifier.Verified() { - return nil, ErrReleaseNotObserved + return ErrReleaseNotObserved } - return rls, nil + return nil } // VerifyRelease verifies that the data of the given release matches the given // chart metadata, and the provided values match the Current.ConfigDigest. // It returns either an error of type ErrReleaseNotFound, ErrChartChanged or // ErrConfigDigest, or nil. -func VerifyRelease(rls *helmrelease.Release, obj *v2.HelmRelease, chrt *helmchart.Metadata, vals helmchartutil.Values) error { +func VerifyRelease(rls *helmrelease.Release, info *v2.HelmReleaseInfo, chrt *helmchart.Metadata, vals helmchartutil.Values) error { if rls == nil { return ErrReleaseNotFound } @@ -94,7 +163,7 @@ func VerifyRelease(rls *helmrelease.Release, obj *v2.HelmRelease, chrt *helmchar } } - if !chartutil.VerifyValues(digest.Digest(obj.Status.Current.ConfigDigest), vals) { + if info == nil || !chartutil.VerifyValues(digest.Digest(info.ConfigDigest), vals) { return ErrConfigDigest } return nil diff --git a/internal/action/verify_test.go b/internal/action/verify_test.go new file mode 100644 index 000000000..698d48ac0 --- /dev/null +++ b/internal/action/verify_test.go @@ -0,0 +1,568 @@ +/* +Copyright 2022 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 action + +import ( + "errors" + "testing" + + . "github.com/onsi/gomega" + helmaction "helm.sh/helm/v3/pkg/action" + helmchart "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chartutil" + helmrelease "helm.sh/helm/v3/pkg/release" + helmstorage "helm.sh/helm/v3/pkg/storage" + "helm.sh/helm/v3/pkg/storage/driver" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/release" + "github.com/fluxcd/helm-controller/internal/storage" + "github.com/fluxcd/helm-controller/internal/testutil" +) + +func TestReleaseTargetChanged(t *testing.T) { + const ( + defaultNamespace = "default-ns" + defaultName = "default-name" + defaultChartName = "default-chart" + defaultReleaseName = "default-release" + defaultTargetNamespace = "default-target-ns" + defaultStorageNamespace = "default-storage-ns" + ) + + tests := []struct { + name string + chartName string + spec v2.HelmReleaseSpec + status v2.HelmReleaseStatus + want bool + }{ + { + name: "no change", + chartName: defaultChartName, + spec: v2.HelmReleaseSpec{}, + status: v2.HelmReleaseStatus{ + Current: &v2.HelmReleaseInfo{ + Name: defaultName, + Namespace: defaultNamespace, + ChartName: defaultChartName, + }, + StorageNamespace: defaultNamespace, + }, + want: false, + }, + { + name: "no storage namespace", + chartName: defaultChartName, + spec: v2.HelmReleaseSpec{ + ReleaseName: defaultReleaseName, + }, + status: v2.HelmReleaseStatus{ + Current: &v2.HelmReleaseInfo{ + Name: defaultReleaseName, + Namespace: defaultNamespace, + ChartName: defaultChartName, + }, + }, + want: false, + }, + { + name: "no current", + spec: v2.HelmReleaseSpec{}, + status: v2.HelmReleaseStatus{ + StorageNamespace: defaultNamespace, + Current: nil, + }, + want: false, + }, + { + name: "different storage namespace", + chartName: defaultChartName, + spec: v2.HelmReleaseSpec{ + StorageNamespace: defaultStorageNamespace, + }, + status: v2.HelmReleaseStatus{ + Current: &v2.HelmReleaseInfo{ + Name: defaultName, + Namespace: defaultNamespace, + ChartName: defaultChartName, + }, + StorageNamespace: defaultNamespace, + }, + want: true, + }, + { + name: "different release namespace", + chartName: defaultChartName, + spec: v2.HelmReleaseSpec{ + TargetNamespace: defaultTargetNamespace, + }, + status: v2.HelmReleaseStatus{ + Current: &v2.HelmReleaseInfo{ + Name: defaultName, + Namespace: defaultNamespace, + ChartName: defaultChartName, + }, + StorageNamespace: defaultNamespace, + }, + want: true, + }, + { + name: "different release name", + chartName: defaultChartName, + spec: v2.HelmReleaseSpec{ + ReleaseName: defaultReleaseName, + }, + status: v2.HelmReleaseStatus{ + Current: &v2.HelmReleaseInfo{ + Name: defaultName, + Namespace: defaultNamespace, + ChartName: defaultChartName, + }, + StorageNamespace: defaultNamespace, + }, + want: true, + }, + { + name: "different chart name", + chartName: "other-chart", + spec: v2.HelmReleaseSpec{}, + status: v2.HelmReleaseStatus{ + Current: &v2.HelmReleaseInfo{ + Name: defaultName, + Namespace: defaultNamespace, + ChartName: defaultChartName, + }, + StorageNamespace: defaultNamespace, + }, + want: true, + }, + { + name: "matching shortened release name", + chartName: defaultChartName, + spec: v2.HelmReleaseSpec{ + TargetNamespace: "target-namespace-exceeding-max-characters", + }, + status: v2.HelmReleaseStatus{ + Current: &v2.HelmReleaseInfo{ + Name: "target-namespace-exceeding-max-character-eceb26601388", + Namespace: "target-namespace-exceeding-max-characters", + ChartName: defaultChartName, + }, + StorageNamespace: defaultNamespace, + }, + want: false, + }, + { + name: "different shortened release name", + chartName: defaultChartName, + spec: v2.HelmReleaseSpec{ + TargetNamespace: "target-namespace-exceeding-max-characters", + }, + status: v2.HelmReleaseStatus{ + Current: &v2.HelmReleaseInfo{ + Name: defaultName, + Namespace: "target-namespace-exceeding-max-characters", + ChartName: defaultChartName, + }, + StorageNamespace: defaultNamespace, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := ReleaseTargetChanged(&v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: defaultName, + }, + Spec: tt.spec, + Status: tt.status, + }, tt.chartName) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func TestIsInstalled(t *testing.T) { + var mockError = errors.New("query mock error") + + tests := []struct { + name string + releaseName string + releases []*helmrelease.Release + queryError error + want bool + wantErr error + }{ + { + name: "installed", + releaseName: "release", + releases: []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Version: 1, + Status: helmrelease.StatusDeployed, + Namespace: "default", + }), + }, + want: true, + }, + { + name: "not installed", + releaseName: "release", + want: false, + }, + { + name: "release list error", + queryError: mockError, + wantErr: mockError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + s := helmstorage.Init(driver.NewMemory()) + for _, v := range tt.releases { + g.Expect(s.Create(v)).To(Succeed()) + } + + s.Driver = &storage.Failing{ + Driver: s.Driver, + QueryErr: tt.queryError, + } + + got, err := IsInstalled(&helmaction.Configuration{Releases: s}, tt.releaseName) + + if tt.wantErr != nil { + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(Equal(tt.wantErr)) + g.Expect(got).To(BeFalse()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func TestVerifyReleaseInfo(t *testing.T) { + mock := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Version: 1, + Status: helmrelease.StatusDeployed, + Namespace: "default", + }) + otherMock := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Version: 1, + Status: helmrelease.StatusSuperseded, + Namespace: "default", + }) + mockInfo := release.ObservedToInfo(release.ObserveRelease(mock)) + mockGetErr := errors.New("mock get error") + + tests := []struct { + name string + info *v2.HelmReleaseInfo + release *helmrelease.Release + getError error + want *helmrelease.Release + wantErr error + }{ + { + name: "valid release", + info: mockInfo, + release: mock, + want: mock, + }, + { + name: "invalid release", + info: mockInfo, + release: otherMock, + wantErr: ErrReleaseNotObserved, + }, + { + name: "release not found", + info: mockInfo, + release: nil, + wantErr: ErrReleaseDisappeared, + }, + { + name: "no release info", + info: nil, + release: nil, + wantErr: ErrReleaseNotFound, + }, + { + name: "driver get error", + info: mockInfo, + getError: mockGetErr, + wantErr: mockGetErr, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + s := helmstorage.Init(driver.NewMemory()) + if tt.release != nil { + g.Expect(s.Create(tt.release)).To(Succeed()) + } + + s.Driver = &storage.Failing{ + Driver: s.Driver, + GetErr: tt.getError, + } + + rls, err := VerifyReleaseInfo(&helmaction.Configuration{Releases: s}, tt.info) + if tt.wantErr != nil { + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(Equal(tt.wantErr)) + g.Expect(rls).To(BeNil()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(rls).To(Equal(tt.want)) + }) + } +} + +func TestVerifyLastStorageItem(t *testing.T) { + mockOne := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Version: 1, + Status: helmrelease.StatusSuperseded, + Namespace: "default", + }) + mockTwo := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Version: 2, + Status: helmrelease.StatusDeployed, + Namespace: "default", + }) + mockInfo := release.ObservedToInfo(release.ObserveRelease(mockTwo)) + mockQueryErr := errors.New("mock query error") + + tests := []struct { + name string + info *v2.HelmReleaseInfo + releases []*helmrelease.Release + queryError error + want *helmrelease.Release + wantErr error + }{ + { + name: "valid last release", + info: mockInfo, + releases: []*helmrelease.Release{mockOne, mockTwo}, + want: mockTwo, + }, + { + name: "invalid last release", + info: mockInfo, + releases: []*helmrelease.Release{mockOne}, + wantErr: ErrReleaseNotObserved, + }, + { + name: "no last release", + info: mockInfo, + releases: []*helmrelease.Release{}, + wantErr: ErrReleaseDisappeared, + }, + { + name: "no release info", + info: nil, + releases: nil, + wantErr: ErrReleaseNotFound, + }, + { + name: "driver query error", + info: mockInfo, + queryError: mockQueryErr, + wantErr: mockQueryErr, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + s := helmstorage.Init(driver.NewMemory()) + for _, v := range tt.releases { + g.Expect(s.Create(v)).To(Succeed()) + } + + s.Driver = &storage.Failing{ + Driver: s.Driver, + QueryErr: tt.queryError, + } + + rls, err := VerifyLastStorageItem(&helmaction.Configuration{Releases: s}, tt.info) + if tt.wantErr != nil { + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(Equal(tt.wantErr)) + g.Expect(rls).To(BeNil()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(rls).To(Equal(tt.want)) + }) + } +} + +func TestVerifyReleaseObject(t *testing.T) { + mockRls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Version: 1, + Status: helmrelease.StatusSuperseded, + Namespace: "default", + }) + mockInfo := release.ObservedToInfo(release.ObserveRelease(mockRls)) + mockInfoIllegal := mockInfo.DeepCopy() + mockInfoIllegal.Digest = "illegal" + + tests := []struct { + name string + info *v2.HelmReleaseInfo + rls *helmrelease.Release + wantErr error + }{ + { + name: "valid digest", + info: mockInfo, + rls: mockRls, + }, + { + name: "illegal digest", + info: mockInfoIllegal, + wantErr: ErrReleaseDigest, + }, + { + name: "invalid digest", + info: mockInfo, + rls: testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Version: 1, + Status: helmrelease.StatusDeployed, + Namespace: "default", + }), + wantErr: ErrReleaseNotObserved, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := VerifyReleaseObject(tt.info, tt.rls) + + if tt.wantErr != nil { + g.Expect(got).To(HaveOccurred()) + g.Expect(got).To(Equal(tt.wantErr)) + return + } + + g.Expect(got).NotTo(HaveOccurred()) + }) + } +} + +func TestVerifyRelease(t *testing.T) { + mockRls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Version: 1, + Status: helmrelease.StatusSuperseded, + Namespace: "default", + }) + mockInfo := release.ObservedToInfo(release.ObserveRelease(mockRls)) + + tests := []struct { + name string + rls *helmrelease.Release + info *v2.HelmReleaseInfo + chrt *helmchart.Metadata + vals chartutil.Values + wantErr error + }{ + { + name: "equal", + rls: mockRls, + info: mockInfo, + chrt: mockRls.Chart.Metadata, + vals: mockRls.Config, + }, + { + name: "no release", + rls: nil, + info: mockInfo, + chrt: mockRls.Chart.Metadata, + vals: mockRls.Config, + wantErr: ErrReleaseNotFound, + }, + { + name: "no release info", + rls: mockRls, + info: nil, + chrt: mockRls.Chart.Metadata, + vals: mockRls.Config, + wantErr: ErrConfigDigest, + }, + { + name: "chart meta diff", + rls: mockRls, + info: mockInfo, + chrt: &helmchart.Metadata{ + Name: "some-other-chart", + Version: "1.0.0", + }, + vals: mockRls.Config, + wantErr: ErrChartChanged, + }, + { + name: "chart values diff", + rls: mockRls, + info: mockInfo, + chrt: mockRls.Chart.Metadata, + vals: chartutil.Values{ + "some": "other", + }, + wantErr: ErrConfigDigest, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got := VerifyRelease(tt.rls, tt.info, tt.chrt, tt.vals) + + if tt.wantErr != nil { + g.Expect(got).To(HaveOccurred()) + g.Expect(got).To(Equal(tt.wantErr)) + return + } + + g.Expect(got).ToNot(HaveOccurred()) + }) + } +} diff --git a/internal/reconcile/action.go b/internal/reconcile/action.go index 65bac3601..57e9f314a 100644 --- a/internal/reconcile/action.go +++ b/internal/reconcile/action.go @@ -17,99 +17,155 @@ limitations under the License. package reconcile import ( + "context" "errors" + "fmt" helmrelease "helm.sh/helm/v3/pkg/release" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/fluxcd/pkg/runtime/logger" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" - "github.com/fluxcd/helm-controller/internal/release" ) var ( - // ErrReconcileEnd is returned by NextAction when the reconciliation process - // has reached an end state. - ErrReconcileEnd = errors.New("abort reconcile") + // ErrNoRetriesRemain is returned when there are no remaining retry + // attempts for the provided release config. + ErrNoRetriesRemain = errors.New("no retries remain") ) // NextAction determines the action that should be performed for the release // by verifying the integrity of the Helm storage and further state of the // release, and comparing the Request.Chart and Request.Values to the latest // release. It can be called repeatedly to step through the reconciliation -// process until it ends up in a state as desired by the Request.Object. -func NextAction(factory *action.ConfigFactory, req *Request) (ActionReconciler, error) { - rls, err := action.VerifyStorage(factory.Build(nil), req.Object) - if err != nil { - switch err { - case action.ErrReleaseNotFound, action.ErrReleaseDisappeared: - return &Install{configFactory: factory}, nil - case action.ErrReleaseNotObserved, action.ErrReleaseDigest: - return &Upgrade{configFactory: factory}, nil - default: - return nil, err +// process until it ends up in a state as desired by the Request.Object, +// or no retries remain. +func NextAction(ctx context.Context, cfg *action.ConfigFactory, recorder record.EventRecorder, req *Request) (ActionReconciler, error) { + log := ctrl.LoggerFrom(ctx).V(logger.DebugLevel) + config := cfg.Build(nil) + cur := req.Object.GetCurrent().DeepCopy() + + // If we do not have a current release, we should either install or upgrade + // the release depending on the state of the storage. + if cur == nil { + ok, err := action.IsInstalled(config, req.Object.GetReleaseName()) + if err != nil { + return nil, fmt.Errorf("cannot confirm if release is already installed: %w", err) } + if ok { + return NewUpgrade(cfg, recorder), nil + } + return NewInstall(cfg, recorder), nil } + // Verify the current release is still in storage and unmodified. + rls, err := action.VerifyLastStorageItem(config, cur) + switch err { + case nil: + // Noop + case action.ErrReleaseNotFound, action.ErrReleaseDisappeared: + log.Info(err.Error()) + return NewInstall(cfg, recorder), nil + case action.ErrReleaseNotObserved, action.ErrReleaseDigest: + log.Info(err.Error()) + return NewUpgrade(cfg, recorder), nil + default: + return nil, fmt.Errorf("cannot verify current release in storage: %w", err) + } + + // If the release is in a pending state, the release process likely failed + // unexpectedly. Unlock the release and e.g. retry again. if rls.Info.Status.IsPending() { - return &Unlock{configFactory: factory}, nil + log.Info("observed release is in stale pending state") + return NewUnlock(cfg, recorder), nil + } + + remediation := req.Object.GetActiveRemediation() + + // A release in a failed state is different from any of the other states in + // that the action also needs to happen a last time when no retries remain. + if rls.Info.Status == helmrelease.StatusFailed { + if remediation.GetFailureCount(req.Object) <= 0 { + // If the chart version and/or values have changed, the failure count(s) + // are reset. This short circuits any remediation attempt to force an + // upgrade with the new configuration instead. + return NewUpgrade(cfg, recorder), nil + } + return rollbackOrUninstall(cfg, recorder, req) } - remediation := req.Object.Spec.GetInstall().GetRemediation() - if req.Object.Status.Previous != nil { - remediation = req.Object.Spec.GetUpgrade().GetRemediation() + // Short circuit if we are out of retries. + if remediation.RetriesExhausted(req.Object) { + return nil, fmt.Errorf("%w: ignoring release in %s state", ErrNoRetriesRemain, rls.Info.Status) } - // TODO(hidde): the logic below lacks some implementation details. E.g. - // upgrading a failed release when a newer chart version appears. + // Act on the state of the release. switch rls.Info.Status { - case helmrelease.StatusFailed: - return rollbackOrUninstall(factory, req) - case helmrelease.StatusUninstalled: - return &Install{configFactory: factory}, nil - case helmrelease.StatusSuperseded: - return &Install{configFactory: factory}, nil + case helmrelease.StatusUninstalled, helmrelease.StatusSuperseded: + return NewInstall(cfg, recorder), nil case helmrelease.StatusDeployed: - if err = action.VerifyRelease(rls, req.Object, req.Chart.Metadata, req.Values); err != nil { + // Confirm the current release matches the desired config. + if err = action.VerifyRelease(rls, cur, req.Chart.Metadata, req.Values); err != nil { switch err { - case action.ErrChartChanged: - return &Upgrade{configFactory: factory}, nil - case action.ErrConfigDigest: - return &Upgrade{configFactory: factory}, nil + case action.ErrChartChanged, action.ErrConfigDigest: + return NewUpgrade(cfg, recorder), nil default: + // Error out on any other error as we cannot determine what + // the state and should e.g. retry. return nil, err } } - if testSpec := req.Object.Spec.GetTest(); testSpec.Enable { - if !release.HasBeenTested(rls) { - return &Test{configFactory: factory}, nil + // For the further determination of test results, we look at the + // observed state of the object. As tests can be run manually by + // users running e.g. `helm test`. + if testSpec := req.Object.GetTest(); testSpec.Enable { + // Confirm the release has been tested if enabled. + if !req.Object.GetCurrent().HasBeenTested() { + return NewTest(cfg, recorder), nil } - if release.HasFailedTests(rls) { - if !remediation.MustIgnoreTestFailures(req.Object.Spec.GetTest().IgnoreFailures) { - return rollbackOrUninstall(factory, req) - } + // Act on any observed test failure. + if !remediation.MustIgnoreTestFailures(req.Object.GetTest().IgnoreFailures) && + req.Object.GetCurrent().HasTestInPhase(helmrelease.HookPhaseFailed.String()) { + return rollbackOrUninstall(cfg, recorder, req) } } } - return nil, ErrReconcileEnd + + return nil, nil } -func rollbackOrUninstall(factory *action.ConfigFactory, req *Request) (ActionReconciler, error) { - remediation := req.Object.Spec.GetInstall().GetRemediation() - if req.Object.Status.Previous != nil { - // TODO: determine if previous is still in storage and unmodified - remediation = req.Object.Spec.GetUpgrade().GetRemediation() - } - // TODO: remove dependency on counter, as this shouldn't be used to determine - // if it's enabled. - remediation.IncrementFailureCount(req.Object) - if !remediation.RetriesExhausted(*req.Object) || remediation.MustRemediateLastFailure() { +// rollbackOrUninstall determines if the release should be rolled back or +// uninstalled based on the active remediation strategy. If the release +// must be rolled back, the target revision is verified to be in storage +// before returning the RollbackRemediation. If the verification fails, +// Upgrade is returned as a remediation action to ensure continuity. +func rollbackOrUninstall(cfg *action.ConfigFactory, recorder record.EventRecorder, req *Request) (ActionReconciler, error) { + remediation := req.Object.GetActiveRemediation() + if !remediation.RetriesExhausted(req.Object) || remediation.MustRemediateLastFailure() { switch remediation.GetStrategy() { case v2.RollbackRemediationStrategy: - return &Rollback{configFactory: factory}, nil + // Verify the previous release is still in storage and unmodified + // before instructing to roll back to it. + if _, err := action.VerifyReleaseInfo(cfg.Build(nil), req.Object.GetPrevious()); err != nil { + switch err { + case action.ErrReleaseNotFound, action.ErrReleaseDisappeared, + action.ErrReleaseNotObserved, action.ErrReleaseDigest: + // If the rollback target is not found or is in any other + // way corrupt, the most correct remediation is to reattempt + // the upgrade. + return NewUpgrade(cfg, recorder), nil + default: + return nil, err + } + } + return NewRollbackRemediation(cfg, recorder), nil case v2.UninstallRemediationStrategy: - return &Uninstall{configFactory: factory}, nil + return NewUninstallRemediation(cfg, recorder), nil } } - return nil, ErrReconcileEnd + return nil, fmt.Errorf("%w: can not remediate %s state", ErrNoRetriesRemain, req.Object.GetCurrent().Status) } diff --git a/internal/reconcile/action_test.go b/internal/reconcile/action_test.go index 3247d25d3..91dec9008 100644 --- a/internal/reconcile/action_test.go +++ b/internal/reconcile/action_test.go @@ -17,6 +17,7 @@ limitations under the License. package reconcile import ( + "context" "testing" "github.com/go-logr/logr" @@ -26,6 +27,7 @@ import ( helmrelease "helm.sh/helm/v3/pkg/release" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" + "k8s.io/client-go/tools/record" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" @@ -61,9 +63,8 @@ func Test_NextAction(t *testing.T) { Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), } }, - chart: testutil.BuildChart(), - values: map[string]interface{}{"foo": "bar"}, - wantErr: true, + chart: testutil.BuildChart(), + values: map[string]interface{}{"foo": "bar"}, }, { name: "no release in storage requires install", @@ -189,7 +190,7 @@ func Test_NextAction(t *testing.T) { want: &Test{}, }, { - name: "failed test requires rollback when enabled", + name: "failure test requires rollback when enabled", releases: []*helmrelease.Release{ testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, @@ -207,7 +208,7 @@ func Test_NextAction(t *testing.T) { Chart: testutil.BuildChart(), }, testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"}), - testutil.ReleaseWithHookExecution("failed-tests", []helmrelease.HookEvent{helmrelease.HookTest}, + testutil.ReleaseWithHookExecution("failure-tests", []helmrelease.HookEvent{helmrelease.HookTest}, helmrelease.HookPhaseFailed), ), }, @@ -222,17 +223,20 @@ func Test_NextAction(t *testing.T) { } }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + cur := release.ObservedToInfo(release.ObserveRelease(releases[1])) + cur.SetTestHooks(release.TestHooksFromRelease(releases[1])) + return v2.HelmReleaseStatus{ - Current: release.ObservedToInfo(release.ObserveRelease(releases[1])), + Current: cur, Previous: release.ObservedToInfo(release.ObserveRelease(releases[0])), } }, chart: testutil.BuildChart(), values: map[string]interface{}{"foo": "bar"}, - want: &Rollback{}, + want: &RollbackRemediation{}, }, { - name: "failed test requires uninstall when enabled", + name: "failure test requires uninstall when enabled", releases: []*helmrelease.Release{ testutil.BuildRelease( &helmrelease.MockReleaseOptions{ @@ -243,7 +247,7 @@ func Test_NextAction(t *testing.T) { Chart: testutil.BuildChart(), }, testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"}), - testutil.ReleaseWithHookExecution("failed-tests", []helmrelease.HookEvent{helmrelease.HookTest}, + testutil.ReleaseWithHookExecution("failure-tests", []helmrelease.HookEvent{helmrelease.HookTest}, helmrelease.HookPhaseFailed), ), }, @@ -258,16 +262,19 @@ func Test_NextAction(t *testing.T) { } }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + cur := release.ObservedToInfo(release.ObserveRelease(releases[0])) + cur.SetTestHooks(release.TestHooksFromRelease(releases[0])) + return v2.HelmReleaseStatus{ - Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + Current: cur, } }, chart: testutil.BuildChart(), values: map[string]interface{}{"foo": "bar"}, - want: &Uninstall{}, + want: &UninstallRemediation{}, }, { - name: "failed test is ignored when ignore failures is set", + name: "failure test is ignored when ignore failures is set", releases: []*helmrelease.Release{ testutil.BuildRelease( &helmrelease.MockReleaseOptions{ @@ -278,7 +285,7 @@ func Test_NextAction(t *testing.T) { Chart: testutil.BuildChart(), }, testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"}), - testutil.ReleaseWithHookExecution("failed-tests", []helmrelease.HookEvent{helmrelease.HookTest}, + testutil.ReleaseWithHookExecution("failure-tests", []helmrelease.HookEvent{helmrelease.HookTest}, helmrelease.HookPhaseFailed), ), }, @@ -295,15 +302,52 @@ func Test_NextAction(t *testing.T) { }, } }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + cur := release.ObservedToInfo(release.ObserveRelease(releases[0])) + cur.SetTestHooks(release.TestHooksFromRelease(releases[0])) + + return v2.HelmReleaseStatus{ + Current: cur, + } + }, + }, + { + name: "failure test is ignored when not made by controller", + releases: []*helmrelease.Release{ + testutil.BuildRelease( + &helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 2, + Status: helmrelease.StatusDeployed, + Chart: testutil.BuildChart(), + }, + testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"}), + testutil.ReleaseWithHookExecution("failure-tests", []helmrelease.HookEvent{helmrelease.HookTest}, + helmrelease.HookPhaseFailed), + ), + }, + chart: testutil.BuildChart(), + values: map[string]interface{}{"foo": "bar"}, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Test = &v2.Test{ + Enable: true, + } + spec.Install = &v2.Install{ + Remediation: &v2.InstallRemediation{ + Retries: 1, + }, + } + }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), } }, - wantErr: true, + want: &Test{}, }, { - name: "failed release requires rollback when enabled", + name: "failure release requires rollback when enabled", releases: []*helmrelease.Release{ testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, @@ -331,14 +375,15 @@ func Test_NextAction(t *testing.T) { values: map[string]interface{}{}, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - Current: release.ObservedToInfo(release.ObserveRelease(releases[1])), - Previous: release.ObservedToInfo(release.ObserveRelease(releases[0])), + Current: release.ObservedToInfo(release.ObserveRelease(releases[1])), + Previous: release.ObservedToInfo(release.ObserveRelease(releases[0])), + UpgradeFailures: 1, } }, - want: &Rollback{}, + want: &RollbackRemediation{}, }, { - name: "failed release requires uninstall when enabled", + name: "failure release requires uninstall when enabled", releases: []*helmrelease.Release{ testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, @@ -359,22 +404,138 @@ func Test_NextAction(t *testing.T) { values: map[string]interface{}{}, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + InstallFailures: 1, + } + }, + want: &UninstallRemediation{}, + }, + { + name: "failure release is ignored when no remediation strategy is configured", + releases: []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmrelease.StatusFailed, + Chart: testutil.BuildChart(), + }, testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"})), + }, + chart: testutil.BuildChart(), + values: map[string]interface{}{}, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + InstallFailures: 1, + } + }, + wantErr: true, + }, + { + name: "failure release without install failure count requires upgrade", + releases: []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmrelease.StatusFailed, + Chart: testutil.BuildChart(), + }, testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"})), + }, + chart: testutil.BuildChart(), + values: map[string]interface{}{}, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + UpgradeFailures: 1, } }, - want: &Uninstall{}, + want: &Upgrade{}, }, { - name: "failed release is ignored when no remediation strategy is configured", + name: "failure release without upgrade failure count requires upgrade", releases: []*helmrelease.Release{ testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, Namespace: mockReleaseNamespace, Version: 1, + Status: helmrelease.StatusSuperseded, + Chart: testutil.BuildChart(), + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 2, Status: helmrelease.StatusFailed, Chart: testutil.BuildChart(), }, testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"})), }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Upgrade = &v2.Upgrade{ + Remediation: &v2.UpgradeRemediation{ + Retries: 1, + }, + } + }, + chart: testutil.BuildChart(), + values: map[string]interface{}{}, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[1])), + Previous: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + want: &Upgrade{}, + }, + { + name: "failure release with disappeared previous release requires upgrade", + releases: []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 2, + Status: helmrelease.StatusFailed, + Chart: testutil.BuildChart(), + }, testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"})), + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Upgrade = &v2.Upgrade{ + Remediation: &v2.UpgradeRemediation{ + Retries: 1, + }, + } + }, + chart: testutil.BuildChart(), + values: map[string]interface{}{}, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + prev := *releases[0] + prev.Version = 1 + return v2.HelmReleaseStatus{ + UpgradeFailures: 1, + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + Previous: release.ObservedToInfo(release.ObserveRelease(&prev)), + } + }, + want: &Upgrade{}, + }, + { + name: "superseded release requires install", + releases: []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmrelease.StatusSuperseded, + Chart: testutil.BuildChart(), + }), + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Upgrade = &v2.Upgrade{ + Remediation: &v2.UpgradeRemediation{ + Retries: 1, + }, + } + }, chart: testutil.BuildChart(), values: map[string]interface{}{}, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { @@ -382,6 +543,70 @@ func Test_NextAction(t *testing.T) { Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), } }, + want: &Install{}, + }, + { + name: "exhausted install retries", + releases: []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmrelease.StatusFailed, + Chart: testutil.BuildChart(), + }), + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Install = &v2.Install{ + Remediation: &v2.InstallRemediation{ + Retries: 2, + }, + } + }, + chart: testutil.BuildChart(), + values: map[string]interface{}{}, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + InstallFailures: 3, + } + }, + wantErr: true, + }, + { + name: "exhausted upgrade retries", + releases: []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmrelease.StatusSuperseded, + Chart: testutil.BuildChart(), + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 2, + Status: helmrelease.StatusDeployed, + Chart: testutil.BuildChart(), + }), + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Upgrade = &v2.Upgrade{ + Remediation: &v2.UpgradeRemediation{ + Retries: 2, + }, + } + }, + chart: testutil.BuildChart(), + values: map[string]interface{}{}, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[1])), + Previous: release.ObservedToInfo(release.ObserveRelease(releases[0])), + UpgradeFailures: 3, + } + }, wantErr: true, }, { @@ -479,7 +704,8 @@ func Test_NextAction(t *testing.T) { } } - got, err := NextAction(cfg, &Request{ + recorder := new(record.FakeRecorder) + got, err := NextAction(context.TODO(), cfg, recorder, &Request{ Object: obj, Chart: tt.chart, Values: tt.values, @@ -489,7 +715,13 @@ func Test_NextAction(t *testing.T) { g.Expect(err).To(HaveOccurred()) return } - g.Expect(got).To(BeAssignableToTypeOf(tt.want)) + + want := BeAssignableToTypeOf(tt.want) + if tt.want == nil { + want = BeNil() + } + + g.Expect(got).To(want) g.Expect(err).ToNot(HaveOccurred()) }) } diff --git a/internal/reconcile/atomic_release.go b/internal/reconcile/atomic_release.go new file mode 100644 index 000000000..7095d5cb9 --- /dev/null +++ b/internal/reconcile/atomic_release.go @@ -0,0 +1,242 @@ +/* +Copyright 2022 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 reconcile + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" + "github.com/fluxcd/pkg/runtime/logger" + "github.com/fluxcd/pkg/runtime/patch" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" +) + +// ownedConditions is a list of Condition types owned by the HelmRelease object. +var ownedConditions = []string{ + v2.ReleasedCondition, + v2.RemediatedCondition, + v2.TestSuccessCondition, + meta.ReconcilingCondition, + meta.ReadyCondition, + meta.StalledCondition, +} + +// AtomicRelease is an ActionReconciler which implements an atomic release +// strategy similar to Helm's `--atomic`, but with more advanced state +// determination. It determines the NextAction to take based on the current +// state of the Request.Object and other data, and the state of the Helm +// release. +// +// This process will continue until an action is called multiple times, no +// action remains, or a remediation action is called. In which case the process +// will stop to be resumed at a later time or be checked upon again, by e.g. a +// requeue. +// +// Before running the ActionReconciler for the next action, the object is +// marked with Reconciling=True and the status is patched. +// This condition is removed when the ActionReconciler process is done. +// +// When it determines the object is out of remediation retries, the object +// is marked with Stalled=True. +// +// The status conditions are summarized into a Ready condition when no actions +// to be run remain, to ensure any transient error is cleared. +// +// Any returned error other than ErrNoRetriesRemain should be retried by the +// caller as soon as possible, preferably with a backoff strategy. +// +// The caller is expected to patch the object one last time with the +// Request.Object result to persist the final observation. As there is an +// expectation they will need to patch the object anyway to e.g. update the +// ObservedGeneration. +// +// For more information on the individual ActionReconcilers, refer to their +// documentation. +type AtomicRelease struct { + kubeClient client.Client + configFactory *action.ConfigFactory + eventRecorder record.EventRecorder + strategy releaseStrategy +} + +// NewAtomicRelease returns a new AtomicRelease reconciler configured with the +// provided values. +func NewAtomicRelease(client client.Client, cfg *action.ConfigFactory, recorder record.EventRecorder) *AtomicRelease { + return &AtomicRelease{ + kubeClient: client, + eventRecorder: recorder, + configFactory: cfg, + strategy: &cleanReleaseStrategy{}, + } +} + +// releaseStrategy defines the continue-stop behavior of the reconcile loop. +type releaseStrategy interface { + // MustContinue should be called before running the current action, and + // returns true if the caller must proceed. + MustContinue(current ReconcilerType, previous ReconcilerTypeSet) bool + // MustStop should be called after running the current action, and returns + // true if the caller must stop. + MustStop(current ReconcilerType, previous ReconcilerTypeSet) bool +} + +// cleanReleaseStrategy is a releaseStrategy which will only execute the +// (remaining) actions for a single release. Effectively, this means it will +// only run any action once during a reconcile attempt, and stops after running +// a remediation action. +type cleanReleaseStrategy ReconcilerTypeSet + +// MustContinue returns if previous does not contain current. +func (cleanReleaseStrategy) MustContinue(current ReconcilerType, previous ReconcilerTypeSet) bool { + return !previous.Contains(current) +} + +// MustStop returns true if current equals ReconcilerTypeRemediate. +func (cleanReleaseStrategy) MustStop(current ReconcilerType, _ ReconcilerTypeSet) bool { + switch current { + case ReconcilerTypeRemediate: + return true + default: + return false + } +} + +func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error { + log := ctrl.LoggerFrom(ctx).V(logger.DebugLevel) + patchHelper := patch.NewSerialPatcher(req.Object, r.kubeClient) + + var ( + previous ReconcilerTypeSet + next ActionReconciler + err error + ) + for { + select { + case <-ctx.Done(): + return ctx.Err() + default: + // Determine the next action to run based on the current state. + log.Info("determining next Helm action based on current state") + if next, err = NextAction(ctx, r.configFactory, r.eventRecorder, req); err != nil { + log.Error(err, "cannot determine next action") + if errors.Is(err, ErrNoRetriesRemain) { + conditions.MarkStalled(req.Object, "NoRemainingRetries", "Attempted %d times but failed", req.Object.GetActiveRemediation().GetRetries()) + } else { + conditions.MarkFalse(req.Object, meta.ReadyCondition, "ActionPlanError", fmt.Sprintf("Could not determine Helm action: %s", err.Error())) + } + return err + } + + // Nothing to do... + if next == nil { + log.Info("release in-sync") + + // If we are in-sync, we are no longer reconciling + conditions.Delete(req.Object, meta.ReconcilingCondition) + + // Always summarize, this ensures we e.g. restore transient errors written to Ready + summarize(req) + + return nil + } + + // If we are not allowed to run the next action, we are done for now... + if !r.strategy.MustContinue(next.Type(), previous) { + log.Info("instructed to stop before running %s action reconciler %s", next.Type(), next.Name()) + conditions.Delete(req.Object, meta.ReconcilingCondition) + return nil + } + + // Mark the release as reconciling before we attempt to run the action. + // This to show continuous progress, as Helm actions can be long-running. + conditions.MarkTrue(req.Object, meta.ReconcilingCondition, "Progressing", "Running '%s' %s action with timeout of %s", + next.Name(), next.Type(), timeoutForAction(next, req.Object).String()) + // Patch the object to reflect the new condition. + if err = patchHelper.Patch(ctx, req.Object, patch.WithOwnedConditions{Conditions: ownedConditions}); err != nil { + return err + } + + // Run the action sub-reconciler. + if err = next.Reconcile(ctx, req); err != nil { + if conditions.IsReady(req.Object) { + conditions.MarkFalse(req.Object, meta.ReadyCondition, "ReconcileError", err.Error()) + } + return err + } + + // If we must stop after running the action, we are done for now... + if r.strategy.MustStop(next.Type(), previous) { + log.Info("instructed to stop after running %s action reconciler %s", next.Type(), next.Name()) + conditions.Delete(req.Object, meta.ReconcilingCondition) + return nil + } + + // Append the type to the set of action types we have performed. + previous = append(previous, next.Type()) + + // Patch the release to reflect progress. + if err = patchHelper.Patch(ctx, req.Object, patch.WithOwnedConditions{Conditions: ownedConditions}); err != nil { + return err + } + } + } +} + +func (r *AtomicRelease) Name() string { + return "atomic-release" +} + +func (r *AtomicRelease) Type() ReconcilerType { + return ReconcilerTypeRelease +} + +func inStringSlice(ss []string, str string) (pos int, ok bool) { + for k, s := range ss { + if strings.EqualFold(s, str) { + return k, true + } + } + return -1, false +} + +func timeoutForAction(action ActionReconciler, obj *v2.HelmRelease) time.Duration { + switch action.(type) { + case *Install: + return obj.GetInstall().GetTimeout(obj.GetTimeout()).Duration + case *Upgrade: + return obj.GetUpgrade().GetTimeout(obj.GetTimeout()).Duration + case *Test: + return obj.GetTest().GetTimeout(obj.GetTimeout()).Duration + case *RollbackRemediation: + return obj.GetRollback().GetTimeout(obj.GetTimeout()).Duration + case *UninstallRemediation: + return obj.GetUninstall().GetTimeout(obj.GetTimeout()).Duration + default: + return obj.GetTimeout().Duration + } +} diff --git a/internal/reconcile/atomic_release_test.go b/internal/reconcile/atomic_release_test.go new file mode 100644 index 000000000..b5c4d5d95 --- /dev/null +++ b/internal/reconcile/atomic_release_test.go @@ -0,0 +1,196 @@ +/* +Copyright 2022 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 reconcile + +import ( + "context" + "testing" + "time" + + "github.com/go-logr/logr" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/testutil" +) + +func TestReleaseStrategy_CleanRelease_MustContinue(t *testing.T) { + tests := []struct { + name string + current ReconcilerType + previous ReconcilerTypeSet + want bool + }{ + { + name: "continue if not in previous", + current: ReconcilerTypeRemediate, + previous: []ReconcilerType{ + ReconcilerTypeRelease, + }, + want: true, + }, + { + name: "do not continue if in previous", + current: ReconcilerTypeRemediate, + previous: []ReconcilerType{ + ReconcilerTypeRemediate, + }, + want: false, + }, + { + name: "do continue on nil", + current: ReconcilerTypeRemediate, + previous: nil, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + at := &cleanReleaseStrategy{} + if got := at.MustContinue(tt.current, tt.previous); got != tt.want { + g := NewWithT(t) + g.Expect(got).To(Equal(tt.want)) + } + }) + } +} + +func TestReleaseStrategy_CleanRelease_MustStop(t *testing.T) { + tests := []struct { + name string + current ReconcilerType + previous ReconcilerTypeSet + want bool + }{ + { + name: "stop if current is remediate", + current: ReconcilerTypeRemediate, + want: true, + }, + { + name: "do not stop if current is not remediate", + current: ReconcilerTypeRelease, + want: false, + }, + { + name: "do not stop if current is not remediate", + current: ReconcilerTypeUnlock, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + at := &cleanReleaseStrategy{} + if got := at.MustStop(tt.current, tt.previous); got != tt.want { + g := NewWithT(t) + g.Expect(got).To(Equal(tt.want)) + } + }) + } +} + +func TestAtomicRelease_Reconcile(t *testing.T) { + t.Run("runs a series of actions", func(t *testing.T) { + g := NewWithT(t) + + namedNS, err := testEnv.CreateNamespace(context.TODO(), mockReleaseNamespace) + g.Expect(err).NotTo(HaveOccurred()) + t.Cleanup(func() { + _ = testEnv.Delete(context.TODO(), namedNS) + }) + releaseNamespace := namedNS.Name + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: mockReleaseName, + Namespace: releaseNamespace, + }, + Spec: v2.HelmReleaseSpec{ + ReleaseName: mockReleaseName, + TargetNamespace: releaseNamespace, + Test: &v2.Test{ + Enable: true, + }, + StorageNamespace: releaseNamespace, + Timeout: &metav1.Duration{Duration: 100 * time.Millisecond}, + }, + } + + getter, err := RESTClientGetterFromManager(testEnv.Manager, obj.GetReleaseNamespace()) + g.Expect(err).ToNot(HaveOccurred()) + + cfg, err := action.NewConfigFactory(getter, + action.WithStorage(action.DefaultStorageDriver, obj.GetStorageNamespace()), + action.WithDebugLog(logr.Discard()), + ) + g.Expect(err).ToNot(HaveOccurred()) + + // We use a fake client here to allow us to work with a minimal release + // object mock. As the fake client does not perform any validation. + // However, for the Helm storage driver to work, we need a real client + // which is therefore initialized separately above. + client := fake.NewClientBuilder(). + WithScheme(testEnv.Scheme()). + WithObjects(obj). + WithStatusSubresource(&v2.HelmRelease{}). + Build() + recorder := new(record.FakeRecorder) + + req := &Request{ + Object: obj, + Chart: testutil.BuildChart(testutil.ChartWithTestHook()), + Values: nil, + } + g.Expect(NewAtomicRelease(client, cfg, recorder).Reconcile(context.TODO(), req)).ToNot(HaveOccurred()) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: v2.TestSucceededReason, + Message: "test hook completed successfully", + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Installed release", + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionTrue, + Reason: v2.TestSucceededReason, + Message: "test hook completed successfully", + }, + })) + g.Expect(obj.GetCurrent()).ToNot(BeNil(), "expected current to not be nil") + g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") + + g.Expect(obj.Status.Failures).To(BeZero()) + g.Expect(obj.Status.InstallFailures).To(BeZero()) + g.Expect(obj.Status.UpgradeFailures).To(BeZero()) + + g.Expect(NextAction(context.TODO(), cfg, recorder, req)).To(And(Succeed(), BeNil())) + }) +} diff --git a/internal/reconcile/install.go b/internal/reconcile/install.go index dc8c4a328..33f0abdf1 100644 --- a/internal/reconcile/install.go +++ b/internal/reconcile/install.go @@ -18,37 +18,68 @@ package reconcile import ( "context" + "fmt" "github.com/fluxcd/pkg/runtime/logger" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" ) +// Install is an ActionReconciler which attempts to install a Helm release +// based on the given Request data. +// +// The writes to the Helm storage during the installation process are +// observed, and updates the Status.Current (and possibly Status.Previous) +// field(s). +// +// On installation success, the object is marked with Released=True and emits +// an event. In addition, the object is marked with TestSuccess=False if tests +// are enabled to indicate we are awaiting the results. +// On failure, the object is marked with Released=False and emits a warning +// event. Only an error which resulted in a modification to the Helm storage +// counts towards a failure for the active remediation strategy. +// +// At the end of the reconciliation, the Status.Conditions are summarized and +// propagated to the Ready condition on the Request.Object. +// +// The caller is assumed to have verified the integrity of Request.Object using +// e.g. action.VerifyReleaseInfo before calling Reconcile. type Install struct { configFactory *action.ConfigFactory + eventRecorder record.EventRecorder +} + +// NewInstall returns a new Install reconciler configured with the provided +// values. +func NewInstall(cfg *action.ConfigFactory, recorder record.EventRecorder) *Install { + return &Install{configFactory: cfg, eventRecorder: recorder} } func (r *Install) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.Status.Current.DeepCopy() + cur = req.Object.GetCurrent().DeepCopy() logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.InfoLevel)), 10) cfg = r.configFactory.Build(logBuf.Log, observeRelease(req.Object)) ) - // Run install action. - rls, err := action.Install(ctx, cfg, req.Object, req.Chart, req.Values) + defer summarize(req) + + // Run the Helm install action. + _, err := action.Install(ctx, cfg, req.Object, req.Chart, req.Values) if err != nil { - // Mark failure on object. - req.Object.Status.Failures++ - conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.InstallFailedReason, err.Error()) + r.failure(req, logBuf, err) // Return error if we did not store a release, as this does not // require remediation and the caller should e.g. retry. - if newCur := req.Object.Status.Current; newCur == nil || cur == newCur { + if newCur := req.Object.GetCurrent(); newCur == nil || (cur != nil && cur.Digest == newCur.Digest) { return err } @@ -58,14 +89,11 @@ func (r *Install) Reconcile(ctx context.Context, req *Request) error { // without a new release in storage there is nothing to remediate, // and the action can be retried immediately without causing // storage drift. - req.Object.Status.InstallFailures++ + req.Object.GetActiveRemediation().IncrementFailureCount(req.Object) return nil } - // Mark release success and delete any test success, as the current release - // isn't tested (yet). - conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.InstallSucceededReason, rls.Info.Description) - conditions.Delete(req.Object, v2.TestSuccessCondition) + r.success(req) return nil } @@ -76,3 +104,66 @@ func (r *Install) Name() string { func (r *Install) Type() ReconcilerType { return ReconcilerTypeRelease } + +const ( + // fmtInstallFailure is the message format for an installation failure. + fmtInstallFailure = "Install of release %s/%s with chart %s@%s failed: %s" + // fmtInstallSuccess is the message format for a successful installation. + fmtInstallSuccess = "Installed release %s with chart %s" +) + +// failure records the failure of a Helm installation action in the status of +// the given Request.Object by marking ReleasedCondition=False and increasing +// the failure counter. In addition, it emits a warning event for the +// Request.Object. +// +// Increase of the failure counter for the active remediation strategy should +// be done conditionally by the caller after verifying the failed action has +// modified the Helm storage. This to avoid counting failures which do not +// result in Helm storage drift. +func (r *Install) failure(req *Request, buffer *action.LogBuffer, err error) { + // Compose failure message. + msg := fmt.Sprintf(fmtInstallFailure, req.Object.GetReleaseNamespace(), req.Object.GetReleaseName(), req.Chart.Name(), + req.Chart.Metadata.Version, err.Error()) + + // Mark install failure on object. + req.Object.Status.Failures++ + conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.InstallFailedReason, msg) + + // Record warning event, this message contains more data than the + // Condition summary. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(req.Chart.Metadata.Version, chartutil.DigestValues(digest.Canonical, req.Values).String()), + corev1.EventTypeWarning, + v2.InstallFailedReason, + eventMessageWithLog(msg, buffer), + ) +} + +// success records the success of a Helm installation action in the status of +// the given Request.Object by marking ReleasedCondition=True and emitting an +// event. In addition, it marks TestSuccessCondition=False when tests are +// enabled to indicate we are awaiting test results after having made the +// release. +func (r *Install) success(req *Request) { + // Compose success message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtInstallSuccess, cur.FullReleaseName(), cur.VersionedChartName()) + + // Mark install success on object. + conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.InstallSucceededReason, msg) + if req.Object.GetTest().Enable && !cur.HasBeenTested() { + conditions.MarkUnknown(req.Object, v2.TestSuccessCondition, "Pending", fmtTestPending, + cur.FullReleaseName(), cur.VersionedChartName()) + } + + // Record event. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeNormal, + v2.InstallSucceededReason, + msg, + ) +} diff --git a/internal/reconcile/install_test.go b/internal/reconcile/install_test.go index e13ea62f7..dd7ec3967 100644 --- a/internal/reconcile/install_test.go +++ b/internal/reconcile/install_test.go @@ -18,6 +18,7 @@ package reconcile import ( "context" + "errors" "fmt" "testing" "time" @@ -25,17 +26,23 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/gomega" "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/chartutil" + helmchartutil "helm.sh/helm/v3/pkg/chartutil" helmrelease "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/releaseutil" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" + "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" "github.com/fluxcd/helm-controller/internal/release" "github.com/fluxcd/helm-controller/internal/storage" "github.com/fluxcd/helm-controller/internal/testutil" @@ -52,7 +59,7 @@ func TestInstall_Reconcile(t *testing.T) { // chart to install. chart *chart.Chart // values to use during install. - values chartutil.Values + values helmchartutil.Values // spec modifies the HelmRelease object spec before install. spec func(spec *v2.HelmReleaseSpec) // status to configure on the HelmRelease object before install. @@ -81,8 +88,10 @@ func TestInstall_Reconcile(t *testing.T) { name: "install success", chart: testutil.BuildChart(), expectConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, v2.InstallSucceededReason, + "Installed release"), *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, - "Install complete"), + "Installed release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[0])) @@ -92,6 +101,8 @@ func TestInstall_Reconcile(t *testing.T) { name: "install failure", chart: testutil.BuildChart(testutil.ChartWithFailingHook()), expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.InstallFailedReason, + "failed post-install"), *conditions.FalseCondition(v2.ReleasedCondition, v2.InstallFailedReason, "failed post-install"), }, @@ -112,6 +123,8 @@ func TestInstall_Reconcile(t *testing.T) { chart: testutil.BuildChart(), wantErr: fmt.Errorf("storage create error"), expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.InstallFailedReason, + "storage create error"), *conditions.FalseCondition(v2.ReleasedCondition, v2.InstallFailedReason, "storage create error"), }, @@ -143,8 +156,10 @@ func TestInstall_Reconcile(t *testing.T) { }, chart: testutil.BuildChart(), expectConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, v2.InstallSucceededReason, + "Installed release"), *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, - "Install complete"), + "Installed release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[1])) @@ -168,8 +183,10 @@ func TestInstall_Reconcile(t *testing.T) { }, chart: testutil.BuildChart(), expectConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, v2.InstallSucceededReason, + "Installed release"), *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, - "Install complete"), + "Installed release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[0])) @@ -226,7 +243,8 @@ func TestInstall_Reconcile(t *testing.T) { cfg.Driver = tt.driver(cfg.Driver) } - got := (&Install{configFactory: cfg}).Reconcile(context.TODO(), &Request{ + recorder := new(record.FakeRecorder) + got := (NewInstall(cfg, recorder)).Reconcile(context.TODO(), &Request{ Object: obj, Chart: tt.chart, Values: tt.values, @@ -243,15 +261,15 @@ func TestInstall_Reconcile(t *testing.T) { releaseutil.SortByRevision(releases) if tt.expectCurrent != nil { - g.Expect(obj.Status.Current).To(testutil.Equal(tt.expectCurrent(releases))) + g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releases))) } else { - g.Expect(obj.Status.Current).To(BeNil(), "expected current to be nil") + g.Expect(obj.GetCurrent()).To(BeNil(), "expected current to be nil") } if tt.expectPrevious != nil { - g.Expect(obj.Status.Previous).To(testutil.Equal(tt.expectPrevious(releases))) + g.Expect(obj.GetPrevious()).To(testutil.Equal(tt.expectPrevious(releases))) } else { - g.Expect(obj.Status.Previous).To(BeNil(), "expected previous to be nil") + g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -260,3 +278,143 @@ func TestInstall_Reconcile(t *testing.T) { }) } } + +func TestInstall_failure(t *testing.T) { + var ( + obj = &v2.HelmRelease{ + Spec: v2.HelmReleaseSpec{ + ReleaseName: mockReleaseName, + TargetNamespace: mockReleaseNamespace, + }, + } + chrt = testutil.BuildChart() + err = errors.New("installation error") + ) + + t.Run("records failure", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Install{ + eventRecorder: recorder, + } + + req := &Request{Object: obj.DeepCopy(), Chart: chrt, Values: map[string]interface{}{"foo": "bar"}} + r.failure(req, nil, err) + + expectMsg := fmt.Sprintf(fmtInstallFailure, mockReleaseNamespace, mockReleaseName, chrt.Name(), + chrt.Metadata.Version, err.Error()) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.ReleasedCondition, v2.InstallFailedReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(1))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: v2.InstallFailedReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): chrt.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, req.Values).String(), + }, + }, + }, + })) + }) + + t.Run("records failure with logs", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Install{ + eventRecorder: recorder, + } + req := &Request{Object: obj.DeepCopy(), Chart: chrt} + r.failure(req, mockLogBuffer(5, 10), err) + + expectSubStr := "Last Helm logs" + g.Expect(conditions.IsFalse(req.Object, v2.ReleasedCondition)).To(BeTrue()) + g.Expect(conditions.GetMessage(req.Object, v2.ReleasedCondition)).ToNot(ContainSubstring(expectSubStr)) + + events := recorder.GetEvents() + g.Expect(events).To(HaveLen(1)) + g.Expect(events[0].Message).To(ContainSubstring(expectSubStr)) + }) +} + +func TestInstall_success(t *testing.T) { + var ( + cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Chart: testutil.BuildChart(), + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + ) + + t.Run("records success", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Install{ + eventRecorder: recorder, + } + + req := &Request{ + Object: obj.DeepCopy(), + } + r.success(req) + + expectMsg := fmt.Sprintf(fmtInstallSuccess, + fmt.Sprintf("%s/%s.%d", mockReleaseNamespace, mockReleaseName, obj.Status.Current.Version), + fmt.Sprintf("%s@%s", obj.Status.Current.ChartName, obj.Status.Current.ChartVersion)) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, expectMsg), + })) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: v2.InstallSucceededReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): obj.Status.Current.ChartVersion, + eventMetaGroupKey(eventv1.MetaTokenKey): obj.Status.Current.ConfigDigest, + }, + }, + }, + })) + }) + + t.Run("records success with TestSuccess=False", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Install{ + eventRecorder: recorder, + } + + obj := obj.DeepCopy() + obj.Spec.Test = &v2.Test{Enable: true} + + req := &Request{Object: obj} + r.success(req) + + g.Expect(conditions.IsTrue(req.Object, v2.ReleasedCondition)).To(BeTrue()) + + cond := conditions.Get(req.Object, v2.TestSuccessCondition) + g.Expect(cond).ToNot(BeNil()) + + expectMsg := fmt.Sprintf(fmtTestPending, + fmt.Sprintf("%s/%s.%d", mockReleaseNamespace, mockReleaseName, obj.Status.Current.Version), + fmt.Sprintf("%s@%s", obj.Status.Current.ChartName, obj.Status.Current.ChartVersion)) + g.Expect(cond.Message).To(Equal(expectMsg)) + }) +} diff --git a/internal/reconcile/reconcile.go b/internal/reconcile/reconcile.go index 762c9204d..2565e57fc 100644 --- a/internal/reconcile/reconcile.go +++ b/internal/reconcile/reconcile.go @@ -45,6 +45,30 @@ const ( // in a single reconciliation. type ReconcilerType string +// ReconcilerTypeSet is a set of ReconcilerType. +type ReconcilerTypeSet []ReconcilerType + +// Contains returns true if the set contains the given type. +func (s ReconcilerTypeSet) Contains(t ReconcilerType) bool { + for _, r := range s { + if r == t { + return true + } + } + return false +} + +// Count returns the number of elements matching the given type. +func (s ReconcilerTypeSet) Count(t ReconcilerType) int { + count := 0 + for _, r := range s { + if r == t { + count++ + } + } + return count +} + // Request is a request to be performed by an ActionReconciler. The reconciler // writes the result of the request to the Object's status. type Request struct { diff --git a/internal/reconcile/release.go b/internal/reconcile/release.go index 94224a599..d2b04587a 100644 --- a/internal/reconcile/release.go +++ b/internal/reconcile/release.go @@ -18,10 +18,17 @@ package reconcile import ( "errors" + "sort" helmrelease "helm.sh/helm/v3/pkg/release" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" "github.com/fluxcd/helm-controller/internal/release" "github.com/fluxcd/helm-controller/internal/storage" ) @@ -46,20 +53,173 @@ var ( // the Helm storage. func observeRelease(obj *v2.HelmRelease) storage.ObserveFunc { return func(rls *helmrelease.Release) { - cur := obj.Status.Current.DeepCopy() + cur := obj.GetCurrent().DeepCopy() obs := release.ObserveRelease(rls) if cur != nil && obs.Targets(cur.Name, cur.Namespace, 0) && cur.Version < obs.Version { // Add current to previous when we observe the first write of a // newer release. - obj.Status.Previous = obj.Status.Current + obj.Status.Previous = obj.GetCurrent() } if cur == nil || !obs.Targets(cur.Name, cur.Namespace, 0) || obs.Version >= cur.Version { // Overwrite current with newer release, or update it. obj.Status.Current = release.ObservedToInfo(obs) } - if prev := obj.Status.Previous; prev != nil && obs.Targets(prev.Name, prev.Namespace, prev.Version) { + if prev := obj.GetPrevious(); prev != nil && obs.Targets(prev.Name, prev.Namespace, prev.Version) { // Write latest state of previous (e.g. status updates) to status. obj.Status.Previous = release.ObservedToInfo(obs) } } } + +// summarize composes a Ready condition out of the Remediated, TestSuccess and +// Released conditions of the given Request.Object, and sets it on the object. +// +// The composition is made by sorting them by highest generation and priority +// of the summary conditions, taking the first result. +// +// Not taking the generation of the object itself into account ensures that if +// the change in generation of the resource does not result in a release, the +// Ready condition is still reflected for the current generation based on a +// release made for the previous generation. +// +// It takes the current specification of the object into account, and deals +// with the conditional handling of TestSuccess. Deleting the condition when +// tests are not enabled, and excluding it when failures must be ignored. +// +// If Ready=True, any Stalled condition is removed. +func summarize(req *Request) { + var sumConds = []string{v2.RemediatedCondition, v2.ReleasedCondition} + if req.Object.GetTest().Enable && !req.Object.GetTest().IgnoreFailures { + sumConds = []string{v2.RemediatedCondition, v2.TestSuccessCondition, v2.ReleasedCondition} + } + + // Remove any stale TestSuccess condition as soon as tests are disabled. + if !req.Object.GetTest().Enable { + conditions.Delete(req.Object, v2.TestSuccessCondition) + } + + // Remove any stale Remediation observation as soon as the release is + // Released and (optionally) has TestSuccess. + conditionallyDeleteRemediated(req) + + conds := req.Object.Status.Conditions + if len(conds) == 0 { + // Nothing to summarize if there are no conditions. + return + } + + sort.SliceStable(conds, func(i, j int) bool { + iPos, ok := inStringSlice(sumConds, conds[i].Type) + if !ok { + return false + } + + jPos, ok := inStringSlice(sumConds, conds[j].Type) + if !ok { + return true + } + + return (conds[i].ObservedGeneration >= conds[j].ObservedGeneration) && (iPos < jPos) + }) + + status := conds[0].Status + // Unknown is considered False within the context of Readiness. + if status == metav1.ConditionUnknown { + status = metav1.ConditionFalse + } + + // Any remediated state is considered an error. + if conds[0].Type == v2.RemediatedCondition { + status = metav1.ConditionFalse + } + + if status == metav1.ConditionTrue { + conditions.Delete(req.Object, meta.StalledCondition) + } + + conditions.Set(req.Object, &metav1.Condition{ + Type: meta.ReadyCondition, + Status: status, + Reason: conds[0].Reason, + Message: conds[0].Message, + ObservedGeneration: req.Object.Generation, + }) +} + +// conditionallyDeleteRemediated removes the Remediated condition if the +// release is Released and (optionally) has TestSuccess. But only if +// the observed generation of these conditions is equal or higher than +// the generation of the Remediated condition. +func conditionallyDeleteRemediated(req *Request) { + remediated := conditions.Get(req.Object, v2.RemediatedCondition) + if remediated == nil { + // If the object is not marked as Remediated, there is nothing to + // remove. + return + } + + released := conditions.Get(req.Object, v2.ReleasedCondition) + if released == nil || released.Status != metav1.ConditionTrue { + // If the release is not marked as Released, we must still be + // Remediated. + return + } + + if !req.Object.GetTest().Enable || req.Object.GetTest().IgnoreFailures { + // If tests are not enabled, or failures are ignored, and the + // generation is equal or higher than the generation of the + // Remediated condition, we are not in a Remediated state anymore. + if released.Status == metav1.ConditionTrue && released.ObservedGeneration >= remediated.ObservedGeneration { + conditions.Delete(req.Object, v2.RemediatedCondition) + } + return + } + + testSuccess := conditions.Get(req.Object, v2.TestSuccessCondition) + if testSuccess == nil || testSuccess.Status != metav1.ConditionTrue { + // If the release is not marked as TestSuccess, we must still be + // Remediated. + return + } + + if testSuccess.Status == metav1.ConditionTrue && testSuccess.ObservedGeneration >= remediated.ObservedGeneration { + // If the release is marked as TestSuccess, and the generation of + // the TestSuccess condition is equal or higher than the generation + // of the Remediated condition, we are not in a Remediated state. + conditions.Delete(req.Object, v2.RemediatedCondition) + return + } + + return +} + +// eventMessageWithLog returns an event message composed out of the given +// message and any log messages by appending them to the message. +func eventMessageWithLog(msg string, log *action.LogBuffer) string { + if log != nil && log.Len() > 0 { + msg = msg + "\n\nLast Helm logs:\n\n" + log.String() + } + return msg +} + +// eventMeta returns the event (annotation) metadata based on the given +// parameters. +func eventMeta(revision, token string) map[string]string { + var metadata map[string]string + if revision != "" || token != "" { + metadata = make(map[string]string) + if revision != "" { + metadata[eventMetaGroupKey(eventv1.MetaRevisionKey)] = revision + } + if token != "" { + metadata[eventMetaGroupKey(eventv1.MetaTokenKey)] = token + } + } + return metadata +} + +// eventMetaGroupKey returns the event (annotation) metadata key prefixed with +// the group. +func eventMetaGroupKey(key string) string { + return v2.GroupVersion.Group + "/" + key +} diff --git a/internal/reconcile/release_test.go b/internal/reconcile/release_test.go index 1cccc5f0c..1a11916e0 100644 --- a/internal/reconcile/release_test.go +++ b/internal/reconcile/release_test.go @@ -19,10 +19,16 @@ package reconcile import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" helmrelease "helm.sh/helm/v3/pkg/release" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" "github.com/fluxcd/helm-controller/internal/release" ) @@ -51,9 +57,9 @@ func Test_observeRelease(t *testing.T) { observeRelease(obj)(mock) - g.Expect(obj.Status.Previous).To(BeNil()) - g.Expect(obj.Status.Current).ToNot(BeNil()) - g.Expect(obj.Status.Current).To(Equal(expect)) + g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.GetCurrent()).ToNot(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) }) t.Run("release with current", func(t *testing.T) { @@ -78,10 +84,10 @@ func Test_observeRelease(t *testing.T) { expect := release.ObservedToInfo(release.ObserveRelease(mock)) observeRelease(obj)(mock) - g.Expect(obj.Status.Previous).ToNot(BeNil()) - g.Expect(obj.Status.Previous).To(Equal(current)) - g.Expect(obj.Status.Current).ToNot(BeNil()) - g.Expect(obj.Status.Current).To(Equal(expect)) + g.Expect(obj.GetPrevious()).ToNot(BeNil()) + g.Expect(obj.GetPrevious()).To(Equal(current)) + g.Expect(obj.GetCurrent()).ToNot(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) }) t.Run("release with current with different name", func(t *testing.T) { @@ -106,9 +112,9 @@ func Test_observeRelease(t *testing.T) { expect := release.ObservedToInfo(release.ObserveRelease(mock)) observeRelease(obj)(mock) - g.Expect(obj.Status.Previous).To(BeNil()) - g.Expect(obj.Status.Current).ToNot(BeNil()) - g.Expect(obj.Status.Current).To(Equal(expect)) + g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.GetCurrent()).ToNot(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) }) t.Run("release with update to previous", func(t *testing.T) { @@ -141,8 +147,621 @@ func Test_observeRelease(t *testing.T) { expect := release.ObservedToInfo(release.ObserveRelease(mock)) observeRelease(obj)(mock) - g.Expect(obj.Status.Previous).ToNot(BeNil()) - g.Expect(obj.Status.Previous).To(Equal(expect)) - g.Expect(obj.Status.Current).To(Equal(current)) + g.Expect(obj.GetPrevious()).ToNot(BeNil()) + g.Expect(obj.GetPrevious()).To(Equal(expect)) + g.Expect(obj.GetCurrent()).To(Equal(current)) }) } + +func Test_summarize(t *testing.T) { + tests := []struct { + name string + generation int64 + spec *v2.HelmReleaseSpec + conditions []metav1.Condition + expect []metav1.Condition + }{ + { + name: "summarize conditions", + generation: 1, + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionFalse, + Reason: v2.TestFailedReason, + Message: "test hook(s) failure", + ObservedGeneration: 1, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + }, + }, + { + name: "with tests enabled", + generation: 1, + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionTrue, + Reason: v2.TestSucceededReason, + Message: "test hook(s) succeeded", + ObservedGeneration: 1, + }, + }, + spec: &v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: v2.TestSucceededReason, + Message: "test hook(s) succeeded", + ObservedGeneration: 1, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionTrue, + Reason: v2.TestSucceededReason, + Message: "test hook(s) succeeded", + ObservedGeneration: 1, + }, + }, + }, + { + name: "with tests enabled and failure tests", + generation: 1, + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionFalse, + Reason: v2.TestFailedReason, + Message: "test hook(s) failure", + ObservedGeneration: 1, + }, + }, + spec: &v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionFalse, + Reason: v2.TestFailedReason, + Message: "test hook(s) failure", + ObservedGeneration: 1, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionFalse, + Reason: v2.TestFailedReason, + Message: "test hook(s) failure", + ObservedGeneration: 1, + }, + }, + }, + { + name: "with test hooks enabled and pending tests", + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionUnknown, + Reason: "Pending", + Message: "Release is awaiting tests", + ObservedGeneration: 1, + }, + }, + spec: &v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionFalse, + Reason: "Pending", + Message: "Release is awaiting tests", + ObservedGeneration: 1, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionUnknown, + Reason: "Pending", + Message: "Release is awaiting tests", + ObservedGeneration: 1, + }, + }, + }, + { + name: "with remediation failure", + generation: 1, + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionFalse, + Reason: v2.TestFailedReason, + Message: "test hook(s) failure", + ObservedGeneration: 1, + }, + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionFalse, + Reason: v2.UninstallFailedReason, + Message: "Uninstall failure", + ObservedGeneration: 1, + }, + }, + spec: &v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionFalse, + Reason: v2.UninstallFailedReason, + Message: "Uninstall failure", + ObservedGeneration: 1, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.InstallSucceededReason, + Message: "Install complete", + ObservedGeneration: 1, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionFalse, + Reason: v2.TestFailedReason, + Message: "test hook(s) failure", + ObservedGeneration: 1, + }, + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionFalse, + Reason: v2.UninstallFailedReason, + Message: "Uninstall failure", + ObservedGeneration: 1, + }, + }, + }, + { + name: "with remediation success", + generation: 1, + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionFalse, + Reason: v2.UpgradeFailedReason, + Message: "Upgrade failure", + ObservedGeneration: 1, + }, + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "Uninstall complete", + ObservedGeneration: 1, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionFalse, + Reason: v2.RollbackSucceededReason, + Message: "Uninstall complete", + ObservedGeneration: 1, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionFalse, + Reason: v2.UpgradeFailedReason, + Message: "Upgrade failure", + ObservedGeneration: 1, + }, + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "Uninstall complete", + ObservedGeneration: 1, + }, + }, + }, + { + name: "with stale ready", + generation: 1, + conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionFalse, + Reason: "ChartNotFound", + Message: "chart not found", + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "Upgrade finished", + ObservedGeneration: 1, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "Upgrade finished", + ObservedGeneration: 1, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "Upgrade finished", + ObservedGeneration: 1, + }, + }, + }, + { + name: "with stale observed generation", + generation: 5, + spec: &v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + conditions: []metav1.Condition{ + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "Upgrade finished", + ObservedGeneration: 4, + }, + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "Rollback finished", + ObservedGeneration: 3, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionFalse, + Reason: v2.TestFailedReason, + Message: "test hook(s) failure", + ObservedGeneration: 2, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "Upgrade finished", + ObservedGeneration: 5, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "Upgrade finished", + ObservedGeneration: 4, + }, + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "Rollback finished", + ObservedGeneration: 3, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionFalse, + Reason: v2.TestFailedReason, + Message: "test hook(s) failure", + ObservedGeneration: 2, + }, + }, + }, + { + name: "with stale remediation", + spec: &v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + conditions: []metav1.Condition{ + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "Rollback finished", + ObservedGeneration: 2, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "Upgrade finished", + ObservedGeneration: 2, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionTrue, + Reason: v2.TestSucceededReason, + Message: "test hooks succeeded", + ObservedGeneration: 2, + }, + }, + expect: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: v2.TestSucceededReason, + Message: "test hooks succeeded", + ObservedGeneration: 2, + }, + { + Type: v2.ReleasedCondition, + Status: metav1.ConditionTrue, + Reason: v2.UpgradeSucceededReason, + Message: "Upgrade finished", + ObservedGeneration: 2, + }, + { + Type: v2.TestSuccessCondition, + Status: metav1.ConditionTrue, + Reason: v2.TestSucceededReason, + Message: "test hooks succeeded", + ObservedGeneration: 2, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Generation: tt.generation, + }, + Status: v2.HelmReleaseStatus{ + Conditions: tt.conditions, + }, + } + if tt.spec != nil { + obj.Spec = *tt.spec.DeepCopy() + } + summarize(&Request{Object: obj}) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.expect)) + }) + } +} + +func Test_conditionallyDeleteRemediated(t *testing.T) { + tests := []struct { + name string + spec v2.HelmReleaseSpec + conditions []metav1.Condition + expectDelete bool + }{ + { + name: "no Remediated condition", + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, "Install finished"), + }, + expectDelete: false, + }, + { + name: "no Released condition", + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "Rollback finished"), + }, + expectDelete: false, + }, + { + name: "Released=True without tests enabled", + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "Rollback finished"), + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Upgrade finished"), + }, + expectDelete: true, + }, + { + name: "Stale Released=True with newer Remediated", + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Upgrade finished"), + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "Rollback finished", + ObservedGeneration: 2, + }, + }, + expectDelete: false, + }, + { + name: "Released=False", + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "Rollback finished"), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, "Upgrade failed"), + }, + expectDelete: false, + }, + { + name: "TestSuccess=True with tests enabled", + spec: v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "Rollback finished"), + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Upgrade finished"), + *conditions.TrueCondition(v2.TestSuccessCondition, v2.TestSucceededReason, "Test hooks succeeded"), + }, + expectDelete: true, + }, + { + name: "TestSuccess=False with tests enabled", + spec: v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "Rollback finished"), + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Upgrade finished"), + *conditions.FalseCondition(v2.TestSuccessCondition, v2.TestSucceededReason, "Test hooks succeeded"), + }, + expectDelete: false, + }, + { + name: "TestSuccess=False with tests ignored", + spec: v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + IgnoreFailures: true, + }, + }, + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "Rollback finished"), + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Upgrade finished"), + *conditions.FalseCondition(v2.TestSuccessCondition, v2.TestFailedReason, "Test hooks failed"), + }, + expectDelete: true, + }, + { + name: "Stale TestSuccess=True with newer Remediated", + spec: v2.HelmReleaseSpec{ + Test: &v2.Test{ + Enable: true, + }, + }, + conditions: []metav1.Condition{ + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Upgrade finished"), + *conditions.TrueCondition(v2.TestSuccessCondition, v2.TestSucceededReason, "Test hooks succeeded"), + { + Type: v2.RemediatedCondition, + Status: metav1.ConditionTrue, + Reason: v2.RollbackSucceededReason, + Message: "Rollback finished", + ObservedGeneration: 2, + }, + }, + expectDelete: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{ + Spec: tt.spec, + Status: v2.HelmReleaseStatus{ + Conditions: tt.conditions, + }, + } + isRemediated := conditions.Has(obj, v2.RemediatedCondition) + + conditionallyDeleteRemediated(&Request{Object: obj}) + + if tt.expectDelete { + g.Expect(isRemediated).ToNot(Equal(conditions.Has(obj, v2.RemediatedCondition))) + return + } + + g.Expect(conditions.Has(obj, v2.RemediatedCondition)).To(Equal(isRemediated)) + }) + } +} + +func mockLogBuffer(size int, lines int) *action.LogBuffer { + log := action.NewLogBuffer(action.NewDebugLog(logr.Discard()), size) + for i := 0; i < lines; i++ { + log.Log("line %d", i+1) + } + return log +} diff --git a/internal/reconcile/rollback.go b/internal/reconcile/rollback.go deleted file mode 100644 index 319f80d7a..000000000 --- a/internal/reconcile/rollback.go +++ /dev/null @@ -1,94 +0,0 @@ -/* -Copyright 2022 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 reconcile - -import ( - "context" - "fmt" - - helmrelease "helm.sh/helm/v3/pkg/release" - ctrl "sigs.k8s.io/controller-runtime" - - "github.com/fluxcd/pkg/runtime/conditions" - "github.com/fluxcd/pkg/runtime/logger" - - v2 "github.com/fluxcd/helm-controller/api/v2beta2" - "github.com/fluxcd/helm-controller/internal/action" - "github.com/fluxcd/helm-controller/internal/release" - "github.com/fluxcd/helm-controller/internal/storage" -) - -type Rollback struct { - configFactory *action.ConfigFactory -} - -func (r *Rollback) Name() string { - return "rollback" -} - -func (r *Rollback) Type() ReconcilerType { - return ReconcilerTypeRemediate -} - -func (r *Rollback) Reconcile(ctx context.Context, req *Request) error { - var ( - cur = req.Object.Status.Current.DeepCopy() - logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.InfoLevel)), 10) - ) - - // Previous is required to determine what version to roll back to. - if req.Object.Status.Previous == nil { - return fmt.Errorf("%w: required to rollback", ErrNoPrevious) - } - - // Run rollback action. - if err := action.Rollback(r.configFactory.Build(logBuf.Log, observeRollback(req.Object)), req.Object); err != nil { - // Mark failure on object. - req.Object.Status.Failures++ - conditions.MarkFalse(req.Object, v2.RemediatedCondition, v2.RollbackFailedReason, err.Error()) - - // Return error if we did not store a release, as this does not - // affect state and the caller should e.g. retry. - if newCur := req.Object.Status.Current; newCur == nil || newCur == cur { - return err - } - return nil - } - - // Mark remediation success. - condMsg := "Rolled back to previous version" - if prev := req.Object.Status.Previous; prev != nil { - condMsg = fmt.Sprintf("Rolled back to version %d", prev.Version) - } - conditions.MarkTrue(req.Object, v2.RemediatedCondition, v2.RollbackSucceededReason, condMsg) - return nil -} - -// observeRollback returns a storage.ObserveFunc that can be used to observe -// and record the result of a rollback action in the status of the given release. -// It updates the Status.Current field of the release if it equals the target -// of the rollback action, and version >= Current.Version. -func observeRollback(obj *v2.HelmRelease) storage.ObserveFunc { - return func(rls *helmrelease.Release) { - cur := obj.Status.Current.DeepCopy() - obs := release.ObserveRelease(rls) - if cur == nil || !obs.Targets(cur.Name, cur.Namespace, 0) || obs.Version >= cur.Version { - // Overwrite current with newer release, or update it. - obj.Status.Current = release.ObservedToInfo(obs) - } - } -} diff --git a/internal/reconcile/rollback_remediation.go b/internal/reconcile/rollback_remediation.go new file mode 100644 index 000000000..d73781594 --- /dev/null +++ b/internal/reconcile/rollback_remediation.go @@ -0,0 +1,189 @@ +/* +Copyright 2022 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 reconcile + +import ( + "context" + "fmt" + + helmrelease "helm.sh/helm/v3/pkg/release" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/fluxcd/pkg/runtime/conditions" + "github.com/fluxcd/pkg/runtime/logger" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" + "github.com/fluxcd/helm-controller/internal/release" + "github.com/fluxcd/helm-controller/internal/storage" +) + +// RollbackRemediation is an ActionReconciler which attempts to roll back +// a Request.Object to the version specified in Status.Previous. +// +// The writes to the Helm storage during the rollback are observed, and update +// the Status.Current field. On an upgrade reattempt, the UpgradeReconciler +// will move the Status.Current field to Status.Previous, effectively updating +// the previous version to the version which was rolled back to. Ensuring +// repetitive failures continue to be able to roll back to a version existing +// in the storage when the history is pruned. +// +// After a successful rollback, the object is marked with Remediated=True and +// an event is emitted. When the rollback fails, the object is marked with +// Remediated=False and a warning event is emitted. +// +// When the Request.Object does not have a Status.Previous, it returns an +// error of type ErrNoPrevious. In addition, it returns ErrReleaseMismatch +// if the name and/or namespace of Status.Current and Status.Previous point +// towards a different release. Any other returned error indicates the caller +// should retry as it did not cause a change to the Helm storage. +// +// At the end of the reconciliation, the Status.Conditions are summarized and +// propagated to the Ready condition on the Request.Object. +// +// The caller is assumed to have verified the integrity of Request.Object using +// e.g. action.VerifyReleaseInfo before calling Reconcile. +type RollbackRemediation struct { + configFactory *action.ConfigFactory + eventRecorder record.EventRecorder +} + +// NewRollbackRemediation returns a new RollbackRemediation reconciler +// configured with the provided values. +func NewRollbackRemediation(configFactory *action.ConfigFactory, eventRecorder record.EventRecorder) *RollbackRemediation { + return &RollbackRemediation{ + configFactory: configFactory, + eventRecorder: eventRecorder, + } +} + +func (r *RollbackRemediation) Reconcile(ctx context.Context, req *Request) error { + var ( + cur = req.Object.GetCurrent().DeepCopy() + logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.InfoLevel)), 10) + cfg = r.configFactory.Build(logBuf.Log, observeRollback(req.Object)) + ) + + defer summarize(req) + + // Previous is required to determine what version to roll back to. + if !req.Object.HasPrevious() { + return fmt.Errorf("%w: required to rollback", ErrNoPrevious) + } + + // Confirm previous and current point to the same release. + prev := req.Object.GetPrevious() + if prev.Name != cur.Name || prev.Namespace != cur.Namespace { + return fmt.Errorf("%w: previous release name or namespace %s does not match current %s", + ErrReleaseMismatch, prev.FullReleaseName(), cur.FullReleaseName()) + } + + // Run the Helm rollback action. + if err := action.Rollback(cfg, req.Object, prev.Name, action.RollbackToVersion(prev.Version)); err != nil { + r.failure(req, logBuf, err) + + // Return error if we did not store a release, as this does not + // affect state and the caller should e.g. retry. + if newCur := req.Object.GetCurrent(); newCur == nil || (cur != nil && newCur.Digest == cur.Digest) { + return err + } + + return nil + } + + r.success(req) + return nil +} + +func (r *RollbackRemediation) Name() string { + return "rollback" +} + +func (r *RollbackRemediation) Type() ReconcilerType { + return ReconcilerTypeRemediate +} + +const ( + // fmtRollbackRemediationFailure is the message format for a rollback + // remediation failure. + fmtRollbackRemediationFailure = "Rollback to previous release %s with chart %s failed: %s" + // fmtRollbackRemediationSuccess is the message format for a successful + // rollback remediation. + fmtRollbackRemediationSuccess = "Rolled back to previous release %s with chart %s" +) + +// failure records the failure of a Helm rollback action in the status of the +// given Request.Object by marking Remediated=False and emitting a warning +// event. +func (r *RollbackRemediation) failure(req *Request, buffer *action.LogBuffer, err error) { + // Compose failure message. + prev := req.Object.GetPrevious() + msg := fmt.Sprintf(fmtRollbackRemediationFailure, prev.FullReleaseName(), prev.VersionedChartName(), err.Error()) + + // Mark remediation failure on object. + req.Object.Status.Failures++ + conditions.MarkFalse(req.Object, v2.RemediatedCondition, v2.RollbackFailedReason, msg) + + // Record warning event, this message contains more data than the + // Condition summary. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(prev.ChartVersion, chartutil.DigestValues(digest.Canonical, req.Values).String()), + corev1.EventTypeWarning, + v2.RollbackFailedReason, + eventMessageWithLog(msg, buffer), + ) +} + +// success records the success of a Helm rollback action in the status of the +// given Request.Object by marking Remediated=True and emitting an event. +func (r *RollbackRemediation) success(req *Request) { + // Compose success message. + prev := req.Object.GetPrevious() + msg := fmt.Sprintf(fmtRollbackRemediationSuccess, prev.FullReleaseName(), prev.VersionedChartName()) + + // Mark remediation success on object. + conditions.MarkTrue(req.Object, v2.RemediatedCondition, v2.RollbackSucceededReason, msg) + + // Record event. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(prev.ChartVersion, chartutil.DigestValues(digest.Canonical, req.Values).String()), + corev1.EventTypeNormal, + v2.RollbackSucceededReason, + msg, + ) +} + +// observeRollback returns a storage.ObserveFunc that can be used to observe +// and record the result of a rollback action in the status of the given release. +// It updates the Status.Current field of the release if it equals the target +// of the rollback action, and version >= Current.Version. +func observeRollback(obj *v2.HelmRelease) storage.ObserveFunc { + return func(rls *helmrelease.Release) { + cur := obj.GetCurrent().DeepCopy() + obs := release.ObserveRelease(rls) + if cur == nil || !obs.Targets(cur.Name, cur.Namespace, 0) || obs.Version >= cur.Version { + // Overwrite current with newer release, or update it. + obj.Status.Current = release.ObservedToInfo(obs) + } + } +} diff --git a/internal/reconcile/rollback_test.go b/internal/reconcile/rollback_remediation_test.go similarity index 55% rename from internal/reconcile/rollback_test.go rename to internal/reconcile/rollback_remediation_test.go index cc5725f55..f346e52d5 100644 --- a/internal/reconcile/rollback_test.go +++ b/internal/reconcile/rollback_remediation_test.go @@ -19,6 +19,8 @@ package reconcile import ( "context" "errors" + "fmt" + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" "testing" "time" @@ -28,17 +30,28 @@ import ( helmreleaseutil "helm.sh/helm/v3/pkg/releaseutil" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" "github.com/fluxcd/helm-controller/internal/release" + "github.com/fluxcd/helm-controller/internal/storage" "github.com/fluxcd/helm-controller/internal/testutil" ) -func TestRollback_Reconcile(t *testing.T) { +func TestRollbackRemediation_Reconcile(t *testing.T) { + var ( + mockCreateErr = fmt.Errorf("storage create error") + mockUpdateErr = fmt.Errorf("storage update error") + ) + tests := []struct { name string // driver allows for modifying the Helm storage driver. @@ -97,8 +110,8 @@ func TestRollback_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{ - *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, - "Rolled back to version 1"), + *conditions.FalseCondition(meta.ReadyCondition, v2.RollbackSucceededReason, "Rolled back to"), + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "Rolled back to"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[2])) @@ -164,6 +177,8 @@ func TestRollback_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.RollbackFailedReason, + "timed out waiting for the condition"), *conditions.FalseCondition(v2.RemediatedCondition, v2.RollbackFailedReason, "timed out waiting for the condition"), }, @@ -175,6 +190,99 @@ func TestRollback_Reconcile(t *testing.T) { }, expectFailures: 1, }, + { + name: "rollback with storage create error", + driver: func(driver helmdriver.Driver) helmdriver.Driver { + return &storage.Failing{ + Driver: driver, + CreateErr: mockCreateErr, + } + }, + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + Namespace: namespace, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusFailed, + Namespace: namespace, + }), + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[1])), + Previous: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + wantErr: mockCreateErr, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.RollbackFailedReason, + mockCreateErr.Error()), + *conditions.FalseCondition(v2.RemediatedCondition, v2.RollbackFailedReason, + mockCreateErr.Error()), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[1])) + }, + expectPrevious: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + expectFailures: 1, + }, + { + name: "rollback with storage update error", + driver: func(driver helmdriver.Driver) helmdriver.Driver { + return &storage.Failing{ + Driver: driver, + UpdateErr: mockUpdateErr, + } + }, + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + Namespace: namespace, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusFailed, + Namespace: namespace, + }), + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[1])), + Previous: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.RollbackFailedReason, + "storage update error"), + *conditions.FalseCondition(v2.RemediatedCondition, v2.RollbackFailedReason, + "storage update error"), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[2])) + }, + expectPrevious: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + expectFailures: 1, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -223,7 +331,8 @@ func TestRollback_Reconcile(t *testing.T) { cfg.Driver = tt.driver(cfg.Driver) } - got := (&Rollback{configFactory: cfg}).Reconcile(context.TODO(), &Request{ + recorder := new(record.FakeRecorder) + got := (NewRollbackRemediation(cfg, recorder)).Reconcile(context.TODO(), &Request{ Object: obj, }) if tt.wantErr != nil { @@ -238,15 +347,15 @@ func TestRollback_Reconcile(t *testing.T) { helmreleaseutil.SortByRevision(releases) if tt.expectCurrent != nil { - g.Expect(obj.Status.Current).To(testutil.Equal(tt.expectCurrent(releases))) + g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releases))) } else { - g.Expect(obj.Status.Current).To(BeNil(), "expected current to be nil") + g.Expect(obj.GetCurrent()).To(BeNil(), "expected current to be nil") } if tt.expectPrevious != nil { - g.Expect(obj.Status.Previous).To(testutil.Equal(tt.expectPrevious(releases))) + g.Expect(obj.GetPrevious()).To(testutil.Equal(tt.expectPrevious(releases))) } else { - g.Expect(obj.Status.Previous).To(BeNil(), "expected previous to be nil") + g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -256,6 +365,122 @@ func TestRollback_Reconcile(t *testing.T) { } } +func TestRollbackRemediation_failure(t *testing.T) { + var ( + prev = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Chart: testutil.BuildChart(), + Version: 4, + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Previous: release.ObservedToInfo(release.ObserveRelease(prev)), + }, + } + err = errors.New("rollback error") + ) + + t.Run("records failure", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &RollbackRemediation{ + eventRecorder: recorder, + } + + req := &Request{Object: obj.DeepCopy()} + r.failure(req, nil, err) + + expectMsg := fmt.Sprintf(fmtRollbackRemediationFailure, + fmt.Sprintf("%s/%s.%d", prev.Namespace, prev.Name, prev.Version), + fmt.Sprintf("%s@%s", prev.Chart.Name(), prev.Chart.Metadata.Version), + err.Error()) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.RemediatedCondition, v2.RollbackFailedReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(1))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: v2.RollbackFailedReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): prev.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, req.Values).String(), + }, + }, + }, + })) + }) + + t.Run("records failure with logs", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &RollbackRemediation{ + eventRecorder: recorder, + } + req := &Request{Object: obj.DeepCopy()} + r.failure(req, mockLogBuffer(5, 10), err) + + expectSubStr := "Last Helm logs" + g.Expect(conditions.IsFalse(req.Object, v2.RemediatedCondition)).To(BeTrue()) + g.Expect(conditions.GetMessage(req.Object, v2.RemediatedCondition)).ToNot(ContainSubstring(expectSubStr)) + + events := recorder.GetEvents() + g.Expect(events).To(HaveLen(1)) + g.Expect(events[0].Message).To(ContainSubstring(expectSubStr)) + }) +} + +func TestRollbackRemediation_success(t *testing.T) { + g := NewWithT(t) + + var prev = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Chart: testutil.BuildChart(), + Version: 4, + }) + + recorder := testutil.NewFakeRecorder(10, false) + r := &RollbackRemediation{ + eventRecorder: recorder, + } + + obj := &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Previous: release.ObservedToInfo(release.ObserveRelease(prev)), + }, + } + + req := &Request{Object: obj, Values: map[string]interface{}{"foo": "bar"}} + r.success(req) + + expectMsg := fmt.Sprintf(fmtRollbackRemediationSuccess, + fmt.Sprintf("%s/%s.%d", prev.Namespace, prev.Name, prev.Version), + fmt.Sprintf("%s@%s", prev.Chart.Name(), prev.Chart.Metadata.Version)) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(0))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: v2.RollbackSucceededReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): prev.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, req.Values).String(), + }, + }, + }, + })) +} + func Test_observeRollback(t *testing.T) { t.Run("rollback", func(t *testing.T) { g := NewWithT(t) @@ -270,8 +495,8 @@ func Test_observeRollback(t *testing.T) { observeRollback(obj)(rls) expect := release.ObservedToInfo(release.ObserveRelease(rls)) - g.Expect(obj.Status.Previous).To(BeNil()) - g.Expect(obj.Status.Current).To(Equal(expect)) + g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) }) t.Run("rollback with current", func(t *testing.T) { @@ -297,9 +522,9 @@ func Test_observeRollback(t *testing.T) { expect := release.ObservedToInfo(release.ObserveRelease(rls)) observeRollback(obj)(rls) - g.Expect(obj.Status.Current).ToNot(BeNil()) - g.Expect(obj.Status.Current).To(Equal(expect)) - g.Expect(obj.Status.Previous).To(BeNil()) + g.Expect(obj.GetCurrent()).ToNot(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) + g.Expect(obj.GetPrevious()).To(BeNil()) }) t.Run("rollback with current with higher version", func(t *testing.T) { @@ -324,8 +549,8 @@ func Test_observeRollback(t *testing.T) { }) observeRollback(obj)(rls) - g.Expect(obj.Status.Previous).To(BeNil()) - g.Expect(obj.Status.Current).To(Equal(current)) + g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(current)) }) t.Run("rollback with current with different name", func(t *testing.T) { @@ -351,7 +576,7 @@ func Test_observeRollback(t *testing.T) { expect := release.ObservedToInfo(release.ObserveRelease(rls)) observeRollback(obj)(rls) - g.Expect(obj.Status.Previous).To(BeNil()) - g.Expect(obj.Status.Current).To(Equal(expect)) + g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) }) } diff --git a/internal/reconcile/suite_test.go b/internal/reconcile/suite_test.go index f2dc80b39..d792bd301 100644 --- a/internal/reconcile/suite_test.go +++ b/internal/reconcile/suite_test.go @@ -21,12 +21,12 @@ import ( "os" "testing" - "github.com/fluxcd/pkg/runtime/testenv" + "helm.sh/helm/v3/pkg/kube" "k8s.io/apimachinery/pkg/api/meta" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/discovery" - cached "k8s.io/client-go/discovery/cached" + "k8s.io/client-go/discovery/cached/memory" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" @@ -34,6 +34,8 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/manager" + "github.com/fluxcd/pkg/runtime/testenv" + v2 "github.com/fluxcd/helm-controller/api/v2beta2" ) @@ -55,6 +57,9 @@ func TestMain(m *testing.M) { }() <-testEnv.Manager.Elected() + // Globally configure field manager for all tests. + kube.ManagedFieldsManager = "reconciler-tests" + code := m.Run() fmt.Println("Stopping the test environment") @@ -77,9 +82,8 @@ func RESTClientGetterFromManager(mgr manager.Manager, ns string) (genericcliopti if err != nil { return nil, err } - cdc := cached.NewMemCacheClient(dc) + cdc := memory.NewMemCacheClient(dc) rm := mgr.GetRESTMapper() - return &managerRESTClientGetter{ restConfig: cfg, discoveryClient: cdc, diff --git a/internal/reconcile/test.go b/internal/reconcile/test.go index ed6b556a2..b4a4e4cd6 100644 --- a/internal/reconcile/test.go +++ b/internal/reconcile/test.go @@ -22,6 +22,8 @@ import ( "github.com/fluxcd/pkg/runtime/logger" helmrelease "helm.sh/helm/v3/pkg/release" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "github.com/fluxcd/pkg/runtime/conditions" @@ -32,50 +34,79 @@ import ( "github.com/fluxcd/helm-controller/internal/storage" ) +// Test is an ActionReconciler which attempts to perform a Helm test for +// the Status.Current release of the Request.Object. +// +// The writes to the Helm storage during testing are observed, which causes the +// Status.Current.TestHooks field of the object to be updated with the results +// when the action targets the same release as current. +// +// When all test hooks for the release succeed, the object is marked with +// TestSuccess=True and an event is emitted. When one of the test hooks fails, +// Helm stops running the remaining tests, and the object is marked with +// TestSuccess=False and a warning event is emitted. If test failures are not +// ignored, the failure count for the active remediation strategy is +// incremented. +// +// When the Request.Object does not have a Status.Current, it returns an +// error of type ErrNoCurrent. In addition, it returns ErrReleaseMismatch +// if the test ran for a different release target than Status.Current. +// Any other returned error indicates the caller should retry as it did not cause +// a change to the Helm storage. +// +// At the end of the reconciliation, the Status.Conditions are summarized and +// propagated to the Ready condition on the Request.Object. +// +// The caller is assumed to have verified the integrity of Request.Object using +// e.g. action.VerifyReleaseInfo before calling Reconcile. type Test struct { configFactory *action.ConfigFactory + eventRecorder record.EventRecorder +} + +// NewTest returns a new Test reconciler configured with the provided values. +func NewTest(cfg *action.ConfigFactory, recorder record.EventRecorder) *Test { + return &Test{configFactory: cfg, eventRecorder: recorder} } func (r *Test) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.Status.Current.DeepCopy() + cur = req.Object.GetCurrent().DeepCopy() logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.InfoLevel)), 10) + cfg = r.configFactory.Build(logBuf.Log, observeTest(req.Object)) ) + defer summarize(req) + // We only accept test results for the current release. if cur == nil { return fmt.Errorf("%w: required for test", ErrNoCurrent) } - // Run tests. - rls, err := action.Test(ctx, r.configFactory.Build(logBuf.Log, observeTest(req.Object)), req.Object) + // Run the Helm test action. + rls, err := action.Test(ctx, cfg, req.Object) // The Helm test action does always target the latest release. Before // accepting results, we need to confirm this is actually the release we // have recorded as Current. if rls != nil && !release.ObserveRelease(rls).Targets(cur.Name, cur.Namespace, cur.Version) { - err = fmt.Errorf("%w: tested release %s/%s with version %d != current release %s/%s with version %d", + err = fmt.Errorf("%w: tested release %s/%s.%d != current release %s/%s.%d", ErrReleaseMismatch, rls.Namespace, rls.Name, rls.Version, cur.Namespace, cur.Name, cur.Version) } // Something went wrong. if err != nil { - req.Object.Status.Failures++ - conditions.MarkFalse(req.Object, v2.TestSuccessCondition, v2.TestFailedReason, err.Error()) + r.failure(req, logBuf, err) + // If we failed to observe anything happened at all, we want to retry // and return the error to indicate this. - if req.Object.Status.Current == cur { + if !req.Object.GetCurrent().HasBeenTested() { return err } return nil } - // Compose success condition message. - condMsg := "No test hooks." - if hookLen := len(req.Object.Status.Current.TestHooks); hookLen > 0 { - condMsg = fmt.Sprintf("%d test hook(s) completed successfully.", hookLen) - } - conditions.MarkTrue(req.Object, v2.TestSuccessCondition, v2.TestSucceededReason, condMsg) + r.success(req) return nil } @@ -87,15 +118,87 @@ func (r *Test) Type() ReconcilerType { return ReconcilerTypeTest } +const ( + // fmtTestPending is the message format used when awaiting tests to be run. + fmtTestPending = "Release %s with chart %s is awaiting tests" + // fmtTestFailure is the message format for a test failure. + fmtTestFailure = "Test for release %s with chart %s failed: %s" + // fmtTestSuccess is the message format for a successful test. + fmtTestSuccess = "Tests for release %s with chart %s succeeded: %s" +) + +// failure records the failure of a Helm test action in the status of the given +// Request.Object by marking TestSuccess=False and increasing the failure +// counter. In addition, it emits a warning event for the Request.Object. +// The active remediation failure count is only incremented if test failures +// are not ignored. +func (r *Test) failure(req *Request, buffer *action.LogBuffer, err error) { + // Compose failure message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtTestFailure, cur.FullReleaseName(), cur.VersionedChartName(), err.Error()) + + // Mark test failure on object. + req.Object.Status.Failures++ + conditions.MarkFalse(req.Object, v2.TestSuccessCondition, v2.TestFailedReason, msg) + + // Record warning event, this message contains more data than the + // Condition summary. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeWarning, + v2.TestFailedReason, + eventMessageWithLog(msg, buffer), + ) + + if req.Object.GetCurrent().HasBeenTested() { + // Count the failure of the test for the active remediation strategy if enabled. + remediation := req.Object.GetActiveRemediation() + if !remediation.MustIgnoreTestFailures(req.Object.GetTest().IgnoreFailures) { + remediation.IncrementFailureCount(req.Object) + } + } +} + +// success records the failure of a Helm test action in the status of the given +// Request.Object by marking TestSuccess=True and emitting an event. +func (r *Test) success(req *Request) { + // Compose success message. + cur := req.Object.GetCurrent() + var hookMsg = "no test hooks" + if l := len(cur.GetTestHooks()); l > 0 { + h := "hook" + if l > 1 { + h += "s" + } + hookMsg = fmt.Sprintf("%d test %s completed successfully", l, h) + } + msg := fmt.Sprintf(fmtTestSuccess, cur.FullReleaseName(), cur.VersionedChartName(), hookMsg) + + // Mark test success on object. + conditions.MarkTrue(req.Object, v2.TestSuccessCondition, v2.TestSucceededReason, msg) + + // Record event. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeNormal, + v2.TestSucceededReason, + msg, + ) +} + +// observeTest returns a storage.ObserveFunc that can be used to observe +// and record the result of a Helm test action in the status of the given +// release. It updates the Status.Current and TestHooks fields of the release +// if it equals the test target, and version = Current.Version. func observeTest(obj *v2.HelmRelease) storage.ObserveFunc { return func(rls *helmrelease.Release) { - if cur := obj.Status.Current; cur != nil { + if cur := obj.GetCurrent(); cur != nil { obs := release.ObserveRelease(rls) if obs.Targets(cur.Name, cur.Namespace, cur.Version) { obj.Status.Current = release.ObservedToInfo(obs) - if hooks := release.TestHooksFromRelease(rls); len(hooks) > 0 { - obj.Status.Current.TestHooks = hooks - } + obj.GetCurrent().SetTestHooks(release.TestHooksFromRelease(rls)) } } } diff --git a/internal/reconcile/test_test.go b/internal/reconcile/test_test.go index 5a4959091..d6f3d1a22 100644 --- a/internal/reconcile/test_test.go +++ b/internal/reconcile/test_test.go @@ -19,6 +19,7 @@ package reconcile import ( "context" "errors" + "fmt" "testing" "time" @@ -28,12 +29,18 @@ import ( helmreleaseutil "helm.sh/helm/v3/pkg/releaseutil" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" + "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" "github.com/fluxcd/helm-controller/internal/release" "github.com/fluxcd/helm-controller/internal/testutil" ) @@ -124,12 +131,14 @@ func TestTest_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, v2.TestSucceededReason, + "1 test hook completed successfully"), *conditions.TrueCondition(v2.TestSuccessCondition, v2.TestSucceededReason, - "1 test hook(s) completed successfully."), + "1 test hook completed successfully"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { info := release.ObservedToInfo(release.ObserveRelease(releases[0])) - info.TestHooks = release.TestHooksFromRelease(releases[0]) + info.SetTestHooks(release.TestHooksFromRelease(releases[0])) return info }, }, @@ -152,16 +161,19 @@ func TestTest_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, v2.TestSucceededReason, + "no test hooks"), *conditions.TrueCondition(v2.TestSuccessCondition, v2.TestSucceededReason, - "No test hooks."), + "no test hooks"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { info := release.ObservedToInfo(release.ObserveRelease(releases[0])) + info.SetTestHooks(release.TestHooksFromRelease(releases[0])) return info }, }, { - name: "test failure", + name: "test install failure", releases: func(namespace string) []*helmrelease.Release { return []*helmrelease.Release{ testutil.BuildRelease(&helmrelease.MockReleaseOptions{ @@ -175,19 +187,23 @@ func TestTest_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + InstallFailures: 0, } }, expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.TestFailedReason, + "timed out waiting for the condition"), *conditions.FalseCondition(v2.TestSuccessCondition, v2.TestFailedReason, "timed out waiting for the condition"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { info := release.ObservedToInfo(release.ObserveRelease(releases[0])) - info.TestHooks = release.TestHooksFromRelease(releases[0]) + info.SetTestHooks(release.TestHooksFromRelease(releases[0])) return info }, - expectFailures: 1, + expectFailures: 1, + expectInstallFailures: 1, }, { name: "test without current", @@ -231,6 +247,8 @@ func TestTest_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.TestFailedReason, + ErrReleaseMismatch.Error()), *conditions.FalseCondition(v2.TestSuccessCondition, v2.TestFailedReason, ErrReleaseMismatch.Error()), }, @@ -238,6 +256,7 @@ func TestTest_Reconcile(t *testing.T) { return release.ObservedToInfo(release.ObserveRelease(releases[0])) }, expectFailures: 1, + wantErr: ErrReleaseMismatch, }, } for _, tt := range tests { @@ -263,6 +282,9 @@ func TestTest_Reconcile(t *testing.T) { TargetNamespace: releaseNamespace, StorageNamespace: releaseNamespace, Timeout: &metav1.Duration{Duration: 100 * time.Millisecond}, + Test: &v2.Test{ + Enable: true, + }, }, } if tt.spec != nil { @@ -290,7 +312,8 @@ func TestTest_Reconcile(t *testing.T) { cfg.Driver = tt.driver(cfg.Driver) } - got := (&Test{configFactory: cfg}).Reconcile(context.TODO(), &Request{ + recorder := new(record.FakeRecorder) + got := (NewTest(cfg, recorder)).Reconcile(context.TODO(), &Request{ Object: obj, }) if tt.wantErr != nil { @@ -305,15 +328,15 @@ func TestTest_Reconcile(t *testing.T) { helmreleaseutil.SortByRevision(releases) if tt.expectCurrent != nil { - g.Expect(obj.Status.Current).To(testutil.Equal(tt.expectCurrent(releases))) + g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releases))) } else { - g.Expect(obj.Status.Current).To(BeNil(), "expected current to be nil") + g.Expect(obj.GetCurrent()).To(BeNil(), "expected current to be nil") } if tt.expectPrevious != nil { - g.Expect(obj.Status.Previous).To(testutil.Equal(tt.expectPrevious(releases))) + g.Expect(obj.GetPrevious()).To(testutil.Equal(tt.expectPrevious(releases))) } else { - g.Expect(obj.Status.Previous).To(BeNil(), "expected previous to be nil") + g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -343,11 +366,11 @@ func Test_observeTest(t *testing.T) { }, testutil.ReleaseWithHooks(testHookFixtures)) expect := release.ObservedToInfo(release.ObserveRelease(rls)) - expect.TestHooks = release.TestHooksFromRelease(rls) + expect.SetTestHooks(release.TestHooksFromRelease(rls)) observeTest(obj)(rls) - g.Expect(obj.Status.Current).To(Equal(expect)) - g.Expect(obj.Status.Previous).To(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) + g.Expect(obj.GetPrevious()).To(BeNil()) }) t.Run("test with different current version", func(t *testing.T) { @@ -370,8 +393,8 @@ func Test_observeTest(t *testing.T) { }, testutil.ReleaseWithHooks(testHookFixtures)) observeTest(obj)(rls) - g.Expect(obj.Status.Current).To(Equal(current)) - g.Expect(obj.Status.Previous).To(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(current)) + g.Expect(obj.GetPrevious()).To(BeNil()) }) t.Run("test without current", func(t *testing.T) { @@ -386,7 +409,187 @@ func Test_observeTest(t *testing.T) { }, testutil.ReleaseWithHooks(testHookFixtures)) observeTest(obj)(rls) - g.Expect(obj.Status.Current).To(BeNil()) - g.Expect(obj.Status.Previous).To(BeNil()) + g.Expect(obj.GetCurrent()).To(BeNil()) + g.Expect(obj.GetPrevious()).To(BeNil()) + }) +} + +func TestTest_failure(t *testing.T) { + var ( + cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Chart: testutil.BuildChart(), + Version: 4, + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + err = errors.New("test error") + ) + + t.Run("records failure", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Test{ + eventRecorder: recorder, + } + + req := &Request{Object: obj.DeepCopy()} + r.failure(req, nil, err) + + expectMsg := fmt.Sprintf(fmtTestFailure, + fmt.Sprintf("%s/%s.%d", cur.Namespace, cur.Name, cur.Version), + fmt.Sprintf("%s@%s", cur.Chart.Name(), cur.Chart.Metadata.Version), + err.Error()) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.TestSuccessCondition, v2.TestFailedReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(1))) + g.Expect(req.Object.Status.InstallFailures).To(BeZero()) + g.Expect(req.Object.Status.UpgradeFailures).To(BeZero()) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: v2.TestFailedReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): cur.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, cur.Config).String(), + }, + }, + }, + })) + }) + + t.Run("records failure with logs", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Test{ + eventRecorder: recorder, + } + req := &Request{Object: obj.DeepCopy()} + r.failure(req, mockLogBuffer(5, 10), err) + + expectSubStr := "Last Helm logs" + g.Expect(conditions.IsFalse(req.Object, v2.TestSuccessCondition)).To(BeTrue()) + g.Expect(conditions.GetMessage(req.Object, v2.TestSuccessCondition)).ToNot(ContainSubstring(expectSubStr)) + + events := recorder.GetEvents() + g.Expect(events).To(HaveLen(1)) + g.Expect(events[0].Message).To(ContainSubstring(expectSubStr)) + }) + + t.Run("increases remediation failure count", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Test{ + eventRecorder: recorder, + } + + obj := obj.DeepCopy() + obj.Status.Current.SetTestHooks(map[string]*v2.HelmReleaseTestHook{}) + req := &Request{Object: obj} + r.failure(req, nil, err) + + g.Expect(req.Object.Status.InstallFailures).To(Equal(int64(1))) + }) + + t.Run("follows ignore failure instructions", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Test{ + eventRecorder: recorder, + } + + obj := obj.DeepCopy() + obj.Spec.Test = &v2.Test{IgnoreFailures: true} + obj.Status.Current.SetTestHooks(map[string]*v2.HelmReleaseTestHook{}) + req := &Request{Object: obj} + r.failure(req, nil, err) + + g.Expect(req.Object.Status.InstallFailures).To(BeZero()) + }) +} + +func TestTest_success(t *testing.T) { + g := NewWithT(t) + + var ( + cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Chart: testutil.BuildChart(), + Version: 4, + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + ) + + t.Run("records success", func(t *testing.T) { + recorder := testutil.NewFakeRecorder(10, false) + r := &Test{ + eventRecorder: recorder, + } + + obj := obj.DeepCopy() + obj.Status.Current.SetTestHooks(map[string]*v2.HelmReleaseTestHook{ + "test": { + Phase: helmrelease.HookPhaseSucceeded.String(), + }, + "test-2": { + Phase: helmrelease.HookPhaseSucceeded.String(), + }, + }) + req := &Request{Object: obj} + r.success(req) + + expectMsg := fmt.Sprintf(fmtTestSuccess, + fmt.Sprintf("%s/%s.%d", cur.Namespace, cur.Name, cur.Version), + fmt.Sprintf("%s@%s", cur.Chart.Name(), cur.Chart.Metadata.Version), + "2 test hooks completed successfully") + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(v2.TestSuccessCondition, v2.TestSucceededReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(0))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: v2.TestSucceededReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): cur.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, cur.Config).String(), + }, + }, + }, + })) + }) + + t.Run("records success without hooks", func(t *testing.T) { + r := &Test{ + eventRecorder: new(testutil.FakeRecorder), + } + + obj := obj.DeepCopy() + obj.Status.Current.SetTestHooks(map[string]*v2.HelmReleaseTestHook{}) + req := &Request{Object: obj} + r.success(req) + + g.Expect(conditions.IsTrue(req.Object, v2.TestSuccessCondition)).To(BeTrue()) + g.Expect(req.Object.Status.Conditions[0].Message).To(ContainSubstring("no test hooks")) }) } diff --git a/internal/reconcile/uninstall.go b/internal/reconcile/uninstall.go index c24f2be51..7948e4e1f 100644 --- a/internal/reconcile/uninstall.go +++ b/internal/reconcile/uninstall.go @@ -18,9 +18,13 @@ package reconcile import ( "context" + "errors" "fmt" helmrelease "helm.sh/helm/v3/pkg/release" + helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "github.com/fluxcd/pkg/runtime/conditions" @@ -32,53 +36,92 @@ import ( "github.com/fluxcd/helm-controller/internal/storage" ) +// Uninstall is an ActionReconciler which attempts to uninstall a Helm release +// based on the given Request data. +// +// The writes to the Helm storage during the uninstallation are observed, and +// update the Status.Current field. +// +// After a successful uninstall, the object is marked with Released=False and +// an event is emitted. When the uninstallation fails, the object is marked +// with Released=False and a warning event is emitted. +// +// When the Request.Object does not have a Status.Current, it returns an +// error of type ErrNoCurrent. If the uninstallation targeted a different +// release (version) than Status.Current, it returns an error of type +// ErrReleaseMismatch. In addition, it returns ErrNoStorageUpdate if the +// uninstallation completed without updating the Helm storage. In which case +// the resources for the release will be removed from the cluster, but the +// storage object remains in the cluster. Any other returned error indicates +// the caller should retry as it did not cause a change to the Helm storage or +// the cluster resources. +// +// At the end of the reconciliation, the Status.Conditions are summarized and +// propagated to the Ready condition on the Request.Object. +// +// This reconciler is different from UninstallRemediation, in that it makes +// observations to the Released condition type instead of Remediated. Use this +// reconciler to uninstall a release, and UninstallRemediation to remediate a +// release. +// +// The caller is assumed to have verified the integrity of Request.Object using +// e.g. action.VerifyReleaseInfo before calling Reconcile. type Uninstall struct { configFactory *action.ConfigFactory + eventRecorder record.EventRecorder +} + +// NewUninstall returns a new Uninstall reconciler configured with the provided +// values. +func NewUninstall(cfg *action.ConfigFactory, recorder record.EventRecorder) *Uninstall { + return &Uninstall{configFactory: cfg, eventRecorder: recorder} } func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.Status.Current.DeepCopy() + cur = req.Object.GetCurrent().DeepCopy() logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.InfoLevel)), 10) cfg = r.configFactory.Build(logBuf.Log, observeUninstall(req.Object)) ) + defer summarize(req) + // Require current to run uninstall. if cur == nil { return fmt.Errorf("%w: required to uninstall", ErrNoCurrent) } - // Run the uninstall action. - res, err := action.Uninstall(ctx, cfg, req.Object) + // Run the Helm uninstall action. + res, err := action.Uninstall(ctx, cfg, req.Object, cur.Name) + if errors.Is(err, helmdriver.ErrReleaseNotFound) { + err = nil + } // The Helm uninstall action does always target the latest release. Before // accepting results, we need to confirm this is actually the release we // have recorded as Current. if res != nil && !release.ObserveRelease(res.Release).Targets(cur.Name, cur.Namespace, cur.Version) { - err = fmt.Errorf("%w: uninstalled release %s/%s with version %d != current release %s/%s with version %d", - ErrReleaseMismatch, res.Release.Namespace, res.Release.Name, res.Release.Version, cur.Namespace, cur.Name, - cur.Version) + err = fmt.Errorf("%w: uninstalled release %s/%s.%d != current release %s", + ErrReleaseMismatch, res.Release.Namespace, res.Release.Name, res.Release.Version, cur.FullReleaseName()) } // The Helm uninstall action may return without an error while the update // to the storage failed. Detect this and return an error. - if err == nil && cur == req.Object.Status.Current { - err = fmt.Errorf("uninstallation completed without updating Helm storage") + if err == nil && cur.Digest == req.Object.GetCurrent().Digest { + err = fmt.Errorf("uninstall completed with error: %w", ErrNoStorageUpdate) } // Handle any error. if err != nil { - req.Object.Status.Failures++ - conditions.MarkFalse(req.Object, v2.RemediatedCondition, v2.UninstallFailedReason, err.Error()) - if req.Object.Status.Current == cur { + r.failure(req, logBuf, err) + if req.Object.GetCurrent().Digest == cur.Digest { return err } return nil } // Mark success. - conditions.MarkTrue(req.Object, v2.RemediatedCondition, v2.UninstallSucceededReason, - res.Release.Info.Description) + r.success(req) return nil } @@ -87,7 +130,58 @@ func (r *Uninstall) Name() string { } func (r *Uninstall) Type() ReconcilerType { - return ReconcilerTypeRemediate + return ReconcilerTypeRelease +} + +const ( + // fmtUninstallFailed is the message format for an uninstall failure. + fmtUninstallFailure = "Uninstall of release %s with chart %s failed: %s" + // fmtUninstallSuccess is the message format for a successful uninstall. + fmtUninstallSuccess = "Uninstalled release %s with chart %s" +) + +// failure records the failure of a Helm uninstall action in the status of the +// given Request.Object by marking Released=False and emitting a warning +// event. +func (r *Uninstall) failure(req *Request, buffer *action.LogBuffer, err error) { + // Compose success message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtUninstallFailure, cur.FullReleaseName(), cur.VersionedChartName(), err.Error()) + + // Mark remediation failure on object. + req.Object.Status.Failures++ + conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UninstallFailedReason, msg) + + // Record warning event, this message contains more data than the + // Condition summary. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeWarning, v2.UninstallFailedReason, + eventMessageWithLog(msg, buffer), + ) +} + +// success records the success of a Helm uninstall action in the status of +// the given Request.Object by marking Released=False and emitting an +// event. +func (r *Uninstall) success(req *Request) { + // Compose success message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtUninstallSuccess, cur.FullReleaseName(), cur.VersionedChartName()) + + // Mark remediation success on object. + conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UninstallSucceededReason, msg) + + // Record warning event, this message contains more data than the + // Condition summary. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeNormal, + v2.UninstallSucceededReason, + msg, + ) } // observeUninstall returns a storage.ObserveFunc that can be used to observe @@ -96,7 +190,7 @@ func (r *Uninstall) Type() ReconcilerType { // uninstallation target, and version = Current.Version. func observeUninstall(obj *v2.HelmRelease) storage.ObserveFunc { return func(rls *helmrelease.Release) { - if cur := obj.Status.Current; cur != nil { + if cur := obj.GetCurrent(); cur != nil { if obs := release.ObserveRelease(rls); obs.Targets(cur.Name, cur.Namespace, cur.Version) { obj.Status.Current = release.ObservedToInfo(obs) } diff --git a/internal/reconcile/uninstall_remediation.go b/internal/reconcile/uninstall_remediation.go new file mode 100644 index 000000000..600712906 --- /dev/null +++ b/internal/reconcile/uninstall_remediation.go @@ -0,0 +1,182 @@ +/* +Copyright 2022 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 reconcile + +import ( + "context" + "errors" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/fluxcd/pkg/runtime/conditions" + "github.com/fluxcd/pkg/runtime/logger" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/release" +) + +var ( + ErrNoStorageUpdate = errors.New("release not updated in Helm storage") +) + +// UninstallRemediation is an ActionReconciler which attempts to remediate a +// failed Helm release for the given Request data by uninstalling it. +// +// The writes to the Helm storage during the uninstallation are observed, and +// update the Status.Current field. +// +// After a successful uninstall, the object is marked with Remediated=True and +// an event is emitted. When the uninstallation fails, the object is marked +// with Remediated=False and a warning event is emitted. +// +// When the Request.Object does not have a Status.Current, it returns an +// error of type ErrNoCurrent. If the uninstallation targeted a different +// release (version) than Status.Current, it returns an error of type +// ErrReleaseMismatch. In addition, it returns ErrNoStorageUpdate if the +// uninstallation completed without updating the Helm storage. In which case +// the resources for the release will be removed from the cluster, but the +// storage object remains in the cluster. Any other returned error indicates +// the caller should retry as it did not cause a change to the Helm storage or +// the cluster resources. +// +// At the end of the reconciliation, the Status.Conditions are summarized and +// propagated to the Ready condition on the Request.Object. +// +// This reconciler is different from Uninstall, in that it makes observations +// to the Remediated condition type instead of Released. Use this reconciler +// to remediate a failed release, and Uninstall to uninstall a release. +// +// The caller is assumed to have verified the integrity of Request.Object using +// e.g. action.VerifyReleaseInfo before calling Reconcile. +type UninstallRemediation struct { + configFactory *action.ConfigFactory + eventRecorder record.EventRecorder +} + +// NewUninstallRemediation returns a new UninstallRemediation reconciler +// configured with the provided values. +func NewUninstallRemediation(cfg *action.ConfigFactory, recorder record.EventRecorder) *UninstallRemediation { + return &UninstallRemediation{configFactory: cfg, eventRecorder: recorder} +} + +func (r *UninstallRemediation) Reconcile(ctx context.Context, req *Request) error { + var ( + cur = req.Object.GetCurrent().DeepCopy() + logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.InfoLevel)), 10) + cfg = r.configFactory.Build(logBuf.Log, observeUninstall(req.Object)) + ) + + // Require current to run uninstall. + if cur == nil { + return fmt.Errorf("%w: required to uninstall", ErrNoCurrent) + } + + // Run the Helm uninstall action. + res, err := action.Uninstall(ctx, cfg, req.Object, cur.Name) + + // The Helm uninstall action does always target the latest release. Before + // accepting results, we need to confirm this is actually the release we + // have recorded as Current. + if res != nil && !release.ObserveRelease(res.Release).Targets(cur.Name, cur.Namespace, cur.Version) { + err = fmt.Errorf("%w: uninstalled release %s/%s.%d != current release %s", + ErrReleaseMismatch, res.Release.Namespace, res.Release.Name, res.Release.Version, cur.FullReleaseName()) + } + + // The Helm uninstall action may return without an error while the update + // to the storage failed. Detect this and return an error. + if err == nil && cur.Digest == req.Object.GetCurrent().Digest { + err = fmt.Errorf("uninstall completed with error: %w", ErrNoStorageUpdate) + } + + // Handle any error. + if err != nil { + r.failure(req, logBuf, err) + if cur.Digest == req.Object.GetCurrent().Digest { + return err + } + return nil + } + + // Mark success. + r.success(req) + return nil +} + +func (r *UninstallRemediation) Name() string { + return "uninstall" +} + +func (r *UninstallRemediation) Type() ReconcilerType { + return ReconcilerTypeRemediate +} + +const ( + // fmtUninstallRemediationFailure is the message format for an uninstall + // remediation failure. + fmtUninstallRemediationFailure = "Uninstall remediation for release %s with chart %s failed: %s" + // fmtUninstallRemediationSuccess is the message format for a successful + // uninstall remediation. + fmtUninstallRemediationSuccess = "Uninstall remediation for release %s with chart %s succeeded" +) + +// success records the success of a Helm uninstall remediation action in the +// status of the given Request.Object by marking Remediated=False and emitting +// a warning event. +func (r *UninstallRemediation) failure(req *Request, buffer *action.LogBuffer, err error) { + // Compose success message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtUninstallRemediationFailure, cur.FullReleaseName(), cur.VersionedChartName(), err.Error()) + + // Mark uninstall failure on object. + req.Object.Status.Failures++ + conditions.MarkFalse(req.Object, v2.RemediatedCondition, v2.UninstallFailedReason, msg) + + // Record warning event, this message contains more data than the + // Condition summary. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeWarning, + v2.UninstallFailedReason, + eventMessageWithLog(msg, buffer), + ) +} + +// success records the success of a Helm uninstall remediation action in the +// status of the given Request.Object by marking Remediated=True and emitting +// an event. +func (r *UninstallRemediation) success(req *Request) { + // Compose success message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtUninstallRemediationSuccess, cur.FullReleaseName(), cur.VersionedChartName()) + + // Mark remediation success on object. + conditions.MarkTrue(req.Object, v2.RemediatedCondition, v2.UninstallSucceededReason, msg) + + // Record event. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeNormal, + v2.UninstallSucceededReason, + msg, + ) +} diff --git a/internal/reconcile/uninstall_remediation_test.go b/internal/reconcile/uninstall_remediation_test.go new file mode 100644 index 000000000..f2543899e --- /dev/null +++ b/internal/reconcile/uninstall_remediation_test.go @@ -0,0 +1,485 @@ +/* +Copyright 2022 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 reconcile + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/go-logr/logr" + . "github.com/onsi/gomega" + helmrelease "helm.sh/helm/v3/pkg/release" + "helm.sh/helm/v3/pkg/releaseutil" + helmstorage "helm.sh/helm/v3/pkg/storage" + helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" + "github.com/fluxcd/pkg/runtime/conditions" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" + "github.com/fluxcd/helm-controller/internal/release" + "github.com/fluxcd/helm-controller/internal/storage" + "github.com/fluxcd/helm-controller/internal/testutil" +) + +func TestUninstallRemediation_Reconcile(t *testing.T) { + var ( + mockUpdateErr = fmt.Errorf("storage update error") + mockDeleteErr = fmt.Errorf("storage delete error") + ) + + tests := []struct { + name string + // driver allows for modifying the Helm storage driver. + driver func(helmdriver.Driver) helmdriver.Driver + // releases is the list of releases that are stored in the driver + // before uninstall. + releases func(namespace string) []*helmrelease.Release + // spec modifies the HelmRelease Object spec before uninstall. + spec func(spec *v2.HelmReleaseSpec) + // status to configure on the HelmRelease Object before uninstall. + status func(releases []*helmrelease.Release) v2.HelmReleaseStatus + // wantErr is the error that is expected to be returned. + wantErr error + // expectedConditions are the conditions that are expected to be set on + // the HelmRelease after running rollback. + expectConditions []metav1.Condition + // expectCurrent is the expected Current release information in the + // HelmRelease after uninstall. + expectCurrent func(releases []*helmrelease.Release) *v2.HelmReleaseInfo + // expectPrevious returns the expected Previous release information of + // the HelmRelease after uninstall. + expectPrevious func(releases []*helmrelease.Release) *v2.HelmReleaseInfo + // expectFailures is the expected Failures count of the HelmRelease. + expectFailures int64 + // expectInstallFailures is the expected InstallFailures count of the + // HelmRelease. + expectInstallFailures int64 + // expectUpgradeFailures is the expected UpgradeFailures count of the + // HelmRelease. + expectUpgradeFailures int64 + }{ + { + name: "uninstall success", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(testutil.ChartWithTestHook()), + Status: helmrelease.StatusDeployed, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + expectConditions: []metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.UninstallSucceededReason, + "Uninstall remediation for release"), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + }, + { + name: "uninstall failure", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Status: helmrelease.StatusDeployed, + Chart: testutil.BuildChart(testutil.ChartWithFailingHook()), + }, testutil.ReleaseWithFailingHook()), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, + "uninstallation completed with 1 error(s): 1 error occurred:\n\t* timed out waiting for the condition\n\n"), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + expectFailures: 1, + }, + { + name: "uninstall failure without storage update", + driver: func(driver helmdriver.Driver) helmdriver.Driver { + return &storage.Failing{ + // Explicitly inherit the driver, as we want to rely on the + // Secret storage, as the memory storage does not detach + // objects from the release action. Causing writes post-persist + // to leak to the stored release object. + // xref: https://github.com/helm/helm/issues/11304 + Driver: driver, + UpdateErr: mockUpdateErr, + } + }, + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(testutil.ChartWithTestHook()), + Status: helmrelease.StatusDeployed, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, + ErrNoStorageUpdate.Error()), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + expectFailures: 1, + wantErr: ErrNoStorageUpdate, + }, + { + name: "uninstall failure without storage delete", + driver: func(driver helmdriver.Driver) helmdriver.Driver { + return &storage.Failing{ + // Explicitly inherit the driver, as we want to rely on the + // Secret storage, as the memory storage does not detach + // objects from the release action. Causing writes post-persist + // to leak to the stored release object. + // xref: https://github.com/helm/helm/issues/11304 + Driver: driver, + DeleteErr: mockDeleteErr, + } + }, + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Status: helmrelease.StatusDeployed, + Chart: testutil.BuildChart(), + }), + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, mockDeleteErr.Error()), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + expectFailures: 1, + }, + { + name: "uninstall without current", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusDeployed, + }), + } + }, + expectConditions: []metav1.Condition{}, + wantErr: ErrNoCurrent, + }, + { + name: "uninstall with stale current", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(testutil.ChartWithTestHook()), + Status: helmrelease.StatusSuperseded, + }, testutil.ReleaseWithTestHook()), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusDeployed, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, + ErrReleaseMismatch.Error()), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + expectFailures: 1, + wantErr: ErrReleaseMismatch, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + namedNS, err := testEnv.CreateNamespace(context.TODO(), mockReleaseNamespace) + g.Expect(err).NotTo(HaveOccurred()) + t.Cleanup(func() { + _ = testEnv.Delete(context.TODO(), namedNS) + }) + releaseNamespace := namedNS.Name + + var releases []*helmrelease.Release + if tt.releases != nil { + releases = tt.releases(releaseNamespace) + releaseutil.SortByRevision(releases) + } + + obj := &v2.HelmRelease{ + Spec: v2.HelmReleaseSpec{ + ReleaseName: mockReleaseName, + TargetNamespace: releaseNamespace, + StorageNamespace: releaseNamespace, + Timeout: &metav1.Duration{Duration: 100 * time.Millisecond}, + }, + } + if tt.spec != nil { + tt.spec(&obj.Spec) + } + if tt.status != nil { + obj.Status = tt.status(releases) + } + + getter, err := RESTClientGetterFromManager(testEnv.Manager, obj.GetReleaseNamespace()) + g.Expect(err).ToNot(HaveOccurred()) + + cfg, err := action.NewConfigFactory(getter, + action.WithStorage(action.DefaultStorageDriver, obj.GetStorageNamespace()), + action.WithDebugLog(logr.Discard()), + ) + g.Expect(err).ToNot(HaveOccurred()) + + store := helmstorage.Init(cfg.Driver) + for _, r := range releases { + g.Expect(store.Create(r)).To(Succeed()) + } + + if tt.driver != nil { + cfg.Driver = tt.driver(cfg.Driver) + } + + recorder := new(record.FakeRecorder) + got := NewUninstallRemediation(cfg, recorder).Reconcile(context.TODO(), &Request{ + Object: obj, + }) + if tt.wantErr != nil { + g.Expect(errors.Is(got, tt.wantErr)).To(BeTrue()) + } else { + g.Expect(got).ToNot(HaveOccurred()) + } + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.expectConditions)) + + releases, _ = store.History(mockReleaseName) + releaseutil.SortByRevision(releases) + + if tt.expectCurrent != nil { + g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releases))) + } else { + g.Expect(obj.GetCurrent()).To(BeNil(), "expected current to be nil") + } + + if tt.expectPrevious != nil { + g.Expect(obj.GetPrevious()).To(testutil.Equal(tt.expectPrevious(releases))) + } else { + g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") + } + + g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) + g.Expect(obj.Status.InstallFailures).To(Equal(tt.expectInstallFailures)) + g.Expect(obj.Status.UpgradeFailures).To(Equal(tt.expectUpgradeFailures)) + }) + } +} + +func TestUninstallRemediation_failure(t *testing.T) { + var ( + cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Chart: testutil.BuildChart(), + Version: 4, + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + err = errors.New("uninstall error") + ) + + t.Run("records failure", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &UninstallRemediation{ + eventRecorder: recorder, + } + + req := &Request{Object: obj.DeepCopy()} + r.failure(req, nil, err) + + expectMsg := fmt.Sprintf(fmtUninstallRemediationFailure, + fmt.Sprintf("%s/%s.%d", cur.Namespace, cur.Name, cur.Version), + fmt.Sprintf("%s@%s", cur.Chart.Name(), cur.Chart.Metadata.Version), + err.Error()) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(1))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: v2.UninstallFailedReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): cur.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, cur.Config).String(), + }, + }, + }, + })) + }) + + t.Run("records failure with logs", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &UninstallRemediation{ + eventRecorder: recorder, + } + req := &Request{Object: obj.DeepCopy()} + r.failure(req, mockLogBuffer(5, 10), err) + + expectSubStr := "Last Helm logs" + g.Expect(conditions.IsFalse(req.Object, v2.RemediatedCondition)).To(BeTrue()) + g.Expect(conditions.GetMessage(req.Object, v2.RemediatedCondition)).ToNot(ContainSubstring(expectSubStr)) + + events := recorder.GetEvents() + g.Expect(events).To(HaveLen(1)) + g.Expect(events[0].Message).To(ContainSubstring(expectSubStr)) + }) +} + +func TestUninstallRemediation_success(t *testing.T) { + g := NewWithT(t) + + var cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Chart: testutil.BuildChart(), + Version: 4, + }) + + recorder := testutil.NewFakeRecorder(10, false) + r := &UninstallRemediation{ + eventRecorder: recorder, + } + + obj := &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + + req := &Request{Object: obj} + r.success(req) + + expectMsg := fmt.Sprintf(fmtUninstallRemediationSuccess, + fmt.Sprintf("%s/%s.%d", cur.Namespace, cur.Name, cur.Version), + fmt.Sprintf("%s@%s", cur.Chart.Name(), cur.Chart.Metadata.Version)) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(v2.RemediatedCondition, v2.UninstallSucceededReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(0))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: v2.UninstallSucceededReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): cur.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, cur.Config).String(), + }, + }, + }, + })) +} diff --git a/internal/reconcile/uninstall_test.go b/internal/reconcile/uninstall_test.go index 9e3eda33d..d53b333f4 100644 --- a/internal/reconcile/uninstall_test.go +++ b/internal/reconcile/uninstall_test.go @@ -23,23 +23,32 @@ import ( "testing" "time" - "github.com/fluxcd/pkg/runtime/conditions" "github.com/go-logr/logr" . "github.com/onsi/gomega" helmrelease "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/releaseutil" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" "github.com/fluxcd/helm-controller/internal/release" "github.com/fluxcd/helm-controller/internal/storage" "github.com/fluxcd/helm-controller/internal/testutil" ) -func Test_uninstall(t *testing.T) { +func TestUninstall_Reconcile(t *testing.T) { + mockUpdateErr := errors.New("mock update error") + tests := []struct { name string // driver allows for modifying the Helm storage driver. @@ -95,8 +104,10 @@ func Test_uninstall(t *testing.T) { } }, expectConditions: []metav1.Condition{ - *conditions.TrueCondition(v2.RemediatedCondition, v2.UninstallSucceededReason, - "Uninstallation complete"), + *conditions.FalseCondition(meta.ReadyCondition, v2.UninstallSucceededReason, + "Uninstalled release"), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, + "Uninstalled release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[0])) @@ -126,7 +137,9 @@ func Test_uninstall(t *testing.T) { } }, expectConditions: []metav1.Condition{ - *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, + *conditions.FalseCondition(meta.ReadyCondition, v2.UninstallFailedReason, + "uninstallation completed with 1 error(s): 1 error occurred:\n\t* timed out waiting for the condition\n\n"), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, "uninstallation completed with 1 error(s): 1 error occurred:\n\t* timed out waiting for the condition\n\n"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { @@ -134,6 +147,52 @@ func Test_uninstall(t *testing.T) { }, expectFailures: 1, }, + { + name: "uninstall failure without storage update", + driver: func(driver helmdriver.Driver) helmdriver.Driver { + return &storage.Failing{ + // Explicitly inherit the driver, as we want to rely on the + // Secret storage, as the memory storage does not detach + // objects from the release action. Causing writes post-persist + // to leak to the stored release object. + // xref: https://github.com/helm/helm/issues/11304 + Driver: driver, + UpdateErr: mockUpdateErr, + } + }, + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(testutil.ChartWithTestHook()), + Status: helmrelease.StatusDeployed, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.UninstallFailedReason, + ErrNoStorageUpdate.Error()), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, + ErrNoStorageUpdate.Error()), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + expectFailures: 1, + wantErr: ErrNoStorageUpdate, + }, { name: "uninstall failure without storage delete", driver: func(driver helmdriver.Driver) helmdriver.Driver { @@ -142,6 +201,7 @@ func Test_uninstall(t *testing.T) { // Secret storage, as the memory storage does not detach // objects from the release action. Causing writes post-persist // to leak to the stored release object. + // xref: https://github.com/helm/helm/issues/11304 Driver: driver, DeleteErr: fmt.Errorf("delete error"), } @@ -163,7 +223,9 @@ func Test_uninstall(t *testing.T) { } }, expectConditions: []metav1.Condition{ - *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, + *conditions.FalseCondition(meta.ReadyCondition, v2.UninstallFailedReason, + "delete error"), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, "delete error"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { @@ -218,13 +280,16 @@ func Test_uninstall(t *testing.T) { } }, expectConditions: []metav1.Condition{ - *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, + *conditions.FalseCondition(meta.ReadyCondition, v2.UninstallFailedReason, + ErrReleaseMismatch.Error()), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, ErrReleaseMismatch.Error()), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[0])) }, expectFailures: 1, + wantErr: ErrReleaseMismatch, }, } for _, tt := range tests { @@ -277,7 +342,8 @@ func Test_uninstall(t *testing.T) { cfg.Driver = tt.driver(cfg.Driver) } - got := (&Uninstall{configFactory: cfg}).Reconcile(context.TODO(), &Request{ + recorder := new(record.FakeRecorder) + got := NewUninstall(cfg, recorder).Reconcile(context.TODO(), &Request{ Object: obj, }) if tt.wantErr != nil { @@ -292,15 +358,15 @@ func Test_uninstall(t *testing.T) { releaseutil.SortByRevision(releases) if tt.expectCurrent != nil { - g.Expect(obj.Status.Current).To(testutil.Equal(tt.expectCurrent(releases))) + g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releases))) } else { - g.Expect(obj.Status.Current).To(BeNil(), "expected current to be nil") + g.Expect(obj.GetCurrent()).To(BeNil(), "expected current to be nil") } if tt.expectPrevious != nil { - g.Expect(obj.Status.Previous).To(testutil.Equal(tt.expectPrevious(releases))) + g.Expect(obj.GetPrevious()).To(testutil.Equal(tt.expectPrevious(releases))) } else { - g.Expect(obj.Status.Previous).To(BeNil(), "expected previous to be nil") + g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -310,6 +376,123 @@ func Test_uninstall(t *testing.T) { } } +func TestUninstall_failure(t *testing.T) { + var ( + cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Chart: testutil.BuildChart(), + Version: 4, + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + err = errors.New("uninstall error") + ) + + t.Run("records failure", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Uninstall{ + eventRecorder: recorder, + } + + req := &Request{Object: obj.DeepCopy()} + r.failure(req, nil, err) + + expectMsg := fmt.Sprintf(fmtUninstallFailure, + fmt.Sprintf("%s/%s.%d", cur.Namespace, cur.Name, cur.Version), + fmt.Sprintf("%s@%s", cur.Chart.Name(), cur.Chart.Metadata.Version), + err.Error()) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(1))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: v2.UninstallFailedReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): cur.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, cur.Config).String(), + }, + }, + }, + })) + }) + + t.Run("records failure with logs", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Uninstall{ + eventRecorder: recorder, + } + req := &Request{Object: obj.DeepCopy()} + r.failure(req, mockLogBuffer(5, 10), err) + + expectSubStr := "Last Helm logs" + g.Expect(conditions.IsFalse(req.Object, v2.ReleasedCondition)).To(BeTrue()) + g.Expect(conditions.GetMessage(req.Object, v2.ReleasedCondition)).ToNot(ContainSubstring(expectSubStr)) + + events := recorder.GetEvents() + g.Expect(events).To(HaveLen(1)) + g.Expect(events[0].Message).To(ContainSubstring(expectSubStr)) + }) +} + +func TestUninstall_success(t *testing.T) { + g := NewWithT(t) + + var cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Chart: testutil.BuildChart(), + Version: 4, + }) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Uninstall{ + eventRecorder: recorder, + } + + obj := &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + + req := &Request{Object: obj} + r.success(req) + + expectMsg := fmt.Sprintf(fmtUninstallSuccess, + fmt.Sprintf("%s/%s.%d", cur.Namespace, cur.Name, cur.Version), + fmt.Sprintf("%s@%s", cur.Chart.Name(), cur.Chart.Metadata.Version)) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(0))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: v2.UninstallSucceededReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): cur.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, cur.Config).String(), + }, + }, + }, + })) +} + func Test_observeUninstall(t *testing.T) { t.Run("uninstall of current", func(t *testing.T) { g := NewWithT(t) @@ -334,9 +517,9 @@ func Test_observeUninstall(t *testing.T) { expect := release.ObservedToInfo(release.ObserveRelease(rls)) observeUninstall(obj)(rls) - g.Expect(obj.Status.Current).ToNot(BeNil()) - g.Expect(obj.Status.Current).To(Equal(expect)) - g.Expect(obj.Status.Previous).To(BeNil()) + g.Expect(obj.GetCurrent()).ToNot(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) + g.Expect(obj.GetPrevious()).To(BeNil()) }) t.Run("uninstall without current", func(t *testing.T) { @@ -355,8 +538,8 @@ func Test_observeUninstall(t *testing.T) { }) observeUninstall(obj)(rls) - g.Expect(obj.Status.Current).To(BeNil()) - g.Expect(obj.Status.Previous).To(BeNil()) + g.Expect(obj.GetCurrent()).To(BeNil()) + g.Expect(obj.GetPrevious()).To(BeNil()) }) t.Run("uninstall of different version than current", func(t *testing.T) { @@ -381,8 +564,8 @@ func Test_observeUninstall(t *testing.T) { }) observeUninstall(obj)(rls) - g.Expect(obj.Status.Current).ToNot(BeNil()) - g.Expect(obj.Status.Current).To(Equal(current)) - g.Expect(obj.Status.Previous).To(BeNil()) + g.Expect(obj.GetCurrent()).ToNot(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(current)) + g.Expect(obj.GetPrevious()).To(BeNil()) }) } diff --git a/internal/reconcile/unlock.go b/internal/reconcile/unlock.go index 908bf4d53..adcbc85c3 100644 --- a/internal/reconcile/unlock.go +++ b/internal/reconcile/unlock.go @@ -21,9 +21,12 @@ import ( "errors" "fmt" - "github.com/fluxcd/pkg/runtime/conditions" helmrelease "helm.sh/helm/v3/pkg/release" helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" + + "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" @@ -31,13 +34,44 @@ import ( "github.com/fluxcd/helm-controller/internal/storage" ) +// Unlock is an ActionReconciler which attempts to unlock the Status.Current +// of a Request.Object in the Helm storage if stuck in a pending state, by +// setting the status to release.StatusFailed and persisting it. +// +// This write to the Helm storage is observed, and updates the Status.Current +// field if the persisted object targets the same release version. +// +// Any pending state marks the v2beta2.HelmRelease object with +// ReleasedCondition=False, even if persisting the object to the Helm storage +// fails. +// +// If the Request.Object does not have a Status.Current, an ErrNoCurrent error +// is returned. +// +// At the end of the reconciliation, the Status.Conditions are summarized and +// propagated to the Ready condition on the Request.Object. +// +// The caller is assumed to have verified the integrity of Request.Object using +// e.g. action.VerifyReleaseInfo before calling Reconcile. type Unlock struct { configFactory *action.ConfigFactory + eventRecorder record.EventRecorder +} + +// NewUnlock returns a new Unlock reconciler configured with the provided +// values. +func NewUnlock(cfg *action.ConfigFactory, recorder record.EventRecorder) *Unlock { + return &Unlock{configFactory: cfg, eventRecorder: recorder} } func (r *Unlock) Reconcile(_ context.Context, req *Request) error { + var ( + cur = req.Object.GetCurrent().DeepCopy() + ) + + defer summarize(req) + // We can only unlock a release if we have a current. - cur := req.Object.Status.Current.DeepCopy() if cur == nil { return fmt.Errorf("%w: required for unlock", ErrNoCurrent) } @@ -46,7 +80,7 @@ func (r *Unlock) Reconcile(_ context.Context, req *Request) error { cfg := r.configFactory.Build(nil, observeUnlock(req.Object)) // Retrieve last release object. - rls, err := cfg.Releases.Last(req.Object.GetReleaseName()) + rls, err := cfg.Releases.Get(cur.Name, cur.Version) if err != nil { // Ignore not found error. Assume caller will decide what to do // when it re-assess state to determine the next action. @@ -57,21 +91,15 @@ func (r *Unlock) Reconcile(_ context.Context, req *Request) error { return err } - // Ensure latest is still same as current. - obs := release.ObserveRelease(rls) - if obs.Targets(cur.Name, cur.Namespace, cur.Version) { - if status := rls.Info.Status; status.IsPending() { - // Update pending status to failed and persist. - rls.SetStatus(helmrelease.StatusFailed, fmt.Sprintf("Release unlocked from stale '%s' state", - status.String())) - if err = cfg.Releases.Update(rls); err != nil { - req.Object.Status.Failures++ - conditions.MarkFalse(req.Object, v2.ReleasedCondition, "StalePending", - "Failed to unlock release from stale '%s' state: %s", status.String(), err.Error()) - return err - } - conditions.MarkFalse(req.Object, v2.ReleasedCondition, "StalePending", rls.Info.Description) + // Ensure the release is in a pending state. + if status := rls.Info.Status; status.IsPending() { + // Update pending status to failed and persist. + rls.SetStatus(helmrelease.StatusFailed, fmt.Sprintf("Release unlocked from stale '%s' state", status.String())) + if err = cfg.Releases.Update(rls); err != nil { + r.failure(req, status, err) + return err } + r.success(req, status) } return nil } @@ -84,13 +112,62 @@ func (r *Unlock) Type() ReconcilerType { return ReconcilerTypeUnlock } +const ( + // fmtUnlockFailure is the message format for an unlock failure. + fmtUnlockFailure = "Unlock of release %s with chart %s in %s state failed: %s" + // fmtUnlockSuccess is the message format for a successful unlock. + fmtUnlockSuccess = "Unlocked release %s with chart %s in %s state" +) + +// failure records the failure of an unlock action in the status of the given +// Request.Object by marking ReleasedCondition=False and increasing the failure +// counter. In addition, it emits a warning event for the Request.Object. +func (r *Unlock) failure(req *Request, status helmrelease.Status, err error) { + // Compose failure message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtUnlockFailure, cur.FullReleaseName(), cur.VersionedChartName(), status.String(), err.Error()) + + // Mark unlock failure on object. + req.Object.Status.Failures++ + conditions.MarkFalse(req.Object, v2.ReleasedCondition, "PendingRelease", msg) + + // Record warning event. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeWarning, + "PendingRelease", + msg, + ) +} + +// success records the success of an unlock action in the status of the given +// Request.Object by marking ReleasedCondition=False and emitting an event. +func (r *Unlock) success(req *Request, status helmrelease.Status) { + // Compose success message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtUnlockSuccess, cur.FullReleaseName(), cur.VersionedChartName(), status.String()) + + // Mark unlock success on object. + conditions.MarkFalse(req.Object, v2.ReleasedCondition, "PendingRelease", msg) + + // Record event. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeNormal, + "PendingRelease", + msg, + ) +} + // observeUnlock returns a storage.ObserveFunc that can be used to observe and // record the result of an unlock action in the status of the given release. // It updates the Status.Current field of the release if it equals the target // of the unlock action. func observeUnlock(obj *v2.HelmRelease) storage.ObserveFunc { return func(rls *helmrelease.Release) { - if cur := obj.Status.Current; cur != nil { + if cur := obj.GetCurrent(); cur != nil { obs := release.ObserveRelease(rls) if obs.Targets(cur.Name, cur.Namespace, cur.Version) { obj.Status.Current = release.ObservedToInfo(obs) diff --git a/internal/reconcile/unlock_test.go b/internal/reconcile/unlock_test.go index b53ced76f..163d69459 100644 --- a/internal/reconcile/unlock_test.go +++ b/internal/reconcile/unlock_test.go @@ -19,6 +19,7 @@ package reconcile import ( "context" "errors" + "fmt" "testing" "time" @@ -28,18 +29,24 @@ import ( helmreleaseutil "helm.sh/helm/v3/pkg/releaseutil" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" + "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" "github.com/fluxcd/helm-controller/internal/release" "github.com/fluxcd/helm-controller/internal/storage" "github.com/fluxcd/helm-controller/internal/testutil" ) -func Test_unlock(t *testing.T) { +func TestUnlock_Reconcile(t *testing.T) { var ( mockQueryErr = errors.New("storage query error") mockUpdateErr = errors.New("storage update error") @@ -95,8 +102,8 @@ func Test_unlock(t *testing.T) { } }, expectConditions: []metav1.Condition{ - *conditions.FalseCondition(v2.ReleasedCondition, "StalePending", - "Release unlocked from stale '%s' state", helmrelease.StatusPendingInstall), + *conditions.FalseCondition(meta.ReadyCondition, "PendingRelease", "Unlocked release"), + *conditions.FalseCondition(v2.ReleasedCondition, "PendingRelease", "Unlocked release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[0])) @@ -128,8 +135,8 @@ func Test_unlock(t *testing.T) { }, wantErr: mockUpdateErr, expectConditions: []metav1.Condition{ - *conditions.FalseCondition(v2.ReleasedCondition, "StalePending", - "Failed to unlock release from stale '%s' state", helmrelease.StatusPendingRollback), + *conditions.FalseCondition(meta.ReadyCondition, "PendingRelease", "Unlock of release"), + *conditions.FalseCondition(v2.ReleasedCondition, "PendingRelease", "Unlock of release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[0])) @@ -233,8 +240,8 @@ func Test_unlock(t *testing.T) { name: "unlock with storage query error", driver: func(driver helmdriver.Driver) helmdriver.Driver { return &storage.Failing{ - Driver: driver, - QueryErr: mockQueryErr, + Driver: driver, + GetErr: mockQueryErr, } }, releases: func(namespace string) []*helmrelease.Release { @@ -318,7 +325,8 @@ func Test_unlock(t *testing.T) { cfg.Driver = tt.driver(cfg.Driver) } - got := (&Unlock{configFactory: cfg}).Reconcile(context.TODO(), &Request{ + recorder := new(record.FakeRecorder) + got := NewUnlock(cfg, recorder).Reconcile(context.TODO(), &Request{ Object: obj, }) if tt.wantErr != nil { @@ -333,15 +341,15 @@ func Test_unlock(t *testing.T) { helmreleaseutil.SortByRevision(releases) if tt.expectCurrent != nil { - g.Expect(obj.Status.Current).To(testutil.Equal(tt.expectCurrent(releases))) + g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releases))) } else { - g.Expect(obj.Status.Current).To(BeNil(), "expected current to be nil") + g.Expect(obj.GetCurrent()).To(BeNil(), "expected current to be nil") } if tt.expectPrevious != nil { - g.Expect(obj.Status.Previous).To(testutil.Equal(tt.expectPrevious(releases))) + g.Expect(obj.GetPrevious()).To(testutil.Equal(tt.expectPrevious(releases))) } else { - g.Expect(obj.Status.Previous).To(BeNil(), "expected previous to be nil") + g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -351,6 +359,107 @@ func Test_unlock(t *testing.T) { } } +func TestUnlock_failure(t *testing.T) { + g := NewWithT(t) + + var ( + cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Chart: testutil.BuildChart(), + Version: 4, + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + status = helmrelease.StatusPendingInstall + err = fmt.Errorf("unlock error") + ) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Unlock{ + eventRecorder: recorder, + } + + req := &Request{Object: obj} + r.failure(req, status, err) + + expectMsg := fmt.Sprintf(fmtUnlockFailure, + fmt.Sprintf("%s/%s.%d", cur.Namespace, cur.Name, cur.Version), + fmt.Sprintf("%s@%s", cur.Chart.Name(), cur.Chart.Metadata.Version), + status, err.Error()) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.ReleasedCondition, "PendingRelease", expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(1))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: "PendingRelease", + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): cur.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, cur.Config).String(), + }, + }, + }, + })) +} + +func TestUnlock_success(t *testing.T) { + g := NewWithT(t) + + var ( + cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Chart: testutil.BuildChart(), + Version: 4, + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + status = helmrelease.StatusPendingInstall + ) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Unlock{ + eventRecorder: recorder, + } + + req := &Request{Object: obj} + r.success(req, status) + + expectMsg := fmt.Sprintf(fmtUnlockSuccess, + fmt.Sprintf("%s/%s.%d", cur.Namespace, cur.Name, cur.Version), + fmt.Sprintf("%s@%s", cur.Chart.Name(), cur.Chart.Metadata.Version), + status) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.ReleasedCondition, "PendingRelease", expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(0))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: "PendingRelease", + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): cur.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, cur.Config).String(), + }, + }, + }, + })) +} + func Test_observeUnlock(t *testing.T) { t.Run("unlock", func(t *testing.T) { g := NewWithT(t) @@ -374,8 +483,8 @@ func Test_observeUnlock(t *testing.T) { expect := release.ObservedToInfo(release.ObserveRelease(rls)) observeUnlock(obj)(rls) - g.Expect(obj.Status.Previous).To(BeNil()) - g.Expect(obj.Status.Current).To(Equal(expect)) + g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.GetCurrent()).To(Equal(expect)) }) t.Run("unlock without current", func(t *testing.T) { @@ -390,7 +499,7 @@ func Test_observeUnlock(t *testing.T) { }) observeUnlock(obj)(rls) - g.Expect(obj.Status.Previous).To(BeNil()) - g.Expect(obj.Status.Current).To(BeNil()) + g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.GetCurrent()).To(BeNil()) }) } diff --git a/internal/reconcile/upgrade.go b/internal/reconcile/upgrade.go index ca8cae42b..8b8b3f0ba 100644 --- a/internal/reconcile/upgrade.go +++ b/internal/reconcile/upgrade.go @@ -18,7 +18,10 @@ package reconcile import ( "context" + "fmt" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "github.com/fluxcd/pkg/runtime/conditions" @@ -26,29 +29,57 @@ import ( v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" ) +// Upgrade is an ActionReconciler which attempts to upgrade a Helm release +// based on the given Request data. +// +// The writes to the Helm storage during the installation process are +// observed, and updates the Status.Current (and possibly Status.Previous) +// field(s). +// +// On upgrade success, the object is marked with Released=True and emits an +// event. In addition, the object is marked with TestSuccess=False if tests +// are enabled to indicate we are awaiting the results. +// On failure, the object is marked with Released=False and emits a warning +// event. Only an error which resulted in a modification to the Helm storage +// counts towards a failure for the active remediation strategy. +// +// At the end of the reconciliation, the Status.Conditions are summarized and +// propagated to the Ready condition on the Request.Object. +// +// The caller is assumed to have verified the integrity of Request.Object using +// e.g. action.VerifyReleaseInfo before calling Reconcile. type Upgrade struct { configFactory *action.ConfigFactory + eventRecorder record.EventRecorder +} + +// NewUpgrade returns a new Upgrade reconciler configured with the provided +// values. +func NewUpgrade(cfg *action.ConfigFactory, recorder record.EventRecorder) *Upgrade { + return &Upgrade{configFactory: cfg, eventRecorder: recorder} } func (r *Upgrade) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.Status.Current.DeepCopy() + cur = req.Object.GetCurrent().DeepCopy() logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.InfoLevel)), 10) cfg = r.configFactory.Build(logBuf.Log, observeRelease(req.Object)) ) - // Run upgrade action. - rls, err := action.Upgrade(ctx, cfg, req.Object, req.Chart, req.Values) + defer summarize(req) + + // Run the Helm upgrade action. + _, err := action.Upgrade(ctx, cfg, req.Object, req.Chart, req.Values) if err != nil { - // Mark failure on object. - conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UpgradeFailedReason, err.Error()) - req.Object.Status.Failures++ + r.failure(req, logBuf, err) // Return error if we did not store a release, as this does not // affect state and the caller should e.g. retry. - if newCur := req.Object.Status.Current; newCur == nil || newCur == cur { + if newCur := req.Object.GetCurrent(); newCur == nil || (cur != nil && newCur.Digest == cur.Digest) { return err } @@ -58,12 +89,11 @@ func (r *Upgrade) Reconcile(ctx context.Context, req *Request) error { // without a new release in storage there is nothing to remediate, // and the action can be retried immediately without causing // storage drift. - req.Object.Status.UpgradeFailures++ + req.Object.GetActiveRemediation().IncrementFailureCount(req.Object) return nil } - // Mark success on object. - conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.UpgradeSucceededReason, rls.Info.Description) + r.success(req) return nil } @@ -74,3 +104,65 @@ func (r *Upgrade) Name() string { func (r *Upgrade) Type() ReconcilerType { return ReconcilerTypeRelease } + +const ( + // fmtUpgradeFailure is the message format for an upgrade failure. + fmtUpgradeFailure = "Upgrade of release %s/%s with chart %s@%s failed: %s" + // fmtUpgradeSuccess is the message format for a successful upgrade. + fmtUpgradeSuccess = "Upgraded release %s with chart %s" +) + +// failure records the failure of a Helm upgrade action in the status of the +// given Request.Object by marking ReleasedCondition=False and increasing the +// failure counter. In addition, it emits a warning event for the +// Request.Object. +// +// Increase of the failure counter for the active remediation strategy should +// be done conditionally by the caller after verifying the failed action has +// modified the Helm storage. This to avoid counting failures which do not +// result in Helm storage drift. +func (r *Upgrade) failure(req *Request, buffer *action.LogBuffer, err error) { + // Compose failure message. + msg := fmt.Sprintf(fmtUpgradeFailure, req.Object.GetReleaseNamespace(), req.Object.GetReleaseName(), req.Chart.Name(), req.Chart.Metadata.Version, err.Error()) + + // Mark upgrade failure on object. + req.Object.Status.Failures++ + conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UpgradeFailedReason, msg) + + // Record warning event, this message contains more data than the + // Condition summary. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(req.Chart.Metadata.Version, chartutil.DigestValues(digest.Canonical, req.Values).String()), + corev1.EventTypeWarning, + v2.UpgradeFailedReason, + eventMessageWithLog(msg, buffer), + ) +} + +// success records the success of a Helm upgrade action in the status of the +// given Request.Object by marking ReleasedCondition=True and emitting an +// event. In addition, it marks TestSuccessCondition=False when tests are +// enabled to indicate we are awaiting test results after having made the +// release. +func (r *Upgrade) success(req *Request) { + // Compose success message. + cur := req.Object.GetCurrent() + msg := fmt.Sprintf(fmtUpgradeSuccess, cur.FullReleaseName(), cur.VersionedChartName()) + + // Mark upgrade success on object. + conditions.MarkTrue(req.Object, v2.ReleasedCondition, v2.UpgradeSucceededReason, msg) + if req.Object.GetTest().Enable && !cur.HasBeenTested() { + conditions.MarkUnknown(req.Object, v2.TestSuccessCondition, "Pending", fmtTestPending, + cur.FullReleaseName(), cur.VersionedChartName()) + } + + // Record event. + r.eventRecorder.AnnotatedEventf( + req.Object, + eventMeta(cur.ChartVersion, cur.ConfigDigest), + corev1.EventTypeNormal, + v2.UpgradeSucceededReason, + msg, + ) +} diff --git a/internal/reconcile/upgrade_test.go b/internal/reconcile/upgrade_test.go index a3669d05a..d7407c28d 100644 --- a/internal/reconcile/upgrade_test.go +++ b/internal/reconcile/upgrade_test.go @@ -18,11 +18,11 @@ package reconcile import ( "context" + "errors" "fmt" "testing" "time" - "github.com/fluxcd/pkg/runtime/conditions" "github.com/go-logr/logr" . "github.com/onsi/gomega" helmchart "helm.sh/helm/v3/pkg/chart" @@ -31,16 +31,24 @@ import ( helmreleaseutil "helm.sh/helm/v3/pkg/releaseutil" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + + eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/chartutil" + "github.com/fluxcd/helm-controller/internal/digest" "github.com/fluxcd/helm-controller/internal/release" "github.com/fluxcd/helm-controller/internal/storage" "github.com/fluxcd/helm-controller/internal/testutil" ) -func Test_upgrade(t *testing.T) { +func TestUpgrade_Reconcile(t *testing.T) { var ( mockCreateErr = fmt.Errorf("storage create error") mockUpdateErr = fmt.Errorf("storage update error") @@ -101,8 +109,8 @@ func Test_upgrade(t *testing.T) { } }, expectConditions: []metav1.Condition{ - *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, - "Upgrade complete"), + *conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, "Upgraded release"), + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Upgraded release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[1])) @@ -131,6 +139,8 @@ func Test_upgrade(t *testing.T) { } }, expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.UpgradeFailedReason, + "post-upgrade hooks failed: 1 error occurred:\n\t* timed out waiting for the condition\n\n"), *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, "post-upgrade hooks failed: 1 error occurred:\n\t* timed out waiting for the condition\n\n"), }, @@ -169,6 +179,8 @@ func Test_upgrade(t *testing.T) { } }, expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.UpgradeFailedReason, + mockCreateErr.Error()), *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, mockCreateErr.Error()), }, @@ -176,7 +188,8 @@ func Test_upgrade(t *testing.T) { return release.ObservedToInfo(release.ObserveRelease(releases[0])) }, expectFailures: 1, - expectUpgradeFailures: 1, + expectUpgradeFailures: 0, + wantErr: mockCreateErr, }, { name: "upgrade failure without storage update", @@ -204,6 +217,8 @@ func Test_upgrade(t *testing.T) { } }, expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.UpgradeFailedReason, + mockUpdateErr.Error()), *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, mockUpdateErr.Error()), }, @@ -236,8 +251,8 @@ func Test_upgrade(t *testing.T) { } }, expectConditions: []metav1.Condition{ - *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, - "Upgrade complete"), + *conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, "Upgraded release"), + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Upgraded release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[1])) @@ -275,8 +290,10 @@ func Test_upgrade(t *testing.T) { } }, expectConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, + "Upgraded release"), *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, - "Upgrade complete"), + "Upgraded release"), }, expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { return release.ObservedToInfo(release.ObserveRelease(releases[2])) @@ -341,7 +358,8 @@ func Test_upgrade(t *testing.T) { cfg.Driver = tt.driver(cfg.Driver) } - got := (&Upgrade{configFactory: cfg}).Reconcile(context.TODO(), &Request{ + recorder := new(record.FakeRecorder) + got := NewUpgrade(cfg, recorder).Reconcile(context.TODO(), &Request{ Object: obj, Chart: tt.chart, Values: tt.values, @@ -358,15 +376,15 @@ func Test_upgrade(t *testing.T) { helmreleaseutil.SortByRevision(releases) if tt.expectCurrent != nil { - g.Expect(obj.Status.Current).To(testutil.Equal(tt.expectCurrent(releases))) + g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releases))) } else { - g.Expect(obj.Status.Current).To(BeNil(), "expected current to be nil") + g.Expect(obj.GetCurrent()).To(BeNil(), "expected current to be nil") } if tt.expectPrevious != nil { - g.Expect(obj.Status.Previous).To(testutil.Equal(tt.expectPrevious(releases))) + g.Expect(obj.GetPrevious()).To(testutil.Equal(tt.expectPrevious(releases))) } else { - g.Expect(obj.Status.Previous).To(BeNil(), "expected previous to be nil") + g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -375,3 +393,143 @@ func Test_upgrade(t *testing.T) { }) } } + +func TestUpgrade_failure(t *testing.T) { + var ( + obj = &v2.HelmRelease{ + Spec: v2.HelmReleaseSpec{ + ReleaseName: mockReleaseName, + TargetNamespace: mockReleaseNamespace, + }, + } + chrt = testutil.BuildChart() + err = errors.New("upgrade error") + ) + + t.Run("records failure", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Upgrade{ + eventRecorder: recorder, + } + + req := &Request{Object: obj.DeepCopy(), Chart: chrt, Values: map[string]interface{}{"foo": "bar"}} + r.failure(req, nil, err) + + expectMsg := fmt.Sprintf(fmtUpgradeFailure, mockReleaseNamespace, mockReleaseName, chrt.Name(), + chrt.Metadata.Version, err.Error()) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, expectMsg), + })) + g.Expect(req.Object.Status.Failures).To(Equal(int64(1))) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: v2.UpgradeFailedReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): chrt.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, req.Values).String(), + }, + }, + }, + })) + }) + + t.Run("records failure with logs", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Upgrade{ + eventRecorder: recorder, + } + req := &Request{Object: obj.DeepCopy(), Chart: chrt} + r.failure(req, mockLogBuffer(5, 10), err) + + expectSubStr := "Last Helm logs" + g.Expect(conditions.IsFalse(req.Object, v2.ReleasedCondition)).To(BeTrue()) + g.Expect(conditions.GetMessage(req.Object, v2.ReleasedCondition)).ToNot(ContainSubstring(expectSubStr)) + + events := recorder.GetEvents() + g.Expect(events).To(HaveLen(1)) + g.Expect(events[0].Message).To(ContainSubstring(expectSubStr)) + }) +} + +func TestUpgrade_success(t *testing.T) { + var ( + cur = testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Chart: testutil.BuildChart(), + }) + obj = &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(cur)), + }, + } + ) + + t.Run("records success", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Upgrade{ + eventRecorder: recorder, + } + + req := &Request{ + Object: obj.DeepCopy(), + } + r.success(req) + + expectMsg := fmt.Sprintf(fmtUpgradeSuccess, + fmt.Sprintf("%s/%s.%d", mockReleaseNamespace, mockReleaseName, obj.Status.Current.Version), + fmt.Sprintf("%s@%s", obj.Status.Current.ChartName, obj.Status.Current.ChartVersion)) + + g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, expectMsg), + })) + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: v2.UpgradeSucceededReason, + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(eventv1.MetaRevisionKey): obj.Status.Current.ChartVersion, + eventMetaGroupKey(eventv1.MetaTokenKey): obj.Status.Current.ConfigDigest, + }, + }, + }, + })) + }) + + t.Run("records success with TestSuccess=False", func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &Upgrade{ + eventRecorder: recorder, + } + + obj := obj.DeepCopy() + obj.Spec.Test = &v2.Test{Enable: true} + + req := &Request{Object: obj} + r.success(req) + + g.Expect(conditions.IsTrue(req.Object, v2.ReleasedCondition)).To(BeTrue()) + + cond := conditions.Get(req.Object, v2.TestSuccessCondition) + g.Expect(cond).ToNot(BeNil()) + + expectMsg := fmt.Sprintf(fmtTestPending, + fmt.Sprintf("%s/%s.%d", mockReleaseNamespace, mockReleaseName, obj.Status.Current.Version), + fmt.Sprintf("%s@%s", obj.Status.Current.ChartName, obj.Status.Current.ChartVersion)) + g.Expect(cond.Message).To(Equal(expectMsg)) + }) +} diff --git a/internal/release/name.go b/internal/release/name.go new file mode 100644 index 000000000..c01b68dfb --- /dev/null +++ b/internal/release/name.go @@ -0,0 +1,46 @@ +/* +Copyright 2022 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 release + +import ( + "crypto/sha256" + "fmt" +) + +// ShortenName returns a short release name in the format of +// '-' for the given name +// if it exceeds 53 characters in length. +// +// The shortening is done by hashing the given release name with +// SHA256 and taking the first 12 characters of the resulting hash. +// The hash is then appended to the release name shortened to 40 +// characters divided by a hyphen separator. +// +// For example: 'some-front-appended-namespace-release-wi-1234567890ab' +// where '1234567890ab' are the first 12 characters of the SHA hash. +func ShortenName(name string) string { + if len(name) <= 53 { + return name + } + + const maxLength = 53 + const shortHashLength = 12 + + sum := fmt.Sprintf("%x", sha256.Sum256([]byte(name))) + shortName := name[:maxLength-(shortHashLength+1)] + "-" + return shortName + sum[:shortHashLength] +} diff --git a/internal/release/name_test.go b/internal/release/name_test.go new file mode 100644 index 000000000..416629d12 --- /dev/null +++ b/internal/release/name_test.go @@ -0,0 +1,55 @@ +/* +Copyright 2022 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 release + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestShortName(t *testing.T) { + g := NewWithT(t) + + tests := []struct { + name string + expected string + }{ + { + name: "release-name", + expected: "release-name", + }, + { + name: "release-name-with-very-long-name-which-is-longer-than-53-characters", + expected: "release-name-with-very-long-name-which-i-788ca0d0d7b0", + }, + { + name: "another-release-name-with-very-long-name-which-is-longer-than-53-characters", + expected: "another-release-name-with-very-long-name-7e72150d5a36", + }, + { + name: "", + expected: "", + }, + } + + for _, tt := range tests { + got := ShortenName(tt.name) + g.Expect(got).To(Equal(tt.expected), got) + g.Expect(got).To(Satisfy(func(s string) bool { return len(s) <= 53 })) + } +} diff --git a/internal/release/util.go b/internal/release/util.go index 23dd51455..5ef10718a 100644 --- a/internal/release/util.go +++ b/internal/release/util.go @@ -32,32 +32,6 @@ func GetTestHooks(rls *helmrelease.Release) map[string]*helmrelease.Hook { return th } -// HasBeenTested returns if any of the test hooks for the given release has -// been started. -func HasBeenTested(rls *helmrelease.Release) bool { - for _, h := range rls.Hooks { - if IsHookForEvent(h, helmrelease.HookTest) { - if !h.LastRun.StartedAt.IsZero() { - return true - } - } - } - return false -} - -// HasFailedTests returns if any of the test hooks for the given release has -// failed. -func HasFailedTests(rls *helmrelease.Release) bool { - for _, h := range rls.Hooks { - if IsHookForEvent(h, helmrelease.HookTest) { - if h.LastRun.Phase == helmrelease.HookPhaseFailed { - return true - } - } - } - return false -} - // IsHookForEvent returns if the given hook fires on the provided event. func IsHookForEvent(hook *helmrelease.Hook, event helmrelease.HookEvent) bool { if hook != nil { diff --git a/internal/release/util_test.go b/internal/release/util_test.go index 529dc14f8..c4555379d 100644 --- a/internal/release/util_test.go +++ b/internal/release/util_test.go @@ -64,26 +64,6 @@ func TestGetTestHooks(t *testing.T) { })) } -func TestHasBeenTested(t *testing.T) { - type args struct { - rls *helmrelease.Release - } - tests := []struct { - name string - args args - want bool - }{ - // TODO: Add test cases. - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := HasBeenTested(tt.args.rls); got != tt.want { - t.Errorf("HasBeenTested() = %v, want %v", got, tt.want) - } - }) - } -} - func TestIsHookForEvent(t *testing.T) { g := NewWithT(t) diff --git a/internal/testutil/fake_recorder.go b/internal/testutil/fake_recorder.go new file mode 100644 index 000000000..24d877fb3 --- /dev/null +++ b/internal/testutil/fake_recorder.go @@ -0,0 +1,116 @@ +/* +Copyright 2022 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 testutil + +import ( + "fmt" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + + _ "k8s.io/client-go/tools/record" +) + +// FakeRecorder is used as a fake during tests. +// +// It was invented to be used in tests which require more precise control over +// e.g. assertions of specific event fields like Reason. For which string +// comparisons on the concentrated event message using record.FakeRecorder is +// not sufficient. +// +// To empty the Events channel into a slice of the recorded events, use +// GetEvents(). Not initializing Events will cause the recorder to not record +// any messages. +type FakeRecorder struct { + Events chan corev1.Event + IncludeObject bool +} + +// NewFakeRecorder creates new fake event recorder with an Events channel with +// the given size. Setting includeObject to true will cause the recorder to +// include the object reference in the events. +// +// To initialize a recorder which does not record any events, simply use: +// +// recorder := new(FakeRecorder) +func NewFakeRecorder(bufferSize int, includeObject bool) *FakeRecorder { + return &FakeRecorder{ + Events: make(chan corev1.Event, bufferSize), + IncludeObject: includeObject, + } +} + +// Event emits an event with the given message. +func (f *FakeRecorder) Event(obj runtime.Object, eventType, reason, message string) { + f.Eventf(obj, eventType, reason, message) +} + +// Eventf emits an event with the given message. +func (f *FakeRecorder) Eventf(obj runtime.Object, eventType, reason, message string, args ...any) { + if f.Events != nil { + f.Events <- f.generateEvent(obj, nil, eventType, reason, message, args...) + } +} + +// AnnotatedEventf emits an event with annotations. +func (f *FakeRecorder) AnnotatedEventf(obj runtime.Object, annotations map[string]string, eventType, reason, message string, args ...any) { + if f.Events != nil { + f.Events <- f.generateEvent(obj, annotations, eventType, reason, message, args...) + } +} + +// GetEvents empties the Events channel and returns a slice of recorded events. +// If the Events channel is nil, it returns nil. +func (f *FakeRecorder) GetEvents() (events []corev1.Event) { + if f.Events != nil { + for { + select { + case e := <-f.Events: + events = append(events, e) + default: + return events + } + } + } + return nil +} + +// generateEvent generates a new mocked event with the given parameters. +func (f *FakeRecorder) generateEvent(obj runtime.Object, annotations map[string]string, eventType, reason, message string, args ...any) corev1.Event { + event := corev1.Event{ + InvolvedObject: objectReference(obj, f.IncludeObject), + Type: eventType, + Reason: reason, + Message: fmt.Sprintf(message, args...), + } + if annotations != nil { + event.ObjectMeta.Annotations = annotations + } + return event +} + +// objectReference returns an object reference for the given object with the +// kind and (group) API version set. +func objectReference(obj runtime.Object, includeObject bool) corev1.ObjectReference { + if !includeObject { + return corev1.ObjectReference{} + } + + return corev1.ObjectReference{ + Kind: obj.GetObjectKind().GroupVersionKind().Kind, + APIVersion: obj.GetObjectKind().GroupVersionKind().GroupVersion().String(), + } +}