PR fixes

cmd/entrypoint/main.go

@@ -25,6 +25,25 @@ import (
 	"k8s.io/test-infra/prow/logrusutil"
 )
 
+/*
+The tool is used to rewrite the entrypoint of a container image.
+To override the base shell image update `.ko.yaml` file.
+
+To use it, run
+```
+image: github.com/knative/build/cmd/entrypoint
+```
+
+It used in knative/build as a method of running containers in
+order that are in the same pod this is done by:
+1) for the Pod(containing user Steps) created by a Build,
+create a shared directory with the entrypoint binary
+2) change the entrypoint of all the user specified containers in Steps to be the
+entrypoint binary with configuration to run the user specified entrypoint with some custom logic
+3) one piece of "custom logic" is having the entrypoint binary wait for the previous step
+as seen in knative/build/pkg/entrypoint/run.go -- waitForPrevStep()
+*/
+
 func main() {
 	o := entrypoint.NewOptions()
 	if err := options.Load(o); err != nil {

pkg/entrypoint/options.go

@@ -20,7 +20,6 @@ import (
 	"encoding/json"
 	"errors"
 	"flag"
-	"time"
 
 	"github.com/knative/build/pkg/entrypoint/wrapper"
 )
@@ -38,18 +37,6 @@ func NewOptions() *Options {
 type Options struct {
 	// Args is the process and args to run
 	Args []string `json:"args"`
-	// Timeout determines how long to wait before the
-	// entrypoint sends SIGINT to the process
-	Timeout time.Duration `json:"timeout"`
-	// GracePeriod determines how long to wait after
-	// sending SIGINT before the entrypoint sends
-	// SIGKILL.
-	GracePeriod time.Duration `json:"grace_period"`
-	// ArtifactDir is a directory where test processes can dump artifacts
-	// for upload to persistent storage (courtesy of sidecar).
-	// If specified, it is created by entrypoint before starting the test process.
-	// May be ignored if not using sidecar.
-	ArtifactDir string `json:"artifact_dir,omitempty"`
 
 	*wrapper.Options
 }
@@ -84,21 +71,14 @@ func (o *Options) LoadConfig(config string) error {
 
 // AddFlags binds flags to options
 func (o *Options) AddFlags(flags *flag.FlagSet) {
-	flags.DurationVar(&o.Timeout, "timeout",
-		DefaultTimeout, "Timeout for the test command.")
-	flags.DurationVar(&o.GracePeriod, "grace-period",
-		DefaultGracePeriod, "Grace period after timeout for the test command.")
-	flags.StringVar(&o.ArtifactDir, "artifact-dir",
-		"", "directory where test artifacts should be placed for upload "+
-			"to persistent storage")
 	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 post run step should be run.")
+		DefaultShouldRunPostRun, "If the post run step should be run after execution finishes.")
 	flags.StringVar(&o.PreRunFile, "prerun-file",
-		DefaultPreRunFile, "The prerun file to wait for.")
+		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, "If postrun file to write.")
+		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)
 }
 

pkg/entrypoint/options/load_test.go

@@ -0,0 +1,67 @@
+/*
+Copyright 2017 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 options
+
+import (
+	"flag"
+	"os"
+	"testing"
+
+	"github.com/knative/build/pkg/entrypoint"
+)
+
+const (
+	TestEnvVar = "TEST_ENV_VAR"
+)
+
+type TestOptions struct {
+	*entrypoint.Options
+}
+
+func (o *TestOptions) ConfigVar() string {
+	return TestEnvVar
+}
+
+func (o *TestOptions) AddFlags(flags *flag.FlagSet) {
+	// Required to reset os.Args[1:] values used in Load()
+	os.Args[1] = ""
+	return
+}
+
+func (o *TestOptions) Complete(args []string) {
+	return
+}
+
+func TestOptions_Load(t *testing.T) {
+	tt := []struct {
+		name   string
+		envmap map[string]string
+		in     OptionLoader
+		err    error
+	}{
+		{"successful load", map[string]string{TestEnvVar: "hello"}, &TestOptions{}, nil},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			err := Load(tc.in)
+			if tc.err != err {
+				t.Errorf("expected err to be %v; got %v", tc.err, err)
+			}
+		})
+	}
+}

pkg/entrypoint/options_test.go

@@ -27,37 +27,29 @@ func TestOptions_Validate(t *testing.T) {
 		name        string
 		input       Options
 		expectedErr bool
-	}{
-		{
-			name: "all ok",
-			input: Options{
-				Args: []string{"/usr/bin/true"},
-				Options: &wrapper.Options{
-					ProcessLog: "output.txt",
-					MarkerFile: "marker.txt",
-				},
+	}{{
+		name: "all ok",
+		input: Options{
+			Args: []string{"/usr/bin/true"},
+			Options: &wrapper.Options{
+				ProcessLog: "output.txt",
+				MarkerFile: "marker.txt",
 			},
-			expectedErr: false,
 		},
-		{
-			name: "missing args",
-			input: Options{
-				Options: &wrapper.Options{
-					ProcessLog: "output.txt",
-					MarkerFile: "marker.txt",
-				},
+	}, {
+		name: "missing args",
+		input: Options{
+			Options: &wrapper.Options{
+				ProcessLog: "output.txt",
+				MarkerFile: "marker.txt",
 			},
-			expectedErr: true,
 		},
-	}
+		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)
+		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)
 		}
 	}
 }