From 47fe5ded08d505eb2a4ca389b2f9a8693f034527 Mon Sep 17 00:00:00 2001 From: Bertrand Mermet Date: Mon, 24 Feb 2025 17:12:38 +0100 Subject: [PATCH] (fleet) Remove locking mechanism and add pre-remove hooks to the installer (#34350) --- pkg/fleet/installer/exec/installer_exec.go | 10 +- pkg/fleet/installer/go.mod | 3 - pkg/fleet/installer/go.sum | 11 - pkg/fleet/installer/installer.go | 4 +- pkg/fleet/installer/installer_test.go | 23 +- .../installer/packages/datadog_installer.go | 11 +- .../installer/packages/pre_remove_hooks.go | 13 + pkg/fleet/installer/paths/installer_paths.go | 2 - .../paths/installer_paths_windows.go | 3 - .../installer/repository/repositories.go | 18 +- .../installer/repository/repositories_test.go | 8 +- pkg/fleet/installer/repository/repository.go | 223 +++++++----------- .../installer/repository/repository_test.go | 156 +++++++----- .../installer/unix/package_installer_test.go | 1 - .../tests/installer/windows/consts/consts.go | 1 - 15 files changed, 233 insertions(+), 254 deletions(-) create mode 100644 pkg/fleet/installer/packages/pre_remove_hooks.go diff --git a/pkg/fleet/installer/exec/installer_exec.go b/pkg/fleet/installer/exec/installer_exec.go index c591af79d025d..f2d00965cf5e9 100644 --- a/pkg/fleet/installer/exec/installer_exec.go +++ b/pkg/fleet/installer/exec/installer_exec.go @@ -216,19 +216,19 @@ func (i *InstallerExec) Setup(ctx context.Context) (err error) { // AvailableDiskSpace returns the available disk space. func (i *InstallerExec) AvailableDiskSpace() (uint64, error) { - repositories := repository.NewRepositories(paths.PackagesPath, paths.LocksPath) + repositories := repository.NewRepositories(paths.PackagesPath, nil) return repositories.AvailableDiskSpace() } // State returns the state of a package. func (i *InstallerExec) State(pkg string) (repository.State, error) { - repositories := repository.NewRepositories(paths.PackagesPath, paths.LocksPath) + repositories := repository.NewRepositories(paths.PackagesPath, nil) return repositories.Get(pkg).GetState() } // States returns the states of all packages. func (i *InstallerExec) States() (map[string]repository.State, error) { - repositories := repository.NewRepositories(paths.PackagesPath, paths.LocksPath) + repositories := repository.NewRepositories(paths.PackagesPath, nil) states, err := repositories.GetStates() log.Debugf("repositories states: %v", states) return states, err @@ -236,13 +236,13 @@ func (i *InstallerExec) States() (map[string]repository.State, error) { // ConfigState returns the state of a package's configuration. func (i *InstallerExec) ConfigState(pkg string) (repository.State, error) { - repositories := repository.NewRepositories(paths.ConfigsPath, paths.LocksPath) + repositories := repository.NewRepositories(paths.ConfigsPath, nil) return repositories.Get(pkg).GetState() } // ConfigStates returns the states of all packages' configurations. func (i *InstallerExec) ConfigStates() (map[string]repository.State, error) { - repositories := repository.NewRepositories(paths.ConfigsPath, paths.LocksPath) + repositories := repository.NewRepositories(paths.ConfigsPath, nil) states, err := repositories.GetStates() log.Debugf("config repositories states: %v", states) return states, err diff --git a/pkg/fleet/installer/go.mod b/pkg/fleet/installer/go.mod index 2f9529a822420..0b93c1a80b151 100644 --- a/pkg/fleet/installer/go.mod +++ b/pkg/fleet/installer/go.mod @@ -6,7 +6,6 @@ require ( cloud.google.com/go/compute/metadata v0.6.0 github.com/DataDog/datadog-agent/pkg/util/log v0.0.0-00010101000000-000000000000 github.com/DataDog/datadog-agent/pkg/version v0.62.3 - github.com/DataDog/gopsutil v1.2.2 github.com/Microsoft/go-winio v0.6.2 github.com/google/go-containerregistry v0.20.3 github.com/google/uuid v1.6.0 @@ -24,7 +23,6 @@ require ( require ( github.com/DataDog/datadog-agent/pkg/util/scrubber v0.62.3 // indirect - github.com/StackExchange/wmi v1.2.1 // indirect github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 // indirect github.com/containerd/stargz-snapshotter/estargz v0.16.3 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect @@ -41,7 +39,6 @@ require ( github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect - github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4 // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/tklauser/go-sysconf v0.3.14 // indirect github.com/tklauser/numcpus v0.9.0 // indirect diff --git a/pkg/fleet/installer/go.sum b/pkg/fleet/installer/go.sum index f968322934bd3..be3b7ff880bf2 100644 --- a/pkg/fleet/installer/go.sum +++ b/pkg/fleet/installer/go.sum @@ -1,12 +1,7 @@ cloud.google.com/go/compute/metadata v0.6.0 h1:A6hENjEsCDtC1k8byVsgwvVcioamEHvZ4j01OwKxG9I= cloud.google.com/go/compute/metadata v0.6.0/go.mod h1:FjyFAW1MW0C203CEOMDTu3Dk1FlqW3Rga40jzHL4hfg= -github.com/DataDog/gopsutil v1.2.2 h1:8lmthwyyCXa1NKiYcHlrtl9AAFdfbNI2gPcioCJcBPU= -github.com/DataDog/gopsutil v1.2.2/go.mod h1:glkxNt/qRu9lnpmUEQwOIAXW+COWDTBOTEAHqbgBPts= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= -github.com/StackExchange/wmi v0.0.0-20181212234831-e0a55b97c705/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg= -github.com/StackExchange/wmi v1.2.1 h1:VIkavFPXSjcnS+O8yTq7NI32k0R5Aj+v39y29VYDOSA= -github.com/StackExchange/wmi v1.2.1/go.mod h1:rcmrprowKIVzvc+NUiLncP2uuArMWLCbu9SBzvHz7e8= github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 h1:kHaBemcxl8o/pQ5VM1c8PVE1PubbNx3mjUr09OqWGCs= github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575/go.mod h1:9d6lWj8KzO/fd/NrVaLscBKmPigpZpn5YawRPw+e3Yo= github.com/containerd/stargz-snapshotter/estargz v0.16.3 h1:7evrXtoh1mSbGj/pfRccTampEyKpjpOnS3CyiV1Ebr8= @@ -23,8 +18,6 @@ github.com/docker/docker-credential-helpers v0.8.2 h1:bX3YxiGzFP5sOXWc3bTPEXdEaZ github.com/docker/docker-credential-helpers v0.8.2/go.mod h1:P3ci7E3lwkZg6XiHdRKft1KckHiO9a2rNtyFbZ/ry9M= github.com/ebitengine/purego v0.8.2 h1:jPPGWs2sZ1UgOSgD2bClL0MJIqu58nOmIcBuXr62z1I= github.com/ebitengine/purego v0.8.2/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= -github.com/go-ole/go-ole v1.2.4/go.mod h1:XCwSNxSkXRo4vlyPy93sltvi/qJq0jqQhjqQNIwKuxM= -github.com/go-ole/go-ole v1.2.5/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= github.com/go-ole/go-ole v1.3.0/go.mod h1:5LS6F96DhAwUc7C+1HLexzMXY1xGRSryjyPPKW6zv78= @@ -59,12 +52,9 @@ github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= github.com/shirou/gopsutil/v4 v4.25.1 h1:QSWkTc+fu9LTAWfkZwZ6j8MSUk4A2LV7rbH0ZqmLjXs= github.com/shirou/gopsutil/v4 v4.25.1/go.mod h1:RoUCUpndaJFtT+2zsZzzmhvbfGoDCJ7nFXKJf8GqJbI= -github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4 h1:udFKJ0aHUL60LboW/A+DfgoHVedieIzIXE8uylPue0U= -github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4/go.mod h1:qsXQc7+bwAM3Q1u/4XEfrquwF8Lw7D7y5cD8CuHnfIc= github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= @@ -92,7 +82,6 @@ golang.org/x/sync v0.11.0 h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w= golang.org/x/sync v0.11.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= diff --git a/pkg/fleet/installer/installer.go b/pkg/fleet/installer/installer.go index 9a34faccd55f5..ecc01200105d1 100644 --- a/pkg/fleet/installer/installer.go +++ b/pkg/fleet/installer/installer.go @@ -97,8 +97,8 @@ func NewInstaller(env *env.Env) (Installer, error) { env: env, db: db, downloader: oci.NewDownloader(env, env.HTTPClient()), - packages: repository.NewRepositories(paths.PackagesPath, paths.LocksPath), - configs: repository.NewRepositories(paths.ConfigsPath, paths.LocksPath), + packages: repository.NewRepositories(paths.PackagesPath, packages.PreRemoveHooks), + configs: repository.NewRepositories(paths.ConfigsPath, nil), userConfigsDir: paths.DefaultUserConfigsDir, packagesDir: paths.PackagesPath, diff --git a/pkg/fleet/installer/installer_test.go b/pkg/fleet/installer/installer_test.go index a963a2a9af3bb..f55e774e1dad5 100644 --- a/pkg/fleet/installer/installer_test.go +++ b/pkg/fleet/installer/installer_test.go @@ -25,6 +25,7 @@ import ( "github.com/DataDog/datadog-agent/pkg/fleet/installer/env" "github.com/DataDog/datadog-agent/pkg/fleet/installer/fixtures" "github.com/DataDog/datadog-agent/pkg/fleet/installer/oci" + "github.com/DataDog/datadog-agent/pkg/fleet/installer/packages" "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" ) @@ -37,9 +38,9 @@ type testPackageManager struct { installerImpl } -func newTestPackageManager(t *testing.T, s *fixtures.Server, rootPath string, locksPath string) *testPackageManager { - packages := repository.NewRepositories(rootPath, locksPath) - configs := repository.NewRepositories(t.TempDir(), t.TempDir()) +func newTestPackageManager(t *testing.T, s *fixtures.Server, rootPath string) *testPackageManager { + packages := repository.NewRepositories(rootPath, packages.PreRemoveHooks) + configs := repository.NewRepositories(t.TempDir(), nil) db, err := db.New(filepath.Join(rootPath, "packages.db")) assert.NoError(t, err) return &testPackageManager{ @@ -62,7 +63,7 @@ func (i *testPackageManager) ConfigFS(f fixtures.Fixture) fs.FS { func TestInstallStable(t *testing.T) { doTestInstallers(t, func(instFactory installFnFactory, t *testing.T) { s := fixtures.NewServer(t) - installer := newTestPackageManager(t, s, t.TempDir(), t.TempDir()) + installer := newTestPackageManager(t, s, t.TempDir()) defer installer.db.Close() err := instFactory(installer)(testCtx, s.PackageURL(fixtures.FixtureSimpleV1), nil) @@ -80,7 +81,7 @@ func TestInstallStable(t *testing.T) { func TestInstallExperiment(t *testing.T) { doTestInstallers(t, func(instFactory installFnFactory, t *testing.T) { s := fixtures.NewServer(t) - installer := newTestPackageManager(t, s, t.TempDir(), t.TempDir()) + installer := newTestPackageManager(t, s, t.TempDir()) defer installer.db.Close() err := instFactory(installer)(testCtx, s.PackageURL(fixtures.FixtureSimpleV1), nil) @@ -101,7 +102,7 @@ func TestInstallExperiment(t *testing.T) { func TestInstallPromoteExperiment(t *testing.T) { doTestInstallers(t, func(instFactory installFnFactory, t *testing.T) { s := fixtures.NewServer(t) - installer := newTestPackageManager(t, s, t.TempDir(), t.TempDir()) + installer := newTestPackageManager(t, s, t.TempDir()) defer installer.db.Close() err := instFactory(installer)(testCtx, s.PackageURL(fixtures.FixtureSimpleV1), nil) @@ -123,7 +124,7 @@ func TestInstallPromoteExperiment(t *testing.T) { func TestUninstallExperiment(t *testing.T) { doTestInstallers(t, func(instFactory installFnFactory, t *testing.T) { s := fixtures.NewServer(t) - installer := newTestPackageManager(t, s, t.TempDir(), t.TempDir()) + installer := newTestPackageManager(t, s, t.TempDir()) defer installer.db.Close() err := instFactory(installer)(testCtx, s.PackageURL(fixtures.FixtureSimpleV1), nil) @@ -145,7 +146,7 @@ func TestUninstallExperiment(t *testing.T) { func TestInstallSkippedWhenAlreadyInstalled(t *testing.T) { s := fixtures.NewServer(t) - installer := newTestPackageManager(t, s, t.TempDir(), t.TempDir()) + installer := newTestPackageManager(t, s, t.TempDir()) defer installer.db.Close() err := installer.Install(testCtx, s.PackageURL(fixtures.FixtureSimpleV1), nil) @@ -164,7 +165,7 @@ func TestInstallSkippedWhenAlreadyInstalled(t *testing.T) { func TestForceInstallWhenAlreadyInstalled(t *testing.T) { s := fixtures.NewServer(t) - installer := newTestPackageManager(t, s, t.TempDir(), t.TempDir()) + installer := newTestPackageManager(t, s, t.TempDir()) defer installer.db.Close() err := installer.Install(testCtx, s.PackageURL(fixtures.FixtureSimpleV1), nil) @@ -184,7 +185,7 @@ func TestForceInstallWhenAlreadyInstalled(t *testing.T) { func TestReinstallAfterDBClean(t *testing.T) { doTestInstallers(t, func(instFactory installFnFactory, t *testing.T) { s := fixtures.NewServer(t) - installer := newTestPackageManager(t, s, t.TempDir(), t.TempDir()) + installer := newTestPackageManager(t, s, t.TempDir()) defer installer.db.Close() err := instFactory(installer)(testCtx, s.PackageURL(fixtures.FixtureSimpleV1), nil) @@ -248,7 +249,7 @@ func TestPurge(t *testing.T) { doTestInstallers(t, func(instFactory installFnFactory, t *testing.T) { s := fixtures.NewServer(t) rootPath := t.TempDir() - installer := newTestPackageManager(t, s, rootPath, t.TempDir()) + installer := newTestPackageManager(t, s, rootPath) err := instFactory(installer)(testCtx, s.PackageURL(fixtures.FixtureSimpleV1), nil) assert.NoError(t, err) diff --git a/pkg/fleet/installer/packages/datadog_installer.go b/pkg/fleet/installer/packages/datadog_installer.go index 9d527c11f0757..5bd618f12bb8d 100644 --- a/pkg/fleet/installer/packages/datadog_installer.go +++ b/pkg/fleet/installer/packages/datadog_installer.go @@ -90,16 +90,7 @@ func SetupInstaller(ctx context.Context) (err error) { // Run directory can already be created by the RC client err = os.Chmod("/opt/datadog-packages/run", 0755) if err != nil { - return fmt.Errorf("error changing permissions of /opt/datadog-packages/run/locks: %w", err) - } - err = os.MkdirAll("/opt/datadog-packages/run/locks", 0777) - if err != nil { - return fmt.Errorf("error creating /opt/datadog-packages/run/locks: %w", err) - } - // Locks directory can already be created by a package install - err = os.Chmod("/opt/datadog-packages/run/locks", 0777) - if err != nil { - return fmt.Errorf("error changing permissions of /opt/datadog-packages/run/locks: %w", err) + return fmt.Errorf("error changing permissions of /opt/datadog-packages/run: %w", err) } err = os.Chown("/etc/datadog-agent", ddAgentUID, ddAgentGID) if err != nil { diff --git a/pkg/fleet/installer/packages/pre_remove_hooks.go b/pkg/fleet/installer/packages/pre_remove_hooks.go new file mode 100644 index 0000000000000..a415902758a21 --- /dev/null +++ b/pkg/fleet/installer/packages/pre_remove_hooks.go @@ -0,0 +1,13 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +package packages + +import ( + "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" +) + +// PreRemoveHooks contains the mapping between packages and their pre-remove hooks +var PreRemoveHooks = map[string]repository.PreRemoveHook{} diff --git a/pkg/fleet/installer/paths/installer_paths.go b/pkg/fleet/installer/paths/installer_paths.go index 99b3193dd7a03..703b85b209774 100644 --- a/pkg/fleet/installer/paths/installer_paths.go +++ b/pkg/fleet/installer/paths/installer_paths.go @@ -13,8 +13,6 @@ const ( PackagesPath = "/opt/datadog-packages" // ConfigsPath is the path to the Fleet-managed configuration directory. ConfigsPath = "/etc/datadog-agent/managed" - // LocksPath is the path to the packages locks directory. - LocksPath = "/opt/datadog-packages/run/locks" // RootTmpDir is the temporary path where the bootstrapper will be extracted to. RootTmpDir = "/opt/datadog-packages/tmp" // DefaultUserConfigsDir is the default Agent configuration directory. diff --git a/pkg/fleet/installer/paths/installer_paths_windows.go b/pkg/fleet/installer/paths/installer_paths_windows.go index 2a1d487a28295..a998b116940db 100644 --- a/pkg/fleet/installer/paths/installer_paths_windows.go +++ b/pkg/fleet/installer/paths/installer_paths_windows.go @@ -34,8 +34,6 @@ var ( PackagesPath string // ConfigsPath is the path to the Fleet-managed configuration directory ConfigsPath string - // LocksPath is the path to the locks directory. - LocksPath string // RootTmpDir is the temporary path where the bootstrapper will be extracted to. RootTmpDir string // DefaultUserConfigsDir is the default Agent configuration directory @@ -50,7 +48,6 @@ func init() { datadogInstallerData, _ = getProgramDataDirForProduct("Datadog Installer") PackagesPath = filepath.Join(datadogInstallerData, "packages") ConfigsPath = filepath.Join(datadogInstallerData, "configs") - LocksPath = filepath.Join(datadogInstallerData, "locks") RootTmpDir = filepath.Join(datadogInstallerData, "tmp") datadogInstallerPath := "C:\\Program Files\\Datadog\\Datadog Installer" StableInstallerPath = filepath.Join(datadogInstallerPath, "datadog-installer.exe") diff --git a/pkg/fleet/installer/repository/repositories.go b/pkg/fleet/installer/repository/repositories.go index 54a0ad2dfed65..7622c09843d8a 100644 --- a/pkg/fleet/installer/repository/repositories.go +++ b/pkg/fleet/installer/repository/repositories.go @@ -21,22 +21,22 @@ const ( // Repositories manages multiple repositories. type Repositories struct { - rootPath string - locksPath string + rootPath string + preRemoveHooks map[string]PreRemoveHook } // NewRepositories returns a new Repositories. -func NewRepositories(rootPath, locksPath string) *Repositories { +func NewRepositories(rootPath string, preRemoveHooks map[string]PreRemoveHook) *Repositories { return &Repositories{ - rootPath: rootPath, - locksPath: locksPath, + rootPath: rootPath, + preRemoveHooks: preRemoveHooks, } } func (r *Repositories) newRepository(pkg string) *Repository { return &Repository{ - rootPath: filepath.Join(r.rootPath, pkg), - locksPath: filepath.Join(r.locksPath, pkg), + rootPath: filepath.Join(r.rootPath, pkg), + preRemoveHooks: r.preRemoveHooks, } } @@ -87,8 +87,8 @@ func (r *Repositories) Create(pkg string, version string, stableSourcePath strin // Delete deletes the repository for the given package name. func (r *Repositories) Delete(_ context.Context, pkg string) error { repository := r.newRepository(pkg) - // TODO: locked packages will still be deleted - err := os.RemoveAll(repository.rootPath) + + err := repository.Delete() if err != nil { return fmt.Errorf("could not delete repository for package %s: %w", pkg, err) } diff --git a/pkg/fleet/installer/repository/repositories_test.go b/pkg/fleet/installer/repository/repositories_test.go index ab66494b826ef..c1cdc634e4c74 100644 --- a/pkg/fleet/installer/repository/repositories_test.go +++ b/pkg/fleet/installer/repository/repositories_test.go @@ -15,8 +15,7 @@ import ( func newTestRepositories(t *testing.T) *Repositories { rootPath := t.TempDir() - locksRootPath := t.TempDir() - repositories := NewRepositories(rootPath, locksRootPath) + repositories := NewRepositories(rootPath, nil) return repositories } @@ -53,7 +52,7 @@ func TestRepositoriesReopen(t *testing.T) { err = repositories.Create("repo2", "v1", t.TempDir()) assert.NoError(t, err) - repositories = NewRepositories(repositories.rootPath, repositories.locksPath) + repositories = NewRepositories(repositories.rootPath, nil) state, err := repositories.GetStates() assert.NoError(t, err) @@ -64,14 +63,13 @@ func TestRepositoriesReopen(t *testing.T) { func TestLoadRepositories(t *testing.T) { rootDir := t.TempDir() - runDir := t.TempDir() os.Mkdir(path.Join(rootDir, "datadog-agent"), 0755) os.Mkdir(path.Join(rootDir, tempDirPrefix+"2394812349"), 0755) os.Mkdir(path.Join(rootDir, "run"), 0755) os.Mkdir(path.Join(rootDir, "tmp"), 0755) - repositories, err := NewRepositories(rootDir, runDir).loadRepositories() + repositories, err := NewRepositories(rootDir, nil).loadRepositories() assert.NoError(t, err) assert.Len(t, repositories, 1) assert.Contains(t, repositories, "datadog-agent") diff --git a/pkg/fleet/installer/repository/repository.go b/pkg/fleet/installer/repository/repository.go index 559bf88854641..fdc59fed75d7c 100644 --- a/pkg/fleet/installer/repository/repository.go +++ b/pkg/fleet/installer/repository/repository.go @@ -12,9 +12,6 @@ import ( "io/fs" "os" "path/filepath" - "strconv" - - "github.com/DataDog/gopsutil/process" "github.com/DataDog/datadog-agent/pkg/util/log" ) @@ -28,6 +25,11 @@ var ( errRepositoryNotCreated = errors.New("repository not created") ) +// PreRemoveHook are called before a package is removed. It returns a boolean +// indicating if the package files can be deleted safely and an error if an error happened +// when running the hook. +type PreRemoveHook func(string) (bool, error) + // Repository contains the stable and experimental package of a single artifact managed by the updater. // // On disk the repository is structured as follows: @@ -37,13 +39,6 @@ var ( // ├── stable -> 7.50.0 (symlink) // └── experiment -> 7.51.0 (symlink) // -// and the locks directory (if any) is structured as follows: -// . -// ├── 7.50.0 -// │ └── 1234 -// ├── 7.51.0 -// │ └── 5678 -// // We voluntarily do not load the state of the repository in memory to avoid any bugs where // what's on disk and what's in memory are not in sync. // All the functions of the repository are "atomic" and ensure no invalid state can be reached @@ -51,11 +46,8 @@ var ( // It is possible to end up with garbage left on disk if an error happens during some operations. This // is cleaned up during the next operation. type Repository struct { - rootPath string - - // locksPath is the path to the locks directory - // containing the PIDs of the processes using the packages. - locksPath string + rootPath string + preRemoveHooks map[string]PreRemoveHook } // State is the state of the repository. @@ -86,7 +78,7 @@ func (r *Repository) ExperimentFS() fs.FS { // GetState returns the state of the repository. func (r *Repository) GetState() (State, error) { - repository, err := readRepository(r.rootPath, r.locksPath) + repository, err := readRepository(r.rootPath, r.preRemoveHooks) if errors.Is(err, errRepositoryNotCreated) { return State{}, nil } @@ -118,12 +110,12 @@ func (r *Repository) Create(name string, stableSourcePath string) error { return fmt.Errorf("could not create packages root directory: %w", err) } - repository, err := readRepository(r.rootPath, r.locksPath) + repository, err := readRepository(r.rootPath, r.preRemoveHooks) if err != nil { return err } - // Remove symlinks as we are bootstrapping + // Remove symlinks as we are (re)-installing the package if repository.experiment.Exists() { err = repository.experiment.Delete() if err != nil { @@ -137,22 +129,6 @@ func (r *Repository) Create(name string, stableSourcePath string) error { } } - // Remove left-over locks paths - packageLocksPaths, err := os.ReadDir(r.locksPath) - if err != nil && !errors.Is(err, os.ErrNotExist) { - return fmt.Errorf("could not read locks directory: %w", err) - } - for _, pkg := range packageLocksPaths { - pkgRootPath := filepath.Join(r.rootPath, pkg.Name()) - pkgLocksPath := filepath.Join(r.locksPath, pkg.Name()) - if _, err := os.Stat(pkgRootPath); err != nil && errors.Is(err, os.ErrNotExist) { - err = os.RemoveAll(pkgLocksPath) - if err != nil { - log.Errorf("could not remove package %s locks directory, will retry at next startup: %v", pkgLocksPath, err) - } - } - } - err = repository.cleanup() if err != nil { return fmt.Errorf("could not cleanup repository: %w", err) @@ -169,13 +145,60 @@ func (r *Repository) Create(name string, stableSourcePath string) error { return nil } +// Delete deletes the repository. +// +// 1. Remove the stable and experiment links. +// 2. Cleanup the repository to remove all package versions after running the pre-remove hooks. +// 3. Remove the root directory. +func (r *Repository) Delete() error { + // Remove symlinks first so that cleanup will attempt to remove all package versions + repositoryFiles, err := readRepository(r.rootPath, r.preRemoveHooks) + if err != nil { + return err + } + if repositoryFiles.experiment.Exists() { + err = repositoryFiles.experiment.Delete() + if err != nil { + return fmt.Errorf("could not delete experiment link: %w", err) + } + } + if repositoryFiles.stable.Exists() { + err = repositoryFiles.stable.Delete() + if err != nil { + return fmt.Errorf("could not delete stable link: %w", err) + } + } + + // Delete all package versions + err = r.Cleanup() + if err != nil { + return fmt.Errorf("could not cleanup repository for package %w", err) + } + + files, err := os.ReadDir(r.rootPath) + if err != nil { + return fmt.Errorf("could not read root directory: %w", err) + } + + if len(files) > 0 { + return fmt.Errorf("could not delete root directory, not empty after cleanup") + } + + // Delete the repository directory + err = os.RemoveAll(r.rootPath) + if err != nil { + return fmt.Errorf("could not delete root directory for package %w", err) + } + return nil +} + // SetExperiment moves package files from the given source path to the repository and sets it as the experiment. // // 1. Cleanup the repository. // 2. Move the experiment source to the repository. // 3. Set the experiment link to the experiment package. func (r *Repository) SetExperiment(name string, sourcePath string) error { - repository, err := readRepository(r.rootPath, r.locksPath) + repository, err := readRepository(r.rootPath, r.preRemoveHooks) if err != nil { return err } @@ -202,7 +225,7 @@ func (r *Repository) SetExperiment(name string, sourcePath string) error { // 2. Set the stable link to the experiment package. The experiment link stays in place. // 3. Cleanup the repository to remove the previous stable package. func (r *Repository) PromoteExperiment() error { - repository, err := readRepository(r.rootPath, r.locksPath) + repository, err := readRepository(r.rootPath, r.preRemoveHooks) if err != nil { return err } @@ -223,12 +246,6 @@ func (r *Repository) PromoteExperiment() error { if err != nil { return fmt.Errorf("could not set stable: %w", err) } - - // Read repository again to re-load the list of locked packages - repository, err = readRepository(r.rootPath, r.locksPath) - if err != nil { - return err - } err = repository.cleanup() if err != nil { return fmt.Errorf("could not cleanup repository: %w", err) @@ -242,7 +259,7 @@ func (r *Repository) PromoteExperiment() error { // 2. Sets the experiment link to the stable link. // 3. Cleanup the repository to remove the previous experiment package. func (r *Repository) DeleteExperiment() error { - repository, err := readRepository(r.rootPath, r.locksPath) + repository, err := readRepository(r.rootPath, r.preRemoveHooks) if err != nil { return err } @@ -260,12 +277,6 @@ func (r *Repository) DeleteExperiment() error { if err != nil { return fmt.Errorf("could not set experiment to stable: %w", err) } - - // Read repository again to re-load the list of locked packages - repository, err = readRepository(r.rootPath, r.locksPath) - if err != nil { - return err - } err = repository.cleanup() if err != nil { return fmt.Errorf("could not cleanup repository: %w", err) @@ -275,7 +286,7 @@ func (r *Repository) DeleteExperiment() error { // Cleanup calls the cleanup function of the repository func (r *Repository) Cleanup() error { - repository, err := readRepository(r.rootPath, r.locksPath) + repository, err := readRepository(r.rootPath, r.preRemoveHooks) if err != nil { return err } @@ -284,14 +295,13 @@ func (r *Repository) Cleanup() error { type repositoryFiles struct { rootPath string - locksPath string - lockedPackages map[string]bool + preRemoveHooks map[string]PreRemoveHook stable *link experiment *link } -func readRepository(rootPath string, locksPath string) (*repositoryFiles, error) { +func readRepository(rootPath string, preRemoveHooks map[string]PreRemoveHook) (*repositoryFiles, error) { stableLink, err := newLink(filepath.Join(rootPath, stableVersionLink)) if err != nil { return nil, fmt.Errorf("could not load stable link: %w", err) @@ -301,36 +311,16 @@ func readRepository(rootPath string, locksPath string) (*repositoryFiles, error) return nil, fmt.Errorf("could not load experiment link: %w", err) } - // List locked packages - packages, err := os.ReadDir(rootPath) - if errors.Is(err, os.ErrNotExist) { - return nil, errRepositoryNotCreated - } - if err != nil { - return nil, fmt.Errorf("could not read root directory: %w", err) - } - lockedPackages := map[string]bool{} - for _, pkg := range packages { - pkgLocksPath := filepath.Join(locksPath, pkg.Name()) - isLocked, err := packageLocked(pkgLocksPath) - if err != nil { - log.Errorf("could not check if package version is in use: %v", err) - continue - } - lockedPackages[pkg.Name()] = isLocked - } - return &repositoryFiles{ rootPath: rootPath, - locksPath: locksPath, - lockedPackages: lockedPackages, + preRemoveHooks: preRemoveHooks, stable: stableLink, experiment: experimentLink, }, nil } func (r *repositoryFiles) setExperiment(name string, sourcePath string) error { - path, err := movePackageFromSource(name, r.rootPath, r.lockedPackages, sourcePath) + path, err := movePackageFromSource(name, r.rootPath, sourcePath) if err != nil { return fmt.Errorf("could not move experiment source: %w", err) } @@ -344,7 +334,7 @@ func (r *repositoryFiles) setExperimentToStable() error { } func (r *repositoryFiles) setStable(name string, sourcePath string) error { - path, err := movePackageFromSource(name, r.rootPath, r.lockedPackages, sourcePath) + path, err := movePackageFromSource(name, r.rootPath, sourcePath) if err != nil { return fmt.Errorf("could not move stable source: %w", err) } @@ -352,20 +342,15 @@ func (r *repositoryFiles) setStable(name string, sourcePath string) error { return r.stable.Set(path) } -func movePackageFromSource(packageName string, rootPath string, lockedPackages map[string]bool, sourcePath string) (string, error) { +func movePackageFromSource(packageName string, rootPath string, sourcePath string) (string, error) { if packageName == "" || packageName == stableVersionLink || packageName == experimentVersionLink { return "", fmt.Errorf("invalid package name") } targetPath := filepath.Join(rootPath, packageName) _, err := os.Stat(targetPath) if err == nil { - // Check if we have long running processes using the package - // If yes, the GC left the package in place so we don't reinstall it, but - // we don't throw an error either. - // If not, the GC should have removed the packages so we error. - if lockedPackages[packageName] { - return targetPath, nil - } + // TODO: Do we want to differentiate between packages that cannot be deleted + // due to the pre-remove hook and other reasons? return "", fmt.Errorf("target package already exists") } if !errors.Is(err, os.ErrNotExist) { @@ -396,9 +381,9 @@ func (r *repositoryFiles) cleanup() error { return fmt.Errorf("could not read root directory: %w", err) } - // For each version, get the running PIDs. These PIDs are written by the injector. - // The injector is ran directly with the service as it's a LD_PRELOAD, and has access - // to the PIDs. + // for all versions that are not stable or experiment: + // - if no pre-remove hook is configured, delete the package + // - if a pre-remove hook is configured, run the hook and delete the package only if the hook returns true for _, file := range files { isLink := file.Name() == stableVersionLink || file.Name() == experimentVersionLink isStable := r.stable.Exists() && r.stable.Target() == file.Name() @@ -407,63 +392,27 @@ func (r *repositoryFiles) cleanup() error { continue } - if !r.lockedPackages[file.Name()] { - // Package isn't locked, remove it - pkgRepositoryPath := filepath.Join(r.rootPath, file.Name()) - pkgLocksPath := filepath.Join(r.locksPath, file.Name()) - log.Debugf("package %s isn't locked, removing it", pkgRepositoryPath) - if err := os.RemoveAll(pkgRepositoryPath); err != nil { - log.Errorf("could not remove package %s directory, will retry: %v", pkgRepositoryPath, err) + pkgRepositoryPath := filepath.Join(r.rootPath, file.Name()) + pkgName := filepath.Base(r.rootPath) + + if pkgHook, hasHook := r.preRemoveHooks[pkgName]; hasHook { + canDelete, err := pkgHook(pkgRepositoryPath) + if err != nil { + log.Errorf("Pre-remove hook for package %s returned an error: %v", pkgRepositoryPath, err) } - if err := os.RemoveAll(pkgLocksPath); err != nil { - log.Errorf("could not remove package %s locks directory, will retry: %v", pkgLocksPath, err) + // if there is an error, the hook still decides if the package can be deleted + if !canDelete { + continue } } - } - - return nil -} - -// packageLocked checks if the given package version is in use -// by checking if there are PIDs corresponding to running processes -// in the locks directory. -func packageLocked(packagePIDsPath string) (bool, error) { - pids, err := os.ReadDir(packagePIDsPath) - if errors.Is(err, os.ErrNotExist) { - log.Debugf("package locks directory does not exist, no running PIDs") - return false, nil - } - if err != nil { - return false, fmt.Errorf("could not read locks directory: %w", err) - } - - // For each PID, check if it's running - for _, rawPID := range pids { - pid, err := strconv.ParseInt(rawPID.Name(), 10, 64) - if err != nil { - log.Errorf("could not parse PID: %v", err) - continue - } - - processUsed, err := process.PidExists(int32(pid)) - if err != nil { - log.Errorf("could not find process with PID %d: %v", pid, err) - continue - } - if processUsed { - return true, nil - } - - // PIDs can be re-used, so if the process isn't running we remove the file - log.Debugf("process with PID %d is stopped, removing PID file", pid) - err = os.Remove(filepath.Join(packagePIDsPath, fmt.Sprint(pid))) - if err != nil { - log.Errorf("could not remove PID file: %v", err) + log.Debugf("Removing package %s", pkgRepositoryPath) + if err := os.RemoveAll(pkgRepositoryPath); err != nil { + log.Errorf("could not remove package %s directory, will retry: %v", pkgRepositoryPath, err) } } - return false, nil + return nil } type link struct { diff --git a/pkg/fleet/installer/repository/repository_test.go b/pkg/fleet/installer/repository/repository_test.go index 9e47e4e48dd86..7a578de43a6de 100644 --- a/pkg/fleet/installer/repository/repository_test.go +++ b/pkg/fleet/installer/repository/repository_test.go @@ -6,7 +6,6 @@ package repository import ( - "fmt" "os" "path" "path/filepath" @@ -15,15 +14,15 @@ import ( "github.com/stretchr/testify/assert" ) -func createTestRepository(t *testing.T, dir string, stablePackageName string) *Repository { +func createTestRepository(t *testing.T, dir string, stablePackageName string, preRemoveHook PreRemoveHook) *Repository { repositoryPath := path.Join(dir, "repository") - locksPath := path.Join(dir, "run") os.MkdirAll(repositoryPath, 0755) - os.MkdirAll(locksPath, 0777) stablePackagePath := createTestDownloadedPackage(t, dir, stablePackageName) r := Repository{ - rootPath: repositoryPath, - locksPath: locksPath, + rootPath: repositoryPath, + } + if preRemoveHook != nil { + r.preRemoveHooks = map[string]PreRemoveHook{"repository": preRemoveHook} } err := r.Create(stablePackageName, stablePackagePath) assert.NoError(t, err) @@ -47,7 +46,7 @@ func assertLinkTarget(t *testing.T, repository *Repository, link string, target func TestCreateFresh(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) state, err := repository.GetState() assert.DirExists(t, repository.rootPath) @@ -62,9 +61,9 @@ func TestCreateFresh(t *testing.T) { func TestCreateOverwrite(t *testing.T) { dir := t.TempDir() - oldRepository := createTestRepository(t, dir, "old") + oldRepository := createTestRepository(t, dir, "old", nil) - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) assert.Equal(t, oldRepository.rootPath, repository.rootPath) assert.DirExists(t, repository.rootPath) @@ -72,34 +71,35 @@ func TestCreateOverwrite(t *testing.T) { assert.NoDirExists(t, path.Join(oldRepository.rootPath, "old")) } -func TestCreateOverwriteWithLockedPackage(t *testing.T) { +func TestCreateOverwriteWithHookAllow(t *testing.T) { dir := t.TempDir() - oldRepository := createTestRepository(t, dir, "old") - err := os.MkdirAll(path.Join(oldRepository.locksPath, "garbagetocollect"), 0777) - assert.NoError(t, err) + oldRepository := createTestRepository(t, dir, "old", nil) - // Add a running process... our own! So we're sure it's running. - err = os.MkdirAll(path.Join(oldRepository.locksPath, "old"), 0777) - assert.NoError(t, err) - err = os.WriteFile( - path.Join(oldRepository.locksPath, "old", fmt.Sprint(os.Getpid())), - nil, - 0644, - ) - assert.NoError(t, err) + hook := func(string) (bool, error) { return true, nil } + repository := createTestRepository(t, dir, "v1", hook) + + assert.Equal(t, oldRepository.rootPath, repository.rootPath) + assert.DirExists(t, repository.rootPath) + assert.DirExists(t, path.Join(repository.rootPath, "v1")) + assert.NoDirExists(t, path.Join(repository.rootPath, "old")) +} + +func TestCreateOverwriteWithHookDeny(t *testing.T) { + dir := t.TempDir() + oldRepository := createTestRepository(t, dir, "old", nil) - repository := createTestRepository(t, dir, "v1") + hook := func(string) (bool, error) { return false, nil } + repository := createTestRepository(t, dir, "v1", hook) assert.Equal(t, oldRepository.rootPath, repository.rootPath) assert.DirExists(t, repository.rootPath) assert.DirExists(t, path.Join(repository.rootPath, "v1")) assert.DirExists(t, path.Join(repository.rootPath, "old")) - assert.NoDirExists(t, path.Join(oldRepository.locksPath, "garbagetocollect")) } func TestSetExperiment(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) experimentDownloadPackagePath := createTestDownloadedPackage(t, dir, "v2") err := repository.SetExperiment("v2", experimentDownloadPackagePath) @@ -118,7 +118,7 @@ func TestSetExperiment(t *testing.T) { func TestSetExperimentTwice(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) experiment1DownloadPackagePath := createTestDownloadedPackage(t, dir, "v2") experiment2DownloadPackagePath := createTestDownloadedPackage(t, dir, "v3") @@ -142,7 +142,7 @@ func TestSetExperimentBeforeStable(t *testing.T) { func TestPromoteExperiment(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) experimentDownloadPackagePath := createTestDownloadedPackage(t, dir, "v2") err := repository.SetExperiment("v2", experimentDownloadPackagePath) @@ -163,7 +163,7 @@ func TestPromoteExperiment(t *testing.T) { func TestPromoteExperimentWithoutExperiment(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) err := repository.PromoteExperiment() assert.Error(t, err) @@ -171,7 +171,7 @@ func TestPromoteExperimentWithoutExperiment(t *testing.T) { func TestDeleteExperiment(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) experimentDownloadPackagePath := createTestDownloadedPackage(t, dir, "v2") err := repository.SetExperiment("v2", experimentDownloadPackagePath) @@ -183,51 +183,41 @@ func TestDeleteExperiment(t *testing.T) { func TestDeleteExperimentWithoutExperiment(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) err := repository.DeleteExperiment() assert.NoError(t, err) } -func TestDeleteExperimentWithLockedPackage(t *testing.T) { +func TestDeleteExperimentWithHookAllow(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + hook := func(string) (bool, error) { return true, nil } + repository := createTestRepository(t, dir, "v1", hook) experimentDownloadPackagePath := createTestDownloadedPackage(t, dir, "v2") err := repository.SetExperiment("v2", experimentDownloadPackagePath) assert.NoError(t, err) - - // Add a running process... our own! So we're sure it's running. - err = os.MkdirAll(path.Join(repository.locksPath, "v2"), 0766) - assert.NoError(t, err) - err = os.WriteFile( - path.Join(repository.locksPath, "v2", fmt.Sprint(os.Getpid())), - nil, - 0644, - ) + err = repository.DeleteExperiment() assert.NoError(t, err) + assert.NoDirExists(t, path.Join(repository.rootPath, "v2")) +} - // Add a running process that's not running to check its deletion - err = os.MkdirAll(path.Join(repository.locksPath, "v2"), 0766) - assert.NoError(t, err) - err = os.WriteFile( - path.Join(repository.locksPath, "v2", "-1"), // We're sure not to hit a running process - nil, - 0644, - ) - assert.NoError(t, err) +func TestDeleteExperimentWithHookDeny(t *testing.T) { + dir := t.TempDir() + hook := func(string) (bool, error) { return false, nil } + repository := createTestRepository(t, dir, "v1", hook) + experimentDownloadPackagePath := createTestDownloadedPackage(t, dir, "v2") + err := repository.SetExperiment("v2", experimentDownloadPackagePath) + assert.NoError(t, err) err = repository.DeleteExperiment() assert.NoError(t, err) assert.DirExists(t, path.Join(repository.rootPath, "v2")) - assert.DirExists(t, path.Join(repository.locksPath, "v2")) - assert.NoFileExists(t, path.Join(repository.locksPath, "v2", "-1")) - assert.FileExists(t, path.Join(repository.locksPath, "v2", fmt.Sprint(os.Getpid()))) } func TestMigrateRepositoryWithoutExperiment(t *testing.T) { dir := t.TempDir() - repository := createTestRepository(t, dir, "v1") + repository := createTestRepository(t, dir, "v1", nil) err := os.Remove(path.Join(repository.rootPath, experimentVersionLink)) assert.NoError(t, err) @@ -237,3 +227,61 @@ func TestMigrateRepositoryWithoutExperiment(t *testing.T) { assertLinkTarget(t, repository, stableVersionLink, "v1") assertLinkTarget(t, repository, experimentVersionLink, "v1") } + +func TestDelete(t *testing.T) { + dir := t.TempDir() + repository := createTestRepository(t, dir, "v1", nil) + + err := repository.Delete() + assert.NoError(t, err) + assert.NoDirExists(t, repository.rootPath) +} + +func TestDeleteHookAllow(t *testing.T) { + dir := t.TempDir() + hook := func(string) (bool, error) { return true, nil } + repository := createTestRepository(t, dir, "v1", hook) + + err := repository.Delete() + assert.NoError(t, err) + assert.NoDirExists(t, repository.rootPath) +} + +func TestDeleteHookDeny(t *testing.T) { + dir := t.TempDir() + hook := func(string) (bool, error) { return false, nil } + repository := createTestRepository(t, dir, "v1", hook) + + err := repository.Delete() + assert.Error(t, err) + assert.DirExists(t, repository.rootPath) +} + +func TestDeleteExtraFilesDoNotPreventDeletion(t *testing.T) { + dir := t.TempDir() + repository := createTestRepository(t, dir, "v1", nil) + + extraFilePath := path.Join(repository.rootPath, "extra") + err := os.WriteFile(extraFilePath, []byte("extra"), 0644) + assert.NoError(t, err) + + err = repository.Delete() + assert.NoError(t, err) + assert.NoDirExists(t, repository.rootPath) +} + +func TestDeleteHookDenyDoesNotPreventReinstall(t *testing.T) { + dir := t.TempDir() + hook := func(string) (bool, error) { return false, nil } + oldRepository := createTestRepository(t, dir, "old", hook) + + err := oldRepository.Delete() + assert.Error(t, err) + + repository := createTestRepository(t, dir, "v1", nil) + + assert.Equal(t, oldRepository.rootPath, repository.rootPath) + assert.DirExists(t, repository.rootPath) + assert.DirExists(t, path.Join(repository.rootPath, "v1")) + assert.NoDirExists(t, path.Join(oldRepository.rootPath, "old")) +} diff --git a/test/new-e2e/tests/installer/unix/package_installer_test.go b/test/new-e2e/tests/installer/unix/package_installer_test.go index 942c99fbe6dce..66dcaa2e76af2 100644 --- a/test/new-e2e/tests/installer/unix/package_installer_test.go +++ b/test/new-e2e/tests/installer/unix/package_installer_test.go @@ -42,7 +42,6 @@ func (s *packageInstallerSuite) TestInstall() { state.AssertDirExists("/etc/datadog-agent", 0755, "dd-agent", "dd-agent") state.AssertDirExists("/var/log/datadog", 0755, "dd-agent", "dd-agent") state.AssertDirExists("/opt/datadog-packages/run", 0755, "dd-agent", "dd-agent") - state.AssertDirExists("/opt/datadog-packages/run/locks", 0777, "root", "root") state.AssertDirExists("/opt/datadog-installer", 0755, "root", "root") state.AssertDirExists("/opt/datadog-packages/tmp", 0755, "dd-agent", "dd-agent") diff --git a/test/new-e2e/tests/installer/windows/consts/consts.go b/test/new-e2e/tests/installer/windows/consts/consts.go index f0168e31ec144..ae16515456db5 100644 --- a/test/new-e2e/tests/installer/windows/consts/consts.go +++ b/test/new-e2e/tests/installer/windows/consts/consts.go @@ -45,7 +45,6 @@ var ( InstallerConfigPaths = []string{ path.Join(baseConfigPath, "packages"), path.Join(baseConfigPath, "configs"), - path.Join(baseConfigPath, "locks"), path.Join(baseConfigPath, "tmp"), } )