diff --git a/api/v2beta2/helmrelease_types.go b/api/v2beta2/helmrelease_types.go index 2959711e5..b742d9314 100644 --- a/api/v2beta2/helmrelease_types.go +++ b/api/v2beta2/helmrelease_types.go @@ -858,16 +858,17 @@ type HelmReleaseStatus struct { HelmChart string `json:"helmChart,omitempty"` // StorageNamespace is the namespace of the Helm release storage for the - // Current release. + // current release. // +kubebuilder:validation:MaxLength=63 // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:Optional // +optional StorageNamespace string `json:"storageNamespace,omitempty"` - // History holds the history of Helm releases performed for this HelmRelease. + // History holds the history of Helm releases performed for this HelmRelease + // up to the last successfully completed release. // +optional - History ReleaseHistory `json:"history,omitempty"` + History Snapshots `json:"history,omitempty"` // LastAttemptedReleaseAction is the last release action performed for this // HelmRelease. It is used to determine the active remediation strategy. @@ -909,6 +910,18 @@ type HelmReleaseStatus struct { meta.ReconcileRequestStatus `json:",inline"` } +// ClearHistory clears the History. +func (in *HelmReleaseStatus) ClearHistory() { + in.History = nil +} + +// ClearFailures clears the failure counters. +func (in *HelmReleaseStatus) ClearFailures() { + in.Failures = 0 + in.InstallFailures = 0 + in.UpgradeFailures = 0 +} + // GetHelmChart returns the namespace and name of the HelmChart. func (in HelmReleaseStatus) GetHelmChart() (string, string) { if in.HelmChart == "" { @@ -920,22 +933,6 @@ func (in HelmReleaseStatus) GetHelmChart() (string, string) { return "", "" } -// ReleaseHistory holds the latest observed Snapshot for the current release, -// and the previous (successful) release. The previous release is only -// populated if the current release is not the first release, and if the -// previous release was successful. This is to prevent the previous release -// from being populated with a failed release. -type ReleaseHistory struct { - // Current holds the latest observed Snapshot for the current release. - // +optional - Current *Snapshot `json:"current,omitempty"` - - // Previous holds the latest observed Snapshot for the previous - // (successful) release. - // +optional - Previous *Snapshot `json:"previous,omitempty"` -} - const ( // SourceIndexKey is the key used for indexing HelmReleases based on // their sources. @@ -1006,35 +1003,6 @@ func (in *HelmRelease) GetUninstall() Uninstall { return *in.Spec.Uninstall } -// GetCurrent returns the current Helm release as observed by the controller. -func (in HelmRelease) GetCurrent() *Snapshot { - if in.HasCurrent() { - return in.Status.History.Current - } - return nil -} - -// HasCurrent returns true if the HelmRelease has a current observed Helm -// release. -func (in HelmRelease) HasCurrent() bool { - return in.Status.History.Current != nil -} - -// GetPrevious returns the previous successful Helm release made by the -// controller. -func (in HelmRelease) GetPrevious() *Snapshot { - if in.HasPrevious() { - return in.Status.History.Previous - } - return nil -} - -// HasPrevious returns true if the HelmRelease has a previous successful Helm -// release. -func (in HelmRelease) HasPrevious() bool { - return in.Status.History.Previous != nil -} - // GetActiveRemediation returns the active Remediation configuration for the // HelmRelease. func (in HelmRelease) GetActiveRemediation() Remediation { diff --git a/api/v2beta2/snapshot_types.go b/api/v2beta2/snapshot_types.go index be81c60c0..9e2043f55 100644 --- a/api/v2beta2/snapshot_types.go +++ b/api/v2beta2/snapshot_types.go @@ -18,10 +18,95 @@ package v2beta2 import ( "fmt" + "sort" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + // snapshotStatusDeployed indicates that the release the snapshot was taken + // from is currently deployed. + snapshotStatusDeployed = "deployed" + // snapshotStatusSuperseded indicates that the release the snapshot was taken + // from has been superseded by a newer release. + snapshotStatusSuperseded = "superseded" + + // snapshotTestPhaseFailed indicates that the test of the release the snapshot + // was taken from has failed. + snapshotTestPhaseFailed = "Failed" +) + +// Snapshots is a list of Snapshot objects. +type Snapshots []*Snapshot + +// Len returns the number of Snapshots. +func (in Snapshots) Len() int { + return len(in) +} + +// SortByVersion sorts the Snapshots by version, in descending order. +func (in Snapshots) SortByVersion() { + sort.Slice(in, func(i, j int) bool { + return in[i].Version > in[j].Version + }) +} + +// Latest returns the most recent Snapshot. +func (in Snapshots) Latest() *Snapshot { + if len(in) == 0 { + return nil + } + in.SortByVersion() + return in[0] +} + +// Previous returns the most recent Snapshot before the Latest that has a +// status of "deployed" or "superseded", or nil if there is no such Snapshot. +// Unless ignoreTests is true, Snapshots with a test in the "Failed" phase are +// ignored. +func (in Snapshots) Previous(ignoreTests bool) *Snapshot { + if len(in) < 2 { + return nil + } + in.SortByVersion() + for i := range in[1:] { + s := in[i+1] + if s.Status == snapshotStatusDeployed || s.Status == snapshotStatusSuperseded { + if ignoreTests || !s.HasTestInPhase(snapshotTestPhaseFailed) { + return s + } + } + } + return nil +} + +// KeepLatest removes all Snapshots except the most recent. +func (in *Snapshots) KeepLatest() { + if in.Len() == 0 { + return + } + in.SortByVersion() + *in = (*in)[:1] +} + +// KeepPrevious removes all Snapshots up to the Previous deployed Snapshot. +// If there is no previous-deployed Snapshot, no Snapshots are removed. +func (in *Snapshots) KeepPrevious(ignoreTests bool) { + if in.Len() < 2 { + return + } + in.SortByVersion() + for i := range (*in)[1:] { + s := (*in)[i+1] + if s.Status == snapshotStatusDeployed || s.Status == snapshotStatusSuperseded { + if ignoreTests || !s.HasTestInPhase(snapshotTestPhaseFailed) { + *in = (*in)[:i+2] + return + } + } + } +} + // Snapshot captures a point-in-time copy of the status information for a Helm release, // as managed by the controller. type Snapshot struct { @@ -76,12 +161,18 @@ type Snapshot struct { // FullReleaseName returns the full name of the release in the format // of '/. func (in *Snapshot) FullReleaseName() string { + if in == nil { + return "" + } return fmt.Sprintf("%s/%s.v%d", in.Namespace, in.Name, in.Version) } // VersionedChartName returns the full name of the chart in the format of // '@'. func (in *Snapshot) VersionedChartName() string { + if in == nil { + return "" + } return fmt.Sprintf("%s@%s", in.ChartName, in.ChartVersion) } @@ -93,7 +184,7 @@ func (in *Snapshot) HasBeenTested() bool { // GetTestHooks returns the TestHooks for the release if not nil. func (in *Snapshot) GetTestHooks() map[string]*TestHookStatus { - if in == nil { + if in == nil || in.TestHooks == nil { return nil } return *in.TestHooks @@ -113,9 +204,20 @@ func (in *Snapshot) HasTestInPhase(phase string) bool { // SetTestHooks sets the TestHooks for the release. func (in *Snapshot) SetTestHooks(hooks map[string]*TestHookStatus) { + if in == nil || hooks == nil { + return + } in.TestHooks = &hooks } +// Targets returns true if the Snapshot targets the given release data. +func (in *Snapshot) Targets(name, namespace string, version int) bool { + if in != nil { + return in.Name == name && in.Namespace == namespace && in.Version == version + } + return false +} + // TestHookStatus holds the status information for a test hook as observed // to be run by the controller. type TestHookStatus struct { diff --git a/api/v2beta2/snapshot_types_test.go b/api/v2beta2/snapshot_types_test.go new file mode 100644 index 000000000..f347f51e6 --- /dev/null +++ b/api/v2beta2/snapshot_types_test.go @@ -0,0 +1,314 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v2beta2 + +import ( + "reflect" + "testing" +) + +func TestSnapshots_Sort(t *testing.T) { + tests := []struct { + name string + in Snapshots + want Snapshots + }{ + { + name: "sorts by descending version", + in: Snapshots{ + {Version: 1}, + {Version: 3}, + {Version: 2}, + }, + want: Snapshots{ + {Version: 3}, + {Version: 2}, + {Version: 1}, + }, + }, + { + name: "already sorted", + in: Snapshots{ + {Version: 3}, + {Version: 2}, + {Version: 1}, + }, + want: Snapshots{ + {Version: 3}, + {Version: 2}, + {Version: 1}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.in.SortByVersion() + + if !reflect.DeepEqual(tt.in, tt.want) { + t.Errorf("SortByVersion() got %v, want %v", tt.in, tt.want) + } + }) + } +} + +func TestSnapshots_Latest(t *testing.T) { + tests := []struct { + name string + in Snapshots + want *Snapshot + }{ + { + name: "returns most recent snapshot", + in: Snapshots{ + {Version: 1}, + {Version: 3}, + {Version: 2}, + }, + want: &Snapshot{Version: 3}, + }, + { + name: "returns nil if empty", + in: Snapshots{}, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.in.Latest(); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Latest() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestSnapshots_Previous(t *testing.T) { + tests := []struct { + name string + in Snapshots + ignoreTests bool + want *Snapshot + }{ + { + name: "returns previous snapshot", + in: Snapshots{ + {Version: 2, Status: "deployed"}, + {Version: 3, Status: "failed"}, + {Version: 1, Status: "superseded"}, + }, + want: &Snapshot{Version: 2, Status: "deployed"}, + }, + { + name: "includes snapshots with failed tests", + in: Snapshots{ + {Version: 4, Status: "deployed"}, + {Version: 1, Status: "superseded"}, + {Version: 2, Status: "superseded"}, + {Version: 3, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "test": {Phase: "Failed"}, + }}, + }, + ignoreTests: true, + want: &Snapshot{Version: 3, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "test": {Phase: "Failed"}, + }}, + }, + { + name: "ignores snapshots with failed tests", + in: Snapshots{ + {Version: 4, Status: "deployed"}, + {Version: 1, Status: "superseded"}, + {Version: 2, Status: "superseded"}, + {Version: 3, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "test": {Phase: "Failed"}, + }}, + }, + ignoreTests: false, + want: &Snapshot{Version: 2, Status: "superseded"}, + }, + { + name: "returns nil without previous snapshot", + in: Snapshots{ + {Version: 1, Status: "deployed"}, + }, + want: nil, + }, + { + name: "returns nil without snapshot matching criteria", + in: Snapshots{ + {Version: 4, Status: "deployed"}, + {Version: 3, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "test": {Phase: "Failed"}, + }}, + }, + ignoreTests: false, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.in.Previous(tt.ignoreTests); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Previous() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestSnapshots_KeepLatest(t *testing.T) { + tests := []struct { + name string + in Snapshots + want Snapshots + }{ + { + name: "keeps latest snapshot", + in: Snapshots{ + {Version: 1, Status: "superseded"}, + {Version: 2, Status: "superseded"}, + {Version: 3, Status: "deployed"}, + }, + want: Snapshots{ + {Version: 3, Status: "deployed"}, + }, + }, + { + name: "without latest snapshot", + in: nil, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.in.KeepLatest() + + if !reflect.DeepEqual(tt.in, tt.want) { + t.Errorf("KeepLatest() got %v, want %v", tt.in, tt.want) + } + }) + } +} + +func TestSnapshots_KeepPrevious(t *testing.T) { + tests := []struct { + name string + in Snapshots + ignoreTests bool + want Snapshots + }{ + { + name: "keeps previous snapshot", + in: Snapshots{ + {Version: 1, Status: "superseded"}, + {Version: 3, Status: "failed"}, + {Version: 2, Status: "superseded"}, + {Version: 4, Status: "deployed"}, + }, + want: Snapshots{ + {Version: 4, Status: "deployed"}, + {Version: 3, Status: "failed"}, + {Version: 2, Status: "superseded"}, + }, + }, + { + name: "ignores snapshots with failed tests", + in: Snapshots{ + {Version: 4, Status: "deployed"}, + {Version: 3, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "upgrade-test-fail-podinfo-fault-test-tiz9x": {Phase: "Failed"}, + "upgrade-test-fail-podinfo-grpc-test-gddcw": {}, + }}, + {Version: 2, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "upgrade-test-fail-podinfo-grpc-test-h0tc2": { + Phase: "Succeeded", + }, + "upgrade-test-fail-podinfo-jwt-test-vzusa": { + Phase: "Succeeded", + }, + "upgrade-test-fail-podinfo-service-test-b647e": { + Phase: "Succeeded", + }, + }}, + }, + ignoreTests: false, + want: Snapshots{ + {Version: 4, Status: "deployed"}, + {Version: 3, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "upgrade-test-fail-podinfo-fault-test-tiz9x": {Phase: "Failed"}, + "upgrade-test-fail-podinfo-grpc-test-gddcw": {}, + }}, + {Version: 2, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "upgrade-test-fail-podinfo-grpc-test-h0tc2": { + Phase: "Succeeded", + }, + "upgrade-test-fail-podinfo-jwt-test-vzusa": { + Phase: "Succeeded", + }, + "upgrade-test-fail-podinfo-service-test-b647e": { + Phase: "Succeeded", + }, + }}, + }, + }, + { + name: "keeps previous snapshot with failed tests", + in: Snapshots{ + {Version: 4, Status: "deployed"}, + {Version: 3, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "upgrade-test-fail-podinfo-fault-test-tiz9x": {Phase: "Failed"}, + "upgrade-test-fail-podinfo-grpc-test-gddcw": {}, + }}, + {Version: 2, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "upgrade-test-fail-podinfo-grpc-test-h0tc2": { + Phase: "Succeeded", + }, + "upgrade-test-fail-podinfo-jwt-test-vzusa": { + Phase: "Succeeded", + }, + "upgrade-test-fail-podinfo-service-test-b647e": { + Phase: "Succeeded", + }, + }}, + {Version: 1, Status: "superseded"}, + }, + ignoreTests: true, + want: Snapshots{ + {Version: 4, Status: "deployed"}, + {Version: 3, Status: "superseded", TestHooks: &map[string]*TestHookStatus{ + "upgrade-test-fail-podinfo-fault-test-tiz9x": {Phase: "Failed"}, + "upgrade-test-fail-podinfo-grpc-test-gddcw": {}, + }}, + }, + }, + { + name: "without previous snapshot", + in: Snapshots{ + {Version: 1, Status: "deployed"}, + }, + want: Snapshots{ + {Version: 1, Status: "deployed"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.in.KeepPrevious(tt.ignoreTests) + + if !reflect.DeepEqual(tt.in, tt.want) { + t.Errorf("KeepPrevious() got %v, want %v", tt.in, tt.want) + } + }) + } +} diff --git a/api/v2beta2/zz_generated.deepcopy.go b/api/v2beta2/zz_generated.deepcopy.go index fe38578f4..a2f3b8f65 100644 --- a/api/v2beta2/zz_generated.deepcopy.go +++ b/api/v2beta2/zz_generated.deepcopy.go @@ -313,7 +313,17 @@ func (in *HelmReleaseStatus) DeepCopyInto(out *HelmReleaseStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - in.History.DeepCopyInto(&out.History) + if in.History != nil { + in, out := &in.History, &out.History + *out = make(Snapshots, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Snapshot) + (*in).DeepCopyInto(*out) + } + } + } out.ReconcileRequestStatus = in.ReconcileRequestStatus } @@ -438,31 +448,6 @@ func (in *PostRenderer) DeepCopy() *PostRenderer { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ReleaseHistory) DeepCopyInto(out *ReleaseHistory) { - *out = *in - if in.Current != nil { - in, out := &in.Current, &out.Current - *out = new(Snapshot) - (*in).DeepCopyInto(*out) - } - if in.Previous != nil { - in, out := &in.Previous, &out.Previous - *out = new(Snapshot) - (*in).DeepCopyInto(*out) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ReleaseHistory. -func (in *ReleaseHistory) DeepCopy() *ReleaseHistory { - if in == nil { - return nil - } - out := new(ReleaseHistory) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Rollback) DeepCopyInto(out *Rollback) { *out = *in @@ -520,6 +505,31 @@ func (in *Snapshot) DeepCopy() *Snapshot { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in Snapshots) DeepCopyInto(out *Snapshots) { + { + in := &in + *out = make(Snapshots, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Snapshot) + (*in).DeepCopyInto(*out) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Snapshots. +func (in Snapshots) DeepCopy() Snapshots { + if in == nil { + return nil + } + out := new(Snapshots) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Test) DeepCopyInto(out *Test) { *out = *in diff --git a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml index 82bc7e067..e008803ce 100644 --- a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml +++ b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml @@ -1809,185 +1809,95 @@ spec: type: string history: description: History holds the history of Helm releases performed - for this HelmRelease. - properties: - current: - description: Current holds the latest observed Snapshot for the - current release. - properties: - apiVersion: - description: 'APIVersion is the API version of the Snapshot. - Provisional: when the calculation method of the Digest field - is changed, this field will be used to distinguish between - the old and new methods.' - type: string - chartName: - description: ChartName is the chart name of the release object - in storage. - type: string - chartVersion: - description: ChartVersion is the chart version of the release - object in storage. - type: string - configDigest: - description: ConfigDigest is the checksum of the config (better - known as "values") of the release object in storage. It - has the format of `:`. - type: string - deleted: - description: Deleted is when the release was deleted. - format: date-time - type: string - digest: - description: Digest is the checksum of the release object - in storage. It has the format of `:`. - type: string - firstDeployed: - description: FirstDeployed is when the release was first deployed. - format: date-time - type: string - lastDeployed: - description: LastDeployed is when the release was last deployed. - format: date-time - type: string - name: - description: Name is the name of the release. - type: string - namespace: - description: Namespace is the namespace the release is deployed - to. - type: string - status: - description: Status is the current state of the release. - type: string - testHooks: - additionalProperties: - description: TestHookStatus holds the status information - for a test hook as observed to be run by the controller. - properties: - lastCompleted: - description: LastCompleted is the time the test hook - last completed. - format: date-time - type: string - lastStarted: - description: LastStarted is the time the test hook was - last started. - format: date-time - type: string - phase: - description: Phase the test hook was observed to be - in. - type: string - type: object - description: TestHooks is the list of test hooks for the release - as observed to be run by the controller. - type: object - version: - description: Version is the version of the release object - in storage. - type: integer - required: - - chartName - - chartVersion - - configDigest - - digest - - firstDeployed - - lastDeployed - - name - - namespace - - status - - version - type: object - previous: - description: Previous holds the latest observed Snapshot for the - previous (successful) release. - properties: - apiVersion: - description: 'APIVersion is the API version of the Snapshot. - Provisional: when the calculation method of the Digest field - is changed, this field will be used to distinguish between - the old and new methods.' - type: string - chartName: - description: ChartName is the chart name of the release object - in storage. - type: string - chartVersion: - description: ChartVersion is the chart version of the release - object in storage. - type: string - configDigest: - description: ConfigDigest is the checksum of the config (better - known as "values") of the release object in storage. It - has the format of `:`. - type: string - deleted: - description: Deleted is when the release was deleted. - format: date-time - type: string - digest: - description: Digest is the checksum of the release object - in storage. It has the format of `:`. - type: string - firstDeployed: - description: FirstDeployed is when the release was first deployed. - format: date-time - type: string - lastDeployed: - description: LastDeployed is when the release was last deployed. - format: date-time - type: string - name: - description: Name is the name of the release. - type: string - namespace: - description: Namespace is the namespace the release is deployed - to. - type: string - status: - description: Status is the current state of the release. - type: string - testHooks: - additionalProperties: - description: TestHookStatus holds the status information - for a test hook as observed to be run by the controller. - properties: - lastCompleted: - description: LastCompleted is the time the test hook - last completed. - format: date-time - type: string - lastStarted: - description: LastStarted is the time the test hook was - last started. - format: date-time - type: string - phase: - description: Phase the test hook was observed to be - in. - type: string - type: object - description: TestHooks is the list of test hooks for the release - as observed to be run by the controller. + for this HelmRelease up to the last successfully completed release. + items: + description: Snapshot captures a point-in-time copy of the status + information for a Helm release, as managed by the controller. + properties: + apiVersion: + description: 'APIVersion is the API version of the Snapshot. + Provisional: when the calculation method of the Digest field + is changed, this field will be used to distinguish between + the old and new methods.' + type: string + chartName: + description: ChartName is the chart name of the release object + in storage. + type: string + chartVersion: + description: ChartVersion is the chart version of the release + object in storage. + type: string + configDigest: + description: ConfigDigest is the checksum of the config (better + known as "values") of the release object in storage. It has + the format of `:`. + type: string + deleted: + description: Deleted is when the release was deleted. + format: date-time + type: string + digest: + description: Digest is the checksum of the release object in + storage. It has the format of `:`. + type: string + firstDeployed: + description: FirstDeployed is when the release was first deployed. + format: date-time + type: string + lastDeployed: + description: LastDeployed is when the release was last deployed. + format: date-time + type: string + name: + description: Name is the name of the release. + type: string + namespace: + description: Namespace is the namespace the release is deployed + to. + type: string + status: + description: Status is the current state of the release. + type: string + testHooks: + additionalProperties: + description: TestHookStatus holds the status information for + a test hook as observed to be run by the controller. + properties: + lastCompleted: + description: LastCompleted is the time the test hook last + completed. + format: date-time + type: string + lastStarted: + description: LastStarted is the time the test hook was + last started. + format: date-time + type: string + phase: + description: Phase the test hook was observed to be in. + type: string type: object - version: - description: Version is the version of the release object - in storage. - type: integer - required: - - chartName - - chartVersion - - configDigest - - digest - - firstDeployed - - lastDeployed - - name - - namespace - - status - - version - type: object - type: object + description: TestHooks is the list of test hooks for the release + as observed to be run by the controller. + type: object + version: + description: Version is the version of the release object in + storage. + type: integer + required: + - chartName + - chartVersion + - configDigest + - digest + - firstDeployed + - lastDeployed + - name + - namespace + - status + - version + type: object + type: array installFailures: description: InstallFailures is the install failure count against the latest desired state. It is reset after a successful reconciliation. @@ -2030,7 +1940,7 @@ spec: type: integer storageNamespace: description: StorageNamespace is the namespace of the Helm release - storage for the Current release. + storage for the current release. maxLength: 63 minLength: 1 type: string diff --git a/docs/api/v2beta2/helm.md b/docs/api/v2beta2/helm.md index f1a65297e..d40960150 100644 --- a/docs/api/v2beta2/helm.md +++ b/docs/api/v2beta2/helm.md @@ -1292,21 +1292,22 @@ string (Optional)

StorageNamespace is the namespace of the Helm release storage for the -Current release.

+current release.

history
- -ReleaseHistory + +Snapshots (Optional) -

History holds the history of Helm releases performed for this HelmRelease.

+

History holds the history of Helm releases performed for this HelmRelease +up to the last successfully completed release.

@@ -1776,60 +1777,6 @@ Kustomize HelmReleaseStatus)

