From e897bd60497844716119f3e0a09bd710449a26c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paolo=20Chil=C3=A0?= Date: Mon, 5 Feb 2024 10:40:45 +0100 Subject: [PATCH] Support version in watcher cleanup rollback (#4176) * extract directory paths to allow unit testing (instead of nested paths.* calls) * introduce tests for watcher cleanup and rollback * close data directory after cleanup --- .../pkg/agent/application/paths/common.go | 9 +- .../pkg/agent/application/upgrade/rollback.go | 40 +- .../application/upgrade/rollback_test.go | 498 ++++++++++++++++++ .../agent/application/upgrade/step_mark.go | 34 +- .../agent/application/upgrade/step_relink.go | 16 +- .../agent/application/upgrade/step_unpack.go | 10 +- .../application/upgrade/step_unpack_test.go | 207 ++++---- .../pkg/agent/application/upgrade/upgrade.go | 30 +- internal/pkg/agent/cmd/run.go | 2 +- internal/pkg/agent/cmd/watch.go | 9 +- 10 files changed, 670 insertions(+), 185 deletions(-) create mode 100644 internal/pkg/agent/application/upgrade/rollback_test.go diff --git a/internal/pkg/agent/application/paths/common.go b/internal/pkg/agent/application/paths/common.go index 5a90712744a..871435e65a1 100644 --- a/internal/pkg/agent/application/paths/common.go +++ b/internal/pkg/agent/application/paths/common.go @@ -180,11 +180,16 @@ func ExternalInputs() string { // Data returns the data directory for Agent func Data() string { + return DataFrom(Top()) +} + +// DataFrom returns the data directory for Agent using the passed directory as top path +func DataFrom(topDirPath string) string { if unversionedHome { // unversioned means the topPath is the data path - return topPath + return topDirPath } - return filepath.Join(Top(), "data") + return filepath.Join(topDirPath, "data") } // Run returns the run directory for Agent diff --git a/internal/pkg/agent/application/upgrade/rollback.go b/internal/pkg/agent/application/upgrade/rollback.go index e3f4a4faf90..9f730c5b68c 100644 --- a/internal/pkg/agent/application/upgrade/rollback.go +++ b/internal/pkg/agent/application/upgrade/rollback.go @@ -33,55 +33,64 @@ const ( ) // Rollback rollbacks to previous version which was functioning before upgrade. -func Rollback(ctx context.Context, log *logger.Logger, prevVersionedHome, prevHash string, currentHash string) error { - symlinkPath := filepath.Join(paths.Top(), agentName) +func Rollback(ctx context.Context, log *logger.Logger, c client.Client, topDirPath, prevVersionedHome, prevHash string) error { + symlinkPath := filepath.Join(topDirPath, agentName) var symlinkTarget string if prevVersionedHome != "" { - symlinkTarget = paths.BinaryPath(filepath.Join(paths.Top(), prevVersionedHome), agentName) + symlinkTarget = paths.BinaryPath(filepath.Join(topDirPath, prevVersionedHome), agentName) } else { // fallback for upgrades that didn't use the manifest and path remapping hashedDir := fmt.Sprintf("%s-%s", agentName, prevHash) // paths.BinaryPath properly derives the binary directory depending on the platform. The path to the binary for macOS is inside of the app bundle. - symlinkTarget = paths.BinaryPath(filepath.Join(paths.Top(), "data", hashedDir), agentName) + symlinkTarget = paths.BinaryPath(filepath.Join(paths.DataFrom(topDirPath), hashedDir), agentName) } // change symlink - if err := changeSymlinkInternal(log, symlinkPath, symlinkTarget); err != nil { + if err := changeSymlinkInternal(log, topDirPath, symlinkPath, symlinkTarget); err != nil { return err } // revert active commit - if err := UpdateActiveCommit(log, prevHash); err != nil { + if err := UpdateActiveCommit(log, topDirPath, prevHash); err != nil { return err } // Restart log.Info("Restarting the agent after rollback") - if err := restartAgent(ctx, log); err != nil { + if err := restartAgent(ctx, log, c); err != nil { return err } // cleanup everything except version we're rolling back into - return Cleanup(log, prevVersionedHome, prevHash, true, true) + return Cleanup(log, topDirPath, prevVersionedHome, prevHash, true, true) } // Cleanup removes all artifacts and files related to a specified version. -func Cleanup(log *logger.Logger, currentVersionedHome, currentHash string, removeMarker bool, keepLogs bool) error { +func Cleanup(log *logger.Logger, topDirPath, currentVersionedHome, currentHash string, removeMarker, keepLogs bool) error { log.Infow("Cleaning up upgrade", "hash", currentHash, "remove_marker", removeMarker) <-time.After(afterRestartDelay) + // data directory path + dataDirPath := paths.DataFrom(topDirPath) + // remove upgrade marker if removeMarker { - if err := CleanMarker(log); err != nil { + if err := CleanMarker(log, dataDirPath); err != nil { return err } } // remove data/elastic-agent-{hash} - dataDir, err := os.Open(paths.Data()) + dataDir, err := os.Open(dataDirPath) if err != nil { return err } + defer func(dataDir *os.File) { + err := dataDir.Close() + if err != nil { + log.Errorw("Error closing data directory", "file.directory", dataDirPath) + } + }(dataDir) subdirs, err := dataDir.Readdirnames(0) if err != nil { @@ -89,8 +98,8 @@ func Cleanup(log *logger.Logger, currentVersionedHome, currentHash string, remov } // remove symlink to avoid upgrade failures, ignore error - prevSymlink := prevSymlinkPath() - log.Infow("Removing previous symlink path", "file.path", prevSymlinkPath()) + prevSymlink := prevSymlinkPath(topDirPath) + log.Infow("Removing previous symlink path", "file.path", prevSymlinkPath(topDirPath)) _ = os.Remove(prevSymlink) dirPrefix := fmt.Sprintf("%s-", agentName) @@ -113,7 +122,7 @@ func Cleanup(log *logger.Logger, currentVersionedHome, currentHash string, remov continue } - hashedDir := filepath.Join(paths.Data(), dir) + hashedDir := filepath.Join(dataDirPath, dir) log.Infow("Removing hashed data directory", "file.path", hashedDir) var ignoredDirs []string if keepLogs { @@ -155,9 +164,8 @@ func InvokeWatcher(log *logger.Logger) error { return nil } -func restartAgent(ctx context.Context, log *logger.Logger) error { +func restartAgent(ctx context.Context, log *logger.Logger, c client.Client) error { restartViaDaemonFn := func(ctx context.Context) error { - c := client.New() connectCtx, connectCancel := context.WithTimeout(ctx, 3*time.Second) defer connectCancel() err := c.Connect(connectCtx, grpc.WithBlock(), grpc.WithDisableRetry()) diff --git a/internal/pkg/agent/application/upgrade/rollback_test.go b/internal/pkg/agent/application/upgrade/rollback_test.go new file mode 100644 index 00000000000..30e259c9d2a --- /dev/null +++ b/internal/pkg/agent/application/upgrade/rollback_test.go @@ -0,0 +1,498 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package upgrade + +import ( + "context" + "fmt" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/elastic/elastic-agent/internal/pkg/agent/application/paths" + "github.com/elastic/elastic-agent/pkg/control/v2/client/mocks" + "github.com/elastic/elastic-agent/pkg/core/logger" +) + +type hookFunc func(t *testing.T, topDir string) + +type agentVersion struct { + version string + hash string +} + +type agentInstall struct { + version agentVersion + useVersionInPath bool +} + +type setupAgentInstallations struct { + installedAgents []agentInstall + upgradeFrom agentVersion + upgradeTo agentVersion + currentAgent agentVersion +} + +var ( + version123Snapshot = agentVersion{ + version: "1.2.3-SNAPSHOT", + hash: "abcdef", + } + version456Snapshot = agentVersion{ + version: "4.5.6-SNAPSHOT", + hash: "ghijkl", + } +) + +func TestCleanup(t *testing.T) { + type args struct { + currentVersionedHome string + currentHash string + removeMarker bool + keepLogs bool + } + + tests := map[string]struct { + args args + agentInstallsSetup setupAgentInstallations + additionalSetup hookFunc + wantErr assert.ErrorAssertionFunc + checkAfterCleanup hookFunc + }{ + "cleanup without versionedHome (legacy upgrade process)": { + args: args{ + currentVersionedHome: "data/elastic-agent-ghijkl", + currentHash: "ghijkl", + removeMarker: true, + keepLogs: false, + }, + agentInstallsSetup: setupAgentInstallations{ + installedAgents: []agentInstall{ + { + version: version123Snapshot, + useVersionInPath: false, + }, + { + version: version456Snapshot, + useVersionInPath: false, + }, + }, + upgradeFrom: version123Snapshot, + upgradeTo: version456Snapshot, + currentAgent: version456Snapshot, + }, + wantErr: assert.NoError, + checkAfterCleanup: func(t *testing.T, topDir string) { + oldAgentHome := filepath.Join("data", "elastic-agent-abcdef") + newAgentHome := filepath.Join("data", "elastic-agent-ghijkl") + checkFilesAfterCleanup(t, topDir, newAgentHome, oldAgentHome) + }, + }, + "cleanup with versionedHome (new upgrade process)": { + args: args{ + currentVersionedHome: "data/elastic-agent-4.5.6-SNAPSHOT-ghijkl", + currentHash: "ghijkl", + removeMarker: true, + keepLogs: false, + }, + agentInstallsSetup: setupAgentInstallations{ + installedAgents: []agentInstall{ + { + version: version123Snapshot, + useVersionInPath: true, + }, + { + version: version456Snapshot, + useVersionInPath: true, + }, + }, + upgradeFrom: version123Snapshot, + upgradeTo: version456Snapshot, + currentAgent: version456Snapshot, + }, + wantErr: assert.NoError, + checkAfterCleanup: func(t *testing.T, topDir string) { + oldAgentHome := filepath.Join("data", "elastic-agent-1.2.3-SNAPSHOT-abcdef") + newAgentHome := filepath.Join("data", "elastic-agent-4.5.6-SNAPSHOT-ghijkl") + checkFilesAfterCleanup(t, topDir, newAgentHome, oldAgentHome) + }, + }, + "cleanup with versionedHome only on the new agent (new upgrade process from an old agent upgraded with legacy process)": { + args: args{ + currentVersionedHome: "data/elastic-agent-4.5.6-SNAPSHOT-ghijkl", + currentHash: "ghijkl", + removeMarker: true, + keepLogs: false, + }, + agentInstallsSetup: setupAgentInstallations{ + installedAgents: []agentInstall{ + { + version: version123Snapshot, + useVersionInPath: false, + }, + { + version: version456Snapshot, + useVersionInPath: true, + }, + }, + upgradeFrom: version123Snapshot, + upgradeTo: version456Snapshot, + currentAgent: version456Snapshot, + }, + wantErr: assert.NoError, + checkAfterCleanup: func(t *testing.T, topDir string) { + oldAgentHome := filepath.Join("data", "elastic-agent-abcdef") + newAgentHome := filepath.Join("data", "elastic-agent-4.5.6-SNAPSHOT-ghijkl") + checkFilesAfterCleanup(t, topDir, newAgentHome, oldAgentHome) + }, + }, + "cleanup with versionedHome only on the new agent + extra old agent installs": { + args: args{ + currentVersionedHome: "data/elastic-agent-4.5.6-SNAPSHOT-ghijkl", + currentHash: "ghijkl", + removeMarker: true, + keepLogs: false, + }, + agentInstallsSetup: setupAgentInstallations{ + installedAgents: []agentInstall{ + { + version: agentVersion{ + version: "0.9.9", + hash: "aaaaaa", + }, + useVersionInPath: false, + }, + { + version: agentVersion{ + version: "1.1.1", + hash: "aaabbb", + }, + useVersionInPath: false, + }, + { + version: version123Snapshot, + useVersionInPath: false, + }, + { + version: version456Snapshot, + useVersionInPath: true, + }, + }, + upgradeFrom: version123Snapshot, + upgradeTo: version456Snapshot, + currentAgent: version456Snapshot, + }, + wantErr: assert.NoError, + checkAfterCleanup: func(t *testing.T, topDir string) { + newAgentHome := filepath.Join("data", "elastic-agent-4.5.6-SNAPSHOT-ghijkl") + oldAgentHomes := []string{ + filepath.Join("data", "elastic-agent-abcdef"), + filepath.Join("data", "elastic-agent-aaabbb"), + filepath.Join("data", "elastic-agent-aaaaaa"), + } + + checkFilesAfterCleanup(t, topDir, newAgentHome, oldAgentHomes...) + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + testLogger, _ := logger.NewTesting(t.Name()) + testTop := t.TempDir() + setupAgents(t, testLogger, testTop, tt.agentInstallsSetup) + if tt.additionalSetup != nil { + tt.additionalSetup(t, testTop) + } + marker, err := LoadMarker(paths.DataFrom(testTop)) + require.NoError(t, err, "error loading update marker") + require.NotNil(t, marker, "loaded marker must not be nil") + t.Logf("Loaded update marker %+v", marker) + tt.wantErr(t, Cleanup(testLogger, testTop, marker.VersionedHome, marker.Hash, tt.args.removeMarker, tt.args.keepLogs), fmt.Sprintf("Cleanup(%v, %v, %v, %v)", marker.VersionedHome, marker.Hash, tt.args.removeMarker, tt.args.keepLogs)) + tt.checkAfterCleanup(t, testTop) + }) + } +} + +func TestRollback(t *testing.T) { + tests := map[string]struct { + agentInstallsSetup setupAgentInstallations + additionalSetup hookFunc + wantErr assert.ErrorAssertionFunc + checkAfterRollback hookFunc + }{ + "rollback without versionedHome (legacy upgrade process)": { + agentInstallsSetup: setupAgentInstallations{ + installedAgents: []agentInstall{ + { + version: version123Snapshot, + useVersionInPath: false, + }, + { + version: version456Snapshot, + useVersionInPath: false, + }, + }, + upgradeFrom: version123Snapshot, + upgradeTo: version456Snapshot, + currentAgent: version456Snapshot, + }, + wantErr: assert.NoError, + checkAfterRollback: func(t *testing.T, topDir string) { + oldAgentHome := filepath.Join("data", "elastic-agent-abcdef") + newAgentHome := filepath.Join("data", "elastic-agent-ghijkl") + checkFilesAfterRollback(t, topDir, oldAgentHome, newAgentHome) + }, + }, + "rollback with versionedHome (new upgrade process)": { + agentInstallsSetup: setupAgentInstallations{ + installedAgents: []agentInstall{ + { + version: version123Snapshot, + useVersionInPath: true, + }, + { + version: version456Snapshot, + useVersionInPath: true, + }, + }, + upgradeFrom: version123Snapshot, + upgradeTo: version456Snapshot, + currentAgent: version456Snapshot, + }, + wantErr: assert.NoError, + checkAfterRollback: func(t *testing.T, topDir string) { + oldAgentHome := filepath.Join("data", "elastic-agent-1.2.3-SNAPSHOT-abcdef") + newAgentHome := filepath.Join("data", "elastic-agent-4.5.6-SNAPSHOT-ghijkl") + checkFilesAfterRollback(t, topDir, oldAgentHome, newAgentHome) + }, + }, + "rollback with versionedHome only on the new agent (new upgrade process from an old agent upgraded with legacy process)": { + agentInstallsSetup: setupAgentInstallations{ + installedAgents: []agentInstall{ + { + version: version123Snapshot, + useVersionInPath: false, + }, + { + version: version456Snapshot, + useVersionInPath: true, + }, + }, + upgradeFrom: version123Snapshot, + upgradeTo: version456Snapshot, + currentAgent: version456Snapshot, + }, + wantErr: assert.NoError, + checkAfterRollback: func(t *testing.T, topDir string) { + oldAgentHome := filepath.Join("data", "elastic-agent-abcdef") + newAgentHome := filepath.Join("data", "elastic-agent-4.5.6-SNAPSHOT-ghijkl") + checkFilesAfterRollback(t, topDir, oldAgentHome, newAgentHome) + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + testLogger, _ := logger.NewTesting(t.Name()) + testTop := t.TempDir() + setupAgents(t, testLogger, testTop, tt.agentInstallsSetup) + if tt.additionalSetup != nil { + tt.additionalSetup(t, testTop) + } + marker, err := LoadMarker(paths.DataFrom(testTop)) + require.NoError(t, err, "error loading update marker") + require.NotNil(t, marker, "loaded marker must not be nil") + t.Logf("Loaded update marker %+v", marker) + + // mock client + mockClient := mocks.NewClient(t) + 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() + + ctx := context.TODO() + tt.wantErr(t, Rollback(ctx, testLogger, mockClient, testTop, marker.PrevVersionedHome, marker.PrevHash), fmt.Sprintf("Rollback(%v, %v, %v, %v, %v, %v)", ctx, testLogger, mockClient, testTop, marker.PrevVersionedHome, marker.PrevHash)) + tt.checkAfterRollback(t, testTop) + }) + } +} + +// checkFilesAfterCleanup is a convenience function to check the file structure within topDir. +// *AgentHome paths must be the expected old and new agent paths relative to topDir (typically in the form of "data/elastic-agent-*") +func checkFilesAfterCleanup(t *testing.T, topDir, newAgentHome string, oldAgentHomes ...string) { + t.Helper() + // the old agent directories must not exist anymore + for _, oldAgentHome := range oldAgentHomes { + assert.NoDirExistsf(t, filepath.Join(topDir, oldAgentHome), "old agent directory %q should be deleted after cleanup", oldAgentHome) + } + + // check the new agent home + assert.DirExists(t, filepath.Join(topDir, newAgentHome), "new agent directory should exist after cleanup") + agentExecutable := agentName + if runtime.GOOS == "windows" { + agentExecutable += ".exe" + } + symlinkPath := filepath.Join(topDir, agentExecutable) + linkTarget, err := os.Readlink(symlinkPath) + if assert.NoError(t, err, "unable to read symbolic link") { + assert.Equal(t, paths.BinaryPath(newAgentHome, agentExecutable), linkTarget, "symbolic link should point to the new agent executable after cleanup") + } + + // read the elastic agent placeholder via the symlink + elasticAgentBytes, err := os.ReadFile(symlinkPath) + if assert.NoError(t, err, "error reading elastic-agent content through the symlink") { + assert.Equal(t, []byte("Placeholder for agent 4.5.6-SNAPSHOT"), elasticAgentBytes, "reading elastic-agent content through symbolic link should point to the new version") + } + + assert.NoFileExists(t, filepath.Join(topDir, "data", markerFilename), "update marker should have been cleaned up") +} + +// checkFilesAfterRollback is a convenience function to check the file structure within topDir. +// *AgentHome paths must be the expected old and new agent paths relative to topDir (typically in the form of "data/elastic-agent-*") +func checkFilesAfterRollback(t *testing.T, topDir, oldAgentHome, newAgentHome string) { + t.Helper() + // the new agent directory must still exist (for the logs) + assert.DirExists(t, filepath.Join(topDir, newAgentHome), "new agent directory should still exist after rollback") + assert.DirExists(t, filepath.Join(topDir, newAgentHome, "logs"), "new agent logs directory should still exist after rollback") + // some things should have been removed from the new agent directory + assert.NoDirExists(t, filepath.Join(topDir, newAgentHome, "components"), "new agent components directory should have been cleaned up in the rollback") + assert.NoDirExists(t, filepath.Join(topDir, newAgentHome, "run"), "new agent run directory should have been cleaned up in the rollback") + assert.NoFileExists(t, filepath.Join(topDir, newAgentHome, agentName), "new agent binary should have been cleaned up in the rollback") + + // check the old agent home + assert.DirExists(t, filepath.Join(topDir, oldAgentHome), "old agent directory should exist after rollback") + agentExecutable := agentName + if runtime.GOOS == "windows" { + agentExecutable += ".exe" + } + symlinkPath := filepath.Join(topDir, agentExecutable) + linkTarget, err := os.Readlink(symlinkPath) + if assert.NoError(t, err, "unable to read symbolic link") { + // Due to the unique way the rollback process works we end up with an absolute path in the link + expectedLinkTargetAfterRollback := paths.BinaryPath(filepath.Join(topDir, oldAgentHome), agentExecutable) + assert.Equal(t, expectedLinkTargetAfterRollback, linkTarget, "symbolic link should point to the old agent executable after rollback") + } + + // read the elastic agent placeholder via the symlink + elasticAgentBytes, err := os.ReadFile(symlinkPath) + if assert.NoError(t, err, "error reading elastic-agent content through the symlink") { + assert.Equal(t, []byte("Placeholder for agent 1.2.3-SNAPSHOT"), elasticAgentBytes, "reading elastic-agent content through symbolic link should point to the old version") + } + + assert.NoFileExists(t, filepath.Join(topDir, "data", markerFilename), "update marker should have been cleaned up") +} + +// setupAgents create fake agent installs, update marker file and symlink pointing to one of the installations' elastic-agent placeholder +func setupAgents(t *testing.T, log *logger.Logger, topDir string, installations setupAgentInstallations) { + + var ( + oldAgentVersion agentVersion + oldAgentVersionedHome string + newAgentVersion agentVersion + newAgentVersionedHome string + useNewMarker bool + ) + for _, ia := range installations.installedAgents { + versionedHome := createFakeAgentInstall(t, topDir, ia.version.version, ia.version.hash, ia.useVersionInPath) + t.Logf("Created fake agent install for %+v located at %q", ia, versionedHome) + if installations.upgradeFrom == ia.version { + t.Logf("Setting version %+v as FROM version for the update marker", ia.version) + oldAgentVersion = ia.version + oldAgentVersionedHome = versionedHome + } + + if installations.upgradeTo == ia.version { + t.Logf("Setting version %+v as TO version for the update marker", ia.version) + newAgentVersion = ia.version + newAgentVersionedHome = versionedHome + useNewMarker = ia.useVersionInPath + } + + if installations.currentAgent == ia.version { + t.Logf("Creating symlink pointing to %q", versionedHome) + createLink(t, topDir, versionedHome) + } + } + + t.Logf("Creating upgrade marker from %+v located at %q to %+v located at %q", oldAgentVersion, oldAgentVersionedHome, newAgentVersion, newAgentVersionedHome) + createUpdateMarker(t, log, topDir, newAgentVersion.version, newAgentVersion.hash, newAgentVersionedHome, oldAgentVersion.version, oldAgentVersion.hash, oldAgentVersionedHome, useNewMarker) +} + +// createFakeAgentInstall will create a mock agent install within topDir, possibly using the version in the directory name, depending on useVersionInPath +// it MUST return the path to the created versionedHome relative to topDir, to mirror what step_unpack returns +func createFakeAgentInstall(t *testing.T, topDir, version, hash string, useVersionInPath bool) string { + + // create versioned home + versionedHome := fmt.Sprintf("elastic-agent-%s", hash[:hashLen]) + if useVersionInPath { + // use the version passed as parameter + versionedHome = fmt.Sprintf("elastic-agent-%s-%s", version, hash[:hashLen]) + } + relVersionedHomePath := filepath.Join("data", versionedHome) + absVersionedHomePath := filepath.Join(topDir, relVersionedHomePath) + + // recalculate the binary path and launch a mkDirAll to account for MacOS weirdness + // (the extra nesting of elastic agent binary within versionedHome) + absVersioneHomeBinaryPath := paths.BinaryPath(absVersionedHomePath, "") + err := os.MkdirAll(absVersioneHomeBinaryPath, 0o750) + require.NoError(t, err, "error creating fake install versioned home directory (including binary path) %q", absVersioneHomeBinaryPath) + + // place a few directories in the fake install + absComponentsDirPath := filepath.Join(absVersionedHomePath, "components") + err = os.MkdirAll(absComponentsDirPath, 0o750) + require.NoError(t, err, "error creating fake install components directory %q", absVersionedHomePath) + + absLogsDirPath := filepath.Join(absVersionedHomePath, "logs") + err = os.MkdirAll(absLogsDirPath, 0o750) + require.NoError(t, err, "error creating fake install logs directory %q", absLogsDirPath) + + absRunDirPath := filepath.Join(absVersionedHomePath, "run") + err = os.MkdirAll(absRunDirPath, 0o750) + require.NoError(t, err, "error creating fake install run directory %q", absRunDirPath) + + // put some placeholder for files + agentExecutableName := agentName + if runtime.GOOS == "windows" { + agentExecutableName += ".exe" + } + err = os.WriteFile(paths.BinaryPath(absVersionedHomePath, agentExecutableName), []byte(fmt.Sprintf("Placeholder for agent %s", version)), 0o750) + require.NoErrorf(t, err, "error writing elastic agent binary placeholder %q", agentExecutableName) + err = os.WriteFile(filepath.Join(absLogsDirPath, "fakelog.ndjson"), []byte(fmt.Sprintf("Sample logs for agent %s", version)), 0o750) + require.NoErrorf(t, err, "error writing fake log placeholder %q") + + // return the path relative to top exactly like the step_unpack does + return relVersionedHomePath +} + +func createLink(t *testing.T, topDir string, currentAgentVersionedHome string) { + linkTarget := paths.BinaryPath(currentAgentVersionedHome, agentName) + linkName := agentName + if runtime.GOOS == "windows" { + linkTarget += ".exe" + linkName += ".exe" + } + err := os.Symlink(linkTarget, filepath.Join(topDir, linkName)) + require.NoError(t, err, "error creating test symlink to fake agent install") +} + +func createUpdateMarker(t *testing.T, log *logger.Logger, topDir, newAgentVersion, newAgentHash, newAgentVersionedHome, oldAgentVersion, oldAgentHash, oldAgentVersionedHome string, useNewMarker bool) { + + if !useNewMarker { + newAgentVersion = "" + newAgentVersionedHome = "" + oldAgentVersionedHome = "" + } + + err := markUpgrade(log, paths.DataFrom(topDir), newAgentVersion, newAgentHash, newAgentVersionedHome, oldAgentVersion, oldAgentHash, oldAgentVersionedHome, nil, nil) + require.NoError(t, err, "error writing fake update marker") +} diff --git a/internal/pkg/agent/application/upgrade/step_mark.go b/internal/pkg/agent/application/upgrade/step_mark.go index 71834f64a90..31f54c83868 100644 --- a/internal/pkg/agent/application/upgrade/step_mark.go +++ b/internal/pkg/agent/application/upgrade/step_mark.go @@ -5,8 +5,6 @@ package upgrade import ( - "context" - "fmt" "os" "path/filepath" "time" @@ -17,7 +15,6 @@ import ( "github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details" "github.com/elastic/elastic-agent/internal/pkg/agent/errors" "github.com/elastic/elastic-agent/internal/pkg/fleetapi" - "github.com/elastic/elastic-agent/internal/pkg/release" "github.com/elastic/elastic-agent/pkg/core/logger" ) @@ -119,13 +116,8 @@ func newMarkerSerializer(m *UpdateMarker) *updateMarkerSerializer { } // markUpgrade marks update happened so we can handle grace period -func (u *Upgrader) markUpgrade(_ context.Context, log *logger.Logger, version, hash, versionedHome string, action *fleetapi.ActionUpgrade, upgradeDetails *details.Details) error { - prevVersion := release.VersionWithSnapshot() - prevHash := release.Commit() - prevVersionedHome, err := filepath.Rel(paths.Top(), paths.Home()) - if err != nil { - return fmt.Errorf("calculating home path relative to top, home: %q top: %q : %w", paths.Home(), paths.Top(), err) - } +func markUpgrade(log *logger.Logger, dataDirPath, version, hash, versionedHome, prevVersion, prevHash, prevVersionedHome string, action *fleetapi.ActionUpgrade, upgradeDetails *details.Details) error { + if len(prevHash) > hashLen { prevHash = prevHash[:hashLen] } @@ -147,13 +139,13 @@ func (u *Upgrader) markUpgrade(_ context.Context, log *logger.Logger, version, h return errors.New(err, errors.TypeConfig, "failed to parse marker file") } - markerPath := markerFilePath() + markerPath := markerFilePath(dataDirPath) log.Infow("Writing upgrade marker file", "file.path", markerPath, "hash", marker.Hash, "prev_hash", prevHash) if err := os.WriteFile(markerPath, markerBytes, 0600); err != nil { return errors.New(err, errors.TypeFilesystem, "failed to create update marker file", errors.M(errors.MetaKeyPath, markerPath)) } - if err := UpdateActiveCommit(log, hash); err != nil { + if err := UpdateActiveCommit(log, paths.Top(), hash); err != nil { return err } @@ -161,8 +153,8 @@ func (u *Upgrader) markUpgrade(_ context.Context, log *logger.Logger, version, h } // UpdateActiveCommit updates active.commit file to point to active version. -func UpdateActiveCommit(log *logger.Logger, hash string) error { - activeCommitPath := filepath.Join(paths.Top(), agentCommitFile) +func UpdateActiveCommit(log *logger.Logger, topDirPath, hash string) error { + activeCommitPath := filepath.Join(topDirPath, agentCommitFile) log.Infow("Updating active commit", "file.path", activeCommitPath, "hash", hash) if err := os.WriteFile(activeCommitPath, []byte(hash), 0600); err != nil { return errors.New(err, errors.TypeFilesystem, "failed to update active commit", errors.M(errors.MetaKeyPath, activeCommitPath)) @@ -172,8 +164,8 @@ func UpdateActiveCommit(log *logger.Logger, hash string) error { } // CleanMarker removes a marker from disk. -func CleanMarker(log *logger.Logger) error { - markerFile := markerFilePath() +func CleanMarker(log *logger.Logger, dataDirPath string) error { + markerFile := markerFilePath(dataDirPath) log.Infow("Removing marker file", "file.path", markerFile) if err := os.Remove(markerFile); !os.IsNotExist(err) { return err @@ -184,8 +176,8 @@ func CleanMarker(log *logger.Logger) error { // LoadMarker loads the update marker. If the file does not exist it returns nil // and no error. -func LoadMarker() (*UpdateMarker, error) { - return loadMarker(markerFilePath()) +func LoadMarker(dataDirPath string) (*UpdateMarker, error) { + return loadMarker(markerFilePath(dataDirPath)) } func loadMarker(markerFile string) (*UpdateMarker, error) { @@ -238,9 +230,9 @@ func SaveMarker(marker *UpdateMarker, shouldFsync bool) error { return err } - return writeMarkerFile(markerFilePath(), markerBytes, shouldFsync) + return writeMarkerFile(markerFilePath(paths.Data()), markerBytes, shouldFsync) } -func markerFilePath() string { - return filepath.Join(paths.Data(), markerFilename) +func markerFilePath(dataDirPath string) string { + return filepath.Join(dataDirPath, markerFilename) } diff --git a/internal/pkg/agent/application/upgrade/step_relink.go b/internal/pkg/agent/application/upgrade/step_relink.go index f272083bfe9..dc528a90cf0 100644 --- a/internal/pkg/agent/application/upgrade/step_relink.go +++ b/internal/pkg/agent/application/upgrade/step_relink.go @@ -23,19 +23,19 @@ const ( ) // ChangeSymlink updates symlink paths to match current version. -func ChangeSymlink(ctx context.Context, log *logger.Logger, targetHash string) error { +func ChangeSymlink(ctx context.Context, log *logger.Logger, topDirPath, targetHash string) error { // create symlink to elastic-agent-{hash} hashedDir := fmt.Sprintf("%s-%s", agentName, targetHash) - symlinkPath := filepath.Join(paths.Top(), agentName) + symlinkPath := filepath.Join(topDirPath, agentName) // paths.BinaryPath properly derives the binary directory depending on the platform. The path to the binary for macOS is inside of the app bundle. - newPath := paths.BinaryPath(filepath.Join(paths.Top(), "data", hashedDir), agentName) + newPath := paths.BinaryPath(filepath.Join(paths.DataFrom(topDirPath), hashedDir), agentName) - return changeSymlinkInternal(log, symlinkPath, newPath) + return changeSymlinkInternal(log, topDirPath, symlinkPath, newPath) } -func changeSymlinkInternal(log *logger.Logger, symlinkPath, newTarget string) error { +func changeSymlinkInternal(log *logger.Logger, topDirPath, symlinkPath, newTarget string) error { // handle windows suffixes if runtime.GOOS == windows { @@ -43,7 +43,7 @@ func changeSymlinkInternal(log *logger.Logger, symlinkPath, newTarget string) er newTarget += exe } - prevNewPath := prevSymlinkPath() + prevNewPath := prevSymlinkPath(topDirPath) log.Infow("Changing symlink", "symlink_path", symlinkPath, "new_path", newTarget, "prev_path", prevNewPath) // remove symlink to avoid upgrade failures @@ -59,7 +59,7 @@ func changeSymlinkInternal(log *logger.Logger, symlinkPath, newTarget string) er return file.SafeFileRotate(symlinkPath, prevNewPath) } -func prevSymlinkPath() string { +func prevSymlinkPath(topDirPath string) string { agentPrevName := agentName + ".prev" // handle windows suffixes @@ -67,5 +67,5 @@ func prevSymlinkPath() string { agentPrevName = agentName + ".exe.prev" } - return filepath.Join(paths.Top(), agentPrevName) + return filepath.Join(topDirPath, agentPrevName) } diff --git a/internal/pkg/agent/application/upgrade/step_unpack.go b/internal/pkg/agent/application/upgrade/step_unpack.go index b9af3ed5620..79b647dec62 100644 --- a/internal/pkg/agent/application/upgrade/step_unpack.go +++ b/internal/pkg/agent/application/upgrade/step_unpack.go @@ -28,7 +28,7 @@ import ( type UnpackResult struct { // Hash contains the unpacked agent commit hash, limited to a length of 6 for backward compatibility Hash string `json:"hash" yaml:"hash"` - // VersionedHome indicates the path (forward slash separated) where to find the unpacked agent files + // VersionedHome indicates the path (relative to topPath, formatted in os-dependent fashion) where to find the unpacked agent files // The value depends on the mappings specified in manifest.yaml, if no manifest is found it assumes the legacy data/elastic-agent- format VersionedHome string `json:"versioned-home" yaml:"versioned-home"` } @@ -81,7 +81,7 @@ func unzip(log *logger.Logger, archivePath, dataDir string) (UnpackResult, error return UnpackResult{}, fmt.Errorf("parsing package manifest: %w", err) } pm.mappings = manifest.Package.PathMappings - versionedHome = path.Clean(pm.Map(manifest.Package.VersionedHome)) + versionedHome = filepath.Clean(pm.Map(manifest.Package.VersionedHome)) } // Load hash, the use of path.Join is intentional since in .zip file paths use slash ('/') as separator @@ -210,8 +210,8 @@ func untar(log *logger.Logger, version string, archivePath, dataDir string) (Unp return UnpackResult{}, fmt.Errorf("looking for package metadata files: %w", err) } - manifestReader := fileContents["manifest.yaml"] - if manifestReader != nil { + manifestReader, ok := fileContents["manifest.yaml"] + if ok && manifestReader != nil { manifest, err := v1.ParseManifest(manifestReader) if err != nil { return UnpackResult{}, fmt.Errorf("parsing package manifest: %w", err) @@ -466,5 +466,5 @@ func getFilesContentFromTar(archivePath string, files ...string) (map[string]io. } func createVersionedHomeFromHash(hash string) string { - return fmt.Sprintf("data/elastic-agent-%s", hash[:hashLen]) + return filepath.Join("data", fmt.Sprintf("elastic-agent-%s", hash[:hashLen])) } diff --git a/internal/pkg/agent/application/upgrade/step_unpack_test.go b/internal/pkg/agent/application/upgrade/step_unpack_test.go index 493190d3454..909fc07d6c0 100644 --- a/internal/pkg/agent/application/upgrade/step_unpack_test.go +++ b/internal/pkg/agent/application/upgrade/step_unpack_test.go @@ -14,6 +14,7 @@ import ( "os" "path" "path/filepath" + "runtime" "strings" "testing" "time" @@ -58,6 +59,33 @@ inputs: - baz ` +var archiveFilesWithManifestNoSymlink = []files{ + {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/manifest.yaml", content: ea_123_manifest, mode: fs.ModePerm & 0o640}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/" + agentCommitFile, content: "abcdefghijklmnopqrstuvwxyz", mode: fs.ModePerm & 0o640}, + {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, + {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/" + agentName, content: agentBinaryPlaceholderContent, mode: fs.ModePerm & 0o750}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/package.version", content: "1.2.3", mode: fs.ModePerm & 0o640}, + {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/components", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/components/comp1", content: "Placeholder for component", mode: fs.ModePerm & 0o750}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/components/comp1.spec.yml", content: foo_component_spec, mode: fs.ModePerm & 0o640}, +} + +var outOfOrderArchiveFilesNoManifestNoSymlink = []files{ + {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/" + agentCommitFile, content: "abcdefghijklmnopqrstuvwxyz", mode: fs.ModePerm & 0o640}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/package.version", content: "1.2.3", mode: fs.ModePerm & 0o640}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/" + agentName, content: agentBinaryPlaceholderContent, mode: fs.ModePerm & 0o750}, + {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, + {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef", mode: fs.ModeDir | (fs.ModePerm & 0o700)}, + {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/components", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/components/comp1", content: "Placeholder for component", mode: fs.ModePerm & 0o750}, + {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/data/elastic-agent-abcdef/components/comp1.spec.yml", content: foo_component_spec, mode: fs.ModePerm & 0o640}, +} + +var agentArchiveSymLink = files{fType: SYMLINK, path: "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64/" + agentName, content: "data/elastic-agent-abcdef/" + agentName, mode: fs.ModeSymlink | (fs.ModePerm & 0o750)} + type fileType uint const ( @@ -117,21 +145,10 @@ func TestUpgrader_unpackTarGz(t *testing.T) { { name: "file before containing folder", args: args{ - version: "1.2.3", - archiveFiles: []files{ - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/" + agentCommitFile, content: "abcdefghijklmnopqrstuvwxyz", mode: fs.ModePerm & 0o640}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/package.version", content: "1.2.3", mode: fs.ModePerm & 0o640}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef", mode: fs.ModeDir | (fs.ModePerm & 0o700)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/" + agentName, content: agentBinaryPlaceholderContent, mode: fs.ModePerm & 0o750}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components/comp1", content: "Placeholder for component", mode: fs.ModePerm & 0o750}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components/comp1.spec.yml", content: foo_component_spec, mode: fs.ModePerm & 0o640}, - {fType: SYMLINK, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/" + agentName, content: "data/elastic-agent-abcdef/" + agentName, mode: fs.ModeSymlink | (fs.ModePerm & 0o750)}, - }, + version: "1.2.3", + archiveFiles: append(outOfOrderArchiveFilesNoManifestNoSymlink, agentArchiveSymLink), archiveGenerator: func(t *testing.T, i []files) (string, error) { - return createTarArchive(t, "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64.tar.gz", i) + return createTarArchive(t, "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64.tar.gz", i) }, }, want: UnpackResult{ @@ -141,58 +158,24 @@ func TestUpgrader_unpackTarGz(t *testing.T) { wantErr: assert.NoError, checkFiles: func(t *testing.T, testDataDir string) { versionedHome := filepath.Join(testDataDir, "elastic-agent-abcdef") - require.DirExists(t, versionedHome, "directory for package.version does not exists") - stat, err := os.Stat(versionedHome) - require.NoErrorf(t, err, "error calling Stat() for versionedHome %q", versionedHome) - expectedPermissions := fs.ModePerm & 0o700 - actualPermissions := fs.ModePerm & stat.Mode() - assert.Equalf(t, expectedPermissions, actualPermissions, "Wrong permissions set on versioned home %q: expected %O, got %O", versionedHome, expectedPermissions, actualPermissions) + checkExtractedFilesOutOfOrder(t, versionedHome) }, }, { name: "package with manifest file", args: args{ - version: "1.2.3", - archiveFiles: []files{ - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/manifest.yaml", content: ea_123_manifest, mode: fs.ModePerm & 0o640}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/" + agentCommitFile, content: "abcdefghijklmnopqrstuvwxyz", mode: fs.ModePerm & 0o640}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/" + agentName, content: agentBinaryPlaceholderContent, mode: fs.ModePerm & 0o750}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/package.version", content: "1.2.3", mode: fs.ModePerm & 0o640}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components/comp1", content: "Placeholder for component", mode: fs.ModePerm & 0o750}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components/comp1.spec.yml", content: foo_component_spec, mode: fs.ModePerm & 0o640}, - {fType: SYMLINK, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/" + agentName, content: "data/elastic-agent-abcdef/" + agentName, mode: fs.ModeSymlink | (fs.ModePerm & 0o750)}, - }, + version: "1.2.3", + archiveFiles: append(archiveFilesWithManifestNoSymlink, agentArchiveSymLink), archiveGenerator: func(t *testing.T, i []files) (string, error) { - return createTarArchive(t, "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64.tar.gz", i) + return createTarArchive(t, "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64.tar.gz", i) }, }, want: UnpackResult{ Hash: "abcdef", - VersionedHome: "data/elastic-agent-1.2.3-SNAPSHOT-abcdef", - }, - wantErr: assert.NoError, - checkFiles: func(t *testing.T, testDataDir string) { - versionedHome := filepath.Join(testDataDir, "elastic-agent-1.2.3-SNAPSHOT-abcdef") - require.DirExists(t, versionedHome, "mapped versioned home directory does not exists") - mappedAgentExecutable := filepath.Join(versionedHome, agentName) - if assert.FileExistsf(t, mappedAgentExecutable, "agent executable %q is not found in mapped versioned home directory %q", mappedAgentExecutable, versionedHome) { - fileBytes, err := os.ReadFile(mappedAgentExecutable) - if assert.NoErrorf(t, err, "error reading elastic-agent executable %q", mappedAgentExecutable) { - assert.Equal(t, agentBinaryPlaceholderContent, string(fileBytes), "agent binary placeholder content does not match") - } - } - mappedPackageManifest := filepath.Join(versionedHome, "manifest.yaml") - if assert.FileExistsf(t, mappedPackageManifest, "package manifest %q is not found in mapped versioned home directory %q", mappedPackageManifest, versionedHome) { - fileBytes, err := os.ReadFile(mappedPackageManifest) - if assert.NoErrorf(t, err, "error reading package manifest %q", mappedPackageManifest) { - assert.Equal(t, ea_123_manifest, string(fileBytes), "package manifest content does not match") - } - } + VersionedHome: filepath.Join("data", "elastic-agent-1.2.3-SNAPSHOT-abcdef"), }, + wantErr: assert.NoError, + checkFiles: checkExtractedFilesWithManifest, }, } for _, tt := range tests { @@ -234,19 +217,9 @@ func TestUpgrader_unpackZip(t *testing.T) { { name: "file before containing folder", args: args{ - archiveFiles: []files{ - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/" + agentCommitFile, content: "abcdefghijklmnopqrstuvwxyz", mode: fs.ModePerm & 0o640}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/package.version", content: "1.2.3", mode: fs.ModePerm & 0o640}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef", mode: fs.ModeDir | (fs.ModePerm & 0o700)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/" + agentName, content: agentBinaryPlaceholderContent, mode: fs.ModePerm & 0o750}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/components", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/components/comp1", content: "Placeholder for component", mode: fs.ModePerm & 0o750}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/components/comp1.spec.yml", content: foo_component_spec, mode: fs.ModePerm & 0o640}, - }, + archiveFiles: outOfOrderArchiveFilesNoManifestNoSymlink, archiveGenerator: func(t *testing.T, i []files) (string, error) { - return createZipArchive(t, "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64.zip", i) + return createZipArchive(t, "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64.zip", i) }, }, want: UnpackResult{ @@ -256,63 +229,23 @@ func TestUpgrader_unpackZip(t *testing.T) { wantErr: assert.NoError, checkFiles: func(t *testing.T, testDataDir string) { versionedHome := filepath.Join(testDataDir, "elastic-agent-abcdef") - require.DirExists(t, versionedHome, "directory for package.version does not exists") - stat, err := os.Stat(versionedHome) - require.NoErrorf(t, err, "error calling Stat() for versionedHome %q", versionedHome) - expectedPermissions := fs.ModePerm & 0o700 - actualPermissions := fs.ModePerm & stat.Mode() - assert.Equalf(t, expectedPermissions, actualPermissions, "Wrong permissions set on versioned home %q: expected %O, got %O", versionedHome, expectedPermissions, actualPermissions) - agentExecutable := filepath.Join(versionedHome, agentName) - if assert.FileExistsf(t, agentExecutable, "agent executable %q is not found in versioned home directory %q", agentExecutable, versionedHome) { - fileBytes, err := os.ReadFile(agentExecutable) - if assert.NoErrorf(t, err, "error reading elastic-agent executable %q", agentExecutable) { - assert.Equal(t, agentBinaryPlaceholderContent, string(fileBytes), "agent binary placeholder content does not match") - } - } + checkExtractedFilesOutOfOrder(t, versionedHome) }, }, { name: "package with manifest file", args: args{ - archiveFiles: []files{ - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/manifest.yaml", content: ea_123_manifest, mode: fs.ModePerm & 0o640}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/" + agentCommitFile, content: "abcdefghijklmnopqrstuvwxyz", mode: fs.ModePerm & 0o640}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/" + agentName, content: agentBinaryPlaceholderContent, mode: fs.ModePerm & 0o750}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/package.version", content: "1.2.3", mode: fs.ModePerm & 0o640}, - {fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/components", mode: fs.ModeDir | (fs.ModePerm & 0o750)}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/components/comp1", content: "Placeholder for component", mode: fs.ModePerm & 0o750}, - {fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64/data/elastic-agent-abcdef/components/comp1.spec.yml", content: foo_component_spec, mode: fs.ModePerm & 0o640}, - }, + archiveFiles: archiveFilesWithManifestNoSymlink, archiveGenerator: func(t *testing.T, i []files) (string, error) { - return createZipArchive(t, "elastic-agent-1.2.3-SNAPSHOT-windows-x86_64.zip", i) + return createZipArchive(t, "elastic-agent-1.2.3-SNAPSHOT-someos-x86_64.zip", i) }, }, want: UnpackResult{ Hash: "abcdef", VersionedHome: filepath.Join("data", "elastic-agent-1.2.3-SNAPSHOT-abcdef"), }, - wantErr: assert.NoError, - checkFiles: func(t *testing.T, testDataDir string) { - versionedHome := filepath.Join(testDataDir, "elastic-agent-1.2.3-SNAPSHOT-abcdef") - require.DirExists(t, versionedHome, "mapped versioned home directory does not exists") - mappedAgentExecutable := filepath.Join(versionedHome, agentName) - if assert.FileExistsf(t, mappedAgentExecutable, "agent executable %q is not found in mapped versioned home directory %q", mappedAgentExecutable, versionedHome) { - fileBytes, err := os.ReadFile(mappedAgentExecutable) - if assert.NoErrorf(t, err, "error reading elastic-agent executable %q", mappedAgentExecutable) { - assert.Equal(t, agentBinaryPlaceholderContent, string(fileBytes), "agent binary placeholder content does not match") - } - } - mappedPackageManifest := filepath.Join(versionedHome, "manifest.yaml") - if assert.FileExistsf(t, mappedPackageManifest, "package manifest %q is not found in mapped versioned home directory %q", mappedPackageManifest, versionedHome) { - fileBytes, err := os.ReadFile(mappedPackageManifest) - if assert.NoErrorf(t, err, "error reading package manifest %q", mappedPackageManifest) { - assert.Equal(t, ea_123_manifest, string(fileBytes), "package manifest content does not match") - } - } - }, + wantErr: assert.NoError, + checkFiles: checkExtractedFilesWithManifest, }, } for _, tt := range tests { @@ -339,8 +272,46 @@ func TestUpgrader_unpackZip(t *testing.T) { } } +func checkExtractedFilesOutOfOrder(t *testing.T, versionedHome string) { + require.DirExists(t, versionedHome, "directory for package.version does not exists") + stat, err := os.Stat(versionedHome) + require.NoErrorf(t, err, "error calling Stat() for versionedHome %q", versionedHome) + expectedPermissions := fs.ModePerm & 0o700 + if runtime.GOOS == "windows" { + // windows permissions are not very fine grained :/ + expectedPermissions = fs.ModePerm & 0o777 + } + actualPermissions := fs.ModePerm & stat.Mode() + assert.Equalf(t, expectedPermissions, actualPermissions, "Wrong permissions set on versioned home %q: expected %O, got %O", versionedHome, expectedPermissions, actualPermissions) + agentExecutable := filepath.Join(versionedHome, agentName) + if assert.FileExistsf(t, agentExecutable, "agent executable %q is not found in versioned home directory %q", agentExecutable, versionedHome) { + fileBytes, err := os.ReadFile(agentExecutable) + if assert.NoErrorf(t, err, "error reading elastic-agent executable %q", agentExecutable) { + assert.Equal(t, agentBinaryPlaceholderContent, string(fileBytes), "agent binary placeholder content does not match") + } + } +} + +func checkExtractedFilesWithManifest(t *testing.T, testDataDir string) { + versionedHome := filepath.Join(testDataDir, "elastic-agent-1.2.3-SNAPSHOT-abcdef") + require.DirExists(t, versionedHome, "mapped versioned home directory does not exists") + mappedAgentExecutable := filepath.Join(versionedHome, agentName) + if assert.FileExistsf(t, mappedAgentExecutable, "agent executable %q is not found in mapped versioned home directory %q", mappedAgentExecutable, versionedHome) { + fileBytes, err := os.ReadFile(mappedAgentExecutable) + if assert.NoErrorf(t, err, "error reading elastic-agent executable %q", mappedAgentExecutable) { + assert.Equal(t, agentBinaryPlaceholderContent, string(fileBytes), "agent binary placeholder content does not match") + } + } + mappedPackageManifest := filepath.Join(versionedHome, "manifest.yaml") + if assert.FileExistsf(t, mappedPackageManifest, "package manifest %q is not found in mapped versioned home directory %q", mappedPackageManifest, versionedHome) { + fileBytes, err := os.ReadFile(mappedPackageManifest) + if assert.NoErrorf(t, err, "error reading package manifest %q", mappedPackageManifest) { + assert.Equal(t, ea_123_manifest, string(fileBytes), "package manifest content does not match") + } + } +} + func createTarArchive(t *testing.T, archiveName string, archiveFiles []files) (string, error) { - t.Helper() outDir := t.TempDir() outFilePath := filepath.Join(outDir, archiveName) @@ -370,13 +341,13 @@ func createTarArchive(t *testing.T, archiveName string, archiveFiles []files) (s func addEntryToTarArchive(af files, writer *tar.Writer) error { header, err := tar.FileInfoHeader(&af, af.content) if err != nil { - return err + return fmt.Errorf("creating header for %q: %w", af.path, err) } - header.Name = af.path + header.Name = filepath.ToSlash(af.path) if err := writer.WriteHeader(header); err != nil { - return err + return fmt.Errorf("writing header for %q: %w", af.path, err) } if af.IsDir() || af.fType == SYMLINK { @@ -425,9 +396,9 @@ func addEntryToZipArchive(af files, writer *zip.Writer) error { } header.SetMode(af.Mode() & os.ModePerm) - header.Name = af.path + header.Name = filepath.ToSlash(af.path) if af.IsDir() { - header.Name += string(filepath.Separator) + header.Name += "/" } else { header.Method = zip.Deflate } diff --git a/internal/pkg/agent/application/upgrade/upgrade.go b/internal/pkg/agent/application/upgrade/upgrade.go index d8f8662ce11..13a2a9f9b5c 100644 --- a/internal/pkg/agent/application/upgrade/upgrade.go +++ b/internal/pkg/agent/application/upgrade/upgrade.go @@ -74,7 +74,7 @@ func NewUpgrader(log *logger.Logger, settings *artifact.Config, agentInfo info.A settings: settings, agentInfo: agentInfo, upgradeable: IsUpgradeable(), - markerWatcher: newMarkerFileWatcher(markerFilePath(), log), + markerWatcher: newMarkerFileWatcher(markerFilePath(paths.Data()), log), }, nil } @@ -186,6 +186,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string return nil, fmt.Errorf("versionedhome is empty: %v", unpackRes) } + // TODO reintroduce the version check //if strings.HasPrefix(release.Commit(), newHash) { // u.log.Warn("Upgrade action skipped: upgrade did not occur because its the same version") // return nil, nil @@ -214,21 +215,30 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string // paths.BinaryPath properly derives the binary directory depending on the platform. The path to the binary for macOS is inside of the app bundle. newPath := paths.BinaryPath(filepath.Join(paths.Top(), hashedDir), agentName) - if err := changeSymlinkInternal(u.log, symlinkPath, newPath); err != nil { + if err := changeSymlinkInternal(u.log, paths.Top(), symlinkPath, newPath); err != nil { u.log.Errorw("Rolling back: changing symlink failed", "error.message", err) - rollbackInstall(ctx, u.log, hashedDir) + rollbackInstall(ctx, u.log, paths.Top(), hashedDir) return nil, err } - if err := u.markUpgrade(ctx, u.log, version, unpackRes.Hash, unpackRes.VersionedHome, action, det); err != nil { + currentVersionedHome, err := filepath.Rel(paths.Top(), paths.Home()) + if err != nil { + return nil, fmt.Errorf("calculating home path relative to top, home: %q top: %q : %w", paths.Home(), paths.Top(), err) + } + if err := markUpgrade( + u.log, + paths.Data(), //data dir to place the marker in + version, unpackRes.Hash, unpackRes.VersionedHome, //new agent version data + release.VersionWithSnapshot(), release.Commit(), currentVersionedHome, // old agent version data + action, det); err != nil { u.log.Errorw("Rolling back: marking upgrade failed", "error.message", err) - rollbackInstall(ctx, u.log, hashedDir) + rollbackInstall(ctx, u.log, paths.Top(), hashedDir) return nil, err } if err := InvokeWatcher(u.log); err != nil { u.log.Errorw("Rolling back: starting watcher failed", "error.message", err) - rollbackInstall(ctx, u.log, hashedDir) + rollbackInstall(ctx, u.log, paths.Top(), hashedDir) return nil, err } @@ -247,7 +257,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string // Ack acks last upgrade action func (u *Upgrader) Ack(ctx context.Context, acker acker.Acker) error { // get upgrade action - marker, err := LoadMarker() + marker, err := LoadMarker(paths.Data()) if err != nil { return err } @@ -289,14 +299,14 @@ func (u *Upgrader) sourceURI(retrievedURI string) string { return u.settings.SourceURI } -func rollbackInstall(ctx context.Context, log *logger.Logger, versionedHome string) { - err := os.RemoveAll(filepath.Join(paths.Top(), versionedHome)) +func rollbackInstall(ctx context.Context, log *logger.Logger, topDirPath, versionedHome string) { + err := os.RemoveAll(filepath.Join(topDirPath, versionedHome)) if err != nil && !errors.Is(err, fs.ErrNotExist) { // TODO should this be a warning or an error ? log.Warnw("error rolling back install", "error.message", err) } // FIXME update - _ = ChangeSymlink(ctx, log, release.ShortCommit()) + _ = ChangeSymlink(ctx, log, topDirPath, release.ShortCommit()) } func copyActionStore(log *logger.Logger, newHome string) error { diff --git a/internal/pkg/agent/cmd/run.go b/internal/pkg/agent/cmd/run.go index 1c44dfc79b3..e9b29efb399 100644 --- a/internal/pkg/agent/cmd/run.go +++ b/internal/pkg/agent/cmd/run.go @@ -629,7 +629,7 @@ func isProcessStatsEnabled(cfg *monitoringCfg.MonitoringConfig) bool { // ongoing upgrade operation, i.e. being re-exec'd and performs // any upgrade-specific work, if needed. func handleUpgrade() error { - upgradeMarker, err := upgrade.LoadMarker() + upgradeMarker, err := upgrade.LoadMarker(paths.Data()) if err != nil { return fmt.Errorf("unable to load upgrade marker to check if Agent is being upgraded: %w", err) } diff --git a/internal/pkg/agent/cmd/watch.go b/internal/pkg/agent/cmd/watch.go index 91425bec5eb..2d3fb5cedf6 100644 --- a/internal/pkg/agent/cmd/watch.go +++ b/internal/pkg/agent/cmd/watch.go @@ -17,6 +17,7 @@ import ( "github.com/elastic/elastic-agent-libs/logp" "github.com/elastic/elastic-agent-libs/logp/configure" + "github.com/elastic/elastic-agent/pkg/control/v2/client" "github.com/elastic/elastic-agent/internal/pkg/agent/application/filelock" "github.com/elastic/elastic-agent/internal/pkg/agent/application/paths" @@ -65,7 +66,7 @@ func newWatchCommandWithArgs(_ []string, streams *cli.IOStreams) *cobra.Command func watchCmd(log *logp.Logger, cfg *configuration.Configuration) error { log.Infow("Upgrade Watcher started", "process.pid", os.Getpid(), "agent.version", version.GetAgentPackageVersion()) - marker, err := upgrade.LoadMarker() + marker, err := upgrade.LoadMarker(paths.Data()) if err != nil { log.Error("failed to load marker", err) return err @@ -97,7 +98,7 @@ func watchCmd(log *logp.Logger, cfg *configuration.Configuration) error { // if we're not within grace and marker is still there it might mean // that cleanup was not performed ok, cleanup everything except current version // hash is the same as hash of agent which initiated watcher. - if err := upgrade.Cleanup(log, marker.VersionedHome, release.ShortCommit(), true, false); err != nil { + if err := upgrade.Cleanup(log, paths.Top(), marker.VersionedHome, release.ShortCommit(), true, false); err != nil { log.Error("clean up of prior watcher run failed", err) } // exit nicely @@ -114,7 +115,7 @@ func watchCmd(log *logp.Logger, cfg *configuration.Configuration) error { log.Error("Error detected, proceeding to rollback: %v", err) upgradeDetails.SetState(details.StateRollback) - err = upgrade.Rollback(ctx, log, marker.PrevVersionedHome, marker.PrevHash, marker.Hash) + err = upgrade.Rollback(ctx, log, client.New(), paths.Top(), marker.PrevVersionedHome, marker.PrevHash) if err != nil { log.Error("rollback failed", err) upgradeDetails.Fail(err) @@ -132,7 +133,7 @@ func watchCmd(log *logp.Logger, cfg *configuration.Configuration) error { // Why is this being skipped on Windows? The comment above is not clear. // issue: https://github.com/elastic/elastic-agent/issues/3027 removeMarker := !isWindows() - err = upgrade.Cleanup(log, marker.VersionedHome, marker.Hash, removeMarker, false) + err = upgrade.Cleanup(log, paths.Top(), marker.VersionedHome, marker.Hash, removeMarker, false) if err != nil { log.Error("cleanup after successful watch failed", err) }