entrypoint: in case of step command failure, write postfile

The entrypoint package wraps the step commands and execute them. This
allows use to use pods containers with some order. In a step, the
entrypoint binary will wait for the file of the previous step to be
present to execute the actual command.

Before this change, if a command failed (`exit 1` or something),
entrypoint would not write a file, and thus the whole pod would be
stuck running (all the next step would wait forever).

This fixes that by always writing the post-file — and making
the *waiter* a bit smarter :

- it will now look for a `{postfile}.err` to detect if the previous
  step failed or not.
- if the previous steps failed, it will fail too without executing the
  step commands.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>

Author: vdemeester

cmd/entrypoint/main.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"flag"
+	"fmt"
 	"log"
 	"os"
 	"os/exec"
@@ -36,15 +37,34 @@ var (
 func main() {
 	flag.Parse()
 
-	entrypoint.Entrypointer{
+	e := entrypoint.Entrypointer{
 		Entrypoint: *ep,
 		WaitFile:   *waitFile,
 		PostFile:   *postFile,
 		Args:       flag.Args(),
 		Waiter:     &RealWaiter{},
 		Runner:     &RealRunner{},
 		PostWriter: &RealPostWriter{},
-	}.Go()
+	}
+	if err := e.Go(); err != nil {
+		switch err.(type) {
+		case skipError:
+			os.Exit(0)
+		case *exec.ExitError:
+			// Copied from https://stackoverflow.com/questions/10385551/get-exit-code-go
+			// This works on both Unix and Windows. Although
+			// package syscall is generally platform dependent,
+			// WaitStatus is defined for both Unix and Windows and
+			// in both cases has an ExitStatus() method with the
+			// same signature.
+			if status, ok := err.(*exec.ExitError).Sys().(syscall.WaitStatus); ok {
+				os.Exit(status.ExitStatus())
+			}
+			log.Fatalf("Error executing command (ExitError): %v", err)
+		default:
+			log.Fatalf("Error executing command: %v", err)
+		}
+	}
 }
 
 // TODO(jasonhall): Test that original exit code is propagated and that
@@ -55,15 +75,20 @@ type RealWaiter struct{ waitFile string }
 
 var _ entrypoint.Waiter = (*RealWaiter)(nil)
 
-func (*RealWaiter) Wait(file string) {
+func (*RealWaiter) Wait(file string) error {
 	if file == "" {
-		return
+		return nil
 	}
 	for ; ; time.Sleep(time.Second) {
+		// Watch for the post file
 		if _, err := os.Stat(file); err == nil {
-			return
+			return nil
 		} else if !os.IsNotExist(err) {
-			log.Fatalf("Waiting for %q: %v", file, err)
+			return fmt.Errorf("Waiting for %q: %v", file, err)
+		}
+		// Watch for the post error file
+		if _, err := os.Stat(file + ".err"); err == nil {
+			return skipError("error file present, bail and skip the step")
 		}
 	}
 }
@@ -73,9 +98,9 @@ type RealRunner struct{}
 
 var _ entrypoint.Runner = (*RealRunner)(nil)
 
-func (*RealRunner) Run(args ...string) {
+func (*RealRunner) Run(args ...string) error {
 	if len(args) == 0 {
-		return
+		return nil
 	}
 	name, args := args[0], args[1:]
 
@@ -84,20 +109,9 @@ func (*RealRunner) Run(args ...string) {
 	cmd.Stderr = os.Stderr
 
 	if err := cmd.Run(); err != nil {
-		if exiterr, ok := err.(*exec.ExitError); ok {
-			// Copied from https://stackoverflow.com/questions/10385551/get-exit-code-go
-			// This works on both Unix and Windows. Although
-			// package syscall is generally platform dependent,
-			// WaitStatus is defined for both Unix and Windows and
-			// in both cases has an ExitStatus() method with the
-			// same signature.
-			if status, ok := exiterr.Sys().(syscall.WaitStatus); ok {
-				os.Exit(status.ExitStatus())
-			}
-			log.Fatalf("Error executing command (ExitError): %v", err)
-		}
-		log.Fatalf("Error executing command: %v", err)
+		return err
 	}
+	return nil
 }
 
 // RealPostWriter actually writes files.
@@ -113,3 +127,9 @@ func (*RealPostWriter) Write(file string) {
 		log.Fatalf("Creating %q: %v", file, err)
 	}
 }
+
+type skipError string
+
+func (e skipError) Error() string {
+	return string(e)
+}

pkg/entrypoint/entrypointer.go

@@ -16,6 +16,10 @@ limitations under the License.
 
 package entrypoint
 
+import (
+	"fmt"
+)
+
 // Entrypointer holds fields for running commands with redirected
 // entrypoints.
 type Entrypointer struct {
@@ -41,12 +45,12 @@ type Entrypointer struct {
 // Waiter encapsulates waiting for files to exist.
 type Waiter interface {
 	// Wait blocks until the specified file exists.
-	Wait(file string)
+	Wait(file string) error
 }
 
 // Runner encapsulates running commands.
 type Runner interface {
-	Run(args ...string)
+	Run(args ...string) error
 }
 
 // PostWriter encapsulates writing a file when complete.
@@ -57,17 +61,33 @@ type PostWriter interface {
 
 // Go optionally waits for a file, runs the command, and writes a
 // post file.
-func (e Entrypointer) Go() {
+func (e Entrypointer) Go() error {
 	if e.WaitFile != "" {
-		e.Waiter.Wait(e.WaitFile)
+		if err := e.Waiter.Wait(e.WaitFile); err != nil {
+			// An error happened while waiting, so we bail
+			// *but* we write postfile to make next steps bail too
+			e.WritePostFile(e.PostFile, err)
+			return err
+		}
 	}
 
 	if e.Entrypoint != "" {
 		e.Args = append([]string{e.Entrypoint}, e.Args...)
 	}
-	e.Runner.Run(e.Args...)
 
-	if e.PostFile != "" {
-		e.PostWriter.Write(e.PostFile)
+	err := e.Runner.Run(e.Args...)
+
+	// Write the post file *no matter what*
+	e.WritePostFile(e.PostFile, err)
+
+	return err
+}
+
+func (e Entrypointer) WritePostFile(postFile string, err error) {
+	if err != nil && postFile != "" {
+		postFile = fmt.Sprintf("%s.err", postFile)
+	}
+	if postFile != "" {
+		e.PostWriter.Write(postFile)
 	}
 }

pkg/entrypoint/entrypoint_test.go renamed to pkg/entrypoint/entrypointer_test.go

@@ -17,10 +17,80 @@ limitations under the License.
 package entrypoint
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
+func TestEntrypointerFailures(t *testing.T) {
+	for _, c := range []struct {
+		desc, waitFile, postFile string
+		waiter                   Waiter
+		runner                   Runner
+		expectedError            string
+	}{{
+		desc:          "failing runner with no postFile",
+		runner:        &fakeErrorRunner{},
+		expectedError: "runner failed",
+	}, {
+		desc:          "failing runner with postFile",
+		runner:        &fakeErrorRunner{},
+		expectedError: "runner failed",
+		postFile:      "foo",
+	}, {
+		desc:          "failing waiter with no postFile",
+		waitFile:      "foo",
+		waiter:        &fakeErrorWaiter{},
+		expectedError: "waiter failed",
+	}, {
+		desc:          "failing waiter with postFile",
+		waitFile:      "foo",
+		waiter:        &fakeErrorWaiter{},
+		expectedError: "waiter failed",
+		postFile:      "bar",
+	}} {
+		t.Run(c.desc, func(t *testing.T) {
+			fw := c.waiter
+			if fw == nil {
+				fw = &fakeWaiter{}
+			}
+			fr := c.runner
+			if fr == nil {
+				fr = &fakeRunner{}
+			}
+			fpw := &fakePostWriter{}
+			err := Entrypointer{
+				Entrypoint: "echo",
+				WaitFile:   c.waitFile,
+				PostFile:   c.postFile,
+				Args:       []string{"some", "args"},
+				Waiter:     fw,
+				Runner:     fr,
+				PostWriter: fpw,
+			}.Go()
+			if err == nil {
+				t.Fatalf("Entrpointer didn't fail")
+			}
+			if d := cmp.Diff(c.expectedError, err.Error()); d != "" {
+				t.Errorf("Entrypointer error diff -want, +got: %v", d)
+			}
+
+			if c.postFile != "" {
+				if fpw.wrote == nil {
+					t.Error("Wanted post file written, got nil")
+				} else if *fpw.wrote != c.postFile+".err" {
+					t.Errorf("Wrote post file %q, want %q", *fpw.wrote, c.postFile)
+				}
+			}
+			if c.postFile == "" && fpw.wrote != nil {
+				t.Errorf("Wrote post file when not required")
+			}
+		})
+	}
+}
+
 func TestEntrypointer(t *testing.T) {
 	for _, c := range []struct {
 		desc, entrypoint, waitFile, postFile string
@@ -50,7 +120,7 @@ func TestEntrypointer(t *testing.T) {
 	}} {
 		t.Run(c.desc, func(t *testing.T) {
 			fw, fr, fpw := &fakeWaiter{}, &fakeRunner{}, &fakePostWriter{}
-			Entrypointer{
+			err := Entrypointer{
 				Entrypoint: c.entrypoint,
 				WaitFile:   c.waitFile,
 				PostFile:   c.postFile,
@@ -59,6 +129,9 @@ func TestEntrypointer(t *testing.T) {
 				Runner:     fr,
 				PostWriter: fpw,
 			}.Go()
+			if err != nil {
+				t.Fatalf("Entrypointer failed: %v", err)
+			}
 
 			if c.waitFile != "" {
 				if fw.waited == nil {
@@ -102,12 +175,32 @@ func TestEntrypointer(t *testing.T) {
 
 type fakeWaiter struct{ waited *string }
 
-func (f *fakeWaiter) Wait(file string) { f.waited = &file }
+func (f *fakeWaiter) Wait(file string) error {
+	f.waited = &file
+	return nil
+}
 
 type fakeRunner struct{ args *[]string }
 
-func (f *fakeRunner) Run(args ...string) { f.args = &args }
+func (f *fakeRunner) Run(args ...string) error {
+	f.args = &args
+	return nil
+}
 
 type fakePostWriter struct{ wrote *string }
 
 func (f *fakePostWriter) Write(file string) { f.wrote = &file }
+
+type fakeErrorWaiter struct{ waited *string }
+
+func (f *fakeErrorWaiter) Wait(file string) error {
+	f.waited = &file
+	return fmt.Errorf("waiter failed")
+}
+
+type fakeErrorRunner struct{ args *[]string }
+
+func (f *fakeErrorRunner) Run(args ...string) error {
+	f.args = &args
+	return fmt.Errorf("runner failed")
+}