remove entrypoint/wrapper

cmd/entrypoint/main.go

@@ -63,5 +63,5 @@ func main() {
  logger.Fatalf("Invalid options: %v", err)
  }
 
- os.Exit(o.Run())
+ os.Exit(o.Run(logger))
 }

pkg/entrypoint/options.go

@@ -20,25 +20,53 @@ import (
  "encoding/json"
  "errors"
  "flag"
-
- "github.com/knative/build/pkg/entrypoint/wrapper"
+ "fmt"
 )
 
-// NewOptions returns an empty Options with no nil fields
-func NewOptions() *Options {
- return &Options{
- Options: &wrapper.Options{},
- }
-}
-
-// Options exposes the configuration necessary
-// for defining the process being watched and
-// where in the image repository an upload will land.
+// Options exposes the configuration options
+// used when wrapping test execution
 type Options struct {
  // Args is the process and args to run
  Args []string `json:"args"`
 
- *wrapper.Options
+ // ShouldWaitForPrevStep will be written with the exit code
+ // of the test process or an internal error code
+ // if the entrypoint fails.
+ ShouldWaitForPrevStep bool `json:"shouldWaitForPrevStep"`
+
+ // PreRunFile will be written with the exit code
+ // of the test process or an internal error code
+ // if the entrypoint fails.
+ PreRunFile string `json:"preRunFile"`
+
+ // ShouldWaitForPrevStep will be written with the exit code
+ // of the test process or an internal error code
+ // if the entrypoint fails.
+ ShouldRunPostRun bool `json:"shouldRunPostRun"`
+
+ // PostRunFile will be written with the exit code
+ // of the test process or an internal error code
+ // if the entrypoint fails or if it succeeds
+ PostRunFile string `json:"postRunFile"`
+}
+
+// NewOptions returns an empty Options with no nil fields
+func NewOptions() *Options {
+ return &Options{}
+}
+
+// AddFlags adds flags to the FlagSet that populate
+// the wrapper options struct provided.
+func (o *Options) AddFlags(flags *flag.FlagSet) {
+ flags.BoolVar(&o.ShouldWaitForPrevStep, "should-wait-for-prev-step",
+ DefaultShouldWaitForPrevStep, "If we should wait for prev step.")
+ flags.BoolVar(&o.ShouldRunPostRun, "should-run-post-run",
+ DefaultShouldRunPostRun, "If the post run step should be run after execution finishes.")
+ flags.StringVar(&o.PreRunFile, "prerun-file",
+ DefaultPreRunFile, "The path of the file that acts as a lock for the entrypoint. The entrypoint binary will wait until that file is present to launch the specified command.")
+ flags.StringVar(&o.PostRunFile, "postrun-file",
+ DefaultPostRunFile, "The path of the file that will be written once the command finishes for the entrypoint. This can act as a lock for other entrypoint rewritten containers.")
+ return
 }
 
 // Validate ensures that the set of options are
@@ -48,7 +76,13 @@ func (o *Options) Validate() error {
  return errors.New("no process to wrap specified")
  }
 
- return o.Options.Validate()
+ if o.PreRunFile != "" && !o.ShouldWaitForPrevStep {
+ return fmt.Errorf("PreRunFile specified but ShouldWaitForPrevStep is false")
+ }
+ if o.PostRunFile != "" && !o.ShouldRunPostRun {
+ return fmt.Errorf("PostRunFile specified but ShouldRunPostRun is false")
+ }
+ return nil
 }
 
 const (
@@ -69,19 +103,6 @@ func (o *Options) LoadConfig(config string) error {
  return json.Unmarshal([]byte(config), o)
 }
 
-// AddFlags binds flags to options
-func (o *Options) AddFlags(flags *flag.FlagSet) {
- flags.BoolVar(&o.ShouldWaitForPrevStep, "should-wait-for-prev-step",
- DefaultShouldWaitForPrevStep, "If we should wait for prev step.")
- flags.BoolVar(&o.ShouldRunPostRun, "should-run-post-run",
- DefaultShouldRunPostRun, "If the post run step should be run after execution finishes.")
- flags.StringVar(&o.PreRunFile, "prerun-file",
- DefaultPreRunFile, "The path of the file that acts as a lock for the entrypoint. The entrypoint binary will wait until that file is present to launch the specified command.")
- flags.StringVar(&o.PostRunFile, "postrun-file",
- DefaultPostRunFile, "The path of the file that will be written once the command finishes for the entrypoint. This can act as a lock for other entrypoint rewritten containers.")
- o.Options.AddFlags(flags)
-}
-
 // Complete internalizes command line arguments
 func (o *Options) Complete(args []string) {
  o.Args = args

pkg/entrypoint/options_test.go

@@ -18,8 +18,6 @@ package entrypoint
 
 import (
  "testing"
-
- "github.com/knative/build/pkg/entrypoint/wrapper"
 )
 
 func TestOptions_Validate(t *testing.T) {
@@ -30,21 +28,29 @@ func TestOptions_Validate(t *testing.T) {
  }{{
  name: "all ok",
  input: Options{
- Args: []string{"/usr/bin/true"},
- Options: &wrapper.Options{
- ProcessLog: "output.txt",
- MarkerFile: "marker.txt",
- },
+ Args: []string{"/usr/bin/true"},
+ ShouldRunPostRun: true,
  },
- }, {
- name: "missing args",
+ }}
+
+ for _, testCase := range testCases {
+ if err := testCase.input.Validate(); testCase.expectedErr != (err != nil) {
+ t.Errorf("%s: expected error to be %v but got %v", testCase.name, testCase.expectedErr, err)
+ }
+ }
+}
+
+func TestOptions_AddFlags(t *testing.T) {
+ var testCases = []struct {
+ name string
+ input Options
+ expectedErr bool
+ }{{
+ name: "all ok",
  input: Options{
- Options: &wrapper.Options{
- ProcessLog: "output.txt",
- MarkerFile: "marker.txt",
- },
+ Args: []string{"/usr/bin/true"},
+ ShouldRunPostRun: true,
  },
- expectedErr: true,
  }}
 
  for _, testCase := range testCases {

pkg/entrypoint/run.go

@@ -21,11 +21,9 @@ import (
  "io/ioutil"
  "os"
  "os/exec"
- "path/filepath"
  "strconv"
  "syscall"
 
- "github.com/knative/pkg/logging"
  "go.uber.org/zap"
 )
 
@@ -55,7 +53,7 @@ const (
 
 // Run executes the test process then writes the exit code to the marker file.
 // This function returns the status code that should be passed to os.Exit().
-func (o Options) Run(*zap.SugaredLogger logger) int {
+func (o Options) Run(logger *zap.SugaredLogger) int {
  defer logger.Sync()
 
  code, err := o.ExecuteProcess()
@@ -94,10 +92,6 @@ func (o Options) ExecuteProcess() (int, error) {
  }()
  select {
  case commandErr = <-done:
- // execute post run action if specified
- if o.ShouldRunPostRun {
- o.postRunWriteFile(0)
- }
  }
 
  var returnCode int
@@ -112,6 +106,11 @@ func (o Options) ExecuteProcess() (int, error) {
  if returnCode != 0 {
  commandErr = fmt.Errorf("wrapped process failed: %v", commandErr)
  }
+
+ if o.ShouldRunPostRun {
+ o.postRunWriteFile(returnCode)
+ }
+
  return returnCode, commandErr
 }
 
@@ -133,26 +132,10 @@ func (o *Options) waitForPrevStep() error {
 func (o *Options) postRunWriteFile(exitCode int) error {
  content := []byte(strconv.Itoa(exitCode))
 
- // create temp file in the same directory as the desired marker file
- dir := filepath.Dir(o.PostRunFile)
- tempFile, err := ioutil.TempFile(dir, "temp-marker")
+ err := ioutil.WriteFile(o.PostRunFile, content, os.ModePerm)
  if err != nil {
- return fmt.Errorf("could not create temp marker file in %s: %v", dir, err)
- }
- // write the exit code to the tempfile, sync to disk and close
- if _, err = tempFile.Write(content); err != nil {
- return fmt.Errorf("could not write to temp marker file (%s): %v", tempFile.Name(), err)
- }
- if err = tempFile.Sync(); err != nil {
- return fmt.Errorf("could not sync temp marker file (%s): %v", tempFile.Name(), err)
- }
- tempFile.Close()
- // set desired permission bits, then rename to the desired file name
- if err = os.Chmod(tempFile.Name(), os.ModePerm); err != nil {
- return fmt.Errorf("could not chmod (%x) temp marker file (%s): %v", os.ModePerm, tempFile.Name(), err)
- }
- if err := os.Rename(tempFile.Name(), o.PostRunFile); err != nil {
- return fmt.Errorf("could not move marker file to destination path (%s): %v", o.PostRunFile, err)
+ return fmt.Errorf("error writing PostRunFile (%s): %v", o.PostRunFile, err)
  }
+
  return nil
 }

pkg/entrypoint/run_test.go

@@ -19,38 +19,42 @@ package entrypoint
 import (
  "io/ioutil"
  "os"
- "path"
+ "path/filepath"
  "testing"
 
- "github.com/knative/build/pkg/entrypoint/wrapper"
+ "github.com/knative/pkg/logging"
 )
 
 func TestOptions_Run(t *testing.T) {
  var testCases = []struct {
- name string
- args []string
- expectedShouldWaitForPrevStep bool
- expectedPreRunFile string
- expectedPostRunFile string
- expectedShouldRunPostRun bool
+ name string
+ options *Options
+ expectedPreRunFileContents string
+ expectedPostRunFileContents string
  }{
  {
- name: "successful command",
- args: []string{"sh", "-c", "exit 0"},
- expectedShouldRunPostRun: true,
- expectedPostRunFile: "0",
+ name: "successful_command",
+ options: &Options{
+ Args: []string{"sh", "-c", "exit 0"},
+ ShouldWaitForPrevStep: true,
+ PreRunFile: "0",
+ ShouldRunPostRun: true,
+ PostRunFile: "1",
+ },
+ expectedPreRunFileContents: "0",
+ expectedPostRunFileContents: "0",
  },
  {
- name: "successful command with output",
- args: []string{"echo", "test"},
- },
- {
- name: "unsuccessful command",
- args: []string{"sh", "-c", "exit 12"},
- },
- {
- name: "unsuccessful command with output",
- args: []string{"sh", "-c", "echo test && exit 12"},
+ name: "unsuccessful_command",
+ options: &Options{
+  Args: []string{"sh", "-c", "exit 1"},
+  ShouldWaitForPrevStep: true,
+  PreRunFile: "0",
+  ShouldRunPostRun: true,
+  PostRunFile: "1",
+  },
+ expectedPreRunFileContents: "0",
+ expectedPostRunFileContents: "1",
  },
  }
 
@@ -65,22 +69,31 @@ func TestOptions_Run(t *testing.T) {
  t.Errorf("%s: error cleaning up temp dir: %v", testCase.name, err)
  }
  }()
+ options := testCase.options
+
+ // reset paths to new temp dir
+ options.PreRunFile = filepath.Join(tmpDir, options.PreRunFile)
+ options.PostRunFile = filepath.Join(tmpDir, options.PostRunFile)
 
- options := Options{
- Args: testCase.args,
- Options: &wrapper.Options{
- ShouldWaitForPrevStep: false,
- PreRunFile: path.Join(tmpDir, "0"),
- PostRunFile: path.Join(tmpDir, "0"),
- },
+ if options.ShouldWaitForPrevStep {
+ // write the temp file it should wait for
+ err := ioutil.WriteFile(options.PreRunFile, []byte("0"), os.ModePerm)
+ if err != nil {
+ t.Errorf("%s: error writing file to temp dir: %v", testCase.name, err)
+ }
  }
+
+ logger, _ := logging.NewLogger("", "entrypoint")
+ defer logger.Sync()
+
+ options.Run(logger)
  if options.ShouldWaitForPrevStep {
  compareFileContents(testCase.name, options.PreRunFile,
- testCase.expectedPreRunFile, t)
+ testCase.expectedPreRunFileContents, t)
  }
  if options.ShouldRunPostRun {
  compareFileContents(testCase.name, options.PostRunFile,
- testCase.expectedPostRunFile, t)
+ testCase.expectedPostRunFileContents, t)
  }
  })
  }