diff --git a/cmd/entrypoint/main.go b/cmd/entrypoint/main.go index 3f83b8e0..b276c5cd 100644 --- a/cmd/entrypoint/main.go +++ b/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)) } diff --git a/pkg/entrypoint/options.go b/pkg/entrypoint/options.go index e0e34e25..00b65893 100644 --- a/pkg/entrypoint/options.go +++ b/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 diff --git a/pkg/entrypoint/options_test.go b/pkg/entrypoint/options_test.go index b4dcdeab..a02fc6e0 100644 --- a/pkg/entrypoint/options_test.go +++ b/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 { diff --git a/pkg/entrypoint/run.go b/pkg/entrypoint/run.go index dec34033..7383d9f8 100644 --- a/pkg/entrypoint/run.go +++ b/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 } diff --git a/pkg/entrypoint/run_test.go b/pkg/entrypoint/run_test.go index 21d621fd..9c8ea043 100644 --- a/pkg/entrypoint/run_test.go +++ b/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) } }) } diff --git a/pkg/entrypoint/wrapper/doc.go b/pkg/entrypoint/wrapper/doc.go deleted file mode 100644 index 07e77be1..00000000 --- a/pkg/entrypoint/wrapper/doc.go +++ /dev/null @@ -1,19 +0,0 @@ -/* -Copyright 2018 The Knative Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package wrapper contains utilities for the processes that -// wrap the test execution in a ProwJob test container -package wrapper diff --git a/pkg/entrypoint/wrapper/options.go b/pkg/entrypoint/wrapper/options.go deleted file mode 100644 index 1cbfe39b..00000000 --- a/pkg/entrypoint/wrapper/options.go +++ /dev/null @@ -1,57 +0,0 @@ -/* -Copyright 2018 The Knative Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package wrapper - -import ( - "flag" -) - -// Options exposes the configuration options -// used when wrapping test execution -type Options struct { - // 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"` -} - -// AddFlags adds flags to the FlagSet that populate -// the wrapper options struct provided. -func (o *Options) AddFlags(fs *flag.FlagSet) { - return -} - -// Validate ensures that the set of options are -// self-consistent and valid -func (o *Options) Validate() error { - return nil -} diff --git a/pkg/entrypoint/wrapper/options_test.go b/pkg/entrypoint/wrapper/options_test.go deleted file mode 100644 index d15524f2..00000000 --- a/pkg/entrypoint/wrapper/options_test.go +++ /dev/null @@ -1,62 +0,0 @@ -/* -Copyright 2018 The Knative Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package wrapper - -import ( - "testing" -) - -func TestOptions_Validate(t *testing.T) { - var testCases = []struct { - name string - input Options - expectedErr bool - }{ - { - name: "all ok", - input: Options{ - ProcessLog: "output.txt", - MarkerFile: "marker.txt", - }, - expectedErr: false, - }, - { - name: "no process log", - input: Options{ - MarkerFile: "marker.txt", - }, - expectedErr: true, - }, - { - name: "no marker file", - input: Options{ - ProcessLog: "output.txt", - }, - expectedErr: true, - }, - } - - for _, testCase := range testCases { - err := testCase.input.Validate() - if testCase.expectedErr && err == nil { - t.Errorf("%s: expected an error but got none", testCase.name) - } - if !testCase.expectedErr && err != nil { - t.Errorf("%s: expected no error but got one: %v", testCase.name, err) - } - } -}