From 7ccfc93448c571da6f54353ae321cb044144a485 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hidde@hhh.computer>
Date: Thu, 7 Dec 2023 11:28:54 +0100
Subject: [PATCH] reconcile: remove stale `TestSuccess` condition

When a Helm install or upgrade is performed, to prevent confusion due to
reporting a stale test result.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
---
 internal/reconcile/install.go      |  3 ++-
 internal/reconcile/install_test.go | 23 ++++++++++++++++++++
 internal/reconcile/upgrade.go      |  3 ++-
 internal/reconcile/upgrade_test.go | 34 ++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/internal/reconcile/install.go b/internal/reconcile/install.go
index a390b2a..0567b54 100644
--- a/internal/reconcile/install.go
+++ b/internal/reconcile/install.go
@@ -84,7 +84,8 @@ func (r *Install) Reconcile(ctx context.Context, req *Request) error {
 	// before the install.
 	req.Object.Status.ClearHistory()
 
-	// If we are installing, we are no longer remediated.
+	// If we are installing, none of the previous conditions apply.
+	conditions.Delete(req.Object, v2.TestSuccessCondition)
 	conditions.Delete(req.Object, v2.RemediatedCondition)
 
 	// Run the Helm install action.
diff --git a/internal/reconcile/install_test.go b/internal/reconcile/install_test.go
index e15a02e..d156e45 100644
--- a/internal/reconcile/install_test.go
+++ b/internal/reconcile/install_test.go
@@ -197,6 +197,29 @@ func TestInstall_Reconcile(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "install with stale conditions",
+			status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
+				return v2.HelmReleaseStatus{
+					Conditions: []metav1.Condition{
+						*conditions.FalseCondition(v2.TestSuccessCondition, v2.TestFailedReason, ""),
+						*conditions.TrueCondition(v2.RemediatedCondition, v2.UninstallSucceededReason, ""),
+					},
+				}
+			},
+			chart: testutil.BuildChart(),
+			expectConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReadyCondition, v2.InstallSucceededReason,
+					"Helm install succeeded"),
+				*conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason,
+					"Helm install succeeded"),
+			},
+			expectHistory: func(releases []*helmrelease.Release) v2.Snapshots {
+				return v2.Snapshots{
+					release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
+				}
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/internal/reconcile/upgrade.go b/internal/reconcile/upgrade.go
index 95b3459..8cdbb08 100644
--- a/internal/reconcile/upgrade.go
+++ b/internal/reconcile/upgrade.go
@@ -75,7 +75,8 @@ func (r *Upgrade) Reconcile(ctx context.Context, req *Request) error {
 	// Mark upgrade attempt on object.
 	req.Object.Status.LastAttemptedReleaseAction = v2.ReleaseActionUpgrade
 
-	// If we are upgrading, we are no longer remediated.
+	// If we are upgrading, none of the previous conditions apply.
+	conditions.Delete(req.Object, v2.TestSuccessCondition)
 	conditions.Delete(req.Object, v2.RemediatedCondition)
 
 	// Run the Helm upgrade action.
diff --git a/internal/reconcile/upgrade_test.go b/internal/reconcile/upgrade_test.go
index 32ac87c..dbfb85b 100644
--- a/internal/reconcile/upgrade_test.go
+++ b/internal/reconcile/upgrade_test.go
@@ -317,6 +317,40 @@ func TestUpgrade_Reconcile(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "upgrade with stale conditions",
+			releases: func(namespace string) []*helmrelease.Release {
+				return []*helmrelease.Release{
+					testutil.BuildRelease(&helmrelease.MockReleaseOptions{
+						Name:      mockReleaseName,
+						Namespace: namespace,
+						Chart:     testutil.BuildChart(),
+						Version:   2,
+						Status:    helmrelease.StatusDeployed,
+					}),
+				}
+			},
+			chart: testutil.BuildChart(),
+			status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
+				return v2.HelmReleaseStatus{
+					Conditions: []metav1.Condition{
+						*conditions.FalseCondition(v2.TestSuccessCondition, v2.TestFailedReason, ""),
+						*conditions.TrueCondition(v2.RemediatedCondition, v2.RollbackSucceededReason, ""),
+					},
+				}
+			},
+			expectConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason,
+					"Helm upgrade succeeded"),
+				*conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason,
+					"Helm upgrade succeeded"),
+			},
+			expectHistory: func(releases []*helmrelease.Release) v2.Snapshots {
+				return v2.Snapshots{
+					release.ObservedToSnapshot(release.ObserveRelease(releases[1])),
+				}
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {