Move entrypointImage from pkg/…/entrypoint package to cmd/controller

This is part of a set of changes to make sure we don't define CLI
flags in our `pkg/…` packages… so that importing packages from there
do not pollute the cli flags.

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

cmd/controller/main.go

@@ -21,6 +21,7 @@ import (
 
 	"knative.dev/pkg/injection/sharedmain"
 
+	"github.com/tektoncd/pipeline/pkg/reconciler"
 	"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun"
 	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun"
 )
@@ -31,12 +32,17 @@ const (
 )
 
 var (
-	nopImage = flag.String("nop-image", "override-with-nop:latest", "The container image used to kill sidecars")
+	entrypointImage = flag.String("entrypoint-image", "override-with-entrypoint:latest",
+		"The container image containing our entrypoint binary.")
+	nopImage = flag.String("nop-image", "override-with-nop:latest",
+		"The container image used to kill sidecars")
 )
 
 func main() {
-	images := map[string]string{
-		"nopImage": *nopImage,
+	flag.Parse()
+	images := reconciler.Images{
+		EntryPointImage: *entrypointImage,
+		NopImage:        *nopImage,
 	}
 	sharedmain.Main(ControllerLogKey,
 		taskrun.NewController(images),

pkg/reconciler/images.go

@@ -0,0 +1,25 @@
+/*
+Copyright 2019 The Tekton 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 reconciler
+
+// Images holds the images reference for a number of container images used accross tektoncd pipelines
+type Images struct {
+	// EntryPointImage is container image containing our entrypoint binary.
+	EntryPointImage string
+	// NopImage is the container image used to kill sidecars
+	NopImage string
+}

pkg/reconciler/pipelinerun/controller.go

@@ -42,7 +42,7 @@ const (
 	resyncPeriod = 10 * time.Hour
 )
 
-func NewController(images map[string]string) func(context.Context, configmap.Watcher) *controller.Impl {
+func NewController(images reconciler.Images) func(context.Context, configmap.Watcher) *controller.Impl {
 	return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
 		logger := logging.FromContext(ctx)
 		kubeclientset := kubeclient.Get(ctx)

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
+	"github.com/tektoncd/pipeline/pkg/reconciler"
 	"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources"
 	taskrunresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
 	"github.com/tektoncd/pipeline/pkg/system"
@@ -44,8 +45,9 @@ import (
 
 var (
 	ignoreLastTransitionTime = cmpopts.IgnoreTypes(apis.Condition{}.LastTransitionTime.Inner.Time)
-	images                   = map[string]string{
-		"nopImage": "override-with-nop:latest",
+	images                   = reconciler.Images{
+		EntryPointImage: "override-with-entrypoint:latest",
+		NopImage:        "override-with-nop:latest",
 	}
 )
 

pkg/reconciler/reconciler.go

@@ -81,12 +81,12 @@ type Base struct {
 	Logger *zap.SugaredLogger
 
 	// Images contains images to use for certain internal container
-	Images map[string]string
+	Images Images
 }
 
 // NewBase instantiates a new instance of Base implementing
 // the common & boilerplate code between our reconcilers.
-func NewBase(opt Options, controllerAgentName string, images map[string]string) *Base {
+func NewBase(opt Options, controllerAgentName string, images Images) *Base {
 	// Enrich the logs with controller name
 	logger := opt.Logger.Named(controllerAgentName).With(zap.String(logkey.ControllerType, controllerAgentName))
 

pkg/reconciler/reconciler_test.go

@@ -67,7 +67,7 @@ func TestRecorderOptions(t *testing.T) {
 		Logger:            zap.New(observer).Sugar(),
 		KubeClientSet:     c.Kube,
 		PipelineClientSet: c.Pipeline,
-	}, "test", map[string]string{})
+	}, "test", Images{})
 
 	if strings.Compare(reflect.TypeOf(b.Recorder).String(), "*record.recorderImpl") != 0 {
 		t.Errorf("Expected recorder type '*record.recorderImpl' but actual type is: %s", reflect.TypeOf(b.Recorder).String())
@@ -81,7 +81,7 @@ func TestRecorderOptions(t *testing.T) {
 		KubeClientSet:     c.Kube,
 		PipelineClientSet: c.Pipeline,
 		Recorder:          fr,
-	}, "test", map[string]string{})
+	}, "test", Images{})
 
 	if strings.Compare(reflect.TypeOf(b.Recorder).String(), "*record.FakeRecorder") != 0 {
 		t.Errorf("Expected recorder type '*record.FakeRecorder' but actual type is: %s", reflect.TypeOf(b.Recorder).String())

pkg/reconciler/taskrun/controller.go

@@ -42,7 +42,7 @@ const (
 	resyncPeriod = 10 * time.Hour
 )
 
-func NewController(images map[string]string) func(context.Context, configmap.Watcher) *controller.Impl {
+func NewController(images reconciler.Images) func(context.Context, configmap.Watcher) *controller.Impl {
 	return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
 		logger := logging.FromContext(ctx)
 		kubeclientset := kubeclient.Get(ctx)

pkg/reconciler/taskrun/entrypoint/entrypoint.go

@@ -17,7 +17,6 @@ limitations under the License.
 package entrypoint
 
 import (
-	"flag"
 	"fmt"
 	"strconv"
 
@@ -56,10 +55,6 @@ var downwardMount = corev1.VolumeMount{
 	Name:      DownwardMountName,
 	MountPath: DownwardMountPoint,
 }
-var (
-	entrypointImage = flag.String("entrypoint-image", "override-with-entrypoint:latest",
-		"The container image containing our entrypoint binary.")
-)
 
 // Cache is a simple caching mechanism allowing for caching the results of
 // getting the Entrypoint of a container image from a remote registry. The
@@ -96,10 +91,10 @@ func AddToEntrypointCache(c *Cache, sha string, ep []string) {
 // copy the entrypoint binary from the entrypoint image into the
 // volume mounted at MountPoint, so that it can be mounted by
 // subsequent steps and used to capture logs.
-func AddCopyStep(spec *v1alpha1.TaskSpec) {
+func AddCopyStep(entrypointImage string, spec *v1alpha1.TaskSpec) {
 	cp := corev1.Container{
 		Name:    InitContainerName,
-		Image:   *entrypointImage,
+		Image:   entrypointImage,
 		Command: []string{"/bin/sh"},
 		// based on the ko version, the binary could be in one of two different locations
 		Args:         []string{"-c", fmt.Sprintf("cp /ko-app/entrypoint %s", BinaryLocation)},

pkg/reconciler/taskrun/entrypoint/entrypoint_test.go

@@ -461,7 +461,7 @@ func TestAddCopyStep(t *testing.T) {
 	}
 
 	expectedSteps := len(ts.Steps) + 1
-	AddCopyStep(ts)
+	AddCopyStep("override-with-entrypoint:latest", ts)
 	if len(ts.Steps) != 3 {
 		t.Errorf("BuildSpec has the wrong step count: %d should be %d", len(ts.Steps), expectedSteps)
 	}

pkg/reconciler/taskrun/taskrun.go

@@ -126,7 +126,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		c.timeoutHandler.Release(tr)
 		pod, err := c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{})
 		if err == nil {
-			err = sidecars.Stop(pod, c.Images["nopImage"], c.KubeClientSet.CoreV1().Pods(tr.Namespace).Update)
+			err = sidecars.Stop(pod, c.Images.NopImage, c.KubeClientSet.CoreV1().Pods(tr.Namespace).Update)
 		} else if errors.IsNotFound(err) {
 			return merr.ErrorOrNil()
 		}
@@ -448,7 +448,7 @@ func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTask
 		return nil, err
 	}
 
-	ts, err = createRedirectedTaskSpec(c.KubeClientSet, ts, tr, c.cache, c.Logger)
+	ts, err = createRedirectedTaskSpec(c.KubeClientSet, c.Images.EntryPointImage, ts, tr, c.cache, c.Logger)
 	if err != nil {
 		return nil, xerrors.Errorf("couldn't create redirected TaskSpec: %w", err)
 	}
@@ -476,7 +476,7 @@ func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTask
 // an entrypoint cache creates a build where all entrypoints are switched to
 // be the entrypoint redirector binary. This function assumes that it receives
 // its own copy of the TaskSpec and modifies it freely
-func createRedirectedTaskSpec(kubeclient kubernetes.Interface, ts *v1alpha1.TaskSpec, tr *v1alpha1.TaskRun, cache *entrypoint.Cache, logger *zap.SugaredLogger) (*v1alpha1.TaskSpec, error) {
+func createRedirectedTaskSpec(kubeclient kubernetes.Interface, entrypointImage string, ts *v1alpha1.TaskSpec, tr *v1alpha1.TaskRun, cache *entrypoint.Cache, logger *zap.SugaredLogger) (*v1alpha1.TaskSpec, error) {
 	// RedirectSteps the entrypoint in each container so that we can use our custom
 	// entrypoint which copies logs to the volume
 	err := entrypoint.RedirectSteps(cache, ts.Steps, kubeclient, tr, logger)
@@ -486,7 +486,7 @@ func createRedirectedTaskSpec(kubeclient kubernetes.Interface, ts *v1alpha1.Task
 	// Add the step which will copy the entrypoint into the volume
 	// we are going to be using, so that all of the steps will have
 	// access to it.
-	entrypoint.AddCopyStep(ts)
+	entrypoint.AddCopyStep(entrypointImage, ts)
 
 	// Add the volume used for storing the binary and logs
 	ts.Volumes = append(ts.Volumes, corev1.Volume{

pkg/reconciler/taskrun/taskrun_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
+	"github.com/tektoncd/pipeline/pkg/reconciler"
 	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/entrypoint"
 	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
 	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources/cloudevent"
@@ -62,8 +63,9 @@ const (
 )
 
 var (
-	images = map[string]string{
-		"nopImage": "override-with-nop:latest",
+	images = reconciler.Images{
+		EntryPointImage: "override-with-entrypoint:latest",
+		NopImage:        "override-with-nop:latest",
 	}
 	entrypointCache          *entrypoint.Cache
 	ignoreLastTransitionTime = cmpopts.IgnoreTypes(apis.Condition{}.LastTransitionTime.Inner.Time)
@@ -1422,7 +1424,7 @@ func TestCreateRedirectedTaskSpec(t *testing.T) {
 	observer, _ := observer.New(zap.InfoLevel)
 	entrypointCache, _ := entrypoint.NewCache()
 	c := fakekubeclientset.NewSimpleClientset()
-	ts, err := createRedirectedTaskSpec(c, &task.Spec, tr, entrypointCache, zap.New(observer).Sugar())
+	ts, err := createRedirectedTaskSpec(c, "override-with-entrypoint:latest", &task.Spec, tr, entrypointCache, zap.New(observer).Sugar())
 	if err != nil {
 		t.Errorf("expected createRedirectedTaskSpec to pass: %v", err)
 	}