diff --git a/client/driver/docker.go b/client/driver/docker.go index a33487693..7fe21b677 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -508,13 +508,10 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle container, err := d.createContainer(config) if err != nil { - d.logger.Printf("[ERR] driver.docker: failed to create container: %s", err) + wrapped := fmt.Sprintf("Failed to create container: %v", err) + d.logger.Printf("[ERR] driver.docker: %s", wrapped) pluginClient.Kill() - if rerr, ok := err.(*structs.RecoverableError); ok { - rerr.Err = fmt.Sprintf("Failed to create container: %s", rerr.Err) - return nil, rerr - } - return nil, err + return nil, structs.WrapRecoverable(wrapped, err) } d.logger.Printf("[INFO] driver.docker: created container %s", container.ID) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 002b07d59..8a1f439bd 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -364,7 +364,7 @@ func TestDockerDriver_Start_BadPull_Recoverable(t *testing.T) { if rerr, ok := err.(*structs.RecoverableError); !ok { t.Fatalf("want recoverable error: %+v", err) - } else if !rerr.Recoverable { + } else if !rerr.IsRecoverable() { t.Fatalf("error not recoverable: %+v", err) } } diff --git a/client/getter/getter.go b/client/getter/getter.go index 6bd3d53fb..cff9f965e 100644 --- a/client/getter/getter.go +++ b/client/getter/getter.go @@ -88,14 +88,38 @@ func getGetterUrl(taskEnv *env.TaskEnvironment, artifact *structs.TaskArtifact) func GetArtifact(taskEnv *env.TaskEnvironment, artifact *structs.TaskArtifact, taskDir string) error { url, err := getGetterUrl(taskEnv, artifact) if err != nil { - return err + return newGetError(artifact.GetterSource, err, false) } // Download the artifact dest := filepath.Join(taskDir, artifact.RelativeDest) if err := getClient(url, dest).Get(); err != nil { - return structs.NewRecoverableError(fmt.Errorf("GET error: %v", err), true) + return newGetError(url, err, true) } return nil } + +// GetError wraps the underlying artifact fetching error with the URL. It +// implements the RecoverableError interface. +type GetError struct { + URL string + Err error + recoverable bool +} + +func newGetError(url string, err error, recoverable bool) *GetError { + return &GetError{ + URL: url, + Err: err, + recoverable: recoverable, + } +} + +func (g *GetError) Error() string { + return g.Err.Error() +} + +func (g *GetError) IsRecoverable() bool { + return g.recoverable +} diff --git a/client/restarts.go b/client/restarts.go index 2c52cd1c9..b6e49e31c 100644 --- a/client/restarts.go +++ b/client/restarts.go @@ -149,7 +149,7 @@ func (r *RestartTracker) GetState() (string, time.Duration) { // infinitely try to start a task. func (r *RestartTracker) handleStartError() (string, time.Duration) { // If the error is not recoverable, do not restart. - if rerr, ok := r.startErr.(*structs.RecoverableError); !(ok && rerr.Recoverable) { + if !structs.IsRecoverable(r.startErr) { r.reason = ReasonUnrecoverableErrror return structs.TaskNotRestarting, 0 } diff --git a/client/task_runner.go b/client/task_runner.go index 6929752d5..ed9e825c6 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -665,7 +665,7 @@ func (r *TaskRunner) deriveVaultToken() (token string, exit bool) { } // Check if we can't recover from the error - if rerr, ok := err.(*structs.RecoverableError); !ok || !rerr.Recoverable { + if !structs.IsRecoverable(err) { r.logger.Printf("[ERR] client: failed to derive Vault token for task %v on alloc %q: %v", r.task.Name, r.alloc.ID, err) r.Kill("vault", fmt.Sprintf("failed to derive token: %v", err), true) @@ -800,7 +800,7 @@ func (r *TaskRunner) prestart(resultCh chan bool) { r.logger.Printf("[DEBUG] client: %v", wrapped) r.setState(structs.TaskStatePending, structs.NewTaskEvent(structs.TaskArtifactDownloadFailed).SetDownloadError(wrapped)) - r.restartTracker.SetStartError(structs.NewRecoverableError(wrapped, structs.IsRecoverable(err))) + r.restartTracker.SetStartError(structs.WrapRecoverable(wrapped.Error(), err)) goto RESTART } } @@ -1195,31 +1195,19 @@ func (r *TaskRunner) startTask() error { r.createdResourcesLock.Unlock() if err != nil { - wrapped := fmt.Errorf("failed to initialize task %q for alloc %q: %v", + wrapped := fmt.Sprintf("failed to initialize task %q for alloc %q: %v", r.task.Name, r.alloc.ID, err) - - r.logger.Printf("[WARN] client: error from prestart: %v", wrapped) - - if rerr, ok := err.(*structs.RecoverableError); ok { - return structs.NewRecoverableError(wrapped, rerr.Recoverable) - } - - return wrapped + r.logger.Printf("[WARN] client: error from prestart: %s", wrapped) + return structs.WrapRecoverable(wrapped, err) } // Start the job handle, err := drv.Start(ctx, r.task) if err != nil { - wrapped := fmt.Errorf("failed to start task %q for alloc %q: %v", + wrapped := fmt.Sprintf("failed to start task %q for alloc %q: %v", r.task.Name, r.alloc.ID, err) - - r.logger.Printf("[WARN] client: %v", wrapped) - - if rerr, ok := err.(*structs.RecoverableError); ok { - return structs.NewRecoverableError(wrapped, rerr.Recoverable) - } - - return wrapped + r.logger.Printf("[WARN] client: %s", wrapped) + return structs.WrapRecoverable(wrapped, err) } diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 0dd7768c7..4cc5ce476 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -1064,13 +1064,8 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest, secret, err := n.srv.vault.CreateToken(ctx, alloc, task) if err != nil { - wrapped := fmt.Errorf("failed to create token for task %q on alloc %q: %v", task, alloc.ID, err) - if rerr, ok := err.(*structs.RecoverableError); ok && rerr.Recoverable { - // If the error is recoverable, propogate it - return structs.NewRecoverableError(wrapped, true) - } - - return wrapped + wrapped := fmt.Sprintf("failed to create token for task %q on alloc %q: %v", task, alloc.ID, err) + return structs.WrapRecoverable(wrapped, err) } results[task] = secret diff --git a/nomad/node_endpoint_test.go b/nomad/node_endpoint_test.go index 329e39d14..68e2b42e7 100644 --- a/nomad/node_endpoint_test.go +++ b/nomad/node_endpoint_test.go @@ -2030,7 +2030,7 @@ func TestClientEndpoint_DeriveVaultToken_VaultError(t *testing.T) { if err != nil { t.Fatalf("bad: %v", err) } - if resp.Error == nil || !resp.Error.Recoverable { + if resp.Error == nil || !resp.Error.IsRecoverable() { t.Fatalf("bad: %+v", resp.Error) } } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index c35c1a2aa..f85ae1d52 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4193,15 +4193,33 @@ func NewRecoverableError(e error, recoverable bool) error { } } +// WrapRecoverable wraps an existing error in a new RecoverableError with a new +// message. If the error was recoverable before the returned error is as well; +// otherwise it is unrecoverable. +func WrapRecoverable(msg string, err error) error { + return &RecoverableError{Err: msg, Recoverable: IsRecoverable(err)} +} + func (r *RecoverableError) Error() string { return r.Err } +func (r *RecoverableError) IsRecoverable() bool { + return r.Recoverable +} + +// Recoverable is an interface for errors to implement to indicate whether or +// not they are fatal or recoverable. +type Recoverable interface { + error + IsRecoverable() bool +} + // IsRecoverable returns true if error is a RecoverableError with // Recoverable=true. Otherwise false is returned. func IsRecoverable(e error) bool { - if re, ok := e.(*RecoverableError); ok { - return re.Recoverable + if re, ok := e.(Recoverable); ok { + return re.IsRecoverable() } return false } diff --git a/nomad/vault_test.go b/nomad/vault_test.go index 4e613a8ab..b874230b4 100644 --- a/nomad/vault_test.go +++ b/nomad/vault_test.go @@ -925,9 +925,9 @@ func TestVaultClient_CreateToken_Role_Unrecoverable(t *testing.T) { t.Fatalf("CreateToken should have failed: %v", err) } - _, ok := err.(*structs.RecoverableError) + _, ok := err.(structs.Recoverable) if ok { - t.Fatalf("CreateToken should not be a recoverable error type: %v", err) + t.Fatalf("CreateToken should not be a recoverable error type: %v (%T)", err, err) } } @@ -955,7 +955,7 @@ func TestVaultClient_CreateToken_Prestart(t *testing.T) { if rerr, ok := err.(*structs.RecoverableError); !ok { t.Fatalf("Err should have been type recoverable error") - } else if ok && !rerr.Recoverable { + } else if ok && !rerr.IsRecoverable() { t.Fatalf("Err should have been recoverable") } }