fixup! WIP - normalize install descriptors at startup

Author: pchila

internal/pkg/agent/application/application.go

@@ -340,13 +340,20 @@ func normalizeInstallDescriptorAtStartup(log *logger.Logger, topDir string, now
 		log.Warnf("Error getting install descriptor from installDescriptorSource during startup check: %s", err)
 		return
 	}
+
 	if installDescriptor == nil {
 		log.Warnf("Got a nil install descriptor from installDescriptorSource during startup check, skipping")
 		return
 	}
 
 	versionedHomesToCleanup := []string{}
 	for _, installDesc := range installDescriptor.AgentInstalls {
+
+		if installDesc.Active || filepath.Join(topDir, installDesc.VersionedHome) == paths.HomeFrom(topDir) {
+			// skip the current install
+			continue
+		}
+
 		versionedHomeAbsPath := filepath.Join(topDir, installDesc.VersionedHome)
 		_, err = os.Stat(versionedHomeAbsPath)
 		if errors.Is(err, os.ErrNotExist) {
@@ -360,18 +367,14 @@ func normalizeInstallDescriptorAtStartup(log *logger.Logger, topDir string, now
 			continue
 		}
 
-		if installDesc.Active || filepath.Join(topDir, installDesc.VersionedHome) == paths.HomeFrom(topDir) {
-			// skip the current install
-			continue
-		}
-
 		if installDesc.TTL != nil && now.After(*installDesc.TTL) {
 			// the install directory exists but it's expired. Remove the files and add the versioned home to the entries to remove on the install descriptor
 			log.Infof("agent install descriptor %+v is expired, removing directory %q", installDesc, versionedHomeAbsPath)
 			if cleanupErr := installpkg.RemoveBut(versionedHomeAbsPath, true); cleanupErr != nil {
 				log.Warnf("Error removing directory %q: %s", versionedHomeAbsPath, cleanupErr)
 			} else {
 				log.Infof("Directory %q was removed, selecting agent install %+v for removal from the install descriptor", versionedHomeAbsPath, installDesc)
+				versionedHomesToCleanup = append(versionedHomesToCleanup, installDesc.VersionedHome)
 			}
 		}
 	}

internal/pkg/agent/application/application_test.go

@@ -7,17 +7,24 @@ package application
 import (
 	"context"
 	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
 	"github.com/elastic/elastic-agent-libs/logp"
 	"github.com/elastic/elastic-agent/internal/pkg/agent/application/info"
+	"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
 	"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade"
+	"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details"
 	"github.com/elastic/elastic-agent/internal/pkg/config"
 	"github.com/elastic/elastic-agent/internal/pkg/testutils"
+	v1 "github.com/elastic/elastic-agent/pkg/api/v1"
 	"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
 	"github.com/elastic/elastic-agent/pkg/limits"
 )
@@ -305,26 +312,319 @@ func TestInjectOutputOverrides(t *testing.T) {
 }
 
 func Test_normalizeInstallDescriptorAtStartup(t *testing.T) {
-	type args struct {
-		topDir              string
-		now                 time.Time
-		initialUpdateMarker *upgrade.UpdateMarker
-	}
+
+	now := time.Now()
+	tomorrow := now.Add(24 * time.Hour)
+	yesterday := now.Add(-24 * time.Hour)
 
 	tests := []struct {
 		name                    string
-		setup                   func(t *testing.T, topDir string, initialUpdateMarker *upgrade.UpdateMarker, installSource *mockInstallDescriptorSource)
-		args                    args
+		setup                   func(t *testing.T, topDir string) (*upgrade.UpdateMarker, installDescriptorSource)
 		postNormalizeAssertions func(t *testing.T, topDir string, initialUpdateMarker *upgrade.UpdateMarker)
 	}{
+		{
+			name: "happy path: single install, no modifications needed",
+			setup: func(t *testing.T, topDir string) (*upgrade.UpdateMarker, installDescriptorSource) {
+				mockInstallSource := newMockInstallDescriptorSource(t)
+				mockInstallSource.EXPECT().GetInstallDesc().Return(
+					&v1.InstallDescriptor{
+						AgentInstalls: []v1.AgentInstallDesc{
+							{
+								Version:       "1.2.3-current",
+								Hash:          "current",
+								VersionedHome: filepath.Join("data", "elastic-agent-1.2.3-curren"),
+								Flavor:        "basic",
+								Active:        true,
+							},
+						},
+					},
+					nil,
+				)
+				return nil, mockInstallSource
+			},
+
+			postNormalizeAssertions: nil,
+		},
+		{
+			name: "Agent was manually rolled back: rolled back install is removed from the list",
+			setup: func(t *testing.T, topDir string) (*upgrade.UpdateMarker, installDescriptorSource) {
+				newAgentInstallPath := createFakeAgentInstall(t, topDir, "4.5.6", "newversionhash", true)
+				oldAgentInstallPath := createFakeAgentInstall(t, topDir, "1.2.3", "oldversionhash", true)
+
+				mockInstallSource := newMockInstallDescriptorSource(t)
+				fakeInstallDescriptor := v1.InstallDescriptor{
+					AgentInstalls: []v1.AgentInstallDesc{
+						{
+							Version:       "4.5.6",
+							Hash:          "newversionhash",
+							VersionedHome: newAgentInstallPath,
+							Flavor:        "basic",
+							Active:        true,
+						},
+						{
+							Version:         "1.2.3",
+							Hash:            "oldversionhash",
+							VersionedHome:   oldAgentInstallPath,
+							Flavor:          "basic",
+							OptionalTTLItem: v1.OptionalTTLItem{TTL: &tomorrow},
+						},
+					},
+				}
+				mockInstallSource.EXPECT().GetInstallDesc().Return(
+					&fakeInstallDescriptor,
+					nil,
+				)
+				updateMarker := &upgrade.UpdateMarker{
+					Version:           "4.5.6",
+					Hash:              "newversionhash",
+					VersionedHome:     newAgentInstallPath,
+					UpdatedOn:         now,
+					PrevVersion:       "1.2.3",
+					PrevHash:          "oldversionhash",
+					PrevVersionedHome: oldAgentInstallPath,
+					Acked:             false,
+					Action:            nil,
+					Details: &details.Details{
+						TargetVersion: "4.5.6",
+						State:         details.StateRollback,
+						ActionID:      "",
+						Metadata: details.Metadata{
+							Reason: details.ReasonManualRollbackPattern,
+						},
+					},
+				}
+
+				mockInstallSource.EXPECT().ModifyInstallDesc(mock.Anything).RunAndReturn(func(f func(*v1.AgentInstallDesc) error) (*v1.InstallDescriptor, error) {
+
+					for i := range fakeInstallDescriptor.AgentInstalls {
+						err := f(&fakeInstallDescriptor.AgentInstalls[i])
+						assert.NoErrorf(t, err, "unexpected error while modifying install descriptor %+v", i)
+					}
+
+					assert.False(t, fakeInstallDescriptor.AgentInstalls[0].Active, "install we rolled back from should be set to not active")
+					assert.False(t, fakeInstallDescriptor.AgentInstalls[0].Active, "install we rolled back to should be set to active")
+					return &fakeInstallDescriptor, nil
+				})
 
-		// TODO: Add test cases.
+				// returned modified install descriptor content is not important
+				mockInstallSource.EXPECT().RemoveAgentInstallDesc(newAgentInstallPath).Return(&fakeInstallDescriptor, nil)
+
+				return updateMarker, mockInstallSource
+			},
+			postNormalizeAssertions: nil,
+		},
+		{
+			name: "Entries not having a matching install directory will be removed from the list",
+			setup: func(t *testing.T, topDir string) (*upgrade.UpdateMarker, installDescriptorSource) {
+				newAgentInstallPath := createFakeAgentInstall(t, topDir, "4.5.6", "newversionhash", true)
+				oldAgentInstallPath := createFakeAgentInstall(t, topDir, "1.2.3", "oldversionhash", true)
+
+				mockInstallSource := newMockInstallDescriptorSource(t)
+				nonExistingVersionedHome := filepath.Join("data", "thisdirectorydoesnotexist")
+				fakeInstallDescriptor := v1.InstallDescriptor{
+					AgentInstalls: []v1.AgentInstallDesc{
+						{
+							Version:       "4.5.6",
+							Hash:          "currentVersionHash",
+							VersionedHome: newAgentInstallPath,
+							Flavor:        "basic",
+							Active:        true,
+						},
+						{
+							Version:         "1.2.3",
+							Hash:            "oldversionhash",
+							VersionedHome:   oldAgentInstallPath,
+							Flavor:          "basic",
+							OptionalTTLItem: v1.OptionalTTLItem{TTL: &tomorrow},
+						},
+						{
+							Version:       "0.0.0",
+							Hash:          "nonExistingHash",
+							VersionedHome: nonExistingVersionedHome,
+							Flavor:        "basic",
+						},
+					},
+				}
+				mockInstallSource.EXPECT().GetInstallDesc().Return(
+					&fakeInstallDescriptor,
+					nil,
+				)
+
+				// returned modified install descriptor content is not important
+				mockInstallSource.EXPECT().RemoveAgentInstallDesc(nonExistingVersionedHome).Return(&fakeInstallDescriptor, nil)
+
+				return nil, mockInstallSource
+			},
+			postNormalizeAssertions: nil,
+		},
+		{
+			name: "Expired installs still existing on disk will be removed from the install list and removed from disk",
+			setup: func(t *testing.T, topDir string) (*upgrade.UpdateMarker, installDescriptorSource) {
+				newAgentInstallPath := createFakeAgentInstall(t, topDir, "4.5.6", "newversionhash", true)
+				oldAgentInstallPath := createFakeAgentInstall(t, topDir, "1.2.3", "oldversionhash", true)
+
+				// assert that the versionedHome of the old install is the same we check in postNormalizeAssertions
+				assert.Equal(t, oldAgentInstallPath, filepath.Join("data", "elastic-agent-1.2.3-oldver"),
+					"Unexpected old install versioned home. Post normalize assertions may not be working")
+
+				mockInstallSource := newMockInstallDescriptorSource(t)
+				fakeInstallDescriptor := v1.InstallDescriptor{
+					AgentInstalls: []v1.AgentInstallDesc{
+						{
+							Version:       "4.5.6",
+							Hash:          "newversionhash",
+							VersionedHome: newAgentInstallPath,
+							Flavor:        "basic",
+							Active:        true,
+						},
+						{
+							Version:         "1.2.3",
+							Hash:            "oldversionhash",
+							VersionedHome:   oldAgentInstallPath,
+							Flavor:          "basic",
+							OptionalTTLItem: v1.OptionalTTLItem{TTL: &yesterday},
+						},
+					},
+				}
+				mockInstallSource.EXPECT().GetInstallDesc().Return(
+					&fakeInstallDescriptor,
+					nil,
+				)
+
+				mockInstallSource.EXPECT().RemoveAgentInstallDesc(oldAgentInstallPath).Return(&fakeInstallDescriptor, nil)
+
+				return nil, mockInstallSource
+			},
+			postNormalizeAssertions: func(t *testing.T, topDir string, _ *upgrade.UpdateMarker) {
+				assert.NoDirExists(t, filepath.Join(topDir, "data", "elastic-agent-1.2.3-oldver"))
+			},
+		},
+		{
+			name: "If a directory cannot be checked, the entry is left alone in the installDescriptor (with a warning in the logs)",
+			setup: func(t *testing.T, topDir string) (*upgrade.UpdateMarker, installDescriptorSource) {
+
+				if runtime.GOOS == "windows" {
+					t.Skipf("This test rely on permission settings not available on Windows")
+				}
+
+				newAgentInstallPath := createFakeAgentInstall(t, topDir, "4.5.6", "newversionhash", true)
+				oldAgentInstallPath := createFakeAgentInstall(t, topDir, "1.2.3", "oldversionhash", true)
+
+				// assert that the versionedHome of the old install is the same we check in postNormalizeAssertions
+				assert.Equal(t, oldAgentInstallPath, filepath.Join("data", "elastic-agent-1.2.3-oldver"),
+					"Unexpected old install versioned home. Post normalize assertions may not be working")
+
+				// make `data` unreadable
+				dataDir := paths.DataFrom(topDir)
+				dataStat, err := os.Stat(dataDir)
+				require.NoError(t, err, "data should be accessible")
+				err = os.Chmod(dataDir, 0o222)
+				require.NoError(t, err, "Error making data unreadable")
+
+				//restore data permissions on test exit
+				t.Cleanup(func() {
+					cleanupErr := os.Chmod(dataDir, dataStat.Mode())
+					assert.NoError(t, cleanupErr, "error restoring data permissions")
+				})
+
+				_, err = os.Stat(filepath.Join(topDir, newAgentInstallPath))
+				require.Errorf(t, err, "os.Stat on %s shoud not be successful", newAgentInstallPath)
+
+				_, err = os.Stat(filepath.Join(topDir, oldAgentInstallPath))
+				require.Errorf(t, err, "os.Stat on %s shoud not be successful", oldAgentInstallPath)
+
+				mockInstallSource := newMockInstallDescriptorSource(t)
+				fakeInstallDescriptor := v1.InstallDescriptor{
+					AgentInstalls: []v1.AgentInstallDesc{
+						{
+							Version:       "4.5.6",
+							Hash:          "newversionhash",
+							VersionedHome: newAgentInstallPath,
+							Flavor:        "basic",
+							Active:        true,
+						},
+						{
+							Version:         "1.2.3",
+							Hash:            "oldversionhash",
+							VersionedHome:   oldAgentInstallPath,
+							Flavor:          "basic",
+							OptionalTTLItem: v1.OptionalTTLItem{TTL: &yesterday},
+						},
+					},
+				}
+				mockInstallSource.EXPECT().GetInstallDesc().Return(
+					&fakeInstallDescriptor,
+					nil,
+				)
+
+				return nil, mockInstallSource
+			},
+			postNormalizeAssertions: func(t *testing.T, topDir string, _ *upgrade.UpdateMarker) {
+				// make data readable again
+				dataDir := paths.DataFrom(topDir)
+				err := os.Chmod(dataDir, 0o755)
+				require.NoError(t, err, "error reopening data permissions")
+				assert.DirExists(t, filepath.Join(topDir, "data", "elastic-agent-1.2.3-oldver"))
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			logger, _ := loggertest.New(t.Name())
-			installSource := newMockInstallDescriptorSource(t)
-			normalizeInstallDescriptorAtStartup(logger, tt.args.topDir, tt.args.now, tt.args.initialUpdateMarker, installSource)
+			tmpDir := t.TempDir()
+			updateMarker, installSource := tt.setup(t, tmpDir)
+			normalizeInstallDescriptorAtStartup(logger, tmpDir, now, updateMarker, installSource)
+			if tt.postNormalizeAssertions != nil {
+				tt.postNormalizeAssertions(t, tmpDir, updateMarker)
+			}
 		})
 	}
 }
+
+// createFakeAgentInstall (copied from the upgrade package tests) 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[:upgrade.HashLen])
+	if useVersionInPath {
+		// use the version passed as parameter
+		versionedHome = fmt.Sprintf("elastic-agent-%s-%s", version, hash[:upgrade.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 := upgrade.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)
+	fakeLogPath := filepath.Join(absLogsDirPath, "fakelog.ndjson")
+	err = os.WriteFile(fakeLogPath, []byte(fmt.Sprintf("Sample logs for agent %s", version)), 0o750)
+	require.NoErrorf(t, err, "error writing fake log placeholder %q", fakeLogPath)
+
+	// return the path relative to top exactly like the step_unpack does
+	return relVersionedHomePath
+}