[9.2] (backport #10579) Remove upgrade marker if rolling back to versions older than 9.2.0

internal/pkg/agent/application/upgrade/rollback.go

@@ -41,12 +41,13 @@ func Rollback(ctx context.Context, log *logger.Logger, c client.Client, topDirPa
 var FatalRollbackError = errors.New("fatal rollback error")
 
 type RollbackSettings struct {
-	skipCleanup    bool
-	skipRestart    bool
-	preRestartHook RollbackHook
+	SkipCleanup    bool
+	SkipRestart    bool
+	PreRestartHook RollbackHook
+	RemoveMarker   bool
 }
 
-func newRollbackSettings(opts ...RollbackOpt) *RollbackSettings {
+func NewRollbackSettings(opts ...RollbackOpt) *RollbackSettings {
 	rs := new(RollbackSettings)
 	for _, opt := range opts {
 		opt(rs)
@@ -57,20 +58,24 @@ func newRollbackSettings(opts ...RollbackOpt) *RollbackSettings {
 type RollbackOpt func(*RollbackSettings)
 
 func (r *RollbackSettings) SetSkipCleanup(skipCleanup bool) {
-	r.skipCleanup = skipCleanup
+	r.SkipCleanup = skipCleanup
 }
 
 func (r *RollbackSettings) SetSkipRestart(skipRestart bool) {
-	r.skipRestart = skipRestart
+	r.SkipRestart = skipRestart
 }
 
 func (r *RollbackSettings) SetPreRestartHook(preRestartHook RollbackHook) {
-	r.preRestartHook = preRestartHook
+	r.PreRestartHook = preRestartHook
+}
+
+func (r *RollbackSettings) SetRemoveMarker(removeMarker bool) {
+	r.RemoveMarker = removeMarker
 }
 
 func RollbackWithOpts(ctx context.Context, log *logger.Logger, c client.Client, topDirPath string, prevVersionedHome string, prevHash string, opts ...RollbackOpt) error {
 
-	settings := newRollbackSettings(opts...)
+	settings := NewRollbackSettings(opts...)
 
 	symlinkPath := filepath.Join(topDirPath, agentName)
 
@@ -94,8 +99,8 @@ func RollbackWithOpts(ctx context.Context, log *logger.Logger, c client.Client,
 	}
 
 	// Hook
-	if settings.preRestartHook != nil {
-		hookErr := settings.preRestartHook(ctx, log, topDirPath)
+	if settings.PreRestartHook != nil {
+		hookErr := settings.PreRestartHook(ctx, log, topDirPath)
 		if hookErr != nil {
 			if errors.Is(hookErr, FatalRollbackError) {
 				return fmt.Errorf("pre-restart hook failed: %w", hookErr)
@@ -105,7 +110,7 @@ func RollbackWithOpts(ctx context.Context, log *logger.Logger, c client.Client,
 		}
 	}
 
-	if settings.skipRestart {
+	if settings.SkipRestart {
 		log.Info("Skipping restart")
 		return nil
 	}
@@ -116,13 +121,13 @@ func RollbackWithOpts(ctx context.Context, log *logger.Logger, c client.Client,
 		return err
 	}
 
-	if settings.skipCleanup {
+	if settings.SkipCleanup {
 		log.Info("Skipping cleanup")
 		return nil
 	}
 
 	// cleanup everything except version we're rolling back into
-	return Cleanup(log, topDirPath, prevVersionedHome, prevHash, false, true)
+	return Cleanup(log, topDirPath, prevVersionedHome, prevHash, settings.RemoveMarker, true)
 }
 
 // Cleanup removes all artifacts and files related to a specified version.

internal/pkg/agent/application/upgrade/rollback_opts.go

@@ -16,6 +16,7 @@ type RollbackOptionSetter interface {
 	SetSkipCleanup(skipCleanup bool)
 	SetSkipRestart(skipRestart bool)
 	SetPreRestartHook(preRestartHook RollbackHook)
+	SetRemoveMarker(removeMarker bool)
 }
 
 type RollbackOption func(ros RollbackOptionSetter)

internal/pkg/agent/application/upgrade/rollback_test.go

@@ -389,6 +389,7 @@ func TestRollbackWithOpts(t *testing.T) {
 				linkTarget, err := os.Readlink(filepath.Join(topDir, agentExecutableName))
 				assert.NoError(t, err, "reading topPath elastic-agent link")
 				assert.Equal(t, paths.BinaryPath(filepath.Join(topDir, "data", "elastic-agent-1.2.3-SNAPSHOT-abcdef"), agentExecutableName), linkTarget)
+				assert.FileExists(t, filepath.Join(topDir, "data", markerFilename))
 			},
 		},
 		"SkipRestart: no cleanup, no restart": {
@@ -421,6 +422,7 @@ func TestRollbackWithOpts(t *testing.T) {
 				linkTarget, err := os.Readlink(filepath.Join(topDir, agentExecutableName))
 				assert.NoError(t, err, "reading topPath elastic-agent link")
 				assert.Equal(t, paths.BinaryPath(filepath.Join(topDir, "data", "elastic-agent-1.2.3-SNAPSHOT-abcdef"), agentExecutableName), linkTarget)
+				assert.FileExists(t, filepath.Join(topDir, "data", markerFilename))
 			},
 		},
 		"Prerestart hook not fatal error: rollback, cleanup and restart as normal": {
@@ -467,6 +469,7 @@ func TestRollbackWithOpts(t *testing.T) {
 				if assert.Len(t, snippetLogs, 1) {
 					assert.Equal(t, zapcore.WarnLevel, snippetLogs[0].Level)
 				}
+				assert.FileExists(t, filepath.Join(topDir, "data", markerFilename))
 			},
 		},
 		"Prerestart hook fatal error: rollback then return the error (no restart, no cleanup)": {
@@ -503,14 +506,55 @@ func TestRollbackWithOpts(t *testing.T) {
 				linkTarget, err := os.Readlink(filepath.Join(topDir, agentExecutableName))
 				assert.NoError(t, err, "reading topPath elastic-agent link")
 				assert.Equal(t, paths.BinaryPath(filepath.Join(topDir, "data", "elastic-agent-1.2.3-SNAPSHOT-abcdef"), agentExecutableName), linkTarget)
+				assert.FileExists(t, filepath.Join(topDir, "data", markerFilename))
+			},
+		},
+		"RemoveMarker true: delete the upgrade marker": {
+			agentInstallsSetup: setupAgentInstallations{
+				installedAgents: []testAgentInstall{
+					{
+						version:          version123Snapshot,
+						useVersionInPath: true,
+					},
+					{
+						version:          version456Snapshot,
+						useVersionInPath: true,
+					},
+				},
+			},
+			setupMocks: func(mockClient *client.MockClient) {
+				mockClient.EXPECT().Connect(
+					mock.AnythingOfType("*context.timerCtx"),
+					mock.AnythingOfType("*grpc.funcDialOption"),
+					mock.AnythingOfType("*grpc.funcDialOption"),
+				).Return(nil)
+				mockClient.EXPECT().Disconnect().Return()
+				mockClient.EXPECT().Restart(mock.Anything).Return(nil).Once()
+			},
+			args: args{
+				prevVersionedHome: "data/elastic-agent-1.2.3-SNAPSHOT-abcdef",
+				prevHash:          "abcdef",
+				rollbackOptions: []RollbackOpt{
+					func(rs *RollbackSettings) {
+						rs.SetRemoveMarker(true)
+					},
+				}},
+			wantErr: assert.NoError,
+			checkAfterRollback: func(t *testing.T, logs *observer.ObservedLogs, topDir string) {
+				assertAgentInstallExists(t, filepath.Join(topDir, "data", "elastic-agent-1.2.3-SNAPSHOT-abcdef"), agentExecutableName)
+				assertAgentInstallCleaned(t, filepath.Join(topDir, "data", "elastic-agent-4.5.6-SNAPSHOT-ghijkl"), agentExecutableName)
+				linkTarget, err := os.Readlink(filepath.Join(topDir, agentExecutableName))
+				assert.NoError(t, err, "reading topPath elastic-agent link")
+				assert.Equal(t, paths.BinaryPath(filepath.Join(topDir, "data", "elastic-agent-1.2.3-SNAPSHOT-abcdef"), agentExecutableName), linkTarget)
+				assert.NoFileExists(t, filepath.Join(topDir, "data", markerFilename))
 			},
 		},
 	}
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			testLogger, obsLogs := loggertest.New(t.Name())
 			testTop := t.TempDir()
-			setupAgents(t, testLogger, testTop, tt.agentInstallsSetup, false)
+			setupAgents(t, testLogger, testTop, tt.agentInstallsSetup, true)
 
 			// mock client
 			mockClient := client.NewMockClient(t)

internal/pkg/agent/cmd/watch.go

@@ -13,6 +13,8 @@ import (
 
 	"github.com/spf13/cobra"
 
+	semver "github.com/elastic/elastic-agent/pkg/version"
+
 	"github.com/elastic/elastic-agent-libs/logp"
 	"github.com/elastic/elastic-agent-libs/logp/configure"
 	"github.com/elastic/elastic-agent/pkg/control/v2/client"
@@ -140,6 +142,12 @@ func WithSkipRestart(skipRestart bool) upgrade.RollbackOption {
 	}
 }
 
+func WithRemoveMarker(removeMarker bool) upgrade.RollbackOption {
+	return func(ros upgrade.RollbackOptionSetter) {
+		ros.SetRemoveMarker(removeMarker)
+	}
+}
+
 type installationModifier interface {
 	Cleanup(log *logger.Logger, topDirPath, currentVersionedHome, currentHash string, removeMarker, keepLogs bool) error
 	Rollback(ctx context.Context, log *logger.Logger, c client.Client, topDirPath, prevVersionedHome, prevHash string, opts ...upgrade.RollbackOption) error
@@ -215,7 +223,19 @@ func watchCmd(log *logp.Logger, topDir string, cfg *configuration.UpgradeWatcher
 		log.Error("Error detected, proceeding to rollback: %v", err)
 
 		upgradeDetails.SetStateWithReason(details.StateRollback, details.ReasonWatchFailed)
-		err = installModifier.Rollback(ctx, log, client.New(), paths.Top(), marker.PrevVersionedHome, marker.PrevHash)
+
+		// by default remove marker (backward compatible behaviour)
+		removeMarker := true
+
+		previousVersion, versionParseErr := semver.ParseVersion(marker.PrevVersion)
+		if versionParseErr != nil {
+			log.Errorf("could not parse previous version %s: %s", marker.PrevVersion, versionParseErr)
+		} else if !previousVersion.Less(*semver.NewParsedSemVer(9, 2, 0, "SNAPSHOT", "")) {
+			// leave the marker in place when rolling back to agent >= 9.2.0-SNAPSHOT as it will be used to determine
+			// that agent was rolled back and the reason
+			removeMarker = false
+		}
+		err = installModifier.Rollback(ctx, log, client.New(), paths.Top(), marker.PrevVersionedHome, marker.PrevHash, WithRemoveMarker(removeMarker))
 		if err != nil {
 			log.Error("rollback failed", err)
 			upgradeDetails.Fail(err)

internal/pkg/agent/cmd/watch_test.go

@@ -153,7 +153,48 @@ func Test_watchCmd(t *testing.T) {
 			wantErr: assert.NoError,
 		},
 		{
-			name: "unhappy path: error watching, rollback to previous install",
+			name: "unhappy path: error watching, rollback to previous install, leaving the upgrade marker",
+			setupUpgradeMarker: func(t *testing.T, topDir string, watcher *mockAgentWatcher, installModifier *mockInstallationModifier) {
+				dataDirPath := paths.DataFrom(topDir)
+				err := os.MkdirAll(dataDirPath, 0755)
+				require.NoError(t, err)
+				err = upgrade.SaveMarker(
+					dataDirPath,
+					&upgrade.UpdateMarker{
+						Version:           "9.3.0",
+						Hash:              "newver",
+						VersionedHome:     "elastic-agent-9.3.0-newver",
+						UpdatedOn:         time.Now(),
+						PrevVersion:       "9.2.0",
+						PrevHash:          "prvver",
+						PrevVersionedHome: "elastic-agent-9.2.0-prvver",
+						Acked:             false,
+						Action:            nil,
+						Details:           nil,
+					},
+					true,
+				)
+				require.NoError(t, err)
+
+				watcher.EXPECT().
+					Watch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+					Return(errors.New("some watch error due to agent misbehaving"))
+				installModifier.EXPECT().
+					Rollback(mock.Anything, mock.Anything, mock.Anything, paths.Top(), "elastic-agent-9.2.0-prvver", "prvver", mock.MatchedBy(func(opt upgrade.RollbackOption) bool {
+						settings := upgrade.NewRollbackSettings()
+						opt(settings)
+
+						return !settings.RemoveMarker
+					})).
+					Return(nil)
+			},
+			args: args{
+				cfg: configuration.DefaultUpgradeConfig().Watcher,
+			},
+			wantErr: assert.NoError,
+		},
+		{
+			name: "unhappy path: error watching, rollback to previous install, removing upgrade marker if version is < 9.2.0-SNAPSHOT",
 			setupUpgradeMarker: func(t *testing.T, topDir string, watcher *mockAgentWatcher, installModifier *mockInstallationModifier) {
 				dataDirPath := paths.DataFrom(topDir)
 				err := os.MkdirAll(dataDirPath, 0755)
@@ -167,7 +208,7 @@ func Test_watchCmd(t *testing.T) {
 						UpdatedOn:         time.Now(),
 						PrevVersion:       "1.2.3",
 						PrevHash:          "prvver",
-						PrevVersionedHome: "elastic-agent-prvver",
+						PrevVersionedHome: "elastic-agent-1.2.3-prvver",
 						Acked:             false,
 						Action:            nil,
 						Details:           nil, //details.NewDetails("4.5.6", details.StateReplacing, ""),
@@ -180,8 +221,19 @@ func Test_watchCmd(t *testing.T) {
 					Watch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).
 					Return(errors.New("some watch error due to agent misbehaving"))
 				installModifier.EXPECT().
-					Rollback(mock.Anything, mock.Anything, mock.Anything, paths.Top(), "elastic-agent-prvver", "prvver", mock.Anything).
-					Return(nil)
+					Rollback(
+						mock.Anything,
+						mock.Anything,
+						mock.Anything,
+						paths.Top(),
+						"elastic-agent-1.2.3-prvver",
+						"prvver",
+						mock.MatchedBy(func(opt upgrade.RollbackOption) bool {
+							settings := upgrade.NewRollbackSettings()
+							opt(settings)
+
+							return settings.RemoveMarker
+						})).Return(nil)
 			},
 			args: args{
 				cfg: configuration.DefaultUpgradeConfig().Watcher,