diff --git a/internal/pkg/agent/application/upgrade/manual_rollback.go b/internal/pkg/agent/application/upgrade/manual_rollback.go index 664ba586da1..6b78163b303 100644 --- a/internal/pkg/agent/application/upgrade/manual_rollback.go +++ b/internal/pkg/agent/application/upgrade/manual_rollback.go @@ -102,6 +102,13 @@ func rollbackUsingAgentInstalls(log *logger.Logger, watcherHelper WatcherHelper, return "", "", fmt.Errorf("version %q not listed among the available rollbacks: %w", rollbackVersion, ErrNoRollbacksAvailable) } + if filepath.IsAbs(targetInstall) { + targetInstall, err = filepath.Rel(topDir, targetInstall) + if err != nil { + return "", "", fmt.Errorf("error calculating path of install %q relative to %q: %w", targetInstall, topDir, err) + } + } + prevAgentParsedVersion, err := version.ParseVersion(targetTTLMarker.Version) if err != nil { return "", "", fmt.Errorf("parsing version of target install %+v: %w", targetInstall, err) diff --git a/internal/pkg/agent/application/upgrade/rollback.go b/internal/pkg/agent/application/upgrade/rollback.go index 96f97fed1cc..380e074ab03 100644 --- a/internal/pkg/agent/application/upgrade/rollback.go +++ b/internal/pkg/agent/application/upgrade/rollback.go @@ -174,16 +174,22 @@ func cleanup(log *logger.Logger, topDirPath string, removeMarker, keepLogs bool, dirPrefix := fmt.Sprintf("%s-", AgentName) - relativeHomePaths := make([]string, len(versionedHomesToKeep)) - for i, h := range versionedHomesToKeep { - relHomePath, err := filepath.Rel("data", h) + log.Infof("versioned homes to keep: %v", versionedHomesToKeep) + + var cumulativeError error + relativeHomePaths := make([]string, 0, len(versionedHomesToKeep)) + for _, h := range versionedHomesToKeep { + relHomePath, err := filepath.Rel(dataDirPath, filepath.Join(topDirPath, h)) if err != nil { - return fmt.Errorf("extracting elastic-agent path relative to data directory from %s: %w", h, err) + cumulativeError = goerrors.Join(cumulativeError, fmt.Errorf("extracting elastic-agent path relative to data directory from %s: %w", h, err)) + // best effort: try to use the entry as-is, without calculating the path relative to `data` + relHomePath = h } - relativeHomePaths[i] = relHomePath + relativeHomePaths = append(relativeHomePaths, relHomePath) } - var errs []error + log.Infof("Starting cleanup of versioned homes. Keeping: %v", relativeHomePaths) + for _, dir := range subdirs { if slices.Contains(relativeHomePaths, dir) { continue @@ -200,11 +206,11 @@ func cleanup(log *logger.Logger, topDirPath string, removeMarker, keepLogs bool, ignoredDirs = append(ignoredDirs, "logs") } if cleanupErr := install.RemoveBut(hashedDir, true, ignoredDirs...); cleanupErr != nil { - errs = append(errs, cleanupErr) + cumulativeError = goerrors.Join(cumulativeError, cleanupErr) } } - return goerrors.Join(errs...) + return cumulativeError } // InvokeWatcher invokes an agent instance using watcher argument for watching behavior of diff --git a/internal/pkg/agent/cmd/watch.go b/internal/pkg/agent/cmd/watch.go index 2308a7630d5..d13df5732a3 100644 --- a/internal/pkg/agent/cmd/watch.go +++ b/internal/pkg/agent/cmd/watch.go @@ -102,6 +102,7 @@ func newWatchCommandWithArgs(_ []string, streams *cli.IOStreams) *cobra.Command fmt.Fprintf(streams.Err, "Rollback command failed: %v\n", err) os.Exit(errorRollbackFailed) } + return } if err = withAppLocker(log, func() error { @@ -200,7 +201,19 @@ func watchCmd(log *logp.Logger, topDir string, cfg *configuration.UpgradeWatcher // hash is the same as hash of agent which initiated watcher. versionedHomesToKeep := make([]string, 0, len(marker.RollbacksAvailable)+1) // current version needs to be kept - versionedHomesToKeep = append(versionedHomesToKeep, paths.VersionedHome(topDir)) + if marker.Details != nil && marker.Details.State == details.StateRollback { + // we need to keep the previous versioned home (we have rolled back) + versionedHomesToKeep = append(versionedHomesToKeep, marker.PrevVersionedHome) + } else { + // we need to keep the upgraded version, since it has not been rolled back + absCurrentVersionedHome := paths.VersionedHome(topDir) + currentVersionedHome, err := filepath.Rel(topDir, absCurrentVersionedHome) + if err != nil { + return fmt.Errorf("extracting current home path %q relative to %q: %w", absCurrentVersionedHome, topDir, err) + } + versionedHomesToKeep = append(versionedHomesToKeep, currentVersionedHome) + } + versionedHomesToKeep = appendAvailableRollbacks(log, marker, versionedHomesToKeep) log.Infof("About to clean up upgrade. Keeping versioned homes: %v", versionedHomesToKeep) if err := installModifier.Cleanup(log, paths.Top(), true, false, versionedHomesToKeep...); err != nil { @@ -320,6 +333,14 @@ func rollback(log *logp.Logger, topDir string, client client.Client, installModi // FIXME get the hash from the list of installs or the manifest or the versioned home // This is only a placeholder in case there is no versionedHome defined (which we always have) hash := "" + if filepath.IsAbs(versionedHome) { + // if the versioned home is an absolute path we need to normalize it relative to the current topDir as the + // cleanup() will expect relative paths + versionedHome, err = filepath.Rel(topDir, versionedHome) + if err != nil { + return fmt.Errorf("extract from %q a path relative to %q: %w", versionedHome, topDir, err) + } + } err = installModifier.Rollback(context.Background(), log, client, topDir, versionedHome, hash, WithPreRestartHook(updateMarkerAndDetails)) if err != nil { return fmt.Errorf("rolling back: %w", err) diff --git a/internal/pkg/agent/cmd/watch_test.go b/internal/pkg/agent/cmd/watch_test.go index 0ebce30753f..c6f14772e49 100644 --- a/internal/pkg/agent/cmd/watch_test.go +++ b/internal/pkg/agent/cmd/watch_test.go @@ -122,11 +122,11 @@ func Test_watchCmd(t *testing.T) { &upgrade.UpdateMarker{ Version: "4.5.6", Hash: "newver", - VersionedHome: "elastic-agent-4.5.6-newver", + VersionedHome: filepath.Join("data", "elastic-agent-4.5.6-newver"), UpdatedOn: time.Now(), PrevVersion: "1.2.3", PrevHash: "prvver", - PrevVersionedHome: "elastic-agent-prvver", + PrevVersionedHome: filepath.Join("data", "elastic-agent-prvver"), Acked: false, Action: nil, Details: nil, //details.NewDetails("4.5.6", details.StateReplacing, ""), @@ -143,7 +143,7 @@ func Test_watchCmd(t *testing.T) { expectedRemoveMarkerFlag := runtime.GOOS != "windows" installModifier.EXPECT(). - Cleanup(mock.Anything, topDir, expectedRemoveMarkerFlag, false, "elastic-agent-4.5.6-newver"). + Cleanup(mock.Anything, topDir, expectedRemoveMarkerFlag, false, filepath.Join("data", "elastic-agent-4.5.6-newver")). Return(nil) }, args: args{ @@ -157,16 +157,17 @@ func Test_watchCmd(t *testing.T) { dataDirPath := paths.DataFrom(topDir) err := os.MkdirAll(dataDirPath, 0755) require.NoError(t, err) + previousVersionedHome := filepath.Join("data", "elastic-agent-9.2.0-prvver") err = upgrade.SaveMarker( dataDirPath, &upgrade.UpdateMarker{ Version: "9.3.0", Hash: "newver", - VersionedHome: "elastic-agent-9.3.0-newver", + VersionedHome: filepath.Join("data", "elastic-agent-9.3.0-newver"), UpdatedOn: time.Now(), PrevVersion: "9.2.0", PrevHash: "prvver", - PrevVersionedHome: "elastic-agent-9.2.0-prvver", + PrevVersionedHome: previousVersionedHome, Acked: false, Action: nil, Details: nil, @@ -179,7 +180,7 @@ 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-9.2.0-prvver", "prvver", mock.MatchedBy(func(opt upgrade.RollbackOption) bool { + Rollback(mock.Anything, mock.Anything, mock.Anything, paths.Top(), previousVersionedHome, "prvver", mock.MatchedBy(func(opt upgrade.RollbackOption) bool { settings := upgrade.NewRollbackSettings() opt(settings) @@ -198,16 +199,17 @@ func Test_watchCmd(t *testing.T) { dataDirPath := paths.DataFrom(topDir) err := os.MkdirAll(dataDirPath, 0755) require.NoError(t, err) + previousVersionedHome := filepath.Join("data", "elastic-agent-1.2.3-prvver") err = upgrade.SaveMarker( dataDirPath, &upgrade.UpdateMarker{ Version: "4.5.6", Hash: "newver", - VersionedHome: "elastic-agent-4.5.6-newver", + VersionedHome: filepath.Join("data", "elastic-agent-4.5.6-newver"), UpdatedOn: time.Now(), PrevVersion: "1.2.3", PrevHash: "prvver", - PrevVersionedHome: "elastic-agent-1.2.3-prvver", + PrevVersionedHome: previousVersionedHome, Acked: false, Action: nil, Details: nil, //details.NewDetails("4.5.6", details.StateReplacing, ""), @@ -225,7 +227,7 @@ func Test_watchCmd(t *testing.T) { mock.Anything, mock.Anything, paths.Top(), - "elastic-agent-1.2.3-prvver", + previousVersionedHome, "prvver", mock.MatchedBy(func(opt upgrade.RollbackOption) bool { settings := upgrade.NewRollbackSettings() @@ -245,16 +247,17 @@ func Test_watchCmd(t *testing.T) { dataDirPath := paths.DataFrom(topDir) err := os.MkdirAll(dataDirPath, 0755) require.NoError(t, err) + previousVersionedHome := filepath.Join("data", "elastic-agent-prvver") err = upgrade.SaveMarker( dataDirPath, &upgrade.UpdateMarker{ Version: "4.5.6", Hash: "newver", - VersionedHome: "elastic-agent-4.5.6-newver", + VersionedHome: filepath.Join("data", "elastic-agent-4.5.6-newver"), UpdatedOn: time.Now(), PrevVersion: "1.2.3", PrevHash: "prvver", - PrevVersionedHome: "elastic-agent-prvver", + PrevVersionedHome: previousVersionedHome, Acked: false, Action: nil, Details: &details.Details{ @@ -271,7 +274,7 @@ func Test_watchCmd(t *testing.T) { // topdir, prevVersionedHome and prevHash are not taken from the upgrade marker, otherwise they would be // installModifier.EXPECT(). - Cleanup(mock.Anything, paths.Top(), true, false, paths.VersionedHome(topDir)). + Cleanup(mock.Anything, paths.Top(), true, false, previousVersionedHome). Return(nil) }, args: args{ @@ -291,11 +294,11 @@ func Test_watchCmd(t *testing.T) { &upgrade.UpdateMarker{ Version: "4.5.6", Hash: "newver", - VersionedHome: "elastic-agent-4.5.6-newver", + VersionedHome: filepath.Join("data", "elastic-agent-4.5.6-newver"), UpdatedOn: updatedOn, PrevVersion: "1.2.3", PrevHash: "prvver", - PrevVersionedHome: "elastic-agent-prvver", + PrevVersionedHome: filepath.Join("data", "elastic-agent-prvver"), Acked: false, Action: nil, Details: nil, @@ -304,10 +307,10 @@ func Test_watchCmd(t *testing.T) { ) require.NoError(t, err) - // topdir, prevVersionedHome and prevHash are not taken from the upgrade marker, otherwise they would be - // + // topdir, versionedHome and hash are not taken from the upgrade marker, otherwise they would be + // installModifier.EXPECT(). - Cleanup(mock.Anything, paths.Top(), true, false, paths.VersionedHome(topDir)). + Cleanup(mock.Anything, paths.Top(), true, false, filepath.Join("data", "elastic-agent-unknow")). Return(nil) }, args: args{ @@ -321,7 +324,7 @@ func Test_watchCmd(t *testing.T) { wantErr: assert.NoError, }, { - name: "Desired outcome is rollback no upgrade details, no rollback and simple cleanup", + name: "Default config, no rollback and simple cleanup", setupUpgradeMarker: func(t *testing.T, tmpDir string, watcher *mockAgentWatcher, installModifier *mockInstallationModifier) { dataDirPath := paths.DataFrom(tmpDir) err := os.MkdirAll(dataDirPath, 0755) @@ -333,11 +336,11 @@ func Test_watchCmd(t *testing.T) { &upgrade.UpdateMarker{ Version: "4.5.6", Hash: "newver", - VersionedHome: "elastic-agent-4.5.6-newver", + VersionedHome: filepath.Join("data", "elastic-agent-4.5.6-newver"), UpdatedOn: updatedOn, PrevVersion: "1.2.3", PrevHash: "prvver", - PrevVersionedHome: "elastic-agent-prvver", + PrevVersionedHome: filepath.Join("data", "elastic-agent-prvver"), Acked: false, Action: &fleetapi.ActionUpgrade{ ActionID: "action-id", @@ -351,7 +354,7 @@ func Test_watchCmd(t *testing.T) { require.NoError(t, err) installModifier.EXPECT(). - Cleanup(mock.Anything, paths.Top(), true, false, paths.VersionedHome(tmpDir)). + Cleanup(mock.Anything, paths.Top(), true, false, filepath.Join("data", "elastic-agent-unknow")). Return(nil) }, args: args{