address comments on PR tektoncd#564

- add e2e test to ensure entrypoint runs in the specified order - move entrypoint mount folder to /builder/tools since the /builder folder is already used by other parts of the controller - fix the taskrun order yaml - add more docs about pulling images
nader-ziada · Mar 4, 2019 · d22f5eb · d22f5eb
1 parent 9b72a85
commit d22f5eb
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 60 deletions.
diff --git a/docs/container-contract.md b/docs/container-contract.md
@@ -6,19 +6,18 @@ specific contract.
 ## Entrypoint
 
 When containers are run in a `Task`, the `entrypoint` of the container will be
-overwritten with a custom binary that ensures the containers within the `Task`pod
-are executed in the specified order.
+overwritten with a custom binary that ensures the containers within the `Task`
+pod are executed in the specified order.
 As such, it is always recommended to explicitly specify a command.
 
 When `command` is not explicitly set, the controller will attempt to lookup the
 entrypoint from the remote registry. If the image is a private registry, the
-service account should include an [ImagePullSecret](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#add-imagepullsecrets-to-a-service-account)
-
-Due to this metadata lookup, if you use a private image as a step inside a
-`Task`, the build-pipeline controller needs to be able to access that registry.
-The simplest way to accomplish this is to add a `.docker/config.json` at
-`$HOME/.docker/config.json`, which will then be used by the controller when
-performing the lookup
+service account should include an [ImagePullSecret](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#add-imagepullsecrets-to-a-service-account).
+The build-pipeline controller will use the `ImagePullSecret` of the service
+account, and if service account is empty, `default` is assumed. Next is falling
+back to docker config added in a `.docker/config.json` at `$HOME/.docker/config.json`.
+If none of these credentials are available the controller will try to lookup
+the image anonymously.
 
 For example, in the following Task with the images,
 `gcr.io/cloud-builders/gcloud` and `gcr.io/cloud-builders/docker`, the

diff --git a/examples/taskruns/taskrun-order.yaml b/examples/taskruns/taskrun-order.yaml
@@ -6,8 +6,8 @@ spec:
   taskSpec:
     steps:
     - image: busybox
-      cmd: bash
+      command: ['bash']
       args: ['-c', 'sleep 3 && touch foo']
     - image: busybox
-      cmd: bash
+      command: ['bash']
       args: ['-c', 'ls', 'foo']
diff --git a/pkg/reconciler/v1alpha1/taskrun/entrypoint/entrypoint.go b/pkg/reconciler/v1alpha1/taskrun/entrypoint/entrypoint.go
@@ -41,7 +41,7 @@ const (
 	// MountName is the name of the pvc being mounted (which
 	// will contain the entrypoint binary and eventually the logs)
 	MountName         = "tools"
-	MountPoint        = "/tools"
+	MountPoint        = "/builder/tools"
 	BinaryLocation    = MountPoint + "/entrypoint"
 	JSONConfigEnvVar  = "ENTRYPOINT_OPTIONS"
 	InitContainerName = "place-tools"
@@ -119,7 +119,7 @@ func RedirectSteps(cache *Cache, steps []corev1.Container, kubeclient kubernetes
 			var err error
 			step.Command, err = GetRemoteEntrypoint(cache, step.Image, kubeclient, build)
 			if err != nil {
-				logger.Errorf("**ALERT**: Error getting entry point image", err.Error())
+				logger.Errorf("Error getting entry point image", err.Error())
 				return err
 			}
 		}

diff --git a/pkg/reconciler/v1alpha1/taskrun/entrypoint/entrypoint_test.go b/pkg/reconciler/v1alpha1/taskrun/entrypoint/entrypoint_test.go
@@ -91,7 +91,7 @@ func TestGetArgs(t *testing.T) {
 		args:     []string{"hello", "world"},
 		expectedArgs: []string{
 			"-wait_file", "",
-			"-post_file", "/tools/0",
+			"-post_file", "/builder/tools/0",
 			"-entrypoint", "echo",
 			"--",
 			"hello", "world",
@@ -102,8 +102,8 @@ func TestGetArgs(t *testing.T) {
 		commands: []string{"echo", "hello"},
 		args:     []string{"world"},
 		expectedArgs: []string{
-			"-wait_file", "/tools/3",
-			"-post_file", "/tools/4",
+			"-wait_file", "/builder/tools/3",
+			"-post_file", "/builder/tools/4",
 			"-entrypoint", "echo",
 			"--",
 			"hello", "world",
@@ -114,8 +114,8 @@ func TestGetArgs(t *testing.T) {
 		commands: []string{"ls"},
 		args:     []string{},
 		expectedArgs: []string{
-			"-wait_file", "/tools/3",
-			"-post_file", "/tools/4",
+			"-wait_file", "/builder/tools/3",
+			"-post_file", "/builder/tools/4",
 			"-entrypoint", "ls",
 			"--",
 		},

diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go
@@ -45,7 +45,7 @@ import (
 )
 
 const (
-	entrypointLocation  = "/tools/entrypoint"
+	entrypointLocation  = "/builder/tools/entrypoint"
 	taskNameLabelKey    = pipeline.GroupName + pipeline.TaskLabelKey
 	taskRunNameLabelKey = pipeline.GroupName + pipeline.TaskRunLabelKey
 	workspaceDir        = "/workspace"
@@ -142,7 +142,7 @@ var (
 		tb.Args("-c", fmt.Sprintf("if [[ -d /ko-app ]]; then cp /ko-app/entrypoint %s; else cp /ko-app %s;  fi;", entrypointLocation, entrypointLocation)),
 		tb.WorkingDir(workspaceDir),
 		tb.EnvVar("HOME", "/builder/home"),
-		tb.VolumeMount("tools", "/tools"),
+		tb.VolumeMount("tools", "/builder/tools"),
 		tb.VolumeMount("workspace", workspaceDir),
 		tb.VolumeMount("home", "/builder/home"),
 	)
@@ -299,10 +299,10 @@ func TestReconcile(t *testing.T) {
 				placeToolsInitContainer,
 				tb.PodContainer("build-step-simple-step", "foo",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "", "-post_file", "/tools/0", "-entrypoint", "/mycmd", "--"),
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/mycmd", "--"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
@@ -326,10 +326,10 @@ func TestReconcile(t *testing.T) {
 				placeToolsInitContainer,
 				tb.PodContainer("build-step-sa-step", "foo",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "", "-post_file", "/tools/0", "-entrypoint", "/mycmd", "--"),
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/mycmd", "--"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
@@ -358,32 +358,32 @@ func TestReconcile(t *testing.T) {
 				placeToolsInitContainer,
 				tb.PodContainer("build-step-git-source-git-resource-9l9zj", "override-with-git:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "", "-post_file", "/tools/0", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/ko-app", "--",
 						"-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-mycontainer", "myimage",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/0", "-post_file", "/tools/1", "-entrypoint", "/mycmd", "--",
+					tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "/mycmd", "--",
 						"--my-arg=foo", "--my-arg-with-default=bar", "--my-arg-with-default2=thedefault",
 						"--my-additional-arg=gcr.io/kristoff/sven"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-myothercontainer", "myotherimage",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/1", "-post_file", "/tools/2", "-entrypoint", "/mycmd", "--",
+					tb.Args("-wait_file", "/builder/tools/1", "-post_file", "/builder/tools/2", "-entrypoint", "/mycmd", "--",
 						"--my-other-arg=https://foo.git"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
@@ -414,74 +414,74 @@ func TestReconcile(t *testing.T) {
 				placeToolsInitContainer,
 				tb.PodContainer("build-step-create-dir-another-git-resource-78c5n", "override-with-bash-noop:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "", "-post_file", "/tools/0", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/ko-app", "--",
 						"-args", "mkdir -p /workspace/another-git-resource"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-source-copy-another-git-resource-mssqb", "override-with-bash-noop:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/0", "-post_file", "/tools/1", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "/ko-app", "--",
 						"-args", "cp -r source-folder/. /workspace/another-git-resource"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
 					tb.VolumeMount("test-pvc", "/pvc"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-create-dir-git-resource-mz4c7", "override-with-bash-noop:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/1", "-post_file", "/tools/2", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "/builder/tools/1", "-post_file", "/builder/tools/2", "-entrypoint", "/ko-app", "--",
 						"-args", "mkdir -p /workspace/git-resource"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-source-copy-git-resource-9l9zj", "override-with-bash-noop:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/2", "-post_file", "/tools/3", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "/builder/tools/2", "-post_file", "/builder/tools/3", "-entrypoint", "/ko-app", "--",
 						"-args", "cp -r source-folder/. /workspace/git-resource"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
 					tb.VolumeMount("test-pvc", "/pvc"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-simple-step", "foo",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/3", "-post_file", "/tools/4", "-entrypoint", "/mycmd", "--"),
+					tb.Args("-wait_file", "/builder/tools/3", "-post_file", "/builder/tools/4", "-entrypoint", "/mycmd", "--"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-source-mkdir-git-resource-6nl7g", "override-with-bash-noop:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/4", "-post_file", "/tools/5", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "/builder/tools/4", "-post_file", "/builder/tools/5", "-entrypoint", "/ko-app", "--",
 						"-args", "mkdir -p output-folder"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
 					tb.VolumeMount("test-pvc", "/pvc"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-source-copy-git-resource-j2tds", "override-with-bash-noop:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/5", "-post_file", "/tools/6", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "/builder/tools/5", "-post_file", "/builder/tools/6", "-entrypoint", "/ko-app", "--",
 						"-args", "cp -r /workspace/git-resource/. output-folder"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
 					tb.VolumeMount("test-pvc", "/pvc"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
@@ -502,22 +502,22 @@ func TestReconcile(t *testing.T) {
 				getCredentialsInitContainer("mz4c7"),
 				tb.PodContainer("build-step-git-source-git-resource-9l9zj", "override-with-git:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "", "-post_file", "/tools/0", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/ko-app", "--",
 						"-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				placeToolsInitContainer,
 				tb.PodContainer("build-step-mycontainer", "myimage",
 					tb.Command(entrypointLocation),
 					tb.WorkingDir(workspaceDir),
-					tb.Args("-wait_file", "/tools/0", "-post_file", "/tools/1", "-entrypoint", "/mycmd", "--",
+					tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "/mycmd", "--",
 						"--my-arg=foo"),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
@@ -540,10 +540,10 @@ func TestReconcile(t *testing.T) {
 				placeToolsInitContainer,
 				tb.PodContainer("build-step-simple-step", "foo",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "", "-post_file", "/tools/0", "-entrypoint", "/mycmd", "--"),
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/mycmd", "--"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
@@ -565,21 +565,21 @@ func TestReconcile(t *testing.T) {
 				placeToolsInitContainer,
 				tb.PodContainer("build-step-git-source-workspace-9l9zj", "override-with-git:latest",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "", "-post_file", "/tools/0", "-entrypoint", "/ko-app", "--",
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/ko-app", "--",
 						"-url", "github.com/build-pipeline.git", "-revision", "rel-can", "-path",
 						"/workspace/workspace"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
 				tb.PodContainer("build-step-mystep", "ubuntu",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "/tools/0", "-post_file", "/tools/1", "-entrypoint", "/mycmd", "--"),
+					tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "/mycmd", "--"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),
@@ -603,10 +603,10 @@ func TestReconcile(t *testing.T) {
 				placeToolsInitContainer,
 				tb.PodContainer("build-step-simple-step", "foo",
 					tb.Command(entrypointLocation),
-					tb.Args("-wait_file", "", "-post_file", "/tools/0", "-entrypoint", "/mycmd", "--"),
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/mycmd", "--"),
 					tb.WorkingDir(workspaceDir),
 					tb.EnvVar("HOME", "/builder/home"),
-					tb.VolumeMount("tools", "/tools"),
+					tb.VolumeMount("tools", "/builder/tools"),
 					tb.VolumeMount("workspace", workspaceDir),
 					tb.VolumeMount("home", "/builder/home"),
 				),

diff --git a/test/entrypoint_remote_test.go → test/entrypoint_test.go b/test/entrypoint_remote_test.go → test/entrypoint_test.go
@@ -28,10 +28,11 @@ const (
 	epTaskRunName = "ep-task-run"
 )
 
-// TestEntrypointRemoteImage is an integration test that will
+// TestEntrypointRunningStepsInOrder is an integration test that will
 // verify attempt to the get the entrypoint of a container image
-// that doesn't have a cmd defined.
-func TestEntrypointRemoteImage(t *testing.T) {
+// that doesn't have a cmd defined. In addition to making sure the steps
+// are executed in the order specified
+func TestEntrypointRunningStepsInOrder(t *testing.T) {
 	logger := logging.GetContextLogger(t.Name())
 	c, namespace := setup(t, logger)
 	t.Parallel()
@@ -41,7 +42,8 @@ func TestEntrypointRemoteImage(t *testing.T) {
 
 	logger.Infof("Creating Task and TaskRun in namespace %s", namespace)
 	task := tb.Task(epTaskName, namespace, tb.TaskSpec(
-		tb.Step("foo", "ubuntu", tb.Args("-c", "ls", "/workspace")),
+		tb.Step("step1", "ubuntu", tb.Args("-c", "sleep 3 && touch foo")),
+		tb.Step("step2", "ubuntu", tb.Args("-c", "ls", "foo")),
 	))
 	if _, err := c.TaskClient.Create(task); err != nil {
 		t.Fatalf("Failed to create Task: %s", err)