From b7d5e2198b793ec1294cde541c7696493b3cf58a Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Tue, 5 Mar 2019 16:30:25 -0800 Subject: [PATCH 1/2] Issue#1165 fake outputs don't notify and task completes successfully This PR is addressing the Issue#1165 reported by @alexfrieden. Issue/Bug: Argo is finishing the task successfully even artifact /file does exist. Fix: Validate the created gzip contains artifact or file. if file/artifact doesn't exist, Current step/stage/task will be failed with log message . Sample Log: ''' INFO[0029] Updating node artifact-passing-lkvj8[0].generate-artifact (artifact-passing-lkvj8-1949982165) status Running -> Error INFO[0029] Updating node artifact-passing-lkvj8[0].generate-artifact (artifact-passing-lkvj8-1949982165) message: failed to save outputs: File or Artifact does not exist. /tmp/hello_world.txt INFO[0029] Step group node artifact-passing-lkvj8[0] (artifact-passing-lkvj8-1067333159) deemed failed: child 'artifact-passing-lkvj8-1949982165' failed namespace=default workflow=artifact-passing-lkvj8 INFO[0029] node artifact-passing-lkvj8[0] (artifact-passing-lkvj8-1067333159) phase Running -> Failed namespace=default workflow=artifact-passing-lkvj8 ''' --- workflow/executor/docker/docker.go | 7 +++++ workflow/util/file/fileutil.go | 43 ++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 workflow/util/file/fileutil.go diff --git a/workflow/executor/docker/docker.go b/workflow/executor/docker/docker.go index 980527c523b4..625359435f60 100644 --- a/workflow/executor/docker/docker.go +++ b/workflow/executor/docker/docker.go @@ -7,6 +7,8 @@ import ( "strings" "time" + "github.com/argoproj/argo/workflow/util/file" + "github.com/argoproj/argo/util" "github.com/argoproj/argo/errors" @@ -51,6 +53,11 @@ func (d *DockerExecutor) CopyFile(containerID string, sourcePath string, destPat if err != nil { return err } + if !file.IsFileOrDirExistInGZip(sourcePath, destPath) { + errMsg := fmt.Sprintf("File or Artifact does not exist. %s", sourcePath) + log.Warn(errMsg) + return errors.InternalError(errMsg) + } log.Infof("Archiving completed") return nil } diff --git a/workflow/util/file/fileutil.go b/workflow/util/file/fileutil.go new file mode 100644 index 000000000000..81a05e360229 --- /dev/null +++ b/workflow/util/file/fileutil.go @@ -0,0 +1,43 @@ +package file + +import ( + "archive/tar" + "compress/gzip" + "io" + "os" + "strings" +) + +//IsFileOrDirExistInGZip return true if file or directory exists in GZip file +func IsFileOrDirExistInGZip(sourcePath string, gzipFilePath string) bool { + + fi, err := os.Open(gzipFilePath) + + if os.IsNotExist(err) { + return false + } + defer fi.Close() + + fz, err := gzip.NewReader(fi) + if err != nil { + return false + } + defer fz.Close() + tr := tar.NewReader(fz) + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return false + } + if hdr.FileInfo().IsDir() && strings.Contains(strings.Trim(hdr.Name, "/"), strings.Trim(sourcePath, "/")) { + return true + } + if strings.Contains(sourcePath, hdr.Name) && hdr.Size > 0 { + return true + } + } + return false +} From c354afa41818b78f448e50d257cfd6384bd46352 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Tue, 5 Mar 2019 19:14:55 -0800 Subject: [PATCH 2/2] fixed gometalinter errcheck issue --- workflow/util/file/fileutil.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/workflow/util/file/fileutil.go b/workflow/util/file/fileutil.go index 81a05e360229..ab3b325adee9 100644 --- a/workflow/util/file/fileutil.go +++ b/workflow/util/file/fileutil.go @@ -6,6 +6,8 @@ import ( "io" "os" "strings" + + log "github.com/sirupsen/logrus" ) //IsFileOrDirExistInGZip return true if file or directory exists in GZip file @@ -16,13 +18,12 @@ func IsFileOrDirExistInGZip(sourcePath string, gzipFilePath string) bool { if os.IsNotExist(err) { return false } - defer fi.Close() + defer closeFile(fi) fz, err := gzip.NewReader(fi) if err != nil { return false } - defer fz.Close() tr := tar.NewReader(fz) for { hdr, err := tr.Next() @@ -30,6 +31,7 @@ func IsFileOrDirExistInGZip(sourcePath string, gzipFilePath string) bool { break } if err != nil { + return false } if hdr.FileInfo().IsDir() && strings.Contains(strings.Trim(hdr.Name, "/"), strings.Trim(sourcePath, "/")) { @@ -41,3 +43,10 @@ func IsFileOrDirExistInGZip(sourcePath string, gzipFilePath string) bool { } return false } + +func closeFile(f *os.File) { + err := f.Close() + if err != nil { + log.Warn("Failed to close the file. v%", err) + } +}