Skip to content

Commit

Permalink
fix(worker): improve display err on worker command (#5277)
Browse files Browse the repository at this point in the history
  • Loading branch information
yesnault authored Jun 24, 2020
1 parent 27fd6a6 commit e0842e5
Show file tree
Hide file tree
Showing 11 changed files with 49 additions and 119 deletions.
4 changes: 2 additions & 2 deletions cli/cdsctl/workflow_artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ var workflowArtifactDownloadCmd = cli.Command{
{Name: "number"},
},
OptionalArgs: []cli.Arg{
{Name: "artefact-name"},
{Name: "artifact-name"},
},
Flags: []cli.Flag{
{
Expand Down Expand Up @@ -92,7 +92,7 @@ func workflowArtifactDownloadRun(v cli.Values) error {

var ok bool
for _, a := range artifacts {
if v.GetString("artefact-name") != "" && v.GetString("artefact-name") != a.Name {
if v.GetString("artifact-name") != "" && v.GetString("artifact-name") != a.Name {
continue
}
if v.GetString("exclude") != "" && reg.MatchString(a.Name) {
Expand Down
4 changes: 2 additions & 2 deletions engine/worker/cmd_artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Inside a job, you can list artifacts of a workflow:
}
c.Flags().StringVar(&cmdDownloadWorkflowName, "workflow", "", "Workflow name. Optional, default: current workflow")
c.Flags().StringVar(&cmdDownloadNumber, "number", "", "Workflow Number. Optional, default: current workflow run")
c.Flags().StringVar(&cmdDownloadArtefactName, "pattern", "", "Pattern matching files to list. Optional, default: *")
c.Flags().StringVar(&cmdDownloadArtifactName, "pattern", "", "Pattern matching files to list. Optional, default: *")
c.Flags().StringVar(&cmdDownloadTag, "tag", "", "Tag matching files to list. Optional")

return c
Expand Down Expand Up @@ -64,7 +64,7 @@ func artifactsCmd() func(cmd *cobra.Command, args []string) {
a := workerruntime.DownloadArtifact{
Workflow: cmdDownloadWorkflowName,
Number: number,
Pattern: cmdDownloadArtefactName,
Pattern: cmdDownloadArtifactName,
Tag: cmdDownloadTag,
}

Expand Down
8 changes: 4 additions & 4 deletions engine/worker/cmd_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
var (
cmdDownloadWorkflowName string
cmdDownloadNumber string
cmdDownloadArtefactName string
cmdDownloadArtifactName string
cmdDownloadTag string
)

Expand Down Expand Up @@ -51,7 +51,7 @@ Theses two commands have the same result:
}
c.Flags().StringVar(&cmdDownloadWorkflowName, "workflow", "", "Workflow name to download from. Optional, default: current workflow")
c.Flags().StringVar(&cmdDownloadNumber, "number", "", "Workflow Number to download from. Optional, default: current workflow run")
c.Flags().StringVar(&cmdDownloadArtefactName, "pattern", "", "Pattern matching files to download. Optional, default: *")
c.Flags().StringVar(&cmdDownloadArtifactName, "pattern", "", "Pattern matching files to download. Optional, default: *")
c.Flags().StringVar(&cmdDownloadTag, "tag", "", "Tag matching files to download. Optional")
return c
}
Expand Down Expand Up @@ -81,7 +81,7 @@ func downloadCmd() func(cmd *cobra.Command, args []string) {
a := workerruntime.DownloadArtifact{
Workflow: cmdDownloadWorkflowName,
Number: number,
Pattern: cmdDownloadArtefactName,
Pattern: cmdDownloadArtifactName,
Tag: cmdDownloadTag,
Destination: wd,
}
Expand All @@ -107,7 +107,7 @@ func downloadCmd() func(cmd *cobra.Command, args []string) {
if resp.StatusCode >= 300 {
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
sdk.Exit("cannot artefact download HTTP %v\n", err)
sdk.Exit("cannot artifact download HTTP %v\n", err)
}
cdsError := sdk.DecodeError(body)
sdk.Exit("download failed: %v\n", cdsError)
Expand Down
8 changes: 7 additions & 1 deletion engine/worker/cmd_upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"net/url"
"os"
Expand Down Expand Up @@ -88,7 +89,12 @@ func uploadCmd() func(cmd *cobra.Command, args []string) {
}

if resp.StatusCode >= 300 {
sdk.Exit("cannot artifact upload HTTP %d\n", resp.StatusCode)
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
sdk.Exit("cannot artifact upload HTTP %v\n", err)
}
cdsError := sdk.DecodeError(body)
sdk.Exit("artifact upload failed: %v\n", cdsError)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ func TestRunArtifactDownload(t *testing.T) {
wk, ctx := SetupTest(t)

as := []sdk.WorkflowNodeRunArtifact{
sdk.WorkflowNodeRunArtifact{
{
ID: 1,
Name: "myFile.txt",
Tag: "999",
},
sdk.WorkflowNodeRunArtifact{
{
ID: 2,
Name: "myFile.csv",
Tag: "999",
Expand Down Expand Up @@ -90,7 +90,7 @@ func TestRunArtifactDownloadOutsideWorkspace(t *testing.T) {
fileName := sdk.RandomString(10)

as := []sdk.WorkflowNodeRunArtifact{
sdk.WorkflowNodeRunArtifact{
{
ID: 1,
Name: fileName,
Tag: "999",
Expand Down
10 changes: 4 additions & 6 deletions engine/worker/internal/action/builtin_artifact_upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package action

import (
"context"
"errors"
"fmt"
"path/filepath"
"strings"
Expand Down Expand Up @@ -47,17 +46,17 @@ func RunArtifactUpload(ctx context.Context, wk workerruntime.Runtime, a sdk.Acti

tag := sdk.ParameterFind(a.Parameters, "tag")
if tag == nil {
return res, errors.New("tag variable is empty. aborting")
return res, sdk.NewError(sdk.ErrWorkerErrorCommand, fmt.Errorf("tag variable is empty. aborting"))
}

// Global all files matching filePath
filesPath, err := afero.Glob(afero.NewOsFs(), artifactPath)
if err != nil {
return res, fmt.Errorf("cannot perform globbing of pattern '%s': %s", artifactPath, err)
return res, sdk.NewError(sdk.ErrWorkerErrorCommand, fmt.Errorf("cannot perform globbing of pattern '%s': %s", artifactPath, err))
}

if len(filesPath) == 0 {
return res, fmt.Errorf("pattern '%s' matched no file", artifactPath)
return res, sdk.NewError(sdk.ErrWorkerErrorCommand, fmt.Errorf("pattern '%s' matched no file", artifactPath))
}

var globalError = &sdk.MultiError{}
Expand Down Expand Up @@ -105,8 +104,7 @@ func RunArtifactUpload(ctx context.Context, wk workerruntime.Runtime, a sdk.Acti
wgErrors.Wait()

if !globalError.IsEmpty() {
log.Error(ctx, "Error while uploading artifact: %v", globalError.Error())
return res, fmt.Errorf("error: %v", globalError.Error())
return res, sdk.NewError(sdk.ErrUnknownError, fmt.Errorf("error: %v", globalError.Error()))
}

return res, nil
Expand Down
2 changes: 1 addition & 1 deletion engine/worker/internal/handler_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func downloadHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc {

wg.Wait()
if isInError {
newError := sdk.NewError(sdk.ErrUnknownError, fmt.Errorf("Error while downloading artefacts - see previous logs"))
newError := sdk.NewError(sdk.ErrUnknownError, fmt.Errorf("Error while downloading artifacts - see previous logs"))
writeError(w, r, newError)
}
}
Expand Down
28 changes: 6 additions & 22 deletions engine/worker/internal/handler_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ func keyInstallHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc

body, err := ioutil.ReadAll(r.Body)
if err != nil {
sdkerr := sdk.NewError(sdk.ErrWrongRequest, err).(sdk.Error)
writeJSON(w, sdkerr, sdkerr.Status)
writeError(w, r, sdk.NewError(sdk.ErrWrongRequest, err))
return
}

Expand All @@ -31,23 +30,16 @@ func keyInstallHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc
var mapBody = make(map[string]string)
if len(body) > 0 {
if err := json.Unmarshal(body, &mapBody); err != nil {
sdkerr := sdk.Error{
Status: sdk.ErrWrongRequest.Status,
Message: err.Error()}
writeJSON(w, sdkerr, sdkerr.Status)
writeError(w, r, sdk.NewError(sdk.ErrWrongRequest, err))
return
}
}

var key *sdk.Variable

if wk.currentJob.secrets == nil {
err := sdk.Error{
Message: "Cannot find any keys for your job",
Status: http.StatusBadRequest,
}
log.Error(ctx, "%v", err)
writeJSON(w, err, err.Status)
writeError(w, r, sdk.NewError(sdk.ErrWrongRequest, fmt.Errorf("Cannot find any keys for your job")))
return
}

Expand All @@ -59,12 +51,8 @@ func keyInstallHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc
}

if key == nil {
err := sdk.Error{
Message: fmt.Sprintf("Key %s not found", keyName),
Status: http.StatusNotFound,
}
log.Error(ctx, "%v", err)
writeJSON(w, err, err.Status)
writeError(w, r, sdk.NewError(sdk.ErrNotFound, fmt.Errorf("Cannot find any keys for your job")))
return
}

Expand All @@ -73,13 +61,9 @@ func keyInstallHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc
if err != nil {
log.Error(ctx, "Unable to install key %s: %v", key.Name, err)
if sdkerr, ok := err.(*sdk.Error); ok {
writeJSON(w, sdkerr, sdkerr.Status)
writeError(w, r, sdkerr)
} else {
err := sdk.Error{
Message: err.Error(),
Status: sdk.ErrUnknownError.Status,
}
writeJSON(w, err, err.Status)
writeError(w, r, sdk.NewError(sdk.ErrUnknownError, err))
}
return
}
Expand Down
4 changes: 2 additions & 2 deletions engine/worker/internal/handler_upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func uploadHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc {
workingDir, err := workerruntime.WorkingDirectory(wk.currentJob.context)
if err != nil {
wk.SendLog(ctx, workerruntime.LevelError, fmt.Sprintf("Artifact upload failed: %v", err))
log.Error(ctx, "Artifact upload failed: No woorking directory: %v", err)
log.Error(ctx, "Artifact upload failed: No working directory: %v", err)
writeError(w, r, err)
return
}
Expand All @@ -74,7 +74,7 @@ func uploadHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc {
if result.Status != sdk.StatusSuccess {
wk.SendLog(ctx, workerruntime.LevelError, fmt.Sprintf("Artifact upload failed: %s", result.Reason))
log.Error(ctx, "Artifact upload failed: %v", result)
w.WriteHeader(http.StatusInternalServerError)
writeError(w, r, fmt.Errorf("Artifact upload failed: %s", result.Reason))
return
}
}
Expand Down
Loading

0 comments on commit e0842e5

Please sign in to comment.