Re-invoke watcher after takeover

Author: pchila

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

@@ -420,10 +420,26 @@ func (u *Upgrader) rollbackToPreviousVersion(ctx context.Context, topDir string,
 		return nil, fmt.Errorf("stat() on upgrade marker %q failed: %w", updateMarkerPath, err)
 	}
 
-	watcherExecutable := ""
+	// read the upgrade marker
+	updateMarker, err := LoadMarker(paths.DataFrom(topDir))
+	if err != nil {
+		return nil, fmt.Errorf("loading marker: %w", err)
+	}
+
+	if updateMarker == nil {
+		return nil, ErrNilUpdateMarker
+	}
+
+	// extract the agent installs involved in the upgrade and select the most appropriate watcher executable
+	previous, current, err := extractAgentInstallsFromMarker(updateMarker)
+	if err != nil {
+		return nil, fmt.Errorf("extracting current and previous install details: %w", err)
+	}
+	watcherExecutable := u.watcherHelper.SelectWatcherExecutable(topDir, previous, current)
+
 	err = withTakeOverWatcher(ctx, u.log, topDir, u.watcherHelper, func() error {
 		// read the upgrade marker
-		updateMarker, err := LoadMarker(paths.DataFrom(topDir))
+		updateMarker, err = LoadMarker(paths.DataFrom(topDir))
 		if err != nil {
 			return fmt.Errorf("loading marker: %w", err)
 		}
@@ -452,23 +468,17 @@ func (u *Upgrader) rollbackToPreviousVersion(ctx context.Context, topDir string,
 			return fmt.Errorf("persisting rollback in update marker: %w", err)
 		}
 
-		// extract the agent installs involved in the upgrade and select the most appropriate watcher executable
-		previous, current, err := extractAgentInstallsFromMarker(updateMarker)
-		if err != nil {
-			return fmt.Errorf("extracting current and previous install details: %w", err)
-		}
-		watcherExecutable = u.watcherHelper.SelectWatcherExecutable(topDir, previous, current)
 		return nil
 	})
 
-	if err != nil {
-		return nil, err
+	// Invoke watcher again (now that we released the watcher applocks)
+	_, invokeWatcherErr := u.watcherHelper.InvokeWatcher(u.log, watcherExecutable)
+	if invokeWatcherErr != nil {
+		return nil, goerrors.Join(err, fmt.Errorf("invoking watcher: %w", invokeWatcherErr))
 	}
 
-	// Invoke watcher again (now that we released the watcher applocks)
-	_, err = u.watcherHelper.InvokeWatcher(u.log, watcherExecutable)
 	if err != nil {
-		return nil, fmt.Errorf("invoking watcher: %w", err)
+		return nil, err
 	}
 
 	return nil, nil

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

@@ -1143,6 +1143,22 @@ func TestManualRollback(t *testing.T) {
 			},
 			additionalAsserts: nil,
 		},
+		{
+			name: "update marker is malformed - rollback fails",
+			setup: func(t *testing.T, topDir string, agent *infomocks.Agent, watcherHelper *MockWatcherHelper) {
+				err := os.WriteFile(markerFilePath(paths.DataFrom(topDir)), []byte("this is not a proper YAML file"), 0600)
+				require.NoError(t, err, "error setting up update marker")
+				locker := filelock.NewAppLocker(topDir, "watcher.lock")
+				err = locker.TryLock()
+				require.NoError(t, err, "error locking initial watcher AppLocker")
+				// there's no takeover watcher so no expectation on that or InvokeWatcher
+			},
+			artifactSettings:  artifact.DefaultConfig(),
+			upgradeSettings:   configuration.DefaultUpgradeConfig(),
+			version:           "1.2.3",
+			wantErr:           assert.Error,
+			additionalAsserts: nil,
+		},
 		{
 			name: "update marker ok but rollback available is empty - error",
 			setup: func(t *testing.T, topDir string, agent *infomocks.Agent, watcherHelper *MockWatcherHelper) {
@@ -1152,6 +1168,9 @@ func TestManualRollback(t *testing.T) {
 				err = locker.TryLock()
 				require.NoError(t, err, "error locking initial watcher AppLocker")
 				watcherHelper.EXPECT().TakeOverWatcher(t.Context(), mock.Anything, topDir).Return(locker, nil)
+				newerWatcherExecutable := filepath.Join(topDir, "data", "elastic-agent-4.5.6-newver", "elastic-agent")
+				watcherHelper.EXPECT().SelectWatcherExecutable(topDir, agentInstall123, agentInstall456).Return(newerWatcherExecutable)
+				watcherHelper.EXPECT().InvokeWatcher(mock.Anything, newerWatcherExecutable).Return(&exec.Cmd{Path: newerWatcherExecutable, Args: []string{"watch", "for realsies"}}, nil)
 			},
 			artifactSettings: artifact.DefaultConfig(),
 			upgradeSettings:  configuration.DefaultUpgradeConfig(),
@@ -1178,6 +1197,9 @@ func TestManualRollback(t *testing.T) {
 				err = locker.TryLock()
 				require.NoError(t, err, "error locking initial watcher AppLocker")
 				watcherHelper.EXPECT().TakeOverWatcher(t.Context(), mock.Anything, topDir).Return(locker, nil)
+				newerWatcherExecutable := filepath.Join(topDir, "data", "elastic-agent-4.5.6-newver", "elastic-agent")
+				watcherHelper.EXPECT().SelectWatcherExecutable(topDir, agentInstall123, agentInstall456).Return(newerWatcherExecutable)
+				watcherHelper.EXPECT().InvokeWatcher(mock.Anything, newerWatcherExecutable).Return(&exec.Cmd{Path: newerWatcherExecutable, Args: []string{"watch", "for realsies"}}, nil)
 			},
 			artifactSettings: artifact.DefaultConfig(),
 			upgradeSettings:  configuration.DefaultUpgradeConfig(),
@@ -1204,6 +1226,9 @@ func TestManualRollback(t *testing.T) {
 				err = locker.TryLock()
 				require.NoError(t, err, "error locking initial watcher AppLocker")
 				watcherHelper.EXPECT().TakeOverWatcher(t.Context(), mock.Anything, topDir).Return(locker, nil)
+				newerWatcherExecutable := filepath.Join(topDir, "data", "elastic-agent-4.5.6-newver", "elastic-agent")
+				watcherHelper.EXPECT().SelectWatcherExecutable(topDir, agentInstall123, agentInstall456).Return(newerWatcherExecutable)
+				watcherHelper.EXPECT().InvokeWatcher(mock.Anything, newerWatcherExecutable).Return(&exec.Cmd{Path: newerWatcherExecutable, Args: []string{"watch", "for realsies"}}, nil)
 			},
 			artifactSettings: artifact.DefaultConfig(),
 			upgradeSettings:  configuration.DefaultUpgradeConfig(),