[Elastic Agent] Handling restarts during an upgrade

[kaanyalti]

Currently the agent does not seem to be able to recover from a restart while it is upgrading. Please note that the agent triggers a restart itself in the upgrade process which is expected; the restarts mentioned here are the unexpected ones (process dies and comes back up again).

Definition of done:

• Document what would happen if the agent restarts during an upgrade.
• Document what can be done to improve the current upgrade process so that the agent is more resilient to restarts.

[cmacknz]

We do explicitly test that we can handle a new agent version that restarts or fails to start:

elastic-agent/testing/integration/upgrade_rollback_test.go

Lines 161 to 164 in 4da779d

	// TestStandaloneUpgradeRollbackOnRestarts tests the scenario where upgrading to a new version
	// of Agent fails due to the new Agent binary not starting up. It checks that the Agent is
	// rolled back to the previous version.
	func TestStandaloneUpgradeRollbackOnRestarts(t *testing.T) {

I do think there is a narrow window where we don't clean up properly, after the marker is created but before the watcher is started in the version we are upgrading from:

The critical section is here

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

Lines 333 to 346 in 4da779d

	if err := markUpgrade(u.log,
	paths.Data(), // data dir to place the marker in
	current, // new agent version data
	previous, // old agent version data
	action, det); err != nil {
	u.log.Errorw("Rolling back: marking upgrade failed", "error.message", err)
	rollbackErr := rollbackInstall(ctx, u.log, paths.Top(), hashedDir, currentVersionedHome)
	return nil, goerrors.Join(err, rollbackErr)
	}
	watcherExecutable := selectWatcherExecutable(paths.Top(), previous, current)
	var watcherCmd *exec.Cmd
	if watcherCmd, err = InvokeWatcher(u.log, watcherExecutable); err != nil {

We only ack on start if the marker file exists

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

Line 420 in 4da779d

marker, err := LoadMarker(paths.Data())

Probably there is a way to detect this situation there (e.g. we are reading the marker file for an upgrade to a version that is not the executing one).

An additional problem is the upgrade watcher is a child process on Linux, so when the agent restarts the watcher that would do rollbacks is gone as well. I don’t have a nice way to deal with this, https://github.com/sevlyar/go-daemon is one possibility from the last time I looked.

We used to have a call to process.Release that I removed in #4834 because it did nothing to handle this. There is a bunch more context on this problem in the description of that PR.

[kaanyalti]

We do explicitly test that we can handle a new agent version that restarts or fails to start:

elastic-agent/testing/integration/upgrade_rollback_test.go

Lines 161 to 164 in 4da779d

// TestStandaloneUpgradeRollbackOnRestarts tests the scenario where upgrading to a new version
// of Agent fails due to the new Agent binary not starting up. It checks that the Agent is
// rolled back to the previous version.
func TestStandaloneUpgradeRollbackOnRestarts(t *testing.T) {

The test case only covers a successful restart followed by an agent that cannot start. By successful I mean the old agent is able to start the watcher, migrate its data and exit gracefully. From what I can tell, if the agent restarts, such as during extraction, it will not clean up any upgrade related artifacts or attempt to reinitiate the upgrade process as it starts back up

I don't know if the marker would get cleaned up either. The cleanup seems to be triggered only by the watcher. So it would be nice to figure out if this can have any cascading affects on subsequent upgrades.