From 4bbf66e4aa0009e905e4b5b8bd8d1e65bfe755c3 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Fri, 22 Mar 2019 10:41:40 -0700 Subject: [PATCH 01/11] Issue#896 Workflow steps with non-existant output artifact path will succeed Issue: https://github.com/argoproj/argo/issues/897 Solution: Added new element "optional" in Artifact. The default is false. This flag will make artifact as optional and existence check will be ignored if input/output artifact has optional=true. Output Artifact ( optional=true ): Artifact existence check will be ignored during the save artifact in destination and continued workflow Input Artifact ( optional=true ): Artifact exist check will be ignored during load artifact from source and continued workflow --- api/openapi-spec/swagger.json | 4 ++ errors/errors.go | 2 +- pkg/apis/workflow/v1alpha1/types.go | 3 ++ .../input-artifact-not-optional.yaml | 22 ++++++++++ .../output-artifact-not-optional.yaml | 24 +++++++++++ .../functional/input-artifact-optional.yaml | 22 ++++++++++ .../functional/output-artifact-optional.yaml | 24 +++++++++++ .../output-input-artifact-optional.yaml | 40 +++++++++++++++++++ workflow/common/util.go | 10 +++-- workflow/executor/docker/docker.go | 2 +- workflow/executor/executor.go | 11 ++++- 11 files changed, 158 insertions(+), 6 deletions(-) create mode 100644 test/e2e/expectedfailures/input-artifact-not-optional.yaml create mode 100644 test/e2e/expectedfailures/output-artifact-not-optional.yaml create mode 100644 test/e2e/functional/input-artifact-optional.yaml create mode 100644 test/e2e/functional/output-artifact-optional.yaml create mode 100644 test/e2e/functional/output-input-artifact-optional.yaml diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index eb0e362aad5b..aea0b50d1ddc 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -94,6 +94,10 @@ "s3": { "description": "S3 contains S3 artifact location details", "$ref": "#/definitions/io.argoproj.workflow.v1alpha1.S3Artifact" + }, + "optional": { + "descrrition": "Make Artifacts optional, if Artifacts doesn't generate or exist", + "type": "boolean" } } }, diff --git a/errors/errors.go b/errors/errors.go index 22177ccaa4f1..1fbe662557a7 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -14,7 +14,7 @@ const ( CodeBadRequest = "ERR_BAD_REQUEST" CodeForbidden = "ERR_FORBIDDEN" CodeNotFound = "ERR_NOT_FOUND" - CodeNotImplemented = "ERR_NOT_INPLEMENTED" + CodeNotImplemented = "ERR_NOT_IMPLEMENTED" CodeTimeout = "ERR_TIMEOUT" CodeInternal = "ERR_INTERNAL" ) diff --git a/pkg/apis/workflow/v1alpha1/types.go b/pkg/apis/workflow/v1alpha1/types.go index ff659353d360..d5f13401d2b8 100644 --- a/pkg/apis/workflow/v1alpha1/types.go +++ b/pkg/apis/workflow/v1alpha1/types.go @@ -319,6 +319,9 @@ type Artifact struct { // Archive controls how the artifact will be saved to the artifact repository. Archive *ArchiveStrategy `json:"archive,omitempty"` + + // Make Artifacts optional, if Artifacts doesn't generate or exist + Optional bool `json:"optional,omitempty"` } // ArchiveStrategy describes how to archive files/directory when saving artifacts diff --git a/test/e2e/expectedfailures/input-artifact-not-optional.yaml b/test/e2e/expectedfailures/input-artifact-not-optional.yaml new file mode 100644 index 000000000000..444068c014bd --- /dev/null +++ b/test/e2e/expectedfailures/input-artifact-not-optional.yaml @@ -0,0 +1,22 @@ +# This example demonstrates the input artifacts optionals +# from one step to the next. +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: input-artifact-optional- +spec: + entrypoint: http-artifact-example + templates: + - name: http-artifact-example + inputs: + artifacts: + - name: kubectl + path: /bin/kubectl + mode: 0755 + optional: false + http: + url: "" + container: + image: debian:9.4 + command: [sh, -c] + args: ["echo NoKubectl"] \ No newline at end of file diff --git a/test/e2e/expectedfailures/output-artifact-not-optional.yaml b/test/e2e/expectedfailures/output-artifact-not-optional.yaml new file mode 100644 index 000000000000..8f807611c7a9 --- /dev/null +++ b/test/e2e/expectedfailures/output-artifact-not-optional.yaml @@ -0,0 +1,24 @@ +# This example demonstrates the output artifacts optionals +# from one step to the next. +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: output-artifact-not-optional- +spec: + entrypoint: artifact-example + templates: + - name: artifact-example + steps: + - - name: generate-artifact + template: whalesay + + - name: whalesay + container: + image: docker/whalesay:latest + command: [sh, -c] + args: ["cowsay hello world | tee /tmp/hello_world12.txt"] + outputs: + artifacts: + - name: hello-art + optional: false + path: /tmp/hello_world.txt \ No newline at end of file diff --git a/test/e2e/functional/input-artifact-optional.yaml b/test/e2e/functional/input-artifact-optional.yaml new file mode 100644 index 000000000000..8fa1a29dc084 --- /dev/null +++ b/test/e2e/functional/input-artifact-optional.yaml @@ -0,0 +1,22 @@ +# This example demonstrates the input artifacts optionals +# from one step to the next. +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: input-artifact-optional- +spec: + entrypoint: http-artifact-example + templates: + - name: http-artifact-example + inputs: + artifacts: + - name: kubectl + path: /bin/kubectl + mode: 0755 + optional: true + http: + url: "" + container: + image: debian:9.4 + command: [sh, -c] + args: ["echo NoKubectl"] \ No newline at end of file diff --git a/test/e2e/functional/output-artifact-optional.yaml b/test/e2e/functional/output-artifact-optional.yaml new file mode 100644 index 000000000000..044a09f3e438 --- /dev/null +++ b/test/e2e/functional/output-artifact-optional.yaml @@ -0,0 +1,24 @@ +# This example demonstrates the output artifacts optionals +# from one step to the next. +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: output-artifact-optional- +spec: + entrypoint: artifact-example + templates: + - name: artifact-example + steps: + - - name: generate-artifact + template: whalesay + + - name: whalesay + container: + image: docker/whalesay:latest + command: [sh, -c] + args: ["cowsay hello world | tee /tmp/hello_world12.txt"] + outputs: + artifacts: + - name: hello-art + optional: true + path: /tmp/hello_world.txt \ No newline at end of file diff --git a/test/e2e/functional/output-input-artifact-optional.yaml b/test/e2e/functional/output-input-artifact-optional.yaml new file mode 100644 index 000000000000..a29959fed04d --- /dev/null +++ b/test/e2e/functional/output-input-artifact-optional.yaml @@ -0,0 +1,40 @@ +# This example demonstrates the output and input artifacts are optionals +# from one step to the next. +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: output-input-artifact-optional- +spec: + entrypoint: artifact-example + templates: + - name: artifact-example + steps: + - - name: generate-artifact + template: whalesay + - - name: consume-artifact + template: print-message + arguments: + artifacts: + - name: message + from: "{{steps.generate-artifact.outputs.artifacts.hello-art}}" + - name: whalesay + container: + image: docker/whalesay:latest + command: [sh, -c] + args: ["cowsay hello world | tee /tmp/hello_world123.txt"] + outputs: + artifacts: + - name: hello-art + optional: true + path: /tmp/hello_world.txt + + - name: print-message + inputs: + artifacts: + - name: message + path: /tmp/message + optional: true + container: + image: alpine:latest + command: [sh, -c] + args: ["echo /tmp/message"] diff --git a/workflow/common/util.go b/workflow/common/util.go index 88f3431ecd0e..79f6d2416877 100644 --- a/workflow/common/util.go +++ b/workflow/common/util.go @@ -148,10 +148,10 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, globalParams, localPa } // artifact must be supplied argArt := args.GetArtifactByName(inArt.Name) - if argArt == nil { + if !inArt.Optional && argArt == nil { return nil, errors.Errorf(errors.CodeBadRequest, "inputs.artifacts.%s was not supplied", inArt.Name) } - if !argArt.HasLocation() && !validateOnly { + if !inArt.Optional && !argArt.HasLocation() && !validateOnly { return nil, errors.Errorf(errors.CodeBadRequest, "inputs.artifacts.%s missing location information", inArt.Name) } argArt.Path = inArt.Path @@ -241,13 +241,17 @@ func RunCommand(name string, arg ...string) error { cmd := exec.Command(name, arg...) cmdStr := strings.Join(cmd.Args, " ") log.Info(cmdStr) - _, err := cmd.Output() + cmdOutput, err := cmd.Output() if err != nil { exErr := err.(*exec.ExitError) errOutput := string(exErr.Stderr) log.Errorf("`%s` failed: %s", cmdStr, errOutput) return errors.InternalError(strings.TrimSpace(errOutput)) } + if strings.Contains(string(cmdOutput), "Failed") { + return errors.InternalError(string(cmdOutput)) + + } return nil } diff --git a/workflow/executor/docker/docker.go b/workflow/executor/docker/docker.go index 0d4084062f81..84690d7a49ee 100644 --- a/workflow/executor/docker/docker.go +++ b/workflow/executor/docker/docker.go @@ -56,7 +56,7 @@ func (d *DockerExecutor) CopyFile(containerID string, sourcePath string, destPat if !file.IsFileOrDirExistInGZip(sourcePath, destPath) { errMsg := fmt.Sprintf("File or Artifact does not exist. %s", sourcePath) log.Warn(errMsg) - return errors.InternalError(errMsg) + return errors.Errorf(errors.CodeNotFound, errMsg) } log.Infof("Archiving completed") return nil diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index 955c4e7927b3..e69909ef8b0f 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -117,6 +117,10 @@ func (we *WorkflowExecutor) LoadArtifacts() error { log.Infof("Downloading artifact: %s", art.Name) artDriver, err := we.InitDriver(art) if err != nil { + if art.Optional{ + log.Warnf("Error in loading Artifacts. Artifact configured as an optional so, Error will be ignored. Error=", err) + return nil + } return err } // Determine the file path of where to load the artifact @@ -232,6 +236,10 @@ func (we *WorkflowExecutor) saveArtifact(tempOutArtDir string, mainCtrID string, localArtPath := path.Join(tempOutArtDir, fileName) err := we.RuntimeExecutor.CopyFile(mainCtrID, art.Path, localArtPath) if err != nil { + if art.Optional && errors.IsCode(errors.CodeNotFound,err){ + log.Warnf("Error in saving Artifact. Artifact configured as an optional so, Error will be ignored. Error=", err) + return nil + } return err } fileName, localArtPath, err = stageArchiveFile(fileName, localArtPath, art) @@ -422,7 +430,7 @@ func (we *WorkflowExecutor) SaveLogs() (*wfv1.Artifact, error) { } // InitDriver initializes an instance of an artifact driver -func (we *WorkflowExecutor) InitDriver(art wfv1.Artifact) (artifact.ArtifactDriver, error) { +func (we *WorkflowExecutor) InitDriver(art wfv1.Artifact) (artifact.ArtifactDriver, error) { if art.S3 != nil { var accessKey string var secretKey string @@ -502,6 +510,7 @@ func (we *WorkflowExecutor) InitDriver(art wfv1.Artifact) (artifact.ArtifactDriv if art.Raw != nil { return &raw.RawArtifactDriver{}, nil } + return nil, errors.Errorf(errors.CodeBadRequest, "Unsupported artifact driver for %s", art.Name) } From 3a0eb0f654de07d52e81b6c15fc88cfb726feba4 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Fri, 22 Mar 2019 10:45:13 -0700 Subject: [PATCH 02/11] added end of line --- test/e2e/expectedfailures/input-artifact-not-optional.yaml | 2 +- test/e2e/expectedfailures/output-artifact-not-optional.yaml | 2 +- test/e2e/functional/input-artifact-optional.yaml | 2 +- test/e2e/functional/output-artifact-optional.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/expectedfailures/input-artifact-not-optional.yaml b/test/e2e/expectedfailures/input-artifact-not-optional.yaml index 444068c014bd..8871bf56e695 100644 --- a/test/e2e/expectedfailures/input-artifact-not-optional.yaml +++ b/test/e2e/expectedfailures/input-artifact-not-optional.yaml @@ -19,4 +19,4 @@ spec: container: image: debian:9.4 command: [sh, -c] - args: ["echo NoKubectl"] \ No newline at end of file + args: ["echo NoKubectl"] diff --git a/test/e2e/expectedfailures/output-artifact-not-optional.yaml b/test/e2e/expectedfailures/output-artifact-not-optional.yaml index 8f807611c7a9..1a26bbc57ecf 100644 --- a/test/e2e/expectedfailures/output-artifact-not-optional.yaml +++ b/test/e2e/expectedfailures/output-artifact-not-optional.yaml @@ -21,4 +21,4 @@ spec: artifacts: - name: hello-art optional: false - path: /tmp/hello_world.txt \ No newline at end of file + path: /tmp/hello_world.txt diff --git a/test/e2e/functional/input-artifact-optional.yaml b/test/e2e/functional/input-artifact-optional.yaml index 8fa1a29dc084..9b7a8a051b19 100644 --- a/test/e2e/functional/input-artifact-optional.yaml +++ b/test/e2e/functional/input-artifact-optional.yaml @@ -19,4 +19,4 @@ spec: container: image: debian:9.4 command: [sh, -c] - args: ["echo NoKubectl"] \ No newline at end of file + args: ["echo NoKubectl"] diff --git a/test/e2e/functional/output-artifact-optional.yaml b/test/e2e/functional/output-artifact-optional.yaml index 044a09f3e438..3713b45450de 100644 --- a/test/e2e/functional/output-artifact-optional.yaml +++ b/test/e2e/functional/output-artifact-optional.yaml @@ -21,4 +21,4 @@ spec: artifacts: - name: hello-art optional: true - path: /tmp/hello_world.txt \ No newline at end of file + path: /tmp/hello_world.txt From 1d1b79c73dd652ac589b9c9198e1d3f0c16c62b9 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Fri, 22 Mar 2019 10:47:37 -0700 Subject: [PATCH 03/11] removed unwanted whitespace --- workflow/executor/executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index e69909ef8b0f..57aacffdad05 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -430,7 +430,7 @@ func (we *WorkflowExecutor) SaveLogs() (*wfv1.Artifact, error) { } // InitDriver initializes an instance of an artifact driver -func (we *WorkflowExecutor) InitDriver(art wfv1.Artifact) (artifact.ArtifactDriver, error) { +func (we *WorkflowExecutor) InitDriver(art wfv1.Artifact) (artifact.ArtifactDriver, error) { if art.S3 != nil { var accessKey string var secretKey string From 6b9dc764506365da455f78114378156e968c103d Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Fri, 22 Mar 2019 10:50:00 -0700 Subject: [PATCH 04/11] Deleted test code --- workflow/common/util.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/workflow/common/util.go b/workflow/common/util.go index 79f6d2416877..860c6b47370d 100644 --- a/workflow/common/util.go +++ b/workflow/common/util.go @@ -241,17 +241,13 @@ func RunCommand(name string, arg ...string) error { cmd := exec.Command(name, arg...) cmdStr := strings.Join(cmd.Args, " ") log.Info(cmdStr) - cmdOutput, err := cmd.Output() + _, err := cmd.Output() if err != nil { exErr := err.(*exec.ExitError) errOutput := string(exErr.Stderr) log.Errorf("`%s` failed: %s", cmdStr, errOutput) return errors.InternalError(strings.TrimSpace(errOutput)) } - if strings.Contains(string(cmdOutput), "Failed") { - return errors.InternalError(string(cmdOutput)) - - } return nil } From 20cb9180a7955b0f6dcea8073fc6ec5ababe7034 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Fri, 22 Mar 2019 11:20:34 -0700 Subject: [PATCH 05/11] go formatted --- workflow/executor/executor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index 57aacffdad05..a1206be9052f 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -117,7 +117,7 @@ func (we *WorkflowExecutor) LoadArtifacts() error { log.Infof("Downloading artifact: %s", art.Name) artDriver, err := we.InitDriver(art) if err != nil { - if art.Optional{ + if art.Optional { log.Warnf("Error in loading Artifacts. Artifact configured as an optional so, Error will be ignored. Error=", err) return nil } @@ -236,7 +236,7 @@ func (we *WorkflowExecutor) saveArtifact(tempOutArtDir string, mainCtrID string, localArtPath := path.Join(tempOutArtDir, fileName) err := we.RuntimeExecutor.CopyFile(mainCtrID, art.Path, localArtPath) if err != nil { - if art.Optional && errors.IsCode(errors.CodeNotFound,err){ + if art.Optional && errors.IsCode(errors.CodeNotFound, err) { log.Warnf("Error in saving Artifact. Artifact configured as an optional so, Error will be ignored. Error=", err) return nil } From 3b869694fa36d59384801856227f9a11d8e59ff9 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Fri, 22 Mar 2019 12:01:33 -0700 Subject: [PATCH 06/11] added formatting directives --- workflow/executor/executor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index a1206be9052f..23dd6e0c80cd 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -118,7 +118,7 @@ func (we *WorkflowExecutor) LoadArtifacts() error { artDriver, err := we.InitDriver(art) if err != nil { if art.Optional { - log.Warnf("Error in loading Artifacts. Artifact configured as an optional so, Error will be ignored. Error=", err) + log.Warnf("Error in loading Artifacts. Artifact configured as an optional so, Error will be ignored. Error=%v", err) return nil } return err @@ -237,7 +237,7 @@ func (we *WorkflowExecutor) saveArtifact(tempOutArtDir string, mainCtrID string, err := we.RuntimeExecutor.CopyFile(mainCtrID, art.Path, localArtPath) if err != nil { if art.Optional && errors.IsCode(errors.CodeNotFound, err) { - log.Warnf("Error in saving Artifact. Artifact configured as an optional so, Error will be ignored. Error=", err) + log.Warnf("Error in saving Artifact. Artifact configured as an optional so, Error will be ignored. Error= %v", err) return nil } return err From f42b23c9d655740c3d052dfde3b181d5f213058b Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Fri, 22 Mar 2019 17:08:40 -0700 Subject: [PATCH 07/11] updated Codegen --- api/openapi-spec/swagger.json | 8 ++++---- pkg/apis/workflow/v1alpha1/openapi_generated.go | 7 +++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index aea0b50d1ddc..19079ab7e615 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -83,6 +83,10 @@ "description": "name of the artifact. must be unique within a template's inputs/outputs.", "type": "string" }, + "optional": { + "description": "Make Artifacts optional, if Artifacts doesn't generate or exist", + "type": "boolean" + }, "path": { "description": "Path is the container path to the artifact", "type": "string" @@ -94,10 +98,6 @@ "s3": { "description": "S3 contains S3 artifact location details", "$ref": "#/definitions/io.argoproj.workflow.v1alpha1.S3Artifact" - }, - "optional": { - "descrrition": "Make Artifacts optional, if Artifacts doesn't generate or exist", - "type": "boolean" } } }, diff --git a/pkg/apis/workflow/v1alpha1/openapi_generated.go b/pkg/apis/workflow/v1alpha1/openapi_generated.go index de74605e3487..f41584392087 100644 --- a/pkg/apis/workflow/v1alpha1/openapi_generated.go +++ b/pkg/apis/workflow/v1alpha1/openapi_generated.go @@ -206,6 +206,13 @@ func schema_pkg_apis_workflow_v1alpha1_Artifact(ref common.ReferenceCallback) co Ref: ref("github.com/argoproj/argo/pkg/apis/workflow/v1alpha1.ArchiveStrategy"), }, }, + "optional": { + SchemaProps: spec.SchemaProps{ + Description: "Make Artifacts optional, if Artifacts doesn't generate or exist", + Type: []string{"boolean"}, + Format: "", + }, + }, }, Required: []string{"name"}, }, From 6704a46db8d4e92711a65be978939b08a54ea8b4 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Sun, 24 Mar 2019 12:38:09 -0700 Subject: [PATCH 08/11] Fixed format on merge conflict --- workflow/executor/docker/docker.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/workflow/executor/docker/docker.go b/workflow/executor/docker/docker.go index 003ad05531ea..6e4191554d8c 100644 --- a/workflow/executor/docker/docker.go +++ b/workflow/executor/docker/docker.go @@ -65,10 +65,9 @@ func (d *DockerExecutor) CopyFile(containerID string, sourcePath string, destPat return err } if !file.ExistsInTar(sourcePath, tar.NewReader(gzipReader)) { - errMsg := fmt.Sprintf("path %s does not exist (or %s is empty) in archive %s", sourcePath, sourcePath, destPath) + errMsg := fmt.Sprintf("path %s does not exist (or %s is empty) in archive %s", sourcePath, sourcePath, destPath) log.Warn(errMsg) return errors.Errorf(errors.CodeNotFound, errMsg) - } log.Infof("Archiving completed") return nil From 3f258aab50d33da5851c6d4797d7b6e69ef98893 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Sun, 24 Mar 2019 13:15:00 -0700 Subject: [PATCH 09/11] format fix --- workflow/executor/docker/docker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/executor/docker/docker.go b/workflow/executor/docker/docker.go index 6e4191554d8c..9b0c7d9266bb 100644 --- a/workflow/executor/docker/docker.go +++ b/workflow/executor/docker/docker.go @@ -65,7 +65,7 @@ func (d *DockerExecutor) CopyFile(containerID string, sourcePath string, destPat return err } if !file.ExistsInTar(sourcePath, tar.NewReader(gzipReader)) { - errMsg := fmt.Sprintf("path %s does not exist (or %s is empty) in archive %s", sourcePath, sourcePath, destPath) + errMsg := fmt.Sprintf("path %s does not exist (or %s is empty) in archive %s", sourcePath, sourcePath, destPath) log.Warn(errMsg) return errors.Errorf(errors.CodeNotFound, errMsg) } From 516b3151346d773f504bbd4e149fcea4d6f18628 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Tue, 26 Mar 2019 12:46:57 -0700 Subject: [PATCH 10/11] updated comments --- .../expectedfailures/input-artifact-not-optional.yaml | 4 ++-- .../expectedfailures/output-artifact-not-optional.yaml | 2 +- workflow/executor/executor.go | 10 ++++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/test/e2e/expectedfailures/input-artifact-not-optional.yaml b/test/e2e/expectedfailures/input-artifact-not-optional.yaml index 8871bf56e695..e1a3615c71ad 100644 --- a/test/e2e/expectedfailures/input-artifact-not-optional.yaml +++ b/test/e2e/expectedfailures/input-artifact-not-optional.yaml @@ -1,9 +1,9 @@ -# This example demonstrates the input artifacts optionals +# This example demonstrates the input artifacts not optionals # from one step to the next. apiVersion: argoproj.io/v1alpha1 kind: Workflow metadata: - generateName: input-artifact-optional- + generateName: input-artifact-not-optional- spec: entrypoint: http-artifact-example templates: diff --git a/test/e2e/expectedfailures/output-artifact-not-optional.yaml b/test/e2e/expectedfailures/output-artifact-not-optional.yaml index 1a26bbc57ecf..d6fe97da86b6 100644 --- a/test/e2e/expectedfailures/output-artifact-not-optional.yaml +++ b/test/e2e/expectedfailures/output-artifact-not-optional.yaml @@ -1,4 +1,4 @@ -# This example demonstrates the output artifacts optionals +# This example demonstrates the output artifacts not optionals # from one step to the next. apiVersion: argoproj.io/v1alpha1 kind: Workflow diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index 23dd6e0c80cd..9734ff4f2799 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -114,13 +114,15 @@ func (we *WorkflowExecutor) LoadArtifacts() error { log.Infof("Start loading input artifacts...") for _, art := range we.Template.Inputs.Artifacts { + log.Infof("Downloading artifact: %s", art.Name) + + if !art.HasLocation() && art.Optional { + log.Warnf("Artifact doesn't have location. Artifact configured as an optional so, Artifact will be ignored") + continue + } artDriver, err := we.InitDriver(art) if err != nil { - if art.Optional { - log.Warnf("Error in loading Artifacts. Artifact configured as an optional so, Error will be ignored. Error=%v", err) - return nil - } return err } // Determine the file path of where to load the artifact From 120c621e0a69db240bd45e5c3052104c5fbbcd96 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Wed, 27 Mar 2019 06:46:13 -0700 Subject: [PATCH 11/11] improved error case --- workflow/executor/executor.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index 9734ff4f2799..c361f3f3abc4 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -117,9 +117,13 @@ func (we *WorkflowExecutor) LoadArtifacts() error { log.Infof("Downloading artifact: %s", art.Name) - if !art.HasLocation() && art.Optional { - log.Warnf("Artifact doesn't have location. Artifact configured as an optional so, Artifact will be ignored") - continue + if !art.HasLocation() { + if art.Optional { + log.Warnf("Artifact %s is not supplied. Artifact configured as an optional so, Artifact will be ignored", art.Name) + continue + } else { + return errors.New("required artifact %s not supplied", art.Name) + } } artDriver, err := we.InitDriver(art) if err != nil {