Check that upgrade details have been cleared out upon successful upgrade

testing/upgradetest/upgrader.go

@@ -7,6 +7,7 @@ package upgradetest
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"path/filepath"
 	"time"
@@ -243,6 +244,8 @@ func PerformUpgrade(
 		return fmt.Errorf("failed to start agent upgrade to version %q: %w\n%s", endVersionInfo.Binary.Version, err, upgradeOutput)
 	}
 
+	// Check that, right before the Upgrade Watcher is started, the upgrade details in Agent status
+	// show the state as UPG_REPLACING.
 	if err := checkUpgradeDetailsState(ctx, startFixture, details.StateReplacing); err != nil {
 		// error context added by checkUpgradeDetailsState
 		return err
@@ -256,6 +259,8 @@ func PerformUpgrade(
 	}
 	logger.Logf("upgrade watcher started")
 
+	// Check that, while the Upgrade Watcher is running, the upgrade details in Agent status
+	// show the state as UPG_WATCHING.
 	if err := checkUpgradeDetailsState(ctx, startFixture, details.StateWatching); err != nil {
 		// error context added by checkUpgradeDetailsState
 		return err
@@ -281,6 +286,13 @@ func PerformUpgrade(
 		return err
 	}
 
+	// Check that, upon successful upgrade, the upgrade details have been cleared out
+	// from Agent status.
+	if err := checkUpgradeDetailsState(ctx, startFixture, ""); err != nil {
+		// error context added by checkUpgradeDetailsState
+		return err
+	}
+
 	// it is unstable to continue until the watcher is done
 	// the maximum wait time is 1 minutes (2 minutes for grace) some older versions
 	// do not respect the `ConfigureFastWatcher` so we have to kill the watcher after the
@@ -403,14 +415,20 @@ func checkUpgradeDetailsState(ctx context.Context, f *atesting.Fixture, expected
 		return err
 	}
 
-	if status.UpgradeDetails == nil && expectedState == "" {
-		return nil
+	if expectedState == "" {
+		if status.UpgradeDetails == nil {
+			// Expected and actual match, so we're good
+			return nil
+		}
+
+		return errors.New("upgrade details found in status but they were expected to be absent")
 	}
 
-	if status.UpgradeDetails == nil && expectedState != "" {
+	if status.UpgradeDetails == nil {
 		return fmt.Errorf("upgrade details not found in status but expected upgrade details state was [%s]", expectedState)
 	}
 
+	// Neither expected nor actual are nil, so compare the two
 	if status.UpgradeDetails.State == expectedState {
 		return nil
 	}