Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(worker): improve display err on worker command #5277

Merged
merged 3 commits into from Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions cli/cdsctl/workflow_artifact.go
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
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
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
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
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
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
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
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
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