ReleaseAction is the action to perform a Helm release.

-

ReleaseHistory -

-

-(Appears on: -HelmReleaseStatus) -

-

ReleaseHistory holds the latest observed Snapshot for the current release, -and the previous (successful) release. The previous release is only -populated if the current release is not the first release, and if the -previous release was successful. This is to prevent the previous release -from being populated with a failed release.

-
-
- - - - - - - - - - - - - - - - - -
FieldDescription
-current
- - -Snapshot - - -
-(Optional) -

Current holds the latest observed Snapshot for the current release.

-
-previous
- - -Snapshot - - -
-(Optional) -

Previous holds the latest observed Snapshot for the previous -(successful) release.

-
-
-

Remediation

Remediation defines a consistent interface for InstallRemediation and @@ -1957,10 +1904,6 @@ rollback action when it fails.

Snapshot

-

-(Appears on: -ReleaseHistory) -

Snapshot captures a point-in-time copy of the status information for a Helm release, as managed by the controller.

@@ -2138,6 +2081,13 @@ run by the controller.

+

Snapshots +([]*./api/v2beta2.Snapshot alias)

+

+(Appears on: +HelmReleaseStatus) +

+

Snapshots is a list of Snapshot objects.

Test

diff --git a/internal/action/verify.go b/internal/action/verify.go index 23e6045e1..23645962c 100644 --- a/internal/action/verify.go +++ b/internal/action/verify.go @@ -43,11 +43,12 @@ var ( // 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. +// target as recorded in the Status.History of the object, 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() + cur := obj.Status.History.Latest() switch { case obj.Status.StorageNamespace == "", cur == nil: return false @@ -166,7 +167,7 @@ func VerifyReleaseObject(snapshot *v2.Snapshot, rls *helmrelease.Release) error } // VerifyRelease verifies that the data of the given release matches the given -// chart metadata, and the provided values match the Current.ConfigDigest. +// chart metadata, and the provided values match the Snapshot.ConfigDigest. // It returns either an error of type ErrReleaseNotFound, ErrChartChanged or // ErrConfigDigest, or nil. func VerifyRelease(rls *helmrelease.Release, snapshot *v2.Snapshot, chrt *helmchart.Metadata, vals helmchartutil.Values) error { diff --git a/internal/action/verify_test.go b/internal/action/verify_test.go index c27868a52..80caab639 100644 --- a/internal/action/verify_test.go +++ b/internal/action/verify_test.go @@ -57,8 +57,8 @@ func TestReleaseTargetChanged(t *testing.T) { chartName: defaultChartName, spec: v2.HelmReleaseSpec{}, status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: defaultName, Namespace: defaultNamespace, ChartName: defaultChartName, @@ -75,8 +75,8 @@ func TestReleaseTargetChanged(t *testing.T) { ReleaseName: defaultReleaseName, }, status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: defaultReleaseName, Namespace: defaultNamespace, ChartName: defaultChartName, @@ -90,9 +90,7 @@ func TestReleaseTargetChanged(t *testing.T) { spec: v2.HelmReleaseSpec{}, status: v2.HelmReleaseStatus{ StorageNamespace: defaultNamespace, - History: v2.ReleaseHistory{ - Current: nil, - }, + History: nil, }, want: false, }, @@ -103,8 +101,8 @@ func TestReleaseTargetChanged(t *testing.T) { StorageNamespace: defaultStorageNamespace, }, status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: defaultName, Namespace: defaultNamespace, ChartName: defaultChartName, @@ -121,8 +119,8 @@ func TestReleaseTargetChanged(t *testing.T) { TargetNamespace: defaultTargetNamespace, }, status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: defaultName, Namespace: defaultNamespace, ChartName: defaultChartName, @@ -139,8 +137,8 @@ func TestReleaseTargetChanged(t *testing.T) { ReleaseName: defaultReleaseName, }, status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: defaultName, Namespace: defaultNamespace, ChartName: defaultChartName, @@ -155,8 +153,8 @@ func TestReleaseTargetChanged(t *testing.T) { chartName: "other-chart", spec: v2.HelmReleaseSpec{}, status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: defaultName, Namespace: defaultNamespace, ChartName: defaultChartName, @@ -173,8 +171,8 @@ func TestReleaseTargetChanged(t *testing.T) { TargetNamespace: "target-namespace-exceeding-max-characters", }, status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: "target-namespace-exceeding-max-character-eceb26601388", Namespace: "target-namespace-exceeding-max-characters", ChartName: defaultChartName, @@ -191,8 +189,8 @@ func TestReleaseTargetChanged(t *testing.T) { TargetNamespace: "target-namespace-exceeding-max-characters", }, status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: defaultName, Namespace: "target-namespace-exceeding-max-characters", ChartName: defaultChartName, diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index 2e98bc761..914d546aa 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -286,10 +286,11 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe // fail due to resources already existing. if action.ReleaseTargetChanged(obj, loadedChart.Name()) { log.Info("release target configuration changed: running uninstall for current release") - if err = r.reconcileUninstall(ctx, getter, obj); err != nil && !errors.Is(err, intreconcile.ErrNoCurrent) { + if err = r.reconcileUninstall(ctx, getter, obj); err != nil && !errors.Is(err, intreconcile.ErrNoLatest) { return ctrl.Result{}, err } - obj.Status.History = v2.ReleaseHistory{} + obj.Status.ClearHistory() + obj.Status.ClearFailures() obj.Status.StorageNamespace = "" return ctrl.Result{Requeue: true}, nil } @@ -299,9 +300,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe // Reset the failure count if the chart or values have changed. if action.MustResetFailures(obj, loadedChart.Metadata, values) { - obj.Status.InstallFailures = 0 - obj.Status.UpgradeFailures = 0 - obj.Status.Failures = 0 + obj.Status.ClearFailures() } // Set last attempt values. @@ -438,7 +437,7 @@ func (r *HelmReleaseReconciler) reconcileReleaseDeletion(ctx context.Context, ob } // Attempt to uninstall the release. - if err = r.reconcileUninstall(ctx, getter, obj); err != nil && !errors.Is(err, intreconcile.ErrNoCurrent) { + if err = r.reconcileUninstall(ctx, getter, obj); err != nil && !errors.Is(err, intreconcile.ErrNoLatest) { return err } if err == nil { @@ -446,7 +445,7 @@ func (r *HelmReleaseReconciler) reconcileReleaseDeletion(ctx context.Context, ob } // Truncate the current release details in the status. - obj.Status.History = v2.ReleaseHistory{} + obj.Status.ClearHistory() obj.Status.StorageNamespace = "" return nil diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index 0abdb5d9a..4a9585bc9 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/internal/controller/helmrelease_controller_test.go @@ -474,8 +474,8 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { StorageNamespace: "other", }, Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: "mock", Namespace: "mock", }, @@ -496,7 +496,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { })) // Verify history and storage namespace are cleared. - g.Expect(obj.Status.History.Current).To(BeNil()) + g.Expect(obj.Status.History).To(BeNil()) g.Expect(obj.Status.StorageNamespace).To(BeEmpty()) }) @@ -694,8 +694,8 @@ func TestHelmReleaseReconciler_reconcileDelete(t *testing.T) { }, Status: v2.HelmReleaseStatus{ StorageNamespace: ns.Name, - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(rls)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(rls)), }, HelmChart: hc.Namespace + "/" + hc.Name, }, @@ -803,8 +803,8 @@ func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { }, Status: v2.HelmReleaseStatus{ StorageNamespace: ns.Name, - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(rls)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(rls)), }, }, } @@ -831,7 +831,7 @@ func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { // Verify status of Helm release has been updated. g.Expect(obj.Status.StorageNamespace).To(BeEmpty()) - g.Expect(obj.Status.History).To(Equal(v2.ReleaseHistory{})) + g.Expect(obj.Status.History).To(BeNil()) // Verify Helm release has been uninstalled. _, err = store.History(rls.Name) @@ -865,8 +865,8 @@ func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { }, Status: v2.HelmReleaseStatus{ StorageNamespace: ns.Name, - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(rls)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(rls)), }, }, } @@ -897,7 +897,7 @@ func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { // Verify status of Helm release has not been updated. g.Expect(obj.Status.StorageNamespace).ToNot(BeEmpty()) - g.Expect(obj.GetCurrent()).ToNot(BeNil()) + g.Expect(obj.Status.History.Latest()).ToNot(BeNil()) // Verify Helm release has not been uninstalled. _, err = store.History(rls.Name) @@ -965,8 +965,8 @@ func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { }, Status: v2.HelmReleaseStatus{ StorageNamespace: ns.Name, - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(rls)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(rls)), }, }, } @@ -993,7 +993,7 @@ func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { // Verify status of Helm release has not been updated. g.Expect(obj.Status.StorageNamespace).ToNot(BeEmpty()) - g.Expect(obj.GetCurrent()).ToNot(BeNil()) + g.Expect(obj.Status.History.Latest()).ToNot(BeNil()) // Verify Helm release has not been uninstalled. _, err = store.History(rls.Name) @@ -1070,8 +1070,8 @@ func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { }, Status: v2.HelmReleaseStatus{ StorageNamespace: "mock", - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{}, + History: v2.Snapshots{ + {}, }, }, } @@ -1084,7 +1084,7 @@ func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { })) }) - t.Run("ignores ErrNoCurrent when uninstalling Helm release", func(t *testing.T) { + t.Run("ignores ErrNoLatest when uninstalling Helm release", func(t *testing.T) { g := NewWithT(t) obj := &v2.HelmRelease{ @@ -1179,7 +1179,7 @@ func TestHelmReleaseReconciler_reconcileUninstall(t *testing.T) { // We do not care about the result of the uninstall, only that it was attempted. err := (&HelmReleaseReconciler{}).reconcileUninstall(context.TODO(), getter, obj) g.Expect(err).To(HaveOccurred()) - g.Expect(errors.Is(err, intreconcile.ErrNoCurrent)).To(BeTrue()) + g.Expect(errors.Is(err, intreconcile.ErrNoLatest)).To(BeTrue()) }) t.Run("error on empty storage namespace", func(t *testing.T) { diff --git a/internal/reconcile/atomic_release.go b/internal/reconcile/atomic_release.go index 5271cc52f..d00962ea5 100644 --- a/internal/reconcile/atomic_release.go +++ b/internal/reconcile/atomic_release.go @@ -61,12 +61,12 @@ var ( // 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 +// determination. It determines the next action 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 +// 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. // @@ -172,12 +172,6 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error { return fmt.Errorf("cannot determine release state: %w", err) } - // Conditionally reset the history if we are in a state where we - // need to start over. - if state.MustResetHistory() { - req.Object.Status.History = v2.ReleaseHistory{} - } - // Determine the next action to run based on the current state. log.V(logger.DebugLevel).Info("determining next Helm action based on current state") if next, err = r.actionForState(ctx, req, state); err != nil { @@ -263,6 +257,10 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state switch state.Status { case ReleaseStatusInSync: log.Info("release in-sync with desired state") + + // Remove all history up to the current revision. + req.Object.Status.History.KeepLatest() + return nil, nil case ReleaseStatusLocked: log.Info(msgWithReason("release locked", state.Reason)) @@ -277,6 +275,10 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state return NewInstall(r.configFactory, r.eventRecorder), nil case ReleaseStatusUnmanaged: log.Info(msgWithReason("release not managed by object", state.Reason)) + + // Clear the history as we can no longer rely on it. + req.Object.Status.ClearHistory() + return NewUpgrade(r.configFactory, r.eventRecorder), nil case ReleaseStatusOutOfSync: log.Info(msgWithReason("release out-of-sync with desired state", state.Reason)) @@ -317,11 +319,16 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state return nil, fmt.Errorf("%w: cannot remediate failed release", ErrExceededMaxRetries) } + // Reset the history up to the point where the failure occurred. + // This ensures we do not accumulate a long history of failures. + req.Object.Status.History.KeepPrevious(remediation.MustIgnoreTestFailures(req.Object.GetTest().IgnoreFailures)) + switch remediation.GetStrategy() { case v2.RollbackRemediationStrategy: // Verify the previous release is still in storage and unmodified // before instructing to roll back to it. - if _, err := action.VerifySnapshot(r.configFactory.Build(nil), req.Object.GetPrevious()); err != nil { + prev := req.Object.Status.History.Previous(remediation.MustIgnoreTestFailures(req.Object.GetTest().IgnoreFailures)) + if _, err := action.VerifySnapshot(r.configFactory.Build(nil), prev); err != nil { if interrors.IsOneOf(err, 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 diff --git a/internal/reconcile/atomic_release_test.go b/internal/reconcile/atomic_release_test.go index d9f53bf25..88c2b98b7 100644 --- a/internal/reconcile/atomic_release_test.go +++ b/internal/reconcile/atomic_release_test.go @@ -21,17 +21,20 @@ import ( "testing" "time" - "github.com/fluxcd/pkg/runtime/patch" . "github.com/onsi/gomega" + helmchart "helm.sh/helm/v3/pkg/chart" 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" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" + "github.com/fluxcd/pkg/runtime/patch" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" @@ -167,7 +170,7 @@ func TestAtomicRelease_Reconcile(t *testing.T) { Chart: testutil.BuildChart(testutil.ChartWithTestHook()), Values: nil, } - g.Expect(NewAtomicRelease(patchHelper, cfg, recorder, "helm-controller").Reconcile(context.TODO(), req)).ToNot(HaveOccurred()) + g.Expect(NewAtomicRelease(patchHelper, cfg, recorder, testFieldManager).Reconcile(context.TODO(), req)).ToNot(HaveOccurred()) g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ { @@ -189,8 +192,8 @@ func TestAtomicRelease_Reconcile(t *testing.T) { 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.History.Latest()).ToNot(BeNil(), "expected current to not be nil") + g.Expect(obj.Status.History.Previous(false)).To(BeNil(), "expected previous to be nil") g.Expect(obj.Status.Failures).To(BeZero()) g.Expect(obj.Status.InstallFailures).To(BeZero()) @@ -202,6 +205,806 @@ func TestAtomicRelease_Reconcile(t *testing.T) { }) } +func TestAtomicRelease_Reconcile_Scenarios(t *testing.T) { + tests := []struct { + name string + releases func(namespace string) []*helmrelease.Release + spec func(spec *v2.HelmReleaseSpec) + status func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus + chart *helmchart.Chart + values map[string]interface{} + expectHistory func(releases []*helmrelease.Release) v2.Snapshots + wantErr error + }{ + { + name: "release is in-sync", + 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, + }, testutil.ReleaseWithConfig(nil)), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(), + values: nil, + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + }, + { + name: "release is out-of-sync (chart)", + 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, + }, testutil.ReleaseWithConfig(nil)), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(testutil.ChartWithVersion("0.2.0")), + values: nil, + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[len(releases)-1])), + } + }, + }, + { + name: "release is out-of-sync (values)", + 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, + }, testutil.ReleaseWithConfig(map[string]interface{}{"foo": "bar"})), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(), + values: map[string]interface{}{"foo": "baz"}, + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[len(releases)-1])), + } + }, + }, + { + name: "release is locked (pending-install)", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusPendingInstall, + }, testutil.ReleaseWithConfig(nil)), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: nil, + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[len(releases)-1])), + } + }, + }, + { + name: "release is locked (pending-upgrade)", + 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, + }, testutil.ReleaseWithConfig(nil)), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusPendingUpgrade, + }, testutil.ReleaseWithConfig(nil)), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[len(releases)-1])), + } + }, + }, + { + name: "release is locked (pending-rollback)", + 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, + }, testutil.ReleaseWithConfig(nil)), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusFailed, + }, testutil.ReleaseWithConfig(nil)), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 3, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusPendingRollback, + }, testutil.ReleaseWithConfig(nil)), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[len(releases)-1])), + } + }, + }, + { + name: "release is not installed", + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + }, + { + name: "release exists but is not managed", + 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, + }), + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + } + }, + }, + { + name: "release was upgraded outside of the reconciler", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 3, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusDeployed, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + }), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + previousDeployed := release.ObserveRelease(releases[1]) + previousDeployed.Info.Status = helmrelease.StatusDeployed + + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(previousDeployed), + }, + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[len(releases)-1])), + } + }, + }, + { + name: "release was rolled back outside of the reconciler", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 3, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusDeployed, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + }), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + modifiedRelease := release.ObserveRelease(releases[1]) + modifiedRelease.Info.Status = helmrelease.StatusFailed + + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(modifiedRelease), + }, + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[len(releases)-1])), + } + }, + }, + { + name: "release was deleted outside of the reconciler", + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease( + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusDeployed, + }), + )), + }, + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + }, + { + name: "part of the release history was deleted outside of the reconciler", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 3, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusDeployed, + }), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + deletedRelease := release.ObservedToSnapshot(release.ObserveRelease( + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 4, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusFailed, + }), + )) + + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + deletedRelease, + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + }, + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[len(releases)-1])), + } + }, + }, + { + name: "install failure", + chart: testutil.BuildChart(testutil.ChartWithFailingHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + wantErr: ErrExceededMaxRetries, + }, + { + name: "install failure with remediation", + spec: func(spec *v2.HelmReleaseSpec) { + spec.Install = &v2.Install{ + Remediation: &v2.InstallRemediation{ + RemediateLastFailure: pointer.Bool(true), + }, + } + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + chart: testutil.BuildChart(testutil.ChartWithFailingHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + }, + { + name: "install test failure with remediation", + spec: func(spec *v2.HelmReleaseSpec) { + spec.Install = &v2.Install{ + Remediation: &v2.InstallRemediation{ + RemediateLastFailure: pointer.Bool(true), + }, + } + spec.Test = &v2.Test{ + Enable: true, + } + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + chart: testutil.BuildChart(testutil.ChartWithFailingTestHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + snap := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + snap.SetTestHooks(release.TestHooksFromRelease(releases[0])) + + return v2.Snapshots{ + snap, + } + }, + }, + { + name: "install test failure with test ignore", + spec: func(spec *v2.HelmReleaseSpec) { + spec.Test = &v2.Test{ + Enable: true, + IgnoreFailures: true, + } + }, + chart: testutil.BuildChart(testutil.ChartWithFailingTestHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + snap := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + snap.SetTestHooks(release.TestHooksFromRelease(releases[0])) + + return v2.Snapshots{ + snap, + } + }, + }, + { + name: "install with exhausted retries after remediation", + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease( + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusUninstalling, + }), + )), + }, + LastAttemptedReleaseAction: v2.ReleaseActionInstall, + Failures: 1, + InstallFailures: 1, + } + }, + wantErr: ErrExceededMaxRetries, + }, + { + name: "upgrade failure", + 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, + }), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(testutil.ChartWithFailingHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + wantErr: ErrExceededMaxRetries, + }, + { + name: "upgrade failure with rollback remediation", + 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, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Upgrade = &v2.Upgrade{ + Remediation: &v2.UpgradeRemediation{ + RemediateLastFailure: pointer.Bool(true), + }, + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(testutil.ChartWithFailingHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[2])), + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + }, + { + name: "upgrade failure with uninstall remediation", + 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, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + strategy := v2.UninstallRemediationStrategy + spec.Upgrade = &v2.Upgrade{ + Remediation: &v2.UpgradeRemediation{ + Strategy: &strategy, + RemediateLastFailure: pointer.Bool(true), + }, + } + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(testutil.ChartWithFailingHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + }, + { + name: "upgrade test failure with remediation", + 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, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Upgrade = &v2.Upgrade{ + Remediation: &v2.UpgradeRemediation{ + RemediateLastFailure: pointer.Bool(true), + }, + } + spec.Test = &v2.Test{ + Enable: true, + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(testutil.ChartWithFailingTestHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + testedSnap := release.ObservedToSnapshot(release.ObserveRelease(releases[1])) + testedSnap.SetTestHooks(release.TestHooksFromRelease(releases[1])) + + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[2])), + testedSnap, + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + }, + { + name: "upgrade test failure with test ignore", + 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, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Test = &v2.Test{ + Enable: true, + IgnoreFailures: true, + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + chart: testutil.BuildChart(testutil.ChartWithFailingTestHook()), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + testedSnap := release.ObservedToSnapshot(release.ObserveRelease(releases[1])) + testedSnap.SetTestHooks(release.TestHooksFromRelease(releases[1])) + + return v2.Snapshots{ + testedSnap, + } + }, + }, + { + name: "upgrade with exhausted retries after remediation", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 2, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusSuperseded, + }), + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 3, + Chart: testutil.BuildChart(), + Status: helmrelease.StatusDeployed, + }), + } + }, + status: func(namespace string, releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[2])), + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + LastAttemptedReleaseAction: v2.ReleaseActionUpgrade, + Failures: 1, + UpgradeFailures: 1, + } + }, + chart: testutil.BuildChart(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[2])), + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } + }, + wantErr: ErrExceededMaxRetries, + }, + } + 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{ + ObjectMeta: metav1.ObjectMeta{ + Name: mockReleaseName, + Namespace: releaseNamespace, + }, + 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(releaseNamespace, releases) + } + + getter, err := RESTClientGetterFromManager(testEnv.Manager, obj.GetReleaseNamespace()) + g.Expect(err).ToNot(HaveOccurred()) + + cfg, err := action.NewConfigFactory(getter, + action.WithStorage(action.DefaultStorageDriver, obj.GetStorageNamespace()), + ) + g.Expect(err).ToNot(HaveOccurred()) + + store := helmstorage.Init(cfg.Driver) + for _, r := range releases { + g.Expect(store.Create(r)).To(Succeed()) + } + + // 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() + patchHelper := patch.NewSerialPatcher(obj, client) + recorder := new(record.FakeRecorder) + + req := &Request{ + Object: obj, + Chart: tt.chart, + Values: tt.values, + } + + err = NewAtomicRelease(patchHelper, cfg, recorder, testFieldManager).Reconcile(context.TODO(), req) + wantErr := BeNil() + if tt.wantErr != nil { + wantErr = MatchError(tt.wantErr) + } + g.Expect(err).To(wantErr) + + if tt.expectHistory != nil { + history, _ := store.History(mockReleaseName) + releaseutil.SortByRevision(history) + + g.Expect(req.Object.Status.History).To(testutil.Equal(tt.expectHistory(history))) + } + }) + } +} + func TestAtomicRelease_actionForState(t *testing.T) { tests := []struct { name string @@ -345,8 +1148,9 @@ func TestAtomicRelease_actionForState(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Previous: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, LastAttemptedReleaseAction: v2.ReleaseActionUpgrade, UpgradeFailures: 1, @@ -375,8 +1179,8 @@ func TestAtomicRelease_actionForState(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Previous: release.ObservedToSnapshot(release.ObserveRelease( + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease( testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, Namespace: mockReleaseNamespace, @@ -417,8 +1221,8 @@ func TestAtomicRelease_actionForState(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Previous: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, LastAttemptedReleaseAction: v2.ReleaseActionUpgrade, UpgradeFailures: 1, diff --git a/internal/reconcile/helmchart_template_test.go b/internal/reconcile/helmchart_template_test.go index 58c7f1257..0385997ff 100644 --- a/internal/reconcile/helmchart_template_test.go +++ b/internal/reconcile/helmchart_template_test.go @@ -81,7 +81,7 @@ func TestHelmChartTemplate_Reconcile(t *testing.T) { r := &HelmChartTemplate{ client: testEnv, eventRecorder: recorder, - fieldManager: "helm-controller", + fieldManager: testFieldManager, } ts := metav1.Now() @@ -134,7 +134,7 @@ func TestHelmChartTemplate_Reconcile(t *testing.T) { r := &HelmChartTemplate{ client: testEnv, eventRecorder: record.NewFakeRecorder(32), - fieldManager: "helm-controller", + fieldManager: testFieldManager, } obj := &v2.HelmRelease{ @@ -182,7 +182,7 @@ func TestHelmChartTemplate_Reconcile(t *testing.T) { r := &HelmChartTemplate{ client: testEnv, eventRecorder: recorder, - fieldManager: "helm-controller", + fieldManager: testFieldManager, } releaseName := "not-found" @@ -254,7 +254,7 @@ func TestHelmChartTemplate_Reconcile(t *testing.T) { r := &HelmChartTemplate{ client: testEnv, eventRecorder: recorder, - fieldManager: "helm-controller", + fieldManager: testFieldManager, } obj := &v2.HelmRelease{ @@ -325,7 +325,7 @@ func TestHelmChartTemplate_Reconcile(t *testing.T) { r := &HelmChartTemplate{ client: testEnv, eventRecorder: recorder, - fieldManager: "helm-controller", + fieldManager: testFieldManager, } obj := &v2.HelmRelease{ @@ -368,7 +368,7 @@ func TestHelmChartTemplate_Reconcile(t *testing.T) { r := &HelmChartTemplate{ client: testEnv, eventRecorder: recorder, - fieldManager: "helm-controller", + fieldManager: testFieldManager, } releaseName := "owner-labels" diff --git a/internal/reconcile/install.go b/internal/reconcile/install.go index 16897167f..b9b142d33 100644 --- a/internal/reconcile/install.go +++ b/internal/reconcile/install.go @@ -37,9 +37,12 @@ import ( // 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). +// Before the installation, the History in the Status of the Request.Object is +// cleared to mark the start of a new release lifecycle. This ensures we never +// attempt to roll back to a previous release before the install. +// +// During the installation process, the writes to the Helm storage are +// observed and recorded in the Status.History field of the Request.Object. // // 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 @@ -66,9 +69,9 @@ func NewInstall(cfg *action.ConfigFactory, recorder record.EventRecorder) *Insta func (r *Install) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.GetCurrent().DeepCopy() - logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.DebugLevel)), 10) - cfg = r.configFactory.Build(logBuf.Log, observeRelease(req.Object)) + logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.DebugLevel)), 10) + obsReleases = make(observedReleases) + cfg = r.configFactory.Build(logBuf.Log, observeRelease(obsReleases)) ) defer summarize(req) @@ -76,14 +79,23 @@ func (r *Install) Reconcile(ctx context.Context, req *Request) error { // Mark install attempt on object. req.Object.Status.LastAttemptedReleaseAction = v2.ReleaseActionInstall + // An install is always considered a reset of any previous history. + // This ensures we never attempt to roll back to a previous release + // before the install. + req.Object.Status.ClearHistory() + // Run the Helm install action. _, err := action.Install(ctx, cfg, req.Object, req.Chart, req.Values) + + // Record the history of releases observed during the install. + obsReleases.recordOnObject(req.Object) + if err != nil { 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.GetCurrent(); newCur == nil || (cur != nil && cur.Digest == newCur.Digest) { + if len(obsReleases) == 0 { return err } @@ -152,7 +164,7 @@ func (r *Install) failure(req *Request, buffer *action.LogBuffer, err error) { // release. func (r *Install) success(req *Request) { // Compose success message. - cur := req.Object.GetCurrent() + cur := req.Object.Status.History.Latest() msg := fmt.Sprintf(fmtInstallSuccess, cur.FullReleaseName(), cur.VersionedChartName()) // Mark install success on object. diff --git a/internal/reconcile/install_test.go b/internal/reconcile/install_test.go index 3b13c58e1..e15a02e8d 100644 --- a/internal/reconcile/install_test.go +++ b/internal/reconcile/install_test.go @@ -66,14 +66,11 @@ func TestInstall_Reconcile(t *testing.T) { // 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 install. - expectCurrent func(releases []*helmrelease.Release) *v2.Snapshot - // expectPrevious returns the expected Previous release information of // the HelmRelease after install. - expectPrevious func(releases []*helmrelease.Release) *v2.Snapshot + expectConditions []metav1.Condition + // expectHistory is the expected History of the HelmRelease after + // install. + expectHistory func(releases []*helmrelease.Release) v2.Snapshots // expectFailures is the expected Failures count of the HelmRelease. expectFailures int64 // expectInstallFailures is the expected InstallFailures count of the @@ -92,8 +89,10 @@ func TestInstall_Reconcile(t *testing.T) { *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, "Helm install succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, { @@ -105,8 +104,10 @@ func TestInstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.InstallFailedReason, "failed post-install"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, expectInstallFailures: 1, @@ -150,8 +151,8 @@ func TestInstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -162,19 +163,18 @@ func TestInstall_Reconcile(t *testing.T) { *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, "Helm install succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[1])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + } }, }, { name: "install with stale current", status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, Namespace: "other", Version: 1, @@ -191,8 +191,10 @@ func TestInstall_Reconcile(t *testing.T) { *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, "Helm install succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, } @@ -262,16 +264,10 @@ func TestInstall_Reconcile(t *testing.T) { 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))) + if tt.expectHistory != nil { + g.Expect(obj.Status.History).To(testutil.Equal(tt.expectHistory(releases))) } else { - g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") + g.Expect(obj.Status.History).To(BeEmpty(), "expected history to be empty") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -355,8 +351,8 @@ func TestInstall_success(t *testing.T) { }) obj = &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(cur)), }, }, } @@ -376,8 +372,8 @@ func TestInstall_success(t *testing.T) { r.success(req) expectMsg := fmt.Sprintf(fmtInstallSuccess, - fmt.Sprintf("%s/%s.v%d", mockReleaseNamespace, mockReleaseName, obj.GetCurrent().Version), - fmt.Sprintf("%s@%s", obj.GetCurrent().ChartName, obj.GetCurrent().ChartVersion)) + fmt.Sprintf("%s/%s.v%d", mockReleaseNamespace, mockReleaseName, obj.Status.History.Latest().Version), + fmt.Sprintf("%s@%s", obj.Status.History.Latest().ChartName, obj.Status.History.Latest().ChartVersion)) g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ *conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason, expectMsg), @@ -389,8 +385,8 @@ func TestInstall_success(t *testing.T) { Message: expectMsg, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - eventMetaGroupKey(eventv1.MetaRevisionKey): obj.GetCurrent().ChartVersion, - eventMetaGroupKey(eventv1.MetaTokenKey): obj.GetCurrent().ConfigDigest, + eventMetaGroupKey(eventv1.MetaRevisionKey): obj.Status.History.Latest().ChartVersion, + eventMetaGroupKey(eventv1.MetaTokenKey): obj.Status.History.Latest().ConfigDigest, }, }, }, @@ -417,8 +413,8 @@ func TestInstall_success(t *testing.T) { g.Expect(cond).ToNot(BeNil()) expectMsg := fmt.Sprintf(fmtTestPending, - fmt.Sprintf("%s/%s.v%d", mockReleaseNamespace, mockReleaseName, obj.GetCurrent().Version), - fmt.Sprintf("%s@%s", obj.GetCurrent().ChartName, obj.GetCurrent().ChartVersion)) + fmt.Sprintf("%s/%s.v%d", mockReleaseNamespace, mockReleaseName, obj.Status.History.Latest().Version), + fmt.Sprintf("%s@%s", obj.Status.History.Latest().ChartName, obj.Status.History.Latest().ChartVersion)) g.Expect(cond.Message).To(Equal(expectMsg)) }) } diff --git a/internal/reconcile/release.go b/internal/reconcile/release.go index 79e3d0a72..dc61f8699 100644 --- a/internal/reconcile/release.go +++ b/internal/reconcile/release.go @@ -20,12 +20,11 @@ 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" + helmrelease "helm.sh/helm/v3/pkg/release" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" @@ -34,44 +33,74 @@ import ( ) var ( - // ErrNoCurrent is returned when the HelmRelease has no current release + // ErrNoLatest is returned when the HelmRelease has no latest release // but this is required by the ActionReconciler. - ErrNoCurrent = errors.New("no current release") + ErrNoLatest = errors.New("no latest release") // ErrNoPrevious is returned when the HelmRelease has no previous release // but this is required by the ActionReconciler. ErrNoPrevious = errors.New("no previous release") // ErrReleaseMismatch is returned when the resulting release after running - // an action does not match the expected current and/or previous release. + // an action does not match the expected latest and/or previous release. // This can happen for actions where targeting a release by version is not // possible, for example while running tests. ErrReleaseMismatch = errors.New("release mismatch") ) -// observeRelease returns a storage.ObserveFunc which updates the Status.Current -// and Status.Previous fields of the HelmRelease object. It can be used to -// record Helm install and upgrade actions as - and while - they are written to -// the Helm storage. -func observeRelease(obj *v2.HelmRelease) storage.ObserveFunc { - return func(rls *helmrelease.Release) { - cur := obj.GetCurrent().DeepCopy() - obs := release.ObserveRelease(rls) +// observedReleases is a map of Helm releases as observed to be written to the +// Helm storage. The key is the version of the release. +type observedReleases map[int]release.Observation - 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.History.Previous = obj.Status.History.Current - } - if cur == nil || !obs.Targets(cur.Name, cur.Namespace, 0) || obs.Version >= cur.Version { - // Overwrite current with newer release, or update it. - obj.Status.History.Current = release.ObservedToSnapshot(obs) +// sortedVersions returns the versions of the observed releases in descending +// order. +func (r observedReleases) sortedVersions() (versions []int) { + for ver := range r { + versions = append(versions, ver) + } + sort.Sort(sort.Reverse(sort.IntSlice(versions))) + return +} + +// recordOnObject records the observed releases on the HelmRelease object. +func (r observedReleases) recordOnObject(obj *v2.HelmRelease) { + switch len(r) { + case 0: + return + case 1: + var obs release.Observation + for _, o := range r { + obs = o } - 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.History.Previous = release.ObservedToSnapshot(obs) + obj.Status.History = append(v2.Snapshots{release.ObservedToSnapshot(obs)}, obj.Status.History...) + default: + versions := r.sortedVersions() + + obj.Status.History = append(v2.Snapshots{release.ObservedToSnapshot(r[versions[0]])}, obj.Status.History...) + + for _, ver := range versions[1:] { + for i := range obj.Status.History { + snap := obj.Status.History[i] + if snap.Targets(r[ver].Name, r[ver].Namespace, r[ver].Version) { + newSnap := release.ObservedToSnapshot(r[ver]) + newSnap.SetTestHooks(snap.GetTestHooks()) + obj.Status.History[i] = newSnap + return + } + } } } } +// observeRelease returns a storage.ObserveFunc that stores the observed +// releases in the given observedReleases map. +// It can be used for Helm actions that modify multiple releases in the +// Helm storage, such as install and upgrade. +func observeRelease(observed observedReleases) storage.ObserveFunc { + return func(rls *helmrelease.Release) { + obs := release.ObserveRelease(rls) + observed[obs.Version] = 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. // diff --git a/internal/reconcile/release_test.go b/internal/reconcile/release_test.go index dec0e55ea..778743dcc 100644 --- a/internal/reconcile/release_test.go +++ b/internal/reconcile/release_test.go @@ -17,11 +17,11 @@ limitations under the License. package reconcile import ( + "reflect" "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" @@ -29,7 +29,6 @@ import ( v2 "github.com/fluxcd/helm-controller/api/v2beta2" "github.com/fluxcd/helm-controller/internal/action" - "github.com/fluxcd/helm-controller/internal/release" ) const ( @@ -37,126 +36,21 @@ const ( mockReleaseNamespace = "mock-ns" ) -func Test_observeRelease(t *testing.T) { - const ( - otherReleaseName = "other" - otherReleaseNamespace = "other-ns" - ) - - t.Run("release", func(t *testing.T) { - g := NewWithT(t) - - obj := &v2.HelmRelease{} - mock := helmrelease.Mock(&helmrelease.MockReleaseOptions{ - Name: mockReleaseName, - Namespace: mockReleaseNamespace, - Version: 1, - Status: helmrelease.StatusPendingInstall, - }) - expect := release.ObservedToSnapshot(release.ObserveRelease(mock)) - - observeRelease(obj)(mock) - - 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) { - g := NewWithT(t) - - current := &v2.Snapshot{ - Name: mockReleaseName, - Namespace: mockReleaseNamespace, - Version: 1, - } - obj := &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, - }, - }, - } - mock := helmrelease.Mock(&helmrelease.MockReleaseOptions{ - Name: current.Name, - Namespace: current.Namespace, - Version: current.Version + 1, - Status: helmrelease.StatusPendingInstall, - }) - expect := release.ObservedToSnapshot(release.ObserveRelease(mock)) - - observeRelease(obj)(mock) - 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) { - g := NewWithT(t) - - current := &v2.Snapshot{ - Name: otherReleaseName, - Namespace: otherReleaseNamespace, - Version: 3, - } - obj := &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, - }, - }, - } - mock := helmrelease.Mock(&helmrelease.MockReleaseOptions{ - Name: mockReleaseName, - Namespace: mockReleaseNamespace, - Version: 1, - Status: helmrelease.StatusPendingInstall, - }) - expect := release.ObservedToSnapshot(release.ObserveRelease(mock)) - - observeRelease(obj)(mock) - 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) { - g := NewWithT(t) - - previous := &v2.Snapshot{ - Name: mockReleaseName, - Namespace: mockReleaseNamespace, - Version: 1, - Status: helmrelease.StatusDeployed.String(), - } - current := &v2.Snapshot{ - Name: previous.Name, - Namespace: previous.Namespace, - Version: previous.Version + 1, - Status: helmrelease.StatusPendingInstall.String(), - } - obj := &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, - Previous: previous, - }, - }, - } - mock := helmrelease.Mock(&helmrelease.MockReleaseOptions{ - Name: previous.Name, - Namespace: previous.Namespace, - Version: previous.Version, - Status: helmrelease.StatusSuperseded, +func Test_observedReleases_sortedVersions(t *testing.T) { + tests := []struct { + name string + r observedReleases + wantVersions []int + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if gotVersions := tt.r.sortedVersions(); !reflect.DeepEqual(gotVersions, tt.wantVersions) { + t.Errorf("sortedVersions() = %v, want %v", gotVersions, tt.wantVersions) + } }) - expect := release.ObservedToSnapshot(release.ObserveRelease(mock)) - - observeRelease(obj)(mock) - 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) { diff --git a/internal/reconcile/rollback_remediation.go b/internal/reconcile/rollback_remediation.go index 5900cac12..755bf7434 100644 --- a/internal/reconcile/rollback_remediation.go +++ b/internal/reconcile/rollback_remediation.go @@ -38,23 +38,20 @@ import ( ) // RollbackRemediation is an ActionReconciler which attempts to roll back -// a Request.Object to the version specified in Status.Previous. +// a Request.Object to a previous successful deployed release in the +// Status.History. // // 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. +// the Status.History field. // // 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 +// When the Request.Object does not have a (successful) previous deployed +// release, it returns an error of type ErrNoPrevious. In addition, it +// returns ErrReleaseMismatch if the name and/or namespace of the latest and +// previous release do not match. 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 @@ -78,7 +75,7 @@ func NewRollbackRemediation(configFactory *action.ConfigFactory, eventRecorder r func (r *RollbackRemediation) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.GetCurrent().DeepCopy() + cur = req.Object.Status.History.Latest().DeepCopy() logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.DebugLevel)), 10) cfg = r.configFactory.Build(logBuf.Log, observeRollback(req.Object)) ) @@ -86,12 +83,12 @@ func (r *RollbackRemediation) Reconcile(ctx context.Context, req *Request) error defer summarize(req) // Previous is required to determine what version to roll back to. - if !req.Object.HasPrevious() { + prev := req.Object.Status.History.Previous(req.Object.GetUpgrade().GetRemediation().MustIgnoreTestFailures(req.Object.GetTest().IgnoreFailures)) + if prev == nil { 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()) @@ -99,18 +96,18 @@ func (r *RollbackRemediation) Reconcile(ctx context.Context, req *Request) error // 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) + r.failure(req, prev, 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) { + if newCur := req.Object.Status.History.Latest(); newCur == nil || (cur != nil && newCur.Digest == cur.Digest) { return err } return nil } - r.success(req) + r.success(req, prev) return nil } @@ -134,9 +131,8 @@ const ( // 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) { +func (r *RollbackRemediation) failure(req *Request, prev *v2.Snapshot, buffer *action.LogBuffer, err error) { // Compose failure message. - prev := req.Object.GetPrevious() msg := fmt.Sprintf(fmtRollbackRemediationFailure, prev.FullReleaseName(), prev.VersionedChartName(), strings.TrimSpace(err.Error())) // Mark remediation failure on object. @@ -156,9 +152,8 @@ func (r *RollbackRemediation) failure(req *Request, buffer *action.LogBuffer, er // 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) { +func (r *RollbackRemediation) success(req *Request, prev *v2.Snapshot) { // Compose success message. - prev := req.Object.GetPrevious() msg := fmt.Sprintf(fmtRollbackRemediationSuccess, prev.FullReleaseName(), prev.VersionedChartName()) // Mark remediation success on object. @@ -174,17 +169,27 @@ func (r *RollbackRemediation) success(req *Request) { ) } -// 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. +// observeRollback returns a storage.ObserveFunc to track the rollback history +// of a HelmRelease. +// It observes the rollback action of a Helm release by comparing the release +// history with the recorded snapshots. +// If the rolled-back release matches a snapshot, it updates the snapshot with +// the observed release data. +// If no matching snapshot is found, it creates a new snapshot and prepends it +// to the release history. 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.History.Current = release.ObservedToSnapshot(obs) + for i := range obj.Status.History { + snap := obj.Status.History[i] + if snap.Targets(rls.Name, rls.Namespace, rls.Version) { + newSnap := release.ObservedToSnapshot(release.ObserveRelease(rls)) + newSnap.SetTestHooks(snap.GetTestHooks()) + obj.Status.History[i] = newSnap + return + } } + + obs := release.ObserveRelease(rls) + obj.Status.History = append(v2.Snapshots{release.ObservedToSnapshot(obs)}, obj.Status.History...) } } diff --git a/internal/reconcile/rollback_remediation_test.go b/internal/reconcile/rollback_remediation_test.go index 9849a663d..cf8e23906 100644 --- a/internal/reconcile/rollback_remediation_test.go +++ b/internal/reconcile/rollback_remediation_test.go @@ -24,8 +24,6 @@ import ( "testing" "time" - eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" - . "github.com/onsi/gomega" helmrelease "helm.sh/helm/v3/pkg/release" helmreleaseutil "helm.sh/helm/v3/pkg/releaseutil" @@ -35,6 +33,7 @@ import ( 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" @@ -69,12 +68,9 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { // expectedConditions are the conditions that are expected to be set on // the HelmRelease after rolling back. expectConditions []metav1.Condition - // expectCurrent is the expected Current release information on the - // HelmRelease after rolling back. - expectCurrent func(releases []*helmrelease.Release) *v2.Snapshot - // expectPrevious returns the expected Previous release information of - // the HelmRelease after rolling back. - expectPrevious func(releases []*helmrelease.Release) *v2.Snapshot + // expectHistory is the expected History on the HelmRelease after + // rolling back. + expectHistory func(releases []*helmrelease.Release) v2.Snapshots // expectFailures is the expected Failures count on the HelmRelease. expectFailures int64 // expectInstallFailures is the expected InstallFailures count on the @@ -106,9 +102,9 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[1])), - Previous: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -116,11 +112,12 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { *conditions.FalseCondition(meta.ReadyCondition, v2.RollbackSucceededReason, "succeeded"), *conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, "succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[2])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[2])), + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, { @@ -145,14 +142,16 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), }, } }, wantErr: ErrNoPrevious, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[1])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + } }, }, { @@ -177,9 +176,9 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[1])), - Previous: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -189,11 +188,12 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.RemediatedCondition, v2.RollbackFailedReason, "timed out waiting for the condition"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[2])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[2])), + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, }, @@ -225,9 +225,9 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[1])), - Previous: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -238,11 +238,11 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.RemediatedCondition, v2.RollbackFailedReason, mockCreateErr.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[1])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, }, @@ -274,9 +274,9 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[1])), - Previous: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -286,11 +286,12 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.RemediatedCondition, v2.RollbackFailedReason, "storage update error"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[2])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[2])), + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, }, @@ -356,16 +357,10 @@ func TestRollbackRemediation_Reconcile(t *testing.T) { releases, _ = store.History(mockReleaseName) helmreleaseutil.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))) + if tt.expectHistory != nil { + g.Expect(obj.Status.History).To(testutil.Equal(tt.expectHistory(releases))) } else { - g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") + g.Expect(obj.Status.History).To(BeEmpty(), "expected history to be empty") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -384,8 +379,8 @@ func TestRollbackRemediation_failure(t *testing.T) { }) obj = &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Previous: release.ObservedToSnapshot(release.ObserveRelease(prev)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(prev)), }, }, } @@ -399,9 +394,8 @@ func TestRollbackRemediation_failure(t *testing.T) { r := &RollbackRemediation{ eventRecorder: recorder, } - req := &Request{Object: obj.DeepCopy()} - r.failure(req, nil, err) + r.failure(req, release.ObservedToSnapshot(release.ObserveRelease(prev)), nil, err) expectMsg := fmt.Sprintf(fmtRollbackRemediationFailure, fmt.Sprintf("%s/%s.v%d", prev.Namespace, prev.Name, prev.Version), @@ -435,7 +429,7 @@ func TestRollbackRemediation_failure(t *testing.T) { eventRecorder: recorder, } req := &Request{Object: obj.DeepCopy()} - r.failure(req, mockLogBuffer(5, 10), err) + r.failure(req, release.ObservedToSnapshot(release.ObserveRelease(prev)), mockLogBuffer(5, 10), err) expectSubStr := "Last Helm logs" g.Expect(conditions.IsFalse(req.Object, v2.RemediatedCondition)).To(BeTrue()) @@ -460,17 +454,8 @@ func TestRollbackRemediation_success(t *testing.T) { r := &RollbackRemediation{ eventRecorder: recorder, } - - obj := &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Previous: release.ObservedToSnapshot(release.ObserveRelease(prev)), - }, - }, - } - - req := &Request{Object: obj, Values: map[string]interface{}{"foo": "bar"}} - r.success(req) + req := &Request{Object: &v2.HelmRelease{}, Values: map[string]interface{}{"foo": "bar"}} + r.success(req, release.ObservedToSnapshot(release.ObserveRelease(prev))) expectMsg := fmt.Sprintf(fmtRollbackRemediationSuccess, fmt.Sprintf("%s/%s.v%d", prev.Namespace, prev.Name, prev.Version), @@ -509,14 +494,15 @@ func Test_observeRollback(t *testing.T) { observeRollback(obj)(rls) expect := release.ObservedToSnapshot(release.ObserveRelease(rls)) - g.Expect(obj.GetPrevious()).To(BeNil()) - g.Expect(obj.GetCurrent()).To(Equal(expect)) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + })) }) - t.Run("rollback with current", func(t *testing.T) { + t.Run("rollback with latest", func(t *testing.T) { g := NewWithT(t) - current := &v2.Snapshot{ + latest := &v2.Snapshot{ Name: mockReleaseName, Namespace: mockReleaseNamespace, Version: 2, @@ -524,79 +510,107 @@ func Test_observeRollback(t *testing.T) { } obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, + History: v2.Snapshots{ + latest, }, }, } rls := helmrelease.Mock(&helmrelease.MockReleaseOptions{ - Name: current.Name, - Namespace: current.Namespace, - Version: current.Version + 1, + Name: latest.Name, + Namespace: latest.Namespace, + Version: latest.Version + 1, Status: helmrelease.StatusPendingRollback, }) expect := release.ObservedToSnapshot(release.ObserveRelease(rls)) observeRollback(obj)(rls) - g.Expect(obj.GetCurrent()).ToNot(BeNil()) - g.Expect(obj.GetCurrent()).To(Equal(expect)) - g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + latest, + })) }) - t.Run("rollback with current with higher version", func(t *testing.T) { + t.Run("rollback with update to previous deployed", func(t *testing.T) { g := NewWithT(t) - current := &v2.Snapshot{ + previous := &v2.Snapshot{ Name: mockReleaseName, Namespace: mockReleaseNamespace, Version: 2, - Status: helmrelease.StatusPendingRollback.String(), + Status: helmrelease.StatusFailed.String(), + } + latest := &v2.Snapshot{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 3, + Status: helmrelease.StatusDeployed.String(), } + obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, + History: v2.Snapshots{ + latest, + previous, }, }, } rls := helmrelease.Mock(&helmrelease.MockReleaseOptions{ - Name: current.Name, - Namespace: current.Namespace, - Version: current.Version - 1, + Name: previous.Name, + Namespace: previous.Namespace, + Version: previous.Version, Status: helmrelease.StatusSuperseded, }) + expect := release.ObservedToSnapshot(release.ObserveRelease(rls)) observeRollback(obj)(rls) - g.Expect(obj.GetPrevious()).To(BeNil()) - g.Expect(obj.GetCurrent()).To(Equal(current)) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + latest, + expect, + })) }) - t.Run("rollback with current with different name", func(t *testing.T) { + t.Run("rollback with update to previous deployed copies existing test hooks", func(t *testing.T) { g := NewWithT(t) - current := &v2.Snapshot{ - Name: mockReleaseName + "-other", + previous := &v2.Snapshot{ + Name: mockReleaseName, Namespace: mockReleaseNamespace, Version: 2, Status: helmrelease.StatusFailed.String(), + TestHooks: &map[string]*v2.TestHookStatus{ + "test-hook": { + Phase: helmrelease.HookPhaseSucceeded.String(), + }, + }, } + latest := &v2.Snapshot{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 3, + Status: helmrelease.StatusDeployed.String(), + } + obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, + History: v2.Snapshots{ + latest, + previous, }, }, } rls := helmrelease.Mock(&helmrelease.MockReleaseOptions{ - Name: mockReleaseName, - Namespace: mockReleaseNamespace, - Version: 1, - Status: helmrelease.StatusPendingRollback, + Name: previous.Name, + Namespace: previous.Namespace, + Version: previous.Version, + Status: helmrelease.StatusSuperseded, }) expect := release.ObservedToSnapshot(release.ObserveRelease(rls)) + expect.SetTestHooks(previous.GetTestHooks()) observeRollback(obj)(rls) - g.Expect(obj.GetPrevious()).To(BeNil()) - g.Expect(obj.GetCurrent()).To(Equal(expect)) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + latest, + expect, + })) }) } diff --git a/internal/reconcile/state.go b/internal/reconcile/state.go index 810e76a3c..ca9b04331 100644 --- a/internal/reconcile/state.go +++ b/internal/reconcile/state.go @@ -71,14 +71,6 @@ type ReleaseState struct { Reason string } -// MustResetHistory returns true if the release state indicates that the -// history on the v2beta2.HelmRelease object must be reset. -// This is the case when the release in storage has been mutated in such a way -// that it no longer can be used to roll back to, or perform a diff against. -func (s ReleaseState) MustResetHistory() bool { - return s.Status == ReleaseStatusLocked || s.Status == ReleaseStatusUnmanaged || s.Status == ReleaseStatusAbsent -} - // DetermineReleaseState determines the state of the Helm release as compared // to the v2beta2.HelmRelease object. It returns a ReleaseState that indicates // the status of the release, and an error if the state could not be determined. @@ -98,8 +90,7 @@ func DetermineReleaseState(cfg *action.ConfigFactory, req *Request) (ReleaseStat } // Confirm we have a release object to compare against. - cur := req.Object.GetCurrent() - if cur == nil { + if req.Object.Status.History.Len() == 0 { if rls.Info.Status == helmrelease.StatusUninstalled { return ReleaseState{Status: ReleaseStatusAbsent, Reason: "found uninstalled release in storage"}, nil } @@ -108,13 +99,14 @@ func DetermineReleaseState(cfg *action.ConfigFactory, req *Request) (ReleaseStat // Verify the release object against the state we observed during our // last reconciliation. + cur := req.Object.Status.History.Latest() if err := action.VerifyReleaseObject(cur, rls); err != nil { if interrors.IsOneOf(err, action.ErrReleaseDigest, action.ErrReleaseNotObserved) { // The release object has been mutated in such a way that we are // unable to determine the state of the release. - // Effectively, this means that the release is no longer managed - // by the object, and we should e.g. perform an upgrade to bring - // the release back in sync and under management. + // Effectively, this means that the object no longer manages the + // release, and we should e.g. perform an upgrade to bring + // the release back in-sync and under management. return ReleaseState{Status: ReleaseStatusUnmanaged, Reason: err.Error()}, nil } return ReleaseState{Status: ReleaseStatusUnknown}, fmt.Errorf("failed to verify release object: %w", err) @@ -143,13 +135,13 @@ func DetermineReleaseState(cfg *action.ConfigFactory, req *Request) (ReleaseStat // 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() { + if !cur.HasBeenTested() { return ReleaseState{Status: ReleaseStatusUntested}, nil } // Act on any observed test failure. - remedation := req.Object.GetActiveRemediation() - if remedation != nil && !remedation.MustIgnoreTestFailures(testSpec.IgnoreFailures) && req.Object.GetCurrent().HasTestInPhase(helmrelease.HookPhaseFailed.String()) { + remediation := req.Object.GetActiveRemediation() + if remediation != nil && !remediation.MustIgnoreTestFailures(testSpec.IgnoreFailures) && cur.HasTestInPhase(helmrelease.HookPhaseFailed.String()) { return ReleaseState{Status: ReleaseStatusFailed, Reason: "release has test in failed phase"}, nil } } diff --git a/internal/reconcile/state_test.go b/internal/reconcile/state_test.go index fa061364c..a4a3b9c64 100644 --- a/internal/reconcile/state_test.go +++ b/internal/reconcile/state_test.go @@ -57,8 +57,8 @@ func Test_DetermineReleaseState(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -79,8 +79,8 @@ func Test_DetermineReleaseState(t *testing.T) { name: "release disappeared from storage", status: func(_ []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, Namespace: mockReleaseNamespace, Version: 1, @@ -124,8 +124,8 @@ func Test_DetermineReleaseState(t *testing.T) { cur := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) cur.Digest = "sha256:invalid" return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: cur, + History: v2.Snapshots{ + cur, }, } }, @@ -151,8 +151,8 @@ func Test_DetermineReleaseState(t *testing.T) { // Digest for empty string is always mismatch cur.Digest = "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: cur, + History: v2.Snapshots{ + cur, }, } }, @@ -197,8 +197,8 @@ func Test_DetermineReleaseState(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -241,8 +241,8 @@ func Test_DetermineReleaseState(t *testing.T) { cur.SetTestHooks(release.TestHooksFromRelease(releases[1])) return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: cur, + History: v2.Snapshots{ + cur, }, LastAttemptedReleaseAction: v2.ReleaseActionUpgrade, } @@ -282,8 +282,8 @@ func Test_DetermineReleaseState(t *testing.T) { cur.SetTestHooks(release.TestHooksFromRelease(releases[0])) return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: cur, + History: v2.Snapshots{ + cur, }, LastAttemptedReleaseAction: v2.ReleaseActionInstall, } @@ -317,8 +317,8 @@ func Test_DetermineReleaseState(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -348,9 +348,9 @@ func Test_DetermineReleaseState(t *testing.T) { values: map[string]interface{}{}, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[1])), - Previous: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -373,8 +373,8 @@ func Test_DetermineReleaseState(t *testing.T) { values: map[string]interface{}{}, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -410,8 +410,8 @@ func Test_DetermineReleaseState(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -434,8 +434,8 @@ func Test_DetermineReleaseState(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, diff --git a/internal/reconcile/suite_test.go b/internal/reconcile/suite_test.go index b5a292f09..ab059265c 100644 --- a/internal/reconcile/suite_test.go +++ b/internal/reconcile/suite_test.go @@ -39,10 +39,13 @@ import ( "github.com/fluxcd/pkg/runtime/testenv" - v2 "github.com/fluxcd/helm-controller/api/v2beta2" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" ) +const testFieldManager = "helm-controller" + var ( ctx = ctrl.SetupSignalHandler() testEnv *testenv.Environment diff --git a/internal/reconcile/test.go b/internal/reconcile/test.go index 9e98fd66f..82c399cf2 100644 --- a/internal/reconcile/test.go +++ b/internal/reconcile/test.go @@ -36,11 +36,11 @@ import ( ) // Test is an ActionReconciler which attempts to perform a Helm test for -// the Status.Current release of the Request.Object. +// the latest 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. +// TestHooks field of the latest Snapshot in the Status.History to be updated +// if it matches the target of the test. // // 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, @@ -49,9 +49,9 @@ import ( // 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. +// When the Request.Object does not have a latest release, it returns an +// error of type ErrNoLatest. In addition, it returns ErrReleaseMismatch +// if the test ran for a different release target than the latest release. // Any other returned error indicates the caller should retry as it did not cause // a change to the Helm storage. // @@ -72,7 +72,7 @@ func NewTest(cfg *action.ConfigFactory, recorder record.EventRecorder) *Test { func (r *Test) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.GetCurrent().DeepCopy() + cur = req.Object.Status.History.Latest().DeepCopy() logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.DebugLevel)), 10) cfg = r.configFactory.Build(logBuf.Log, observeTest(req.Object)) ) @@ -81,7 +81,7 @@ func (r *Test) Reconcile(ctx context.Context, req *Request) error { // We only accept test results for the current release. if cur == nil { - return fmt.Errorf("%w: required for test", ErrNoCurrent) + return fmt.Errorf("%w: required for test", ErrNoLatest) } // Run the Helm test action. @@ -89,7 +89,7 @@ func (r *Test) Reconcile(ctx context.Context, req *Request) error { // 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. + // have recorded as latest. if rls != nil && !release.ObserveRelease(rls).Targets(cur.Name, cur.Namespace, cur.Version) { err = fmt.Errorf("%w: tested release %s/%s.v%d != current release %s/%s.v%d", ErrReleaseMismatch, rls.Namespace, rls.Name, rls.Version, cur.Namespace, cur.Name, cur.Version) @@ -101,7 +101,7 @@ func (r *Test) Reconcile(ctx context.Context, req *Request) error { // If we failed to observe anything happened at all, we want to retry // and return the error to indicate this. - if !req.Object.GetCurrent().HasBeenTested() { + if !req.Object.Status.History.Latest().HasBeenTested() { return err } return nil @@ -135,7 +135,7 @@ const ( // are not ignored. func (r *Test) failure(req *Request, buffer *action.LogBuffer, err error) { // Compose failure message. - cur := req.Object.GetCurrent() + cur := req.Object.Status.History.Latest() msg := fmt.Sprintf(fmtTestFailure, cur.FullReleaseName(), cur.VersionedChartName(), strings.TrimSpace(err.Error())) // Mark test failure on object. @@ -152,7 +152,7 @@ func (r *Test) failure(req *Request, buffer *action.LogBuffer, err error) { eventMessageWithLog(msg, buffer), ) - if req.Object.GetCurrent().HasBeenTested() { + if req.Object.Status.History.Latest().HasBeenTested() { // Count the failure of the test for the active remediation strategy if enabled. remediation := req.Object.GetActiveRemediation() if remediation != nil && !remediation.MustIgnoreTestFailures(req.Object.GetTest().IgnoreFailures) { @@ -165,7 +165,7 @@ func (r *Test) failure(req *Request, buffer *action.LogBuffer, err error) { // Request.Object by marking TestSuccess=True and emitting an event. func (r *Test) success(req *Request) { // Compose success message. - cur := req.Object.GetCurrent() + cur := req.Object.Status.History.Latest() var hookMsg = "no test hooks" if l := len(cur.GetTestHooks()); l > 0 { h := "hook" @@ -189,18 +189,20 @@ func (r *Test) success(req *Request) { ) } -// 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. +// observeTest returns a storage.ObserveFunc to track test results of a +// HelmRelease. +// It only accepts test results for the latest release and updates the +// latest snapshot with the observed test results. func observeTest(obj *v2.HelmRelease) storage.ObserveFunc { return func(rls *helmrelease.Release) { - if cur := obj.GetCurrent(); cur != nil { - obs := release.ObserveRelease(rls) - if obs.Targets(cur.Name, cur.Namespace, cur.Version) { - obj.Status.History.Current = release.ObservedToSnapshot(obs) - obj.GetCurrent().SetTestHooks(release.TestHooksFromRelease(rls)) - } + // Only accept test results for the latest release. + if !obj.Status.History.Latest().Targets(rls.Name, rls.Namespace, rls.Version) { + return } + + // Update the latest snapshot with the test result. + tested := release.ObservedToSnapshot(release.ObserveRelease(rls)) + tested.SetTestHooks(release.TestHooksFromRelease(rls)) + obj.Status.History[0] = tested } } diff --git a/internal/reconcile/test_test.go b/internal/reconcile/test_test.go index 6ee683d45..afcc68e45 100644 --- a/internal/reconcile/test_test.go +++ b/internal/reconcile/test_test.go @@ -94,14 +94,11 @@ func TestTest_Reconcile(t *testing.T) { // 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. + // the HelmRelease after running test. expectConditions []metav1.Condition - // expectCurrent is the expected Current release information in the - // HelmRelease after install. - expectCurrent func(releases []*helmrelease.Release) *v2.Snapshot - // expectPrevious returns the expected Previous release information of - // the HelmRelease after install. - expectPrevious func(releases []*helmrelease.Release) *v2.Snapshot + // expectHistory is the expected History on the HelmRelease after + // running test. + expectHistory func(releases []*helmrelease.Release) v2.Snapshots // expectFailures is the expected Failures count of the HelmRelease. expectFailures int64 // expectInstallFailures is the expected InstallFailures count of the @@ -126,8 +123,8 @@ func TestTest_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -137,10 +134,10 @@ func TestTest_Reconcile(t *testing.T) { *conditions.TrueCondition(v2.TestSuccessCondition, v2.TestSucceededReason, "1 test hook completed successfully"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - info := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) - info.SetTestHooks(release.TestHooksFromRelease(releases[0])) - return info + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + withTests := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + withTests.SetTestHooks(release.TestHooksFromRelease(releases[0])) + return v2.Snapshots{withTests} }, }, { @@ -158,8 +155,8 @@ func TestTest_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -169,10 +166,10 @@ func TestTest_Reconcile(t *testing.T) { *conditions.TrueCondition(v2.TestSuccessCondition, v2.TestSucceededReason, "no test hooks"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - info := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) - info.SetTestHooks(release.TestHooksFromRelease(releases[0])) - return info + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + withTests := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + withTests.SetTestHooks(release.TestHooksFromRelease(releases[0])) + return v2.Snapshots{withTests} }, }, { @@ -190,8 +187,8 @@ func TestTest_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, LastAttemptedReleaseAction: v2.ReleaseActionInstall, InstallFailures: 0, @@ -203,10 +200,10 @@ func TestTest_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.TestSuccessCondition, v2.TestFailedReason, "timed out waiting for the condition"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - info := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) - info.SetTestHooks(release.TestHooksFromRelease(releases[0])) - return info + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + withTests := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + withTests.SetTestHooks(release.TestHooksFromRelease(releases[0])) + return v2.Snapshots{withTests} }, expectFailures: 1, expectInstallFailures: 1, @@ -225,7 +222,7 @@ func TestTest_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{}, - wantErr: ErrNoCurrent, + wantErr: ErrNoLatest, }, { name: "test with stale current", @@ -249,8 +246,8 @@ func TestTest_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -260,8 +257,10 @@ func TestTest_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.TestSuccessCondition, v2.TestFailedReason, ErrReleaseMismatch.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, wantErr: ErrReleaseMismatch, @@ -334,16 +333,10 @@ func TestTest_Reconcile(t *testing.T) { releases, _ = store.History(mockReleaseName) helmreleaseutil.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))) + if tt.expectHistory != nil { + g.Expect(obj.Status.History).To(testutil.Equal(tt.expectHistory(releases))) } else { - g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") + g.Expect(obj.Status.History).To(BeEmpty(), "expected history to be empty") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -359,8 +352,8 @@ func Test_observeTest(t *testing.T) { obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + &v2.Snapshot{ Name: mockReleaseName, Namespace: mockReleaseNamespace, Version: 1, @@ -378,34 +371,43 @@ func Test_observeTest(t *testing.T) { expect.SetTestHooks(release.TestHooksFromRelease(rls)) observeTest(obj)(rls) - g.Expect(obj.GetCurrent()).To(Equal(expect)) - g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + })) }) - t.Run("test with different current version", func(t *testing.T) { + t.Run("test targeting different version than latest", func(t *testing.T) { g := NewWithT(t) current := &v2.Snapshot{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 2, + } + previous := &v2.Snapshot{ Name: mockReleaseName, Namespace: mockReleaseNamespace, Version: 1, } obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, + History: v2.Snapshots{ + current, + previous, }, }, } rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, Namespace: mockReleaseNamespace, - Version: 2, + Version: previous.Version, }, testutil.ReleaseWithHooks(testHookFixtures)) observeTest(obj)(rls) - g.Expect(obj.GetCurrent()).To(Equal(current)) - g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + current, + previous, + })) }) t.Run("test without current", func(t *testing.T) { @@ -420,8 +422,7 @@ func Test_observeTest(t *testing.T) { }, testutil.ReleaseWithHooks(testHookFixtures)) observeTest(obj)(rls) - g.Expect(obj.GetCurrent()).To(BeNil()) - g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.Status.History).To(BeEmpty()) }) } @@ -435,8 +436,8 @@ func TestTest_failure(t *testing.T) { }) obj = &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(cur)), }, }, } @@ -509,7 +510,7 @@ func TestTest_failure(t *testing.T) { obj := obj.DeepCopy() obj.Status.LastAttemptedReleaseAction = v2.ReleaseActionInstall - obj.GetCurrent().SetTestHooks(map[string]*v2.TestHookStatus{}) + obj.Status.History.Latest().SetTestHooks(map[string]*v2.TestHookStatus{}) req := &Request{Object: obj} r.failure(req, nil, err) @@ -526,7 +527,7 @@ func TestTest_failure(t *testing.T) { obj := obj.DeepCopy() obj.Spec.Test = &v2.Test{IgnoreFailures: true} - obj.GetCurrent().SetTestHooks(map[string]*v2.TestHookStatus{}) + obj.Status.History.Latest().SetTestHooks(map[string]*v2.TestHookStatus{}) req := &Request{Object: obj} r.failure(req, nil, err) @@ -546,8 +547,8 @@ func TestTest_success(t *testing.T) { }) obj = &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(cur)), }, }, } @@ -560,7 +561,7 @@ func TestTest_success(t *testing.T) { } obj := obj.DeepCopy() - obj.GetCurrent().SetTestHooks(map[string]*v2.TestHookStatus{ + obj.Status.History.Latest().SetTestHooks(map[string]*v2.TestHookStatus{ "test": { Phase: helmrelease.HookPhaseSucceeded.String(), }, @@ -601,7 +602,7 @@ func TestTest_success(t *testing.T) { } obj := obj.DeepCopy() - obj.GetCurrent().SetTestHooks(map[string]*v2.TestHookStatus{}) + obj.Status.History.Latest().SetTestHooks(map[string]*v2.TestHookStatus{}) req := &Request{Object: obj} r.success(req) diff --git a/internal/reconcile/uninstall.go b/internal/reconcile/uninstall.go index c434c3d76..8c8158745 100644 --- a/internal/reconcile/uninstall.go +++ b/internal/reconcile/uninstall.go @@ -41,15 +41,15 @@ import ( // based on the given Request data. // // The writes to the Helm storage during the uninstallation are observed, and -// update the Status.Current field. +// update the Status.History 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 +// When the Request.Object does not have a latest release, it returns an +// error of type ErrNoLatest. If the uninstallation targeted a different +// release (version) than the latest release, 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 @@ -80,7 +80,7 @@ func NewUninstall(cfg *action.ConfigFactory, recorder record.EventRecorder) *Uni func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.GetCurrent().DeepCopy() + cur = req.Object.Status.History.Latest().DeepCopy() logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.DebugLevel)), 10) cfg = r.configFactory.Build(logBuf.Log, observeUninstall(req.Object)) ) @@ -89,7 +89,7 @@ func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { // Require current to run uninstall. if cur == nil { - return fmt.Errorf("%w: required to uninstall", ErrNoCurrent) + return fmt.Errorf("%w: required to uninstall", ErrNoLatest) } // Run the Helm uninstall action. @@ -118,7 +118,7 @@ func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { // 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. + // have recorded as latest. if res != nil && !release.ObserveRelease(res.Release).Targets(cur.Name, cur.Namespace, cur.Version) { err = fmt.Errorf("%w: uninstalled release %s/%s.v%d != current release %s", ErrReleaseMismatch, res.Release.Namespace, res.Release.Name, res.Release.Version, cur.FullReleaseName()) @@ -126,7 +126,7 @@ func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { // 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 { + if err == nil && cur.Digest == req.Object.Status.History.Latest().Digest { // An exception is made for the case where the release was already marked // as uninstalled, which would only result in the release object getting // removed from the storage. @@ -138,7 +138,7 @@ func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { // Handle any error. if err != nil { r.failure(req, logBuf, err) - if req.Object.GetCurrent().Digest == cur.Digest { + if req.Object.Status.History.Latest().Digest == cur.Digest { return err } return nil @@ -169,7 +169,7 @@ const ( // event. func (r *Uninstall) failure(req *Request, buffer *action.LogBuffer, err error) { // Compose success message. - cur := req.Object.GetCurrent() + cur := req.Object.Status.History.Latest() msg := fmt.Sprintf(fmtUninstallFailure, cur.FullReleaseName(), cur.VersionedChartName(), strings.TrimSpace(err.Error())) // Mark remediation failure on object. @@ -191,7 +191,7 @@ func (r *Uninstall) failure(req *Request, buffer *action.LogBuffer, err error) { // event. func (r *Uninstall) success(req *Request) { // Compose success message. - cur := req.Object.GetCurrent() + cur := req.Object.Status.History.Latest() msg := fmt.Sprintf(fmtUninstallSuccess, cur.FullReleaseName(), cur.VersionedChartName()) // Mark remediation success on object. @@ -208,15 +208,26 @@ func (r *Uninstall) success(req *Request) { ) } -// observeUninstall returns a storage.ObserveFunc that can be used to observe -// and record the result of an uninstall action in the status of the given -// release. It updates the Status.Current field of the release if it equals the -// uninstallation target, and version = Current.Version. +// observeUninstall returns a storage.ObserveFunc to track uninstallations of a +// HelmRelease. +// It compares the release history snapshots with the uninstalled release +// information. +// If a matching snapshot for the uninstalled release is found, it updates the +// snapshot with the observed release data. func observeUninstall(obj *v2.HelmRelease) storage.ObserveFunc { + // NB: One could argue that we should only update the latest release in + // the history. + // But like during rollback, Helm may supersede any previous releases. + // As such, we need to update all releases we have in our history. + // xref: https://github.com/helm/helm/pull/12564 return func(rls *helmrelease.Release) { - if cur := obj.GetCurrent(); cur != nil { - if obs := release.ObserveRelease(rls); obs.Targets(cur.Name, cur.Namespace, cur.Version) { - obj.Status.History.Current = release.ObservedToSnapshot(obs) + for i := range obj.Status.History { + snap := obj.Status.History[i] + if snap.Targets(rls.Name, rls.Namespace, rls.Version) { + newSnap := release.ObservedToSnapshot(release.ObserveRelease(rls)) + newSnap.SetTestHooks(snap.GetTestHooks()) + obj.Status.History[i] = newSnap + return } } } diff --git a/internal/reconcile/uninstall_remediation.go b/internal/reconcile/uninstall_remediation.go index d0acdaf62..4e244cdc0 100644 --- a/internal/reconcile/uninstall_remediation.go +++ b/internal/reconcile/uninstall_remediation.go @@ -41,16 +41,16 @@ var ( // 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. +// The writes to the Helm storage during the rollback are observed, and update +// the Status.History 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 +// When the Request.Object does not have a latest release, it returns an +// error of type ErrNoLatest. If the uninstallation targeted a different +// release (version) than the latest release, 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 @@ -80,14 +80,14 @@ func NewUninstallRemediation(cfg *action.ConfigFactory, recorder record.EventRec func (r *UninstallRemediation) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.GetCurrent().DeepCopy() + cur = req.Object.Status.History.Latest().DeepCopy() logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.DebugLevel)), 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) + return fmt.Errorf("%w: required to uninstall", ErrNoLatest) } // Run the Helm uninstall action. @@ -95,7 +95,7 @@ func (r *UninstallRemediation) Reconcile(ctx context.Context, req *Request) erro // 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. + // have recorded as latest. if res != nil && !release.ObserveRelease(res.Release).Targets(cur.Name, cur.Namespace, cur.Version) { err = fmt.Errorf("%w: uninstalled release %s/%s.v%d != current release %s", ErrReleaseMismatch, res.Release.Namespace, res.Release.Name, res.Release.Version, cur.FullReleaseName()) @@ -103,14 +103,14 @@ func (r *UninstallRemediation) Reconcile(ctx context.Context, req *Request) erro // 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 { + if err == nil && cur.Digest == req.Object.Status.History.Latest().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 { + if cur.Digest == req.Object.Status.History.Latest().Digest { return err } return nil @@ -143,7 +143,7 @@ const ( // a warning event. func (r *UninstallRemediation) failure(req *Request, buffer *action.LogBuffer, err error) { // Compose success message. - cur := req.Object.GetCurrent() + cur := req.Object.Status.History.Latest() msg := fmt.Sprintf(fmtUninstallRemediationFailure, cur.FullReleaseName(), cur.VersionedChartName(), strings.TrimSpace(err.Error())) // Mark uninstall failure on object. @@ -166,7 +166,7 @@ func (r *UninstallRemediation) failure(req *Request, buffer *action.LogBuffer, e // an event. func (r *UninstallRemediation) success(req *Request) { // Compose success message. - cur := req.Object.GetCurrent() + cur := req.Object.Status.History.Latest() msg := fmt.Sprintf(fmtUninstallRemediationSuccess, cur.FullReleaseName(), cur.VersionedChartName()) // Mark remediation success on object. diff --git a/internal/reconcile/uninstall_remediation_test.go b/internal/reconcile/uninstall_remediation_test.go index 9e37613dc..f6abe2745 100644 --- a/internal/reconcile/uninstall_remediation_test.go +++ b/internal/reconcile/uninstall_remediation_test.go @@ -66,12 +66,9 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { // 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.Snapshot - // expectPrevious returns the expected Previous release information of - // the HelmRelease after uninstall. - expectPrevious func(releases []*helmrelease.Release) *v2.Snapshot + // expectHistory is the expected History of the HelmRelease after + // uninstall. + expectHistory func(releases []*helmrelease.Release) v2.Snapshots // expectFailures is the expected Failures count of the HelmRelease. expectFailures int64 // expectInstallFailures is the expected InstallFailures count of the @@ -101,8 +98,8 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -110,8 +107,10 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { *conditions.TrueCondition(v2.RemediatedCondition, v2.UninstallSucceededReason, "succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, { @@ -134,8 +133,8 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -143,8 +142,10 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, "uninstallation completed with 1 error(s): 1 error occurred:\n\t* timed out waiting for the condition"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, }, @@ -179,8 +180,8 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -188,8 +189,10 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, ErrNoStorageUpdate.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, wantErr: ErrNoStorageUpdate, @@ -220,16 +223,18 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, expectConditions: []metav1.Condition{ *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, mockDeleteErr.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, }, @@ -247,7 +252,7 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{}, - wantErr: ErrNoCurrent, + wantErr: ErrNoLatest, }, { name: "uninstall with stale current", @@ -276,8 +281,8 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -285,8 +290,10 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.RemediatedCondition, v2.UninstallFailedReason, ErrReleaseMismatch.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, wantErr: ErrReleaseMismatch, @@ -356,16 +363,10 @@ func TestUninstallRemediation_Reconcile(t *testing.T) { 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))) + if tt.expectHistory != nil { + g.Expect(obj.Status.History).To(testutil.Equal(tt.expectHistory(releases))) } else { - g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") + g.Expect(obj.Status.History).To(BeEmpty(), "expected history to be empty") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -384,8 +385,8 @@ func TestUninstallRemediation_failure(t *testing.T) { }) obj = &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(cur)), }, }, } @@ -464,8 +465,8 @@ func TestUninstallRemediation_success(t *testing.T) { obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(cur)), }, }, } diff --git a/internal/reconcile/uninstall_test.go b/internal/reconcile/uninstall_test.go index 8a9036957..ec0a9e23a 100644 --- a/internal/reconcile/uninstall_test.go +++ b/internal/reconcile/uninstall_test.go @@ -64,12 +64,9 @@ func TestUninstall_Reconcile(t *testing.T) { // 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(namespace string, releases []*helmrelease.Release) *v2.Snapshot - // expectPrevious returns the expected Previous release information of - // the HelmRelease after uninstall. - expectPrevious func(releases []*helmrelease.Release) *v2.Snapshot + // expectHistory is the expected History of the HelmRelease after + // uninstall. + expectHistory func(namespace string, releases []*helmrelease.Release) v2.Snapshots // expectFailures is the expected Failures count of the HelmRelease. expectFailures int64 // expectInstallFailures is the expected InstallFailures count of the @@ -99,8 +96,8 @@ func TestUninstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -110,8 +107,10 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, "succeeded"), }, - expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(namespace string, releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, { @@ -134,8 +133,8 @@ func TestUninstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -145,8 +144,10 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, "uninstallation completed with 1 error(s): 1 error occurred:\n\t* timed out waiting for the condition"), }, - expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(namespace string, releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, }, @@ -181,8 +182,8 @@ func TestUninstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -192,8 +193,10 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, ErrNoStorageUpdate.Error()), }, - expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(namespace string, releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, wantErr: ErrNoStorageUpdate, @@ -224,8 +227,8 @@ func TestUninstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -235,8 +238,10 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, "delete error"), }, - expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(namespace string, releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, }, @@ -254,7 +259,7 @@ func TestUninstall_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{}, - wantErr: ErrNoCurrent, + wantErr: ErrNoLatest, }, { name: "uninstall with stale current", @@ -283,8 +288,8 @@ func TestUninstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -294,8 +299,10 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, ErrReleaseMismatch.Error()), }, - expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(namespace string, releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, wantErr: ErrReleaseMismatch, @@ -326,8 +333,8 @@ func TestUninstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -337,8 +344,10 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, "assuming it is uninstalled"), }, - expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(namespace string, releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, { @@ -356,8 +365,8 @@ func TestUninstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -367,7 +376,7 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, "succeeded"), }, - expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { + expectHistory: func(namespace string, releases []*helmrelease.Release) v2.Snapshots { rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ Name: mockReleaseName, Namespace: namespace, @@ -375,7 +384,9 @@ func TestUninstall_Reconcile(t *testing.T) { Chart: testutil.BuildChart(testutil.ChartWithTestHook()), Status: helmrelease.StatusUninstalled, }) - return release.ObservedToSnapshot(release.ObserveRelease(rls)) + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(rls)), + } }, }, { @@ -398,8 +409,8 @@ func TestUninstall_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -409,8 +420,10 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, "was already uninstalled"), }, - expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(namespace string, releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, } @@ -478,16 +491,10 @@ func TestUninstall_Reconcile(t *testing.T) { releases, _ = store.History(mockReleaseName) releaseutil.SortByRevision(releases) - if tt.expectCurrent != nil { - g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releaseNamespace, releases))) + if tt.expectHistory != nil { + g.Expect(obj.Status.History).To(testutil.Equal(tt.expectHistory(releaseNamespace, 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.History).To(BeEmpty(), "expected history to be empty") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -506,8 +513,8 @@ func TestUninstall_failure(t *testing.T) { }) obj = &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(cur)), }, }, } @@ -586,12 +593,11 @@ func TestUninstall_success(t *testing.T) { obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(cur)), }, }, } - req := &Request{Object: obj} r.success(req) @@ -630,8 +636,8 @@ func Test_observeUninstall(t *testing.T) { } obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, + History: v2.Snapshots{ + current, }, }, } @@ -644,9 +650,9 @@ func Test_observeUninstall(t *testing.T) { expect := release.ObservedToSnapshot(release.ObserveRelease(rls)) observeUninstall(obj)(rls) - g.Expect(obj.GetCurrent()).ToNot(BeNil()) - g.Expect(obj.GetCurrent()).To(Equal(expect)) - g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + })) }) t.Run("uninstall without current", func(t *testing.T) { @@ -654,9 +660,7 @@ func Test_observeUninstall(t *testing.T) { obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: nil, - }, + History: nil, }, } rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ @@ -667,8 +671,7 @@ func Test_observeUninstall(t *testing.T) { }) observeUninstall(obj)(rls) - g.Expect(obj.GetCurrent()).To(BeNil()) - g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.Status.History).To(BeNil()) }) t.Run("uninstall of different version than current", func(t *testing.T) { @@ -682,8 +685,8 @@ func Test_observeUninstall(t *testing.T) { } obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: current, + History: v2.Snapshots{ + current, }, }, } @@ -695,8 +698,8 @@ func Test_observeUninstall(t *testing.T) { }) observeUninstall(obj)(rls) - g.Expect(obj.GetCurrent()).ToNot(BeNil()) - g.Expect(obj.GetCurrent()).To(Equal(current)) - g.Expect(obj.GetPrevious()).To(BeNil()) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + current, + })) }) } diff --git a/internal/reconcile/unlock.go b/internal/reconcile/unlock.go index 1b08e159f..7d045856c 100644 --- a/internal/reconcile/unlock.go +++ b/internal/reconcile/unlock.go @@ -34,25 +34,19 @@ 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 +// Unlock is an ActionReconciler which attempts to unlock the latest release +// for 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 +// This write to the Helm storage is observed, and updates the Status.History // 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.VerifySnapshot before calling Reconcile. type Unlock struct { configFactory *action.ConfigFactory eventRecorder record.EventRecorder @@ -83,14 +77,15 @@ func (r *Unlock) Reconcile(_ context.Context, req *Request) error { } // Ensure the release is in a pending state. + cur := release.ObservedToSnapshot(release.ObserveRelease(rls)) 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) + r.failure(req, cur, status, err) return err } - r.success(req, status) + r.success(req, cur, status) } return nil } @@ -113,9 +108,8 @@ const ( // 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) { +func (r *Unlock) failure(req *Request, cur *v2.Snapshot, status helmrelease.Status, err error) { // Compose failure message. - cur := req.Object.GetCurrent() msg := fmt.Sprintf(fmtUnlockFailure, cur.FullReleaseName(), cur.VersionedChartName(), status.String(), strings.TrimSpace(err.Error())) // Mark unlock failure on object. @@ -134,9 +128,8 @@ func (r *Unlock) failure(req *Request, status helmrelease.Status, err error) { // 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) { +func (r *Unlock) success(req *Request, cur *v2.Snapshot, 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. @@ -152,16 +145,17 @@ func (r *Unlock) success(req *Request, status helmrelease.Status) { ) } -// 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. +// observeUnlock returns a storage.ObserveFunc to track unlocking actions on +// a HelmRelease. +// It updates the snapshot of a release when an unlock action is observed for +// that release. func observeUnlock(obj *v2.HelmRelease) storage.ObserveFunc { return func(rls *helmrelease.Release) { - if cur := obj.GetCurrent(); cur != nil { - obs := release.ObserveRelease(rls) - if obs.Targets(cur.Name, cur.Namespace, cur.Version) { - obj.Status.History.Current = release.ObservedToSnapshot(obs) + for i := range obj.Status.History { + snap := obj.Status.History[i] + if snap.Targets(rls.Name, rls.Namespace, rls.Version) { + obj.Status.History[i] = release.ObservedToSnapshot(release.ObserveRelease(rls)) + return } } } diff --git a/internal/reconcile/unlock_test.go b/internal/reconcile/unlock_test.go index 2154a40f2..6799fe198 100644 --- a/internal/reconcile/unlock_test.go +++ b/internal/reconcile/unlock_test.go @@ -67,12 +67,9 @@ func TestUnlock_Reconcile(t *testing.T) { // 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 unlock. - expectCurrent func(releases []*helmrelease.Release) *v2.Snapshot - // expectPrevious returns the expected Previous release information of - // the HelmRelease after unlock. - expectPrevious func(releases []*helmrelease.Release) *v2.Snapshot + // expectHistory is the expected History of the HelmRelease after + // unlock. + expectHistory func(releases []*helmrelease.Release) v2.Snapshots // expectFailures is the expected Failures count of the HelmRelease. expectFailures int64 // expectInstallFailures is the expected InstallFailures count of the @@ -97,8 +94,8 @@ func TestUnlock_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -106,8 +103,10 @@ func TestUnlock_Reconcile(t *testing.T) { *conditions.FalseCondition(meta.ReadyCondition, "PendingRelease", "Unlocked Helm release"), *conditions.FalseCondition(v2.ReleasedCondition, "PendingRelease", "Unlocked Helm release"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, { @@ -131,8 +130,8 @@ func TestUnlock_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -141,8 +140,10 @@ func TestUnlock_Reconcile(t *testing.T) { *conditions.FalseCondition(meta.ReadyCondition, "PendingRelease", "in pending-rollback state failed: storage update error"), *conditions.FalseCondition(v2.ReleasedCondition, "PendingRelease", "in pending-rollback state failed: storage update error"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, }, @@ -161,8 +162,8 @@ func TestUnlock_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + &v2.Snapshot{ Name: mockReleaseName, Namespace: releases[0].Namespace, Version: 1, @@ -172,12 +173,14 @@ func TestUnlock_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{}, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return &v2.Snapshot{ - Name: mockReleaseName, - Namespace: releases[0].Namespace, - Version: 1, - Status: helmrelease.StatusFailed.String(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + &v2.Snapshot{ + Name: mockReleaseName, + Namespace: releases[0].Namespace, + Version: 1, + Status: helmrelease.StatusFailed.String(), + }, } }, }, @@ -196,8 +199,8 @@ func TestUnlock_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + &v2.Snapshot{ Name: mockReleaseName, Namespace: releases[0].Namespace, Version: releases[0].Version - 1, @@ -206,12 +209,14 @@ func TestUnlock_Reconcile(t *testing.T) { }, } }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return &v2.Snapshot{ - Name: mockReleaseName, - Namespace: releases[0].Namespace, - Version: releases[0].Version - 1, - Status: helmrelease.StatusPendingInstall.String(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + &v2.Snapshot{ + Name: mockReleaseName, + Namespace: releases[0].Namespace, + Version: releases[0].Version - 1, + Status: helmrelease.StatusPendingInstall.String(), + }, } }, }, @@ -219,8 +224,8 @@ func TestUnlock_Reconcile(t *testing.T) { name: "unlock without latest", status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + &v2.Snapshot{ Name: mockReleaseName, Version: 1, Status: helmrelease.StatusFailed.String(), @@ -229,11 +234,13 @@ func TestUnlock_Reconcile(t *testing.T) { } }, expectConditions: []metav1.Condition{}, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return &v2.Snapshot{ - Name: mockReleaseName, - Version: 1, - Status: helmrelease.StatusFailed.String(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + &v2.Snapshot{ + Name: mockReleaseName, + Version: 1, + Status: helmrelease.StatusFailed.String(), + }, } }, }, @@ -258,8 +265,8 @@ func TestUnlock_Reconcile(t *testing.T) { }, status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + &v2.Snapshot{ Name: mockReleaseName, Version: 1, Status: helmrelease.StatusFailed.String(), @@ -269,11 +276,13 @@ func TestUnlock_Reconcile(t *testing.T) { }, wantErr: mockQueryErr, expectConditions: []metav1.Condition{}, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return &v2.Snapshot{ - Name: mockReleaseName, - Version: 1, - Status: helmrelease.StatusFailed.String(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + &v2.Snapshot{ + Name: mockReleaseName, + Version: 1, + Status: helmrelease.StatusFailed.String(), + }, } }, }, @@ -342,16 +351,10 @@ func TestUnlock_Reconcile(t *testing.T) { releases, _ = store.History(mockReleaseName) helmreleaseutil.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))) + if tt.expectHistory != nil { + g.Expect(obj.Status.History).To(testutil.Equal(tt.expectHistory(releases))) } else { - g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") + g.Expect(obj.Status.History).To(BeEmpty(), "expected history to be empty") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -371,13 +374,7 @@ func TestUnlock_failure(t *testing.T) { Chart: testutil.BuildChart(), Version: 4, }) - obj = &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), - }, - }, - } + obj = &v2.HelmRelease{} status = helmrelease.StatusPendingInstall err = fmt.Errorf("unlock error") ) @@ -388,7 +385,7 @@ func TestUnlock_failure(t *testing.T) { } req := &Request{Object: obj} - r.failure(req, status, err) + r.failure(req, release.ObservedToSnapshot(release.ObserveRelease(cur)), status, err) expectMsg := fmt.Sprintf(fmtUnlockFailure, fmt.Sprintf("%s/%s.v%d", cur.Namespace, cur.Name, cur.Version), @@ -424,13 +421,7 @@ func TestUnlock_success(t *testing.T) { Chart: testutil.BuildChart(), Version: 4, }) - obj = &v2.HelmRelease{ - Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), - }, - }, - } + obj = &v2.HelmRelease{} status = helmrelease.StatusPendingInstall ) @@ -440,7 +431,7 @@ func TestUnlock_success(t *testing.T) { } req := &Request{Object: obj} - r.success(req, status) + r.success(req, release.ObservedToSnapshot(release.ObserveRelease(cur)), status) expectMsg := fmt.Sprintf(fmtUnlockSuccess, fmt.Sprintf("%s/%s.v%d", cur.Namespace, cur.Name, cur.Version), @@ -472,8 +463,8 @@ func Test_observeUnlock(t *testing.T) { obj := &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: mockReleaseName, Namespace: mockReleaseNamespace, Version: 1, @@ -491,8 +482,9 @@ func Test_observeUnlock(t *testing.T) { expect := release.ObservedToSnapshot(release.ObserveRelease(rls)) observeUnlock(obj)(rls) - g.Expect(obj.GetPrevious()).To(BeNil()) - g.Expect(obj.GetCurrent()).To(Equal(expect)) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + })) }) t.Run("unlock without current", func(t *testing.T) { @@ -507,7 +499,6 @@ func Test_observeUnlock(t *testing.T) { }) observeUnlock(obj)(rls) - g.Expect(obj.GetPrevious()).To(BeNil()) - g.Expect(obj.GetCurrent()).To(BeNil()) + g.Expect(obj.Status.History).To(BeEmpty()) }) } diff --git a/internal/reconcile/upgrade.go b/internal/reconcile/upgrade.go index 2a08bc70d..95fca4135 100644 --- a/internal/reconcile/upgrade.go +++ b/internal/reconcile/upgrade.go @@ -37,9 +37,8 @@ import ( // 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). +// The writes to the Helm storage during the upgrade process are observed, +// and update the Status.History field. // // 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 @@ -66,9 +65,9 @@ func NewUpgrade(cfg *action.ConfigFactory, recorder record.EventRecorder) *Upgra func (r *Upgrade) Reconcile(ctx context.Context, req *Request) error { var ( - cur = req.Object.GetCurrent().DeepCopy() - logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.DebugLevel)), 10) - cfg = r.configFactory.Build(logBuf.Log, observeRelease(req.Object)) + logBuf = action.NewLogBuffer(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.DebugLevel)), 10) + obsReleases = make(observedReleases) + cfg = r.configFactory.Build(logBuf.Log, observeRelease(obsReleases)) ) defer summarize(req) @@ -78,12 +77,16 @@ func (r *Upgrade) Reconcile(ctx context.Context, req *Request) error { // Run the Helm upgrade action. _, err := action.Upgrade(ctx, cfg, req.Object, req.Chart, req.Values) + + // Record the history of releases observed during the upgrade. + obsReleases.recordOnObject(req.Object) + if 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) { + if len(obsReleases) == 0 { return err } @@ -151,7 +154,7 @@ func (r *Upgrade) failure(req *Request, buffer *action.LogBuffer, err error) { // release. func (r *Upgrade) success(req *Request) { // Compose success message. - cur := req.Object.GetCurrent() + cur := req.Object.Status.History.Latest() msg := fmt.Sprintf(fmtUpgradeSuccess, cur.FullReleaseName(), cur.VersionedChartName()) // Mark upgrade success on object. diff --git a/internal/reconcile/upgrade_test.go b/internal/reconcile/upgrade_test.go index 9403120ab..32ac87c8e 100644 --- a/internal/reconcile/upgrade_test.go +++ b/internal/reconcile/upgrade_test.go @@ -73,12 +73,9 @@ func TestUpgrade_Reconcile(t *testing.T) { // expectedConditions are the conditions that are expected to be set on // the HelmRelease after upgrade. expectConditions []metav1.Condition - // expectCurrent is the expected Current release information in the - // HelmRelease after upgrade. - expectCurrent func(releases []*helmrelease.Release) *v2.Snapshot - // expectPrevious returns the expected Previous release information of - // the HelmRelease after upgrade. - expectPrevious func(releases []*helmrelease.Release) *v2.Snapshot + // expectHistory returns the expected History of the HelmRelease after + // upgrade. + expectHistory func(releases []*helmrelease.Release) v2.Snapshots // expectFailures is the expected Failures count of the HelmRelease. expectFailures int64 // expectInstallFailures is the expected InstallFailures count of the @@ -104,8 +101,8 @@ func TestUpgrade_Reconcile(t *testing.T) { chart: testutil.BuildChart(), status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -113,11 +110,11 @@ func TestUpgrade_Reconcile(t *testing.T) { *conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded"), *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[1])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, }, { @@ -136,8 +133,8 @@ func TestUpgrade_Reconcile(t *testing.T) { chart: testutil.BuildChart(testutil.ChartWithFailingHook()), status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -147,11 +144,11 @@ func TestUpgrade_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, "post-upgrade hooks failed: 1 error occurred:\n\t* timed out waiting for the condition"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[1])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, expectUpgradeFailures: 1, @@ -178,8 +175,8 @@ func TestUpgrade_Reconcile(t *testing.T) { chart: testutil.BuildChart(), status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -189,8 +186,10 @@ func TestUpgrade_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, mockCreateErr.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, expectUpgradeFailures: 0, @@ -218,8 +217,8 @@ func TestUpgrade_Reconcile(t *testing.T) { chart: testutil.BuildChart(), status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), }, } }, @@ -229,11 +228,11 @@ func TestUpgrade_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UpgradeFailedReason, mockUpdateErr.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[1])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + } }, expectFailures: 1, expectUpgradeFailures: 1, @@ -254,17 +253,17 @@ func TestUpgrade_Reconcile(t *testing.T) { chart: testutil.BuildChart(), status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: nil, - }, + History: nil, } }, expectConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded"), *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[1])) + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[1])), + } }, }, { @@ -290,8 +289,8 @@ func TestUpgrade_Reconcile(t *testing.T) { chart: testutil.BuildChart(), status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { return v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: &v2.Snapshot{ + History: v2.Snapshots{ + { Name: mockReleaseName, Namespace: releases[0].Namespace, Version: 1, @@ -306,15 +305,15 @@ func TestUpgrade_Reconcile(t *testing.T) { *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { - return release.ObservedToSnapshot(release.ObserveRelease(releases[2])) - }, - expectPrevious: func(releases []*helmrelease.Release) *v2.Snapshot { - return &v2.Snapshot{ - Name: mockReleaseName, - Namespace: releases[0].Namespace, - Version: 1, - Status: helmrelease.StatusDeployed.String(), + expectHistory: func(releases []*helmrelease.Release) v2.Snapshots { + return v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(releases[2])), + { + Name: mockReleaseName, + Namespace: releases[0].Namespace, + Version: 1, + Status: helmrelease.StatusDeployed.String(), + }, } }, }, @@ -385,16 +384,10 @@ func TestUpgrade_Reconcile(t *testing.T) { releases, _ = store.History(mockReleaseName) helmreleaseutil.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))) + if tt.expectHistory != nil { + g.Expect(obj.Status.History).To(testutil.Equal(tt.expectHistory(releases))) } else { - g.Expect(obj.GetPrevious()).To(BeNil(), "expected previous to be nil") + g.Expect(obj.Status.History).To(BeEmpty(), "expected history to be empty") } g.Expect(obj.Status.Failures).To(Equal(tt.expectFailures)) @@ -478,8 +471,8 @@ func TestUpgrade_success(t *testing.T) { }) obj = &v2.HelmRelease{ Status: v2.HelmReleaseStatus{ - History: v2.ReleaseHistory{ - Current: release.ObservedToSnapshot(release.ObserveRelease(cur)), + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(cur)), }, }, } @@ -499,8 +492,8 @@ func TestUpgrade_success(t *testing.T) { r.success(req) expectMsg := fmt.Sprintf(fmtUpgradeSuccess, - fmt.Sprintf("%s/%s.v%d", mockReleaseNamespace, mockReleaseName, obj.GetCurrent().Version), - fmt.Sprintf("%s@%s", obj.GetCurrent().ChartName, obj.GetCurrent().ChartVersion)) + fmt.Sprintf("%s/%s.v%d", mockReleaseNamespace, mockReleaseName, obj.Status.History.Latest().Version), + fmt.Sprintf("%s@%s", obj.Status.History.Latest().ChartName, obj.Status.History.Latest().ChartVersion)) g.Expect(req.Object.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, expectMsg), @@ -512,8 +505,8 @@ func TestUpgrade_success(t *testing.T) { Message: expectMsg, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - eventMetaGroupKey(eventv1.MetaRevisionKey): obj.GetCurrent().ChartVersion, - eventMetaGroupKey(eventv1.MetaTokenKey): obj.GetCurrent().ConfigDigest, + eventMetaGroupKey(eventv1.MetaRevisionKey): obj.Status.History.Latest().ChartVersion, + eventMetaGroupKey(eventv1.MetaTokenKey): obj.Status.History.Latest().ConfigDigest, }, }, }, @@ -540,8 +533,8 @@ func TestUpgrade_success(t *testing.T) { g.Expect(cond).ToNot(BeNil()) expectMsg := fmt.Sprintf(fmtTestPending, - fmt.Sprintf("%s/%s.v%d", mockReleaseNamespace, mockReleaseName, obj.GetCurrent().Version), - fmt.Sprintf("%s@%s", obj.GetCurrent().ChartName, obj.GetCurrent().ChartVersion)) + fmt.Sprintf("%s/%s.v%d", mockReleaseNamespace, mockReleaseName, obj.Status.History.Latest().Version), + fmt.Sprintf("%s@%s", obj.Status.History.Latest().ChartName, obj.Status.History.Latest().ChartVersion)) g.Expect(cond.Message).To(Equal(expectMsg)) }) } diff --git a/internal/testutil/mock_chart.go b/internal/testutil/mock_chart.go index 489a314ca..72c458806 100644 --- a/internal/testutil/mock_chart.go +++ b/internal/testutil/mock_chart.go @@ -129,6 +129,13 @@ func ChartWithName(name string) ChartOption { } } +// ChartWithVersion sets the version of the chart. +func ChartWithVersion(version string) ChartOption { + return func(opts *ChartOptions) { + opts.Metadata.Version = version + } +} + // ChartWithFailingHook appends a failing hook to the chart. func ChartWithFailingHook() ChartOption { return func(opts *ChartOptions) {