diff --git a/api/v2/snapshot_types.go b/api/v2/snapshot_types.go index 4951c7ec3..bce25c1ff 100644 --- a/api/v2/snapshot_types.go +++ b/api/v2/snapshot_types.go @@ -49,6 +49,9 @@ const ( // snapshotTestPhaseFailed indicates that the test of the release the snapshot // was taken from has failed. snapshotTestPhaseFailed = "Failed" + // snapshotTestPhaseSucceeded indicates that the test of the release the snapshot + // was taken from has succeeded. + snapshotTestPhaseSucceeded = "Succeeded" ) // Snapshots is a list of Snapshot objects. @@ -209,10 +212,26 @@ func (in *Snapshot) VersionedChartName() string { return fmt.Sprintf("%s@%s", in.ChartName, in.ChartVersion) } -// HasBeenTested returns true if TestHooks is not nil. This includes an empty -// map, which indicates the chart has no tests. +// HasBeenTested returns true if test results have been observed. This includes +// an empty map, which indicates the chart has no selected tests. func (in *Snapshot) HasBeenTested() bool { - return in != nil && in.TestHooks != nil + if in == nil || in.TestHooks == nil { + return false + } + if len(*in.TestHooks) == 0 { + return true + } + for _, th := range *in.TestHooks { + if th != nil && th.Phase == snapshotTestPhaseFailed { + return true + } + } + for _, th := range *in.TestHooks { + if th == nil || th.Phase != snapshotTestPhaseSucceeded { + return false + } + } + return true } // GetTestHooks returns the TestHooks for the release if not nil. @@ -227,7 +246,7 @@ func (in *Snapshot) GetTestHooks() map[string]*TestHookStatus { func (in *Snapshot) HasTestInPhase(phase string) bool { if in != nil { for _, h := range in.GetTestHooks() { - if h.Phase == phase { + if h != nil && h.Phase == phase { return true } } diff --git a/api/v2/snapshot_types_test.go b/api/v2/snapshot_types_test.go index 4b7d6c7d5..a0cdca8a7 100644 --- a/api/v2/snapshot_types_test.go +++ b/api/v2/snapshot_types_test.go @@ -95,6 +95,98 @@ func TestSnapshots_Latest(t *testing.T) { } } +func TestSnapshots_HasBeenTested(t *testing.T) { + tests := []struct { + name string + in Snapshot + want bool + }{ + { + name: "no testhooks", + in: Snapshot{Version: 2, Status: "deployed"}, + want: false, + }, + { + name: "empty testhooks", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{}}, + want: true, + }, + { + name: "all testhooks are zero", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": &TestHookStatus{}, + }}, + want: false, + }, + { + name: "nil testhook", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": nil, + }}, + want: false, + }, + { + name: "all testhooks succeeded", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": {Phase: "Succeeded"}, + }}, + want: true, + }, + { + name: "all testhooks failed", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": {Phase: "Failed"}, + }}, + want: true, + }, + { + name: "mixed terminal testhooks", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": {Phase: "Succeeded"}, + "test2": {Phase: "Failed"}, + }}, + want: true, + }, + { + name: "mixed failed and zero testhooks", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": {Phase: "Failed"}, + "test2": {}, + }}, + want: true, + }, + { + name: "mixed succeeded and zero testhooks", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": {Phase: "Succeeded"}, + "test2": {}, + }}, + want: false, + }, + { + name: "testhook still running", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": {Phase: "Running"}, + }}, + want: false, + }, + { + name: "testhook phase unknown", + in: Snapshot{Version: 2, Status: "deployed", TestHooks: &map[string]*TestHookStatus{ + "test1": {Phase: "Unknown"}, + }}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.in.HasBeenTested(); got != tt.want { + t.Errorf("HasBeenTested() = %v, want %v", got, tt.want) + } + }) + } +} + func TestSnapshots_Previous(t *testing.T) { tests := []struct { name string diff --git a/internal/reconcile/state_test.go b/internal/reconcile/state_test.go index 9c3fed1ac..f10dd4ac8 100644 --- a/internal/reconcile/state_test.go +++ b/internal/reconcile/state_test.go @@ -220,6 +220,44 @@ func Test_DetermineReleaseState(t *testing.T) { Status: ReleaseStatusUntested, }, }, + { + name: "incomplete test result", + releases: []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmreleasecommon.StatusDeployed, + Chart: testutil.BuildChart(), + }, testutil.ReleaseWithConfig(map[string]any{"foo": "bar"})), + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Test = &v2.Test{ + Enable: true, + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + cur := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + cur.SetTestHooks(map[string]*v2.TestHookStatus{ + "passing-test": { + Phase: helmrelease.HookPhaseSucceeded.String(), + }, + "never-run-test": {}, + }) + + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + cur, + }, + LastAttemptedReleaseAction: v2.ReleaseActionUpgrade, + } + }, + chart: testutil.BuildChart(), + values: map[string]any{"foo": "bar"}, + want: ReleaseState{ + Status: ReleaseStatusUntested, + }, + }, { name: "failed test", releases: []*helmrelease.Release{ @@ -265,6 +303,47 @@ func Test_DetermineReleaseState(t *testing.T) { Status: ReleaseStatusFailed, }, }, + { + name: "failed test with unstarted remaining test hooks", + releases: []*helmrelease.Release{ + testutil.BuildRelease( + &helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 2, + Status: helmreleasecommon.StatusDeployed, + Chart: testutil.BuildChart(), + }, + testutil.ReleaseWithConfig(map[string]any{"foo": "bar"}), + ), + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Test = &v2.Test{ + Enable: true, + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + cur := release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + cur.SetTestHooks(map[string]*v2.TestHookStatus{ + "failure-tests": { + Phase: helmrelease.HookPhaseFailed.String(), + }, + "never-run-test": {}, + }) + + return v2.HelmReleaseStatus{ + History: v2.Snapshots{ + cur, + }, + LastAttemptedReleaseAction: v2.ReleaseActionUpgrade, + } + }, + chart: testutil.BuildChart(), + values: map[string]any{"foo": "bar"}, + want: ReleaseState{ + Status: ReleaseStatusFailed, + }, + }, { name: "failed test with ignore failures set", releases: []*helmrelease.Release{ diff --git a/internal/reconcile/test.go b/internal/reconcile/test.go index a1c4b8599..56127834f 100644 --- a/internal/reconcile/test.go +++ b/internal/reconcile/test.go @@ -207,7 +207,7 @@ func observeTest(obj *v2.HelmRelease) storage.ObserveFunc { // Update the latest snapshot with the test result. latest := obj.Status.History.Latest() tested := release.ObservedToSnapshot(releaseToObservation(rls, latest, latest.GetAction())) - tested.SetTestHooks(release.TestHooksFromRelease(rls)) + tested.SetTestHooks(release.TestHooksFromRelease(rls, obj.GetTest().GetFilters()...)) obj.Status.History[0] = tested } } diff --git a/internal/reconcile/test_test.go b/internal/reconcile/test_test.go index 6267d4a78..346c6fcb4 100644 --- a/internal/reconcile/test_test.go +++ b/internal/reconcile/test_test.go @@ -410,6 +410,46 @@ func Test_observeTest(t *testing.T) { })) }) + t.Run("test with filters", func(t *testing.T) { + g := NewWithT(t) + + filters := []v2.Filter{ + {Name: "passing-test"}, + } + obj := &v2.HelmRelease{ + Spec: v2.HelmReleaseSpec{ + Test: &v2.Test{ + Filters: &filters, + }, + }, + Status: v2.HelmReleaseStatus{ + History: v2.Snapshots{ + &v2.Snapshot{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + }, + }, + }, + } + rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + }, testutil.ReleaseWithHooks(testHookFixtures)) + + expect := release.ObservedToSnapshot(release.ObserveRelease(rls)) + expect.SetTestHooks(release.TestHooksFromRelease(rls, filters...)) + + observeTest(obj)(rls) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + })) + g.Expect(obj.Status.History.Latest().GetTestHooks()).To(HaveKey("passing-test")) + g.Expect(obj.Status.History.Latest().GetTestHooks()).ToNot(HaveKey("never-run-test")) + g.Expect(obj.Status.History.Latest().GetTestHooks()).ToNot(HaveKey("failing-test")) + }) + t.Run("test with current preserves action", func(t *testing.T) { g := NewWithT(t) diff --git a/internal/release/observation.go b/internal/release/observation.go index fa3ac5e72..9f00137d0 100644 --- a/internal/release/observation.go +++ b/internal/release/observation.go @@ -182,10 +182,14 @@ func ObservedToSnapshot(rls Observation) *v2.Snapshot { } // TestHooksFromRelease returns the list of v2.TestHookStatus for the -// given release, indexed by name. -func TestHooksFromRelease(rls *helmrelease.Release) map[string]*v2.TestHookStatus { +// given release, indexed by name. When filters are provided, only hooks +// selected by the filters are returned. +func TestHooksFromRelease(rls *helmrelease.Release, filters ...v2.Filter) map[string]*v2.TestHookStatus { hooks := make(map[string]*v2.TestHookStatus) for k, v := range GetTestHooks(rls) { + if !isTestHookSelected(k, filters) { + continue + } var h *v2.TestHookStatus if v != nil { h = &v2.TestHookStatus{ @@ -198,3 +202,23 @@ func TestHooksFromRelease(rls *helmrelease.Release) map[string]*v2.TestHookStatu } return hooks } + +func isTestHookSelected(name string, filters []v2.Filter) bool { + var ( + hasInclude bool + included bool + ) + for _, f := range filters { + if f.Exclude { + if f.Name == name { + return false + } + continue + } + hasInclude = true + if f.Name == name { + included = true + } + } + return !hasInclude || included +} diff --git a/internal/release/observation_test.go b/internal/release/observation_test.go index a9a88399f..5bbc57031 100644 --- a/internal/release/observation_test.go +++ b/internal/release/observation_test.go @@ -343,8 +343,6 @@ func TestObservedToSnapshot_WithAction(t *testing.T) { } func TestTestHooksFromRelease(t *testing.T) { - g := NewWithT(t) - hooks := []*helmrelease.Hook{ { Name: "never-run-test", @@ -379,7 +377,7 @@ func TestTestHooksFromRelease(t *testing.T) { Chart: testutil.BuildChart(), }, testutil.ReleaseWithHooks(hooks)) - g.Expect(TestHooksFromRelease(rls)).To(testutil.Equal(map[string]*v2.TestHookStatus{ + wantHooks := map[string]*v2.TestHookStatus{ hooks[0].Name: {}, hooks[1].Name: { LastStarted: metav1.Time{Time: hooks[1].LastRun.StartedAt}, @@ -391,5 +389,57 @@ func TestTestHooksFromRelease(t *testing.T) { LastCompleted: metav1.Time{Time: hooks[2].LastRun.CompletedAt}, Phase: hooks[2].LastRun.Phase.String(), }, - })) + } + + tests := []struct { + name string + filters []v2.Filter + want map[string]*v2.TestHookStatus + }{ + { + name: "all test hooks", + want: wantHooks, + }, + { + name: "include filter", + filters: []v2.Filter{ + {Name: hooks[1].Name}, + }, + want: map[string]*v2.TestHookStatus{ + hooks[1].Name: wantHooks[hooks[1].Name], + }, + }, + { + name: "exclude filter", + filters: []v2.Filter{ + {Name: hooks[0].Name, Exclude: true}, + }, + want: map[string]*v2.TestHookStatus{ + hooks[1].Name: wantHooks[hooks[1].Name], + hooks[2].Name: wantHooks[hooks[2].Name], + }, + }, + { + name: "exclude wins over include", + filters: []v2.Filter{ + {Name: hooks[1].Name}, + {Name: hooks[1].Name, Exclude: true}, + }, + want: map[string]*v2.TestHookStatus{}, + }, + { + name: "include filter without match", + filters: []v2.Filter{ + {Name: "missing-test"}, + }, + want: map[string]*v2.TestHookStatus{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(TestHooksFromRelease(rls, tt.filters...)).To(testutil.Equal(tt.want)) + }) + } }