From e3e590f86bf8e880da0dd807ff40bf16ee6a7aed Mon Sep 17 00:00:00 2001 From: Arthur Bellal Date: Fri, 10 May 2024 15:08:07 +0200 Subject: [PATCH 1/8] (fleet) store package installation in a bbolt db --- .../subcommands/installer/command.go | 44 ++++-- pkg/fleet/installer/installer.go | 58 +++++--- pkg/fleet/installer/installer_test.go | 4 + .../installer/service/datadog_installer.go | 10 -- pkg/fleet/internal/db/db.go | 126 ++++++++++++++++++ pkg/fleet/internal/db/db_test.go | 77 +++++++++++ pkg/fleet/internal/exec/installer_exec.go | 10 +- 7 files changed, 285 insertions(+), 44 deletions(-) create mode 100644 pkg/fleet/internal/db/db.go create mode 100644 pkg/fleet/internal/db/db_test.go diff --git a/cmd/installer/subcommands/installer/command.go b/cmd/installer/subcommands/installer/command.go index faadfe9f73217..face6066f1c8f 100644 --- a/cmd/installer/subcommands/installer/command.go +++ b/cmd/installer/subcommands/installer/command.go @@ -77,7 +77,7 @@ type installerCmd struct { installer.Installer } -func newInstallerCmd(operation string) *installerCmd { +func newInstallerCmd(operation string) (*installerCmd, error) { cmd := newCmd(operation) var opts []installer.Option if cmd.registry != "" { @@ -86,11 +86,14 @@ func newInstallerCmd(operation string) *installerCmd { if cmd.registryAuth != "" { opts = append(opts, installer.WithRegistryAuth(cmd.registryAuth)) } - i := installer.NewInstaller(opts...) + i, err := installer.NewInstaller(opts...) + if err != nil { + return nil, err + } return &installerCmd{ Installer: i, cmd: cmd, - } + }, nil } type bootstraperCmd struct { @@ -176,8 +179,11 @@ func installCommand() *cobra.Command { GroupID: "installer", Args: cobra.ExactArgs(1), RunE: func(_ *cobra.Command, args []string) (err error) { - i := newInstallerCmd("install") + i, err := newInstallerCmd("install") defer func() { i.Stop(err) }() + if err != nil { + return err + } i.span.SetTag("params.url", args[0]) return i.Install(i.ctx, args[0]) }, @@ -192,8 +198,11 @@ func removeCommand() *cobra.Command { GroupID: "installer", Args: cobra.ExactArgs(1), RunE: func(_ *cobra.Command, args []string) (err error) { - i := newInstallerCmd("remove") + i, err := newInstallerCmd("remove") defer func() { i.Stop(err) }() + if err != nil { + return err + } i.span.SetTag("params.package", args[0]) return i.Remove(i.ctx, args[0]) }, @@ -208,8 +217,11 @@ func purgeCommand() *cobra.Command { GroupID: "installer", Args: cobra.NoArgs, RunE: func(_ *cobra.Command, _ []string) (err error) { - i := newInstallerCmd("purge") + i, err := newInstallerCmd("purge") defer func() { i.Stop(err) }() + if err != nil { + return err + } i.Purge(i.ctx) return nil }, @@ -224,8 +236,11 @@ func installExperimentCommand() *cobra.Command { GroupID: "installer", Args: cobra.ExactArgs(1), RunE: func(_ *cobra.Command, args []string) (err error) { - i := newInstallerCmd("install_experiment") + i, err := newInstallerCmd("install_experiment") defer func() { i.Stop(err) }() + if err != nil { + return err + } i.span.SetTag("params.url", args[0]) return i.InstallExperiment(i.ctx, args[0]) }, @@ -240,8 +255,11 @@ func removeExperimentCommand() *cobra.Command { GroupID: "installer", Args: cobra.ExactArgs(1), RunE: func(_ *cobra.Command, args []string) (err error) { - i := newInstallerCmd("remove_experiment") + i, err := newInstallerCmd("remove_experiment") defer func() { i.Stop(err) }() + if err != nil { + return err + } i.span.SetTag("params.package", args[0]) return i.RemoveExperiment(i.ctx, args[0]) }, @@ -256,8 +274,11 @@ func promoteExperimentCommand() *cobra.Command { GroupID: "installer", Args: cobra.ExactArgs(1), RunE: func(_ *cobra.Command, args []string) (err error) { - i := newInstallerCmd("promote_experiment") + i, err := newInstallerCmd("promote_experiment") defer func() { i.Stop(err) }() + if err != nil { + return err + } i.span.SetTag("params.package", args[0]) return i.PromoteExperiment(i.ctx, args[0]) }, @@ -272,8 +293,11 @@ func garbageCollectCommand() *cobra.Command { GroupID: "installer", Args: cobra.NoArgs, RunE: func(_ *cobra.Command, _ []string) (err error) { - i := newInstallerCmd("garbage_collect") + i, err := newInstallerCmd("garbage_collect") defer func() { i.Stop(err) }() + if err != nil { + return err + } return i.GarbageCollect(i.ctx) }, } diff --git a/pkg/fleet/installer/installer.go b/pkg/fleet/installer/installer.go index a5fdb62a848e1..ec73a9997d4c5 100644 --- a/pkg/fleet/installer/installer.go +++ b/pkg/fleet/installer/installer.go @@ -16,6 +16,7 @@ import ( "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" "github.com/DataDog/datadog-agent/pkg/fleet/installer/service" + "github.com/DataDog/datadog-agent/pkg/fleet/internal/db" "github.com/DataDog/datadog-agent/pkg/fleet/internal/oci" "github.com/DataDog/datadog-agent/pkg/util/filesystem" "github.com/DataDog/datadog-agent/pkg/util/log" @@ -64,6 +65,7 @@ type Installer interface { type installerImpl struct { m sync.Mutex + db *db.PackagesDB downloader *oci.Downloader repositories *repository.Repositories configsDir string @@ -101,18 +103,27 @@ func WithRegistry(registry string) Option { } // NewInstaller returns a new Package Manager. -func NewInstaller(opts ...Option) Installer { +func NewInstaller(opts ...Option) (Installer, error) { o := newOptions() for _, opt := range opts { opt(o) } + err := ensurePackageDirExists() + if err != nil { + return nil, fmt.Errorf("could not ensure packages directory exists: %w", err) + } + db, err := db.New(filepath.Join(PackagesPath, "packages.db")) + if err != nil { + return nil, fmt.Errorf("could not create packages db: %w", err) + } return &installerImpl{ + db: db, downloader: oci.NewDownloader(http.DefaultClient, o.registry, o.registryAuth), repositories: repository.NewRepositories(PackagesPath, LocksPack), configsDir: defaultConfigsDir, tmpDirPath: TmpDirPath, packagesDir: PackagesPath, - } + }, nil } // State returns the state of a package. @@ -129,11 +140,7 @@ func (i *installerImpl) States() (map[string]repository.State, error) { func (i *installerImpl) Install(ctx context.Context, url string) error { i.m.Lock() defer i.m.Unlock() - err := i.preSetupPackage(ctx, packageDatadogInstaller) - if err != nil { - return fmt.Errorf("could not pre-setup package: %w", err) - } - err = checkAvailableDiskSpace(mininumDiskSpace, i.packagesDir) + err := checkAvailableDiskSpace(mininumDiskSpace, i.packagesDir) if err != nil { return fmt.Errorf("not enough disk space: %w", err) } @@ -159,7 +166,15 @@ func (i *installerImpl) Install(ctx context.Context, url string) error { if err != nil { return fmt.Errorf("could not create repository: %w", err) } - return i.setupPackage(ctx, pkg.Name) + err = i.setupPackage(ctx, pkg.Name) + if err != nil { + return fmt.Errorf("could not setup package: %w", err) + } + err = i.db.CreatePackage(pkg.Name) + if err != nil { + return fmt.Errorf("could not store package installation in db: %w", err) + } + return nil } // InstallExperiment installs an experiment on top of an existing package. @@ -245,7 +260,15 @@ func (i *installerImpl) Remove(ctx context.Context, pkg string) error { i.m.Lock() defer i.m.Unlock() i.removePackage(ctx, pkg) - return i.repositories.Delete(ctx, pkg) + err := i.repositories.Delete(ctx, pkg) + if err != nil { + return fmt.Errorf("could not delete repository: %w", err) + } + err = i.db.DeletePackage(pkg) + if err != nil { + return fmt.Errorf("could not remove package installation in db: %w", err) + } + return nil } // GarbageCollect removes unused packages. @@ -278,15 +301,6 @@ func (i *installerImpl) stopExperiment(ctx context.Context, pkg string) error { } } -func (i *installerImpl) preSetupPackage(_ context.Context, pkg string) error { - switch pkg { - case packageDatadogInstaller: - return service.PreSetupInstaller() - default: - return nil - } -} - func (i *installerImpl) setupPackage(ctx context.Context, pkg string) error { switch pkg { case packageDatadogInstaller: @@ -337,3 +351,11 @@ func checkAvailableDiskSpace(requiredDiskSpace uint64, path string) error { } return nil } + +func ensurePackageDirExists() error { + err := os.MkdirAll(PackagesPath, 0755) + if err != nil { + return fmt.Errorf("error creating packages directory: %w", err) + } + return nil +} diff --git a/pkg/fleet/installer/installer_test.go b/pkg/fleet/installer/installer_test.go index a2eb57f477248..4eb75d7129e3d 100644 --- a/pkg/fleet/installer/installer_test.go +++ b/pkg/fleet/installer/installer_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" + "github.com/DataDog/datadog-agent/pkg/fleet/internal/db" "github.com/DataDog/datadog-agent/pkg/fleet/internal/fixtures" "github.com/DataDog/datadog-agent/pkg/fleet/internal/oci" ) @@ -31,8 +32,11 @@ type testPackageManager struct { func newTestPackageManager(t *testing.T, s *fixtures.Server, rootPath string, locksPath string) *testPackageManager { repositories := repository.NewRepositories(rootPath, locksPath) + db, err := db.New(filepath.Join(rootPath, "packages.db")) + assert.NoError(t, err) return &testPackageManager{ installerImpl{ + db: db, downloader: oci.NewDownloader(s.Client(), "", oci.RegistryAuthDefault), repositories: repositories, configsDir: t.TempDir(), diff --git a/pkg/fleet/installer/service/datadog_installer.go b/pkg/fleet/installer/service/datadog_installer.go index 6f9773d70158b..3b16bd0509bc0 100644 --- a/pkg/fleet/installer/service/datadog_installer.go +++ b/pkg/fleet/installer/service/datadog_installer.go @@ -26,16 +26,6 @@ const ( var installerUnits = []string{installerUnit, installerUnitExp} -// PreSetupInstaller creates the necessary directories for the installer to be installed. -// FIXME: This is a preinst and I feel bad about it -func PreSetupInstaller() error { - err := os.MkdirAll("/opt/datadog-packages", 0755) - if err != nil { - return fmt.Errorf("error creating /opt/datadog-packages: %w", err) - } - return nil -} - func addDDAgentUser(ctx context.Context) error { if _, err := user.Lookup("dd-agent"); err == nil { return nil diff --git a/pkg/fleet/internal/db/db.go b/pkg/fleet/internal/db/db.go new file mode 100644 index 0000000000000..cc33d9e8b0dbe --- /dev/null +++ b/pkg/fleet/internal/db/db.go @@ -0,0 +1,126 @@ +// 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 db provides a database to store information about packages +package db + +import ( + "fmt" + "time" + + "go.etcd.io/bbolt" +) + +var ( + bucketPackages = []byte("packages") +) + +// PackagesDB is a database that stores information about packages +type PackagesDB struct { + db *bbolt.DB +} + +type options struct { + readOnly bool + timeout time.Duration +} + +// Option is a function that sets an option on a PackagesDB +type Option func(*options) + +// ReadOnly sets the database to read-only mode +func ReadOnly() Option { + return func(o *options) { + o.readOnly = true + } +} + +// WithTimeout sets the timeout for opening the database +func WithTimeout(timeout time.Duration) Option { + return func(o *options) { + o.timeout = timeout + } +} + +// New creates a new PackagesDB +func New(dbPath string, opts ...Option) (*PackagesDB, error) { + o := options{} + for _, opt := range opts { + opt(&o) + } + db, err := bbolt.Open(dbPath, 0644, &bbolt.Options{ + ReadOnly: o.readOnly, + Timeout: o.timeout, + NoGrowSync: false, + FreelistType: bbolt.FreelistArrayType, + }) + if err != nil { + return nil, fmt.Errorf("could not open database: %w", err) + } + err = db.Update(func(tx *bbolt.Tx) error { + _, err := tx.CreateBucketIfNotExists(bucketPackages) + return err + }) + if err != nil { + return nil, fmt.Errorf("could not create packages bucket: %w", err) + } + return &PackagesDB{ + db: db, + }, nil +} + +// Close closes the database +func (p *PackagesDB) Close() error { + return p.db.Close() +} + +// CreatePackage sets a package +func (p *PackagesDB) CreatePackage(name string) error { + err := p.db.Update(func(tx *bbolt.Tx) error { + b := tx.Bucket(bucketPackages) + if b == nil { + return fmt.Errorf("bucket not found") + } + return b.Put([]byte(name), []byte{}) + }) + if err != nil { + return fmt.Errorf("could not set package: %w", err) + } + return nil +} + +// DeletePackage deletes a package by name +func (p *PackagesDB) DeletePackage(name string) error { + err := p.db.Update(func(tx *bbolt.Tx) error { + b := tx.Bucket(bucketPackages) + if b == nil { + return fmt.Errorf("bucket not found") + } + return b.Delete([]byte(name)) + }) + if err != nil { + return fmt.Errorf("could not delete package: %w", err) + } + return nil +} + +// ListPackages returns a list of all packages +func (p *PackagesDB) ListPackages() ([]string, error) { + var pkgs []string + err := p.db.View(func(tx *bbolt.Tx) error { + b := tx.Bucket(bucketPackages) + if b == nil { + return fmt.Errorf("bucket not found") + } + return b.ForEach(func(k, v []byte) error { + pkgs = append(pkgs, string(k)) + return nil + }) + }) + if err != nil { + return nil, fmt.Errorf("could not list packages: %w", err) + } + return pkgs, nil +} diff --git a/pkg/fleet/internal/db/db_test.go b/pkg/fleet/internal/db/db_test.go new file mode 100644 index 0000000000000..23f5031e6f196 --- /dev/null +++ b/pkg/fleet/internal/db/db_test.go @@ -0,0 +1,77 @@ +// 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 db + +import ( + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "go.etcd.io/bbolt" +) + +func newTestDB(t *testing.T) *PackagesDB { + db, err := New(filepath.Join(t.TempDir(), "test.db")) + assert.NoError(t, err) + return db +} + +func TestCreatePackage(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + err := db.CreatePackage("test") + assert.NoError(t, err) + packages, err := db.ListPackages() + assert.NoError(t, err) + assert.Len(t, packages, 1) + assert.Equal(t, "test", packages[0]) +} + +func TestDeletePackage(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + err := db.CreatePackage("test") + assert.NoError(t, err) + err = db.DeletePackage("test") + assert.NoError(t, err) + packages, err := db.ListPackages() + assert.NoError(t, err) + assert.Len(t, packages, 0) +} + +func TestListPackages(t *testing.T) { + db := newTestDB(t) + defer db.Close() + + packages, err := db.ListPackages() + assert.NoError(t, err) + assert.Len(t, packages, 0) + + err = db.CreatePackage("test1") + assert.NoError(t, err) + err = db.CreatePackage("test2") + assert.NoError(t, err) + packages, err = db.ListPackages() + assert.NoError(t, err) + assert.Len(t, packages, 2) + assert.Contains(t, packages, "test1") + assert.Contains(t, packages, "test2") +} + +func TestTimeout(t *testing.T) { + dbFile := filepath.Join(t.TempDir(), "test.db") + dbLock, err := New(dbFile) + assert.NoError(t, err) + defer dbLock.Close() + + before := time.Now() + _, err = New(dbFile, WithTimeout(time.Second)) + assert.ErrorIs(t, err, bbolt.ErrTimeout) + assert.GreaterOrEqual(t, time.Since(before), time.Second-100*time.Millisecond) // bbolt timeout can be shorter by up to 50ms +} diff --git a/pkg/fleet/internal/exec/installer_exec.go b/pkg/fleet/internal/exec/installer_exec.go index 5998f7af76c7e..6e470d9b5eb7d 100644 --- a/pkg/fleet/internal/exec/installer_exec.go +++ b/pkg/fleet/internal/exec/installer_exec.go @@ -27,9 +27,6 @@ type InstallerExec struct { // telemetry options apiKey string site string - - // FIXME: decide where we want to host the status logic - pm installer.Installer } // NewInstallerExec returns a new InstallerExec. @@ -40,7 +37,6 @@ func NewInstallerExec(installerBinPath string, registry string, registryAuth str registryAuth: registryAuth, apiKey: apiKey, site: site, - pm: installer.NewInstaller(), } } @@ -127,10 +123,12 @@ func (i *InstallerExec) GarbageCollect(ctx context.Context) (err error) { // State returns the state of a package. func (i *InstallerExec) State(pkg string) (repository.State, error) { - return i.pm.State(pkg) + repositories := repository.NewRepositories(installer.PackagesPath, installer.LocksPack) + return repositories.Get(pkg).GetState() } // States returns the states of all packages. func (i *InstallerExec) States() (map[string]repository.State, error) { - return i.pm.States() + repositories := repository.NewRepositories(installer.PackagesPath, installer.LocksPack) + return repositories.GetState() } From 4649e64a0b75004c60d62d23dc89b5b2ee87a259 Mon Sep 17 00:00:00 2001 From: Arthur Bellal Date: Fri, 10 May 2024 15:18:02 +0200 Subject: [PATCH 2/8] add is-installed --- .../subcommands/installer/command.go | 29 +++++++++++++++++++ pkg/fleet/installer/installer.go | 11 +++++++ 2 files changed, 40 insertions(+) diff --git a/cmd/installer/subcommands/installer/command.go b/cmd/installer/subcommands/installer/command.go index face6066f1c8f..a251965ec98a5 100644 --- a/cmd/installer/subcommands/installer/command.go +++ b/cmd/installer/subcommands/installer/command.go @@ -303,3 +303,32 @@ func garbageCollectCommand() *cobra.Command { } return cmd } + +const ( + returnCodeIsInstalledFalse = 10 +) + +func isInstalled() *cobra.Command { + cmd := &cobra.Command{ + Use: "is-installed ", + Short: "Check if a package is installed", + GroupID: "installer", + Args: cobra.ExactArgs(1), + RunE: func(_ *cobra.Command, args []string) (err error) { + i, err := newInstallerCmd("is_installed") + defer func() { i.Stop(err) }() + if err != nil { + return err + } + installed, err := i.IsInstalled(args[0]) + if err != nil { + return err + } + if !installed { + os.Exit(returnCodeIsInstalledFalse) + } + return nil + }, + } + return cmd +} diff --git a/pkg/fleet/installer/installer.go b/pkg/fleet/installer/installer.go index ec73a9997d4c5..00586ecb9ea85 100644 --- a/pkg/fleet/installer/installer.go +++ b/pkg/fleet/installer/installer.go @@ -12,6 +12,7 @@ import ( "net/http" "os" "path/filepath" + "slices" "sync" "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" @@ -47,6 +48,7 @@ var ( // Installer is a package manager that installs and uninstalls packages. type Installer interface { + IsInstalled(pkg string) (bool, error) State(pkg string) (repository.State, error) States() (map[string]repository.State, error) @@ -136,6 +138,15 @@ func (i *installerImpl) States() (map[string]repository.State, error) { return i.repositories.GetState() } +// IsInstalled checks if a package is installed. +func (i *installerImpl) IsInstalled(pkg string) (bool, error) { + packages, err := i.db.ListPackages() + if err != nil { + return false, fmt.Errorf("could not list packages: %w", err) + } + return slices.Contains(packages, pkg), nil +} + // Install installs or updates a package. func (i *installerImpl) Install(ctx context.Context, url string) error { i.m.Lock() From 70cdcde3600024975af125f0aa27404536950aa0 Mon Sep 17 00:00:00 2001 From: Arthur Bellal Date: Fri, 10 May 2024 15:25:33 +0200 Subject: [PATCH 3/8] feedback --- cmd/installer/subcommands/installer/command.go | 2 ++ pkg/fleet/installer/installer.go | 3 ++- pkg/fleet/internal/db/db.go | 12 +----------- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/cmd/installer/subcommands/installer/command.go b/cmd/installer/subcommands/installer/command.go index a251965ec98a5..38807c74ca4ed 100644 --- a/cmd/installer/subcommands/installer/command.go +++ b/cmd/installer/subcommands/installer/command.go @@ -325,6 +325,8 @@ func isInstalled() *cobra.Command { return err } if !installed { + // Return a specific code to differentiate from other errors + // `return err` will lead to a return code of -1 os.Exit(returnCodeIsInstalledFalse) } return nil diff --git a/pkg/fleet/installer/installer.go b/pkg/fleet/installer/installer.go index 00586ecb9ea85..139c6f0b64739 100644 --- a/pkg/fleet/installer/installer.go +++ b/pkg/fleet/installer/installer.go @@ -14,6 +14,7 @@ import ( "path/filepath" "slices" "sync" + "time" "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" "github.com/DataDog/datadog-agent/pkg/fleet/installer/service" @@ -114,7 +115,7 @@ func NewInstaller(opts ...Option) (Installer, error) { if err != nil { return nil, fmt.Errorf("could not ensure packages directory exists: %w", err) } - db, err := db.New(filepath.Join(PackagesPath, "packages.db")) + db, err := db.New(filepath.Join(PackagesPath, "packages.db"), db.WithTimeout(10*time.Second)) if err != nil { return nil, fmt.Errorf("could not create packages db: %w", err) } diff --git a/pkg/fleet/internal/db/db.go b/pkg/fleet/internal/db/db.go index cc33d9e8b0dbe..b00cd11acbf22 100644 --- a/pkg/fleet/internal/db/db.go +++ b/pkg/fleet/internal/db/db.go @@ -23,20 +23,12 @@ type PackagesDB struct { } type options struct { - readOnly bool - timeout time.Duration + timeout time.Duration } // Option is a function that sets an option on a PackagesDB type Option func(*options) -// ReadOnly sets the database to read-only mode -func ReadOnly() Option { - return func(o *options) { - o.readOnly = true - } -} - // WithTimeout sets the timeout for opening the database func WithTimeout(timeout time.Duration) Option { return func(o *options) { @@ -51,9 +43,7 @@ func New(dbPath string, opts ...Option) (*PackagesDB, error) { opt(&o) } db, err := bbolt.Open(dbPath, 0644, &bbolt.Options{ - ReadOnly: o.readOnly, Timeout: o.timeout, - NoGrowSync: false, FreelistType: bbolt.FreelistArrayType, }) if err != nil { From 9cf4a0c6a71d77293ab0ffd6b02e733f2ef95209 Mon Sep 17 00:00:00 2001 From: Arthur Bellal Date: Fri, 10 May 2024 15:32:55 +0200 Subject: [PATCH 4/8] make use of list packages in purge --- pkg/fleet/installer/installer.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/fleet/installer/installer.go b/pkg/fleet/installer/installer.go index 139c6f0b64739..4b79be306bd36 100644 --- a/pkg/fleet/installer/installer.go +++ b/pkg/fleet/installer/installer.go @@ -255,12 +255,23 @@ func (i *installerImpl) Purge(ctx context.Context) { i.m.Lock() defer i.m.Unlock() - // todo check if agent/injector are installed + packages, err := i.db.ListPackages() + if err != nil { + // if we can't list packages we'll only remove the installer + packages = nil + log.Warnf("could not list packages: %v", err) + } + for _, pkg := range packages { + if pkg == packageDatadogInstaller { + continue + } + i.removePackage(ctx, pkg) + } i.removePackage(ctx, packageDatadogInstaller) // remove all from disk span, _ := tracer.StartSpanFromContext(ctx, "remove_all") - err := os.RemoveAll(PackagesPath) + err = os.RemoveAll(PackagesPath) defer span.Finish(tracer.WithError(err)) if err != nil { log.Warnf("could not remove path: %v", err) From 391431d109f9c2d7838f233a70f6de9265aa80ef Mon Sep 17 00:00:00 2001 From: Arthur Bellal Date: Fri, 10 May 2024 16:42:30 +0200 Subject: [PATCH 5/8] add missing command --- cmd/installer/subcommands/installer/command.go | 5 +++-- pkg/fleet/internal/exec/installer_exec.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/cmd/installer/subcommands/installer/command.go b/cmd/installer/subcommands/installer/command.go index 38807c74ca4ed..d5e6528956cbd 100644 --- a/cmd/installer/subcommands/installer/command.go +++ b/cmd/installer/subcommands/installer/command.go @@ -305,7 +305,8 @@ func garbageCollectCommand() *cobra.Command { } const ( - returnCodeIsInstalledFalse = 10 + // ReturnCodeIsInstalledFalse is the return code when a package is not installed + ReturnCodeIsInstalledFalse = 10 ) func isInstalled() *cobra.Command { @@ -327,7 +328,7 @@ func isInstalled() *cobra.Command { if !installed { // Return a specific code to differentiate from other errors // `return err` will lead to a return code of -1 - os.Exit(returnCodeIsInstalledFalse) + os.Exit(ReturnCodeIsInstalledFalse) } return nil }, diff --git a/pkg/fleet/internal/exec/installer_exec.go b/pkg/fleet/internal/exec/installer_exec.go index 6e470d9b5eb7d..5372fe6d4a57d 100644 --- a/pkg/fleet/internal/exec/installer_exec.go +++ b/pkg/fleet/internal/exec/installer_exec.go @@ -12,6 +12,7 @@ import ( "os" "os/exec" + "github.com/DataDog/datadog-agent/cmd/installer/command" "github.com/DataDog/datadog-agent/pkg/fleet/installer" "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" "github.com/DataDog/datadog-agent/pkg/fleet/telemetry" @@ -121,6 +122,19 @@ func (i *InstallerExec) GarbageCollect(ctx context.Context) (err error) { return cmd.Run() } +func (i *InstallerExec) IsInstalled(ctx context.Context, pkg string) (bool, error) { + cmd := i.newInstallerCmd(ctx, "is-installed", pkg) + defer func() { cmd.span.Finish(tracer.WithError(err)) }() + err := cmd.Run() + if err != nil && cmd.ProcessState.ExitCode() == command.ReturnCodeIsInstalledFalse { + return false, nil + } + if err != nil { + return false, err + } + return true, nil +} + // State returns the state of a package. func (i *InstallerExec) State(pkg string) (repository.State, error) { repositories := repository.NewRepositories(installer.PackagesPath, installer.LocksPack) From 5cd676ae6eead747820a48f505d1c54f15859865 Mon Sep 17 00:00:00 2001 From: Arthur Bellal Date: Fri, 10 May 2024 17:04:39 +0200 Subject: [PATCH 6/8] fixes --- pkg/fleet/daemon/daemon_test.go | 5 +++++ pkg/fleet/installer/installer.go | 4 ++-- pkg/fleet/internal/exec/installer_exec.go | 7 +++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/fleet/daemon/daemon_test.go b/pkg/fleet/daemon/daemon_test.go index 684a8d3aac37d..3b45b185dbec3 100644 --- a/pkg/fleet/daemon/daemon_test.go +++ b/pkg/fleet/daemon/daemon_test.go @@ -27,6 +27,11 @@ type testPackageManager struct { mock.Mock } +func (m *testPackageManager) IsInstalled(ctx context.Context, pkg string) (bool, error) { + args := m.Called(pkg) + return args.Bool(0), args.Error(1) +} + func (m *testPackageManager) State(pkg string) (repository.State, error) { args := m.Called(pkg) return args.Get(0).(repository.State), args.Error(1) diff --git a/pkg/fleet/installer/installer.go b/pkg/fleet/installer/installer.go index 4b79be306bd36..0c5908be0b49c 100644 --- a/pkg/fleet/installer/installer.go +++ b/pkg/fleet/installer/installer.go @@ -49,7 +49,7 @@ var ( // Installer is a package manager that installs and uninstalls packages. type Installer interface { - IsInstalled(pkg string) (bool, error) + IsInstalled(ctx context.Context, pkg string) (bool, error) State(pkg string) (repository.State, error) States() (map[string]repository.State, error) @@ -140,7 +140,7 @@ func (i *installerImpl) States() (map[string]repository.State, error) { } // IsInstalled checks if a package is installed. -func (i *installerImpl) IsInstalled(pkg string) (bool, error) { +func (i *installerImpl) IsInstalled(_ context.Context, pkg string) (bool, error) { packages, err := i.db.ListPackages() if err != nil { return false, fmt.Errorf("could not list packages: %w", err) diff --git a/pkg/fleet/internal/exec/installer_exec.go b/pkg/fleet/internal/exec/installer_exec.go index 5372fe6d4a57d..7e3e0030a454b 100644 --- a/pkg/fleet/internal/exec/installer_exec.go +++ b/pkg/fleet/internal/exec/installer_exec.go @@ -12,7 +12,6 @@ import ( "os" "os/exec" - "github.com/DataDog/datadog-agent/cmd/installer/command" "github.com/DataDog/datadog-agent/pkg/fleet/installer" "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" "github.com/DataDog/datadog-agent/pkg/fleet/telemetry" @@ -122,11 +121,11 @@ func (i *InstallerExec) GarbageCollect(ctx context.Context) (err error) { return cmd.Run() } -func (i *InstallerExec) IsInstalled(ctx context.Context, pkg string) (bool, error) { +func (i *InstallerExec) IsInstalled(ctx context.Context, pkg string) (_ bool, err error) { cmd := i.newInstallerCmd(ctx, "is-installed", pkg) defer func() { cmd.span.Finish(tracer.WithError(err)) }() - err := cmd.Run() - if err != nil && cmd.ProcessState.ExitCode() == command.ReturnCodeIsInstalledFalse { + err = cmd.Run() + if err != nil && cmd.ProcessState.ExitCode() == 10 { return false, nil } if err != nil { From 8677b64c16cc3b8b62e5b6566c58e88da5caec16 Mon Sep 17 00:00:00 2001 From: Arthur Bellal Date: Sat, 11 May 2024 12:54:04 +0200 Subject: [PATCH 7/8] fix lint --- cmd/installer/subcommands/installer/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/installer/subcommands/installer/command.go b/cmd/installer/subcommands/installer/command.go index d5e6528956cbd..f01ead160e317 100644 --- a/cmd/installer/subcommands/installer/command.go +++ b/cmd/installer/subcommands/installer/command.go @@ -321,7 +321,7 @@ func isInstalled() *cobra.Command { if err != nil { return err } - installed, err := i.IsInstalled(args[0]) + installed, err := i.IsInstalled(i.ctx, args[0]) if err != nil { return err } From 3e3e52e8bc4b09f1261a3108fade3055d9e87c0d Mon Sep 17 00:00:00 2001 From: Arthur Bellal Date: Sat, 11 May 2024 13:14:31 +0200 Subject: [PATCH 8/8] more lint fixes --- cmd/installer/subcommands/installer/command.go | 4 ++-- pkg/fleet/daemon/daemon_test.go | 2 +- pkg/fleet/internal/exec/installer_exec.go | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/installer/subcommands/installer/command.go b/cmd/installer/subcommands/installer/command.go index 2a8b4d7efed69..3cb901b3ecb19 100644 --- a/cmd/installer/subcommands/installer/command.go +++ b/cmd/installer/subcommands/installer/command.go @@ -31,7 +31,7 @@ const ( // Commands returns the installer subcommands. func Commands(_ *command.GlobalParams) []*cobra.Command { - return []*cobra.Command{bootstrapCommand(), installCommand(), removeCommand(), installExperimentCommand(), removeExperimentCommand(), promoteExperimentCommand(), garbageCollectCommand(), purgeCommand()} + return []*cobra.Command{bootstrapCommand(), installCommand(), removeCommand(), installExperimentCommand(), removeExperimentCommand(), promoteExperimentCommand(), garbageCollectCommand(), purgeCommand(), isInstalledCommand()} } type cmd struct { @@ -312,7 +312,7 @@ const ( ReturnCodeIsInstalledFalse = 10 ) -func isInstalled() *cobra.Command { +func isInstalledCommand() *cobra.Command { cmd := &cobra.Command{ Use: "is-installed ", Short: "Check if a package is installed", diff --git a/pkg/fleet/daemon/daemon_test.go b/pkg/fleet/daemon/daemon_test.go index 3b45b185dbec3..9183cb4381f99 100644 --- a/pkg/fleet/daemon/daemon_test.go +++ b/pkg/fleet/daemon/daemon_test.go @@ -28,7 +28,7 @@ type testPackageManager struct { } func (m *testPackageManager) IsInstalled(ctx context.Context, pkg string) (bool, error) { - args := m.Called(pkg) + args := m.Called(ctx, pkg) return args.Bool(0), args.Error(1) } diff --git a/pkg/fleet/internal/exec/installer_exec.go b/pkg/fleet/internal/exec/installer_exec.go index 7e3e0030a454b..f12595b6e3547 100644 --- a/pkg/fleet/internal/exec/installer_exec.go +++ b/pkg/fleet/internal/exec/installer_exec.go @@ -121,6 +121,7 @@ func (i *InstallerExec) GarbageCollect(ctx context.Context) (err error) { return cmd.Run() } +// IsInstalled checks if a package is installed. func (i *InstallerExec) IsInstalled(ctx context.Context, pkg string) (_ bool, err error) { cmd := i.newInstallerCmd(ctx, "is-installed", pkg) defer func() { cmd.span.Finish(tracer.WithError(err)) }()