From c6d0c1e0d7d4c9f7e82126a4961333c5fb81a138 Mon Sep 17 00:00:00 2001 From: Baptiste Foy Date: Sun, 1 Dec 2024 11:51:54 +0100 Subject: [PATCH] upgrade(installer): Return specific error codes (#31619) Co-authored-by: arbll --- cmd/installer/main.go | 24 ++++++- pkg/fleet/daemon/daemon.go | 2 +- pkg/fleet/installer/errors/errors.go | 71 +++++++++++++++------ pkg/fleet/installer/errors/errors_test.go | 27 ++++++-- pkg/fleet/installer/installer.go | 76 ++++++++++++++++++----- pkg/fleet/internal/exec/installer_exec.go | 6 +- pkg/fleet/internal/oci/download.go | 7 ++- 7 files changed, 170 insertions(+), 43 deletions(-) diff --git a/cmd/installer/main.go b/cmd/installer/main.go index 15fcd2a8eb91c..1cd198631d420 100644 --- a/cmd/installer/main.go +++ b/cmd/installer/main.go @@ -9,13 +9,33 @@ package main import ( + "fmt" "os" "github.com/DataDog/datadog-agent/cmd/installer/command" "github.com/DataDog/datadog-agent/cmd/installer/subcommands" - "github.com/DataDog/datadog-agent/cmd/internal/runcmd" + "github.com/spf13/cobra" + "go.uber.org/dig" + + installerErrors "github.com/DataDog/datadog-agent/pkg/fleet/installer/errors" ) func main() { - os.Exit(runcmd.Run(command.MakeCommand(subcommands.InstallerSubcommands()))) + os.Exit(runCmd(command.MakeCommand(subcommands.InstallerSubcommands()))) +} + +func runCmd(cmd *cobra.Command) int { + // always silence errors, since they are handled here + cmd.SilenceErrors = true + + err := cmd.Execute() + if err != nil { + if rootCauseErr := dig.RootCause(err); rootCauseErr != err { + fmt.Fprintln(cmd.ErrOrStderr(), installerErrors.FromErr(rootCauseErr).ToJSON()) + } else { + fmt.Fprintln(cmd.ErrOrStderr(), installerErrors.FromErr(err).ToJSON()) + } + return -1 + } + return 0 } diff --git a/pkg/fleet/daemon/daemon.go b/pkg/fleet/daemon/daemon.go index d4d99ff7fd68e..c0e38e5cc545c 100644 --- a/pkg/fleet/daemon/daemon.go +++ b/pkg/fleet/daemon/daemon.go @@ -552,7 +552,7 @@ func setRequestDone(ctx context.Context, err error) { state.State = pbgo.TaskState_DONE if err != nil { state.State = pbgo.TaskState_ERROR - state.Err = installerErrors.From(err) + state.Err = installerErrors.FromErr(err) } } diff --git a/pkg/fleet/installer/errors/errors.go b/pkg/fleet/installer/errors/errors.go index 589d8179a2fa8..736c609af033c 100644 --- a/pkg/fleet/installer/errors/errors.go +++ b/pkg/fleet/installer/errors/errors.go @@ -7,6 +7,7 @@ package errors import ( + "encoding/json" "errors" ) @@ -14,19 +15,15 @@ import ( type InstallerErrorCode uint64 const ( - errUnknown InstallerErrorCode = iota // This error code is purposefully not exported - // ErrInstallFailed is the code for an install failure. - ErrInstallFailed + errUnknown InstallerErrorCode = 0 // This error code is purposefully not exported // ErrDownloadFailed is the code for a download failure. - ErrDownloadFailed - // ErrInvalidHash is the code for an invalid hash. - ErrInvalidHash - // ErrInvalidState is the code for an invalid state. - ErrInvalidState + ErrDownloadFailed InstallerErrorCode = 1 + // ErrNotEnoughDiskSpace is the code for not enough disk space. + ErrNotEnoughDiskSpace InstallerErrorCode = 2 // ErrPackageNotFound is the code for a package not found. - ErrPackageNotFound - // ErrUpdateExperimentFailed is the code for an update experiment failure. - ErrUpdateExperimentFailed + ErrPackageNotFound InstallerErrorCode = 3 + // ErrFilesystemIssue is the code for a filesystem issue (e.g. permission issue). + ErrFilesystemIssue InstallerErrorCode = 4 ) // InstallerError is an error type used by the installer. @@ -35,6 +32,11 @@ type InstallerError struct { code InstallerErrorCode } +type installerErrorJSON struct { + Error string `json:"error"` + Code int `json:"code"` +} + // Error returns the error message. func (e InstallerError) Error() string { return e.err.Error() @@ -60,7 +62,7 @@ func (e InstallerError) Code() InstallerErrorCode { // If the given error is already an installer error, it is not wrapped and // left as it is. Only the deepest InstallerError remains. func Wrap(errCode InstallerErrorCode, err error) error { - if errors.Is(err, &InstallerError{}) { + if FromErr(err).code != errUnknown { return err } return &InstallerError{ @@ -69,18 +71,51 @@ func Wrap(errCode InstallerErrorCode, err error) error { } } -// From returns a new InstallerError from the given error. -func From(err error) *InstallerError { +// FromErr returns a new InstallerError from the given error. +// Unwraps the error until it finds an InstallerError and return unknown error code if not found. +func FromErr(err error) *InstallerError { if err == nil { return nil } - e, ok := err.(*InstallerError) if !ok { - return &InstallerError{ - err: err, - code: errUnknown, + unwrappedErr := errors.Unwrap(err) + if unwrappedErr == nil { + return &InstallerError{ + err: err, + code: errUnknown, + } } + return FromErr(unwrappedErr) } return e } + +// ToJSON returns the error as a JSON string. +func (e InstallerError) ToJSON() string { + tmp := installerErrorJSON{ + Error: e.err.Error(), + Code: int(e.code), + } + jsonErr, err := json.Marshal(tmp) + if err != nil { + return e.err.Error() + } + return string(jsonErr) +} + +// FromJSON returns an InstallerError from a JSON string. +func FromJSON(errStr string) InstallerError { + var jsonError installerErrorJSON + err := json.Unmarshal([]byte(errStr), &jsonError) + if err != nil { + return InstallerError{ + err: errors.New(errStr), + code: errUnknown, + } + } + return InstallerError{ + err: errors.New(jsonError.Error), + code: InstallerErrorCode(jsonError.Code), + } +} diff --git a/pkg/fleet/installer/errors/errors_test.go b/pkg/fleet/installer/errors/errors_test.go index 44a5cdc584b95..5115faaaf8a26 100644 --- a/pkg/fleet/installer/errors/errors_test.go +++ b/pkg/fleet/installer/errors/errors_test.go @@ -12,18 +12,34 @@ import ( "github.com/stretchr/testify/assert" ) -func TestFrom(t *testing.T) { +func TestFromErr(t *testing.T) { var err error = &InstallerError{ err: fmt.Errorf("test: test"), code: ErrDownloadFailed, } - taskErr := From(err) + taskErr := FromErr(err) assert.Equal(t, taskErr, &InstallerError{ err: fmt.Errorf("test: test"), code: ErrDownloadFailed, }) - assert.Nil(t, From(nil)) + assert.Nil(t, FromErr(nil)) +} + +func TestFromErrWithWrap(t *testing.T) { + err := fmt.Errorf("test: %w", &InstallerError{ + err: fmt.Errorf("test: test"), + code: ErrDownloadFailed, + }) + taskErr := FromErr(err) + assert.Equal(t, taskErr, &InstallerError{ + err: fmt.Errorf("test: test"), + code: ErrDownloadFailed, + }) + + taskErr2 := fmt.Errorf("Wrap 2: %w", fmt.Errorf("Wrap 1: %w", taskErr)) + assert.Equal(t, FromErr(taskErr2).Code(), ErrDownloadFailed) + assert.Nil(t, FromErr(nil)) } func TestWrap(t *testing.T) { @@ -36,9 +52,12 @@ func TestWrap(t *testing.T) { // Check that Wrap doesn't change anything if the error // is already an InstallerError - taskErr2 := Wrap(ErrInstallFailed, taskErr) + taskErr2 := Wrap(ErrNotEnoughDiskSpace, taskErr) assert.Equal(t, taskErr2, &InstallerError{ err: err, code: ErrDownloadFailed, }) + + taskErr3 := Wrap(ErrFilesystemIssue, fmt.Errorf("Wrap 2: %w", fmt.Errorf("Wrap 1: %w", taskErr2))) + assert.Equal(t, FromErr(taskErr3).Code(), ErrDownloadFailed) } diff --git a/pkg/fleet/installer/installer.go b/pkg/fleet/installer/installer.go index 8db7bc07954a5..be2706643df76 100644 --- a/pkg/fleet/installer/installer.go +++ b/pkg/fleet/installer/installer.go @@ -21,6 +21,7 @@ import ( "github.com/DataDog/datadog-agent/pkg/fleet/internal/paths" fleetEnv "github.com/DataDog/datadog-agent/pkg/fleet/env" + installerErrors "github.com/DataDog/datadog-agent/pkg/fleet/installer/errors" "github.com/DataDog/datadog-agent/pkg/fleet/installer/packages" "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" "github.com/DataDog/datadog-agent/pkg/fleet/internal/db" @@ -227,30 +228,48 @@ func (i *installerImpl) InstallExperiment(ctx context.Context, url string) error defer i.m.Unlock() pkg, err := i.downloader.Download(ctx, url) if err != nil { - return fmt.Errorf("could not download package: %w", err) + return installerErrors.Wrap( + installerErrors.ErrDownloadFailed, + fmt.Errorf("could not download package: %w", err), + ) } err = checkAvailableDiskSpace(i.packages, pkg) if err != nil { - return fmt.Errorf("not enough disk space: %w", err) + return installerErrors.Wrap( + installerErrors.ErrNotEnoughDiskSpace, + fmt.Errorf("not enough disk space: %w", err), + ) } tmpDir, err := i.packages.MkdirTemp() if err != nil { - return fmt.Errorf("could not create temporary directory: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could create temporary directory: %w", err), + ) } defer os.RemoveAll(tmpDir) configDir := filepath.Join(i.userConfigsDir, pkg.Name) err = pkg.ExtractLayers(oci.DatadogPackageLayerMediaType, tmpDir) if err != nil { - return fmt.Errorf("could not extract package layers: %w", err) + return installerErrors.Wrap( + installerErrors.ErrDownloadFailed, + fmt.Errorf("could not extract package layer: %w", err), + ) } err = pkg.ExtractLayers(oci.DatadogPackageConfigLayerMediaType, configDir) if err != nil { - return fmt.Errorf("could not extract package config layer: %w", err) + return installerErrors.Wrap( + installerErrors.ErrDownloadFailed, + fmt.Errorf("could not extract package config layer: %w", err), + ) } repository := i.packages.Get(pkg.Name) err = repository.SetExperiment(pkg.Version, tmpDir) if err != nil { - return fmt.Errorf("could not set experiment: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could not set experiment: %w", err), + ) } return i.startExperiment(ctx, pkg.Name) @@ -267,7 +286,10 @@ func (i *installerImpl) RemoveExperiment(ctx context.Context, pkg string) error // will kill the current process, delete the experiment first. err := repository.DeleteExperiment() if err != nil { - return fmt.Errorf("could not delete experiment: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could not delete experiment: %w", err), + ) } err = i.stopExperiment(ctx, pkg) if err != nil { @@ -280,7 +302,10 @@ func (i *installerImpl) RemoveExperiment(ctx context.Context, pkg string) error } err = repository.DeleteExperiment() if err != nil { - return fmt.Errorf("could not delete experiment: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could not delete experiment: %w", err), + ) } } return nil @@ -306,27 +331,42 @@ func (i *installerImpl) InstallConfigExperiment(ctx context.Context, pkg string, config, err := i.cdn.Get(ctx, pkg) if err != nil { - return fmt.Errorf("could not get cdn config: %w", err) + return installerErrors.Wrap( + installerErrors.ErrDownloadFailed, + fmt.Errorf("could not get cdn config: %w", err), + ) } if config.State().GetVersion() != version { - return fmt.Errorf("version mismatch: expected %s, got %s", config.State().GetVersion(), version) + return installerErrors.Wrap( + installerErrors.ErrDownloadFailed, + fmt.Errorf("version mismatch: expected %s, got %s", config.State().GetVersion(), version), + ) } tmpDir, err := i.packages.MkdirTemp() if err != nil { - return fmt.Errorf("could not create temporary directory: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could not create temporary directory: %w", err), + ) } defer os.RemoveAll(tmpDir) err = config.Write(tmpDir) if err != nil { - return fmt.Errorf("could not write agent config: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could not write agent config: %w", err), + ) } configRepo := i.configs.Get(pkg) err = configRepo.SetExperiment(version, tmpDir) if err != nil { - return fmt.Errorf("could not set experiment: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could not set experiment: %w", err), + ) } switch runtime.GOOS { @@ -349,7 +389,10 @@ func (i *installerImpl) RemoveConfigExperiment(ctx context.Context, pkg string) repository := i.configs.Get(pkg) err = repository.DeleteExperiment() if err != nil { - return fmt.Errorf("could not delete experiment: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could not delete experiment: %w", err), + ) } return nil } @@ -362,7 +405,10 @@ func (i *installerImpl) PromoteConfigExperiment(ctx context.Context, pkg string) repository := i.configs.Get(pkg) err := repository.PromoteExperiment() if err != nil { - return fmt.Errorf("could not promote experiment: %w", err) + return installerErrors.Wrap( + installerErrors.ErrFilesystemIssue, + fmt.Errorf("could not promote experiment: %w", err), + ) } return i.promoteExperiment(ctx, pkg) } diff --git a/pkg/fleet/internal/exec/installer_exec.go b/pkg/fleet/internal/exec/installer_exec.go index 1841de484b841..34a3cfad4fbcf 100644 --- a/pkg/fleet/internal/exec/installer_exec.go +++ b/pkg/fleet/internal/exec/installer_exec.go @@ -19,6 +19,7 @@ import ( "github.com/DataDog/datadog-agent/pkg/util/log" "github.com/DataDog/datadog-agent/pkg/fleet/env" + installerErrors "github.com/DataDog/datadog-agent/pkg/fleet/installer/errors" "github.com/DataDog/datadog-agent/pkg/fleet/installer/repository" "github.com/DataDog/datadog-agent/pkg/fleet/telemetry" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -247,8 +248,9 @@ func (iCmd *installerCmd) Run() error { } if len(errBuf.Bytes()) == 0 { - return fmt.Errorf("run failed: %s", err.Error()) + return fmt.Errorf("run failed: %w", err) } - return fmt.Errorf("run failed: %s \n%s", strings.TrimSpace(errBuf.String()), err.Error()) + installerError := installerErrors.FromJSON(strings.TrimSpace(errBuf.String())) + return fmt.Errorf("run failed: %v \n%s", installerError, err.Error()) } diff --git a/pkg/fleet/internal/oci/download.go b/pkg/fleet/internal/oci/download.go index f4a19fe29e59c..6bf91468fa0e6 100644 --- a/pkg/fleet/internal/oci/download.go +++ b/pkg/fleet/internal/oci/download.go @@ -33,6 +33,8 @@ import ( "golang.org/x/net/http2" httptrace "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http" + installerErrors "github.com/DataDog/datadog-agent/pkg/fleet/installer/errors" + "github.com/DataDog/datadog-agent/pkg/fleet/env" "github.com/DataDog/datadog-agent/pkg/fleet/internal/tar" "github.com/DataDog/datadog-agent/pkg/util/log" @@ -304,7 +306,10 @@ func (d *Downloader) downloadIndex(index oci.ImageIndex) (oci.Image, error) { } return image, nil } - return nil, fmt.Errorf("no matching image found in the index") + return nil, installerErrors.Wrap( + installerErrors.ErrPackageNotFound, + fmt.Errorf("no matching image found in the index"), + ) } // ExtractLayers extracts the layers of the downloaded package with the given media type to the given directory.