Optimize start time for TaskRuns with no sidecars

config/config-feature-flags.yaml

@@ -41,3 +41,11 @@ data:
   # See https://github.com/tektoncd/pipeline/issues/1836 for more
   # info.
   disable-working-directory-overwrite: "false"
+  # This option should be set to false when Pipelines is running in a
+  # cluster that does not use injected sidecars such as Istio. Setting
+  # it to false should decrease the time it takes for a TaskRun to start
+  # running. For clusters that use injected sidecars, setting this
+  # option to false can lead to unexpected behavior.
+  #
+  # See https://github.com/tektoncd/pipeline/issues/2080 for more info.
+  running-in-environment-with-injected-sidecars: "true"

docs/install.md

@@ -278,6 +278,12 @@ The default value is `false`, which causes Tekton to override the working direct
 for each `Step` that does not have its working directory explicitly set with `/workspace`.
 For more information, see the [associated issue](https://github.com/tektoncd/pipeline/issues/1836).
 
+- `running-in-environment-with-injected-sidecars`: set this flag to `"true"` to allow the
+Tekton controller to set the `tekon.dev/ready` annotation at pod creation time for 
+TaskRuns with no Sidecars specified. Enabling this option should decrease the time it takes for a TaskRun to
+start running. However, for clusters that use injected sidecars e.g. istio
+enabling this option can lead to unexpected behavior.
+
 For example:
 
 ```yaml

pkg/pod/pod.go

@@ -45,8 +45,11 @@ const (
 	// ResultsDir is the folder used by default to create the results file
 	ResultsDir = "/tekton/results"
 
-	featureFlagDisableHomeEnvKey    = "disable-home-env-overwrite"
-	featureFlagDisableWorkingDirKey = "disable-working-directory-overwrite"
+	featureInjectedSidecar                   = "running-in-environment-with-injected-sidecars"
+	featureFlagDisableHomeEnvKey             = "disable-home-env-overwrite"
+	featureFlagDisableWorkingDirKey          = "disable-working-directory-overwrite"
+	featureFlagConfigMapName                 = "feature-flags"
+	featureFlagSetReadyAnnotationOnPodCreate = "enable-ready-annotation-on-pod-create"
 
 	taskRunLabelKey = pipeline.GroupName + pipeline.TaskRunLabelKey
 )
@@ -238,6 +241,10 @@ func MakePod(images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1.
 	podAnnotations := taskRun.Annotations
 	podAnnotations[ReleaseAnnotation] = ReleaseAnnotationValue
 
+	if shouldAddReadyAnnotationOnPodCreate(taskSpec.Sidecars, kubeclient) {
+		podAnnotations[readyAnnotation] = readyAnnotationValue
+	}
+
 	return &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			// We execute the build's pod in the same namespace as where the build was
@@ -351,3 +358,21 @@ func shouldOverrideWorkingDir(kubeclient kubernetes.Interface) bool {
 	}
 	return true
 }
+
+// shouldAddReadyAnnotationonPodCreate returns a bool indicating whether the
+// controller should add the `Ready` annotation when creating the Pod. We cannot
+// add the annotation if Tekton is running in a cluster with injected sidecars
+// or if the Task specifies any sidecars.
+func shouldAddReadyAnnotationOnPodCreate(sidecars []v1beta1.Sidecar, kubeclient kubernetes.Interface) bool {
+	// If the TaskRun has sidecars, we cannot set the READY annotation early
+	if len(sidecars) > 0 {
+		return false
+	}
+	// If the TaskRun has no sidecars, check if we are running in a cluster where sidecars can be injected by other
+	// controllers.
+	configMap, err := kubeclient.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetFeatureFlagsConfigName(), metav1.GetOptions{})
+	if err == nil && configMap != nil && configMap.Data != nil && configMap.Data[featureInjectedSidecar] == "false" {
+		return true
+	}
+	return false
+}

pkg/pod/pod_test.go

@@ -24,7 +24,9 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"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/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/system"
 	"github.com/tektoncd/pipeline/test/diff"
@@ -41,11 +43,13 @@ var (
 		CredsImage:      "override-with-creds:latest",
 		ShellImage:      "busybox",
 	}
+
+	ignoreReleaseAnnotation = func(k string, v string) bool {
+		return k == ReleaseAnnotation
+	}
 )
 
 func TestMakePod(t *testing.T) {
-	names.TestingSeed()
-
 	implicitEnvVars := []corev1.EnvVar{{
 		Name:  "HOME",
 		Value: pipeline.HomeDir,
@@ -58,14 +62,12 @@ func TestMakePod(t *testing.T) {
 		Name:         "tekton-internal-secret-volume-multi-creds-9l9zj",
 		VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "multi-creds"}},
 	}
-
 	placeToolsInit := corev1.Container{
 		Name:         "place-tools",
 		Image:        images.EntrypointImage,
 		Command:      []string{"cp", "/ko-app/entrypoint", "/tekton/tools/entrypoint"},
 		VolumeMounts: []corev1.VolumeMount{toolsMount},
 	}
-
 	runtimeClassName := "gvisor"
 	automountServiceAccountToken := false
 	dnsPolicy := corev1.DNSNone
@@ -76,6 +78,7 @@ func TestMakePod(t *testing.T) {
 		desc            string
 		trs             v1beta1.TaskRunSpec
 		ts              v1beta1.TaskSpec
+		featureFlags    map[string]string
 		want            *corev1.PodSpec
 		wantAnnotations map[string]string
 	}{{
@@ -114,6 +117,48 @@ func TestMakePod(t *testing.T) {
 			}},
 			Volumes: append(implicitVolumes, toolsVolume, downwardVolume),
 		},
+	}, {
+		desc: "simple with running-in-environment-with-injected-sidecar set to false",
+		ts: v1beta1.TaskSpec{
+			Steps: []v1alpha1.Step{{Container: corev1.Container{
+				Name:    "name",
+				Image:   "image",
+				Command: []string{"cmd"}, // avoid entrypoint lookup.
+			}}},
+		},
+		featureFlags: map[string]string{
+			featureInjectedSidecar: "false",
+		},
+		want: &corev1.PodSpec{
+			RestartPolicy:  corev1.RestartPolicyNever,
+			InitContainers: []corev1.Container{placeToolsInit},
+			Containers: []corev1.Container{{
+				Name:    "step-name",
+				Image:   "image",
+				Command: []string{"/tekton/tools/entrypoint"},
+				Args: []string{
+					"-wait_file",
+					"/tekton/downward/ready",
+					"-wait_file_content",
+					"-post_file",
+					"/tekton/tools/0",
+					"-termination_path",
+					"/tekton/termination",
+					"-entrypoint",
+					"cmd",
+					"--",
+				},
+				Env:                    implicitEnvVars,
+				VolumeMounts:           append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...),
+				WorkingDir:             pipeline.WorkspaceDir,
+				Resources:              corev1.ResourceRequirements{Requests: allZeroQty()},
+				TerminationMessagePath: "/tekton/termination",
+			}},
+			Volumes: append(implicitVolumes, toolsVolume, downwardVolume),
+		},
+		wantAnnotations: map[string]string{
+			readyAnnotation: readyAnnotationValue,
+		},
 	}, {
 		desc: "with service account",
 		ts: v1beta1.TaskSpec{
@@ -472,6 +517,58 @@ sidecar-script-heredoc-randomly-generated-mz4c7
 			}},
 			Volumes: append(implicitVolumes, scriptsVolume, toolsVolume, downwardVolume),
 		},
+	}, {
+		desc: "sidecar container with enable-ready-annotation-on-pod-create",
+		ts: v1beta1.TaskSpec{
+			Steps: []v1alpha1.Step{{Container: corev1.Container{
+				Name:    "primary-name",
+				Image:   "primary-image",
+				Command: []string{"cmd"}, // avoid entrypoint lookup.
+			}}},
+			Sidecars: []v1alpha1.Sidecar{{
+				Container: corev1.Container{
+					Name:  "sc-name",
+					Image: "sidecar-image",
+				},
+			}},
+		},
+		featureFlags: map[string]string{
+			featureFlagSetReadyAnnotationOnPodCreate: "true",
+		},
+		wantAnnotations: map[string]string{}, // no ready annotations on pod create since sidecars are present
+		want: &corev1.PodSpec{
+			RestartPolicy:  corev1.RestartPolicyNever,
+			InitContainers: []corev1.Container{placeToolsInit},
+			Containers: []corev1.Container{{
+				Name:    "step-primary-name",
+				Image:   "primary-image",
+				Command: []string{"/tekton/tools/entrypoint"},
+				Args: []string{
+					"-wait_file",
+					"/tekton/downward/ready",
+					"-wait_file_content",
+					"-post_file",
+					"/tekton/tools/0",
+					"-termination_path",
+					"/tekton/termination",
+					"-entrypoint",
+					"cmd",
+					"--",
+				},
+				Env:                    implicitEnvVars,
+				VolumeMounts:           append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...),
+				WorkingDir:             pipeline.WorkspaceDir,
+				Resources:              corev1.ResourceRequirements{Requests: allZeroQty()},
+				TerminationMessagePath: "/tekton/termination",
+			}, {
+				Name:  "sidecar-sc-name",
+				Image: "sidecar-image",
+				Resources: corev1.ResourceRequirements{
+					Requests: nil,
+				},
+			}},
+			Volumes: append(implicitVolumes, toolsVolume, downwardVolume),
+		},
 	}, {
 		desc: "resource request",
 		ts: v1beta1.TaskSpec{
@@ -779,6 +876,10 @@ script-heredoc-randomly-generated-78c5n
 		t.Run(c.desc, func(t *testing.T) {
 			names.TestingSeed()
 			kubeclient := fakek8s.NewSimpleClientset(
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{Name: featureFlagConfigMapName, Namespace: system.GetNamespace()},
+					Data:       c.featureFlags,
+				},
 				&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}},
 				&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "service-account", Namespace: "default"},
 					Secrets: []corev1.ObjectReference{{
@@ -825,6 +926,12 @@ script-heredoc-randomly-generated-78c5n
 			if d := cmp.Diff(c.want, &got.Spec, resourceQuantityCmp); d != "" {
 				t.Errorf("Diff %s", diff.PrintWantGot(d))
 			}
+
+			if c.wantAnnotations != nil {
+				if d := cmp.Diff(c.wantAnnotations, got.ObjectMeta.Annotations, cmpopts.IgnoreMapEntries(ignoreReleaseAnnotation)); d != "" {
+					t.Errorf("Annotation Diff(-want, +got):\n%s", d)
+				}
+			}
 		})
 	}
 }
@@ -964,3 +1071,82 @@ func TestShouldOverrideWorkingDir(t *testing.T) {
 		})
 	}
 }
+
+func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) {
+	sd := v1beta1.Sidecar{
+		Container: corev1.Container{
+			Name: "a-sidecar",
+		},
+	}
+	tcs := []struct {
+		description string
+		sidecars    []v1beta1.Sidecar
+		configMap   *corev1.ConfigMap
+		expected    bool
+	}{{
+		description: "Default behavior with sidecars present: Ready annotation not set on pod create",
+		sidecars:    []v1beta1.Sidecar{sd},
+		configMap: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()},
+			Data:       map[string]string{},
+		},
+		expected: false,
+	}, {
+		description: "Default behavior with no sidecars present: Ready annotation not set on pod create",
+		sidecars:    []v1beta1.Sidecar{},
+		configMap: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()},
+			Data:       map[string]string{},
+		},
+		expected: false,
+	}, {
+		description: "Setting running-in-environment-with-injected-sidecars to true with sidecars present results in false",
+		sidecars:    []v1beta1.Sidecar{sd},
+		configMap: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()},
+			Data: map[string]string{
+				featureInjectedSidecar: "true",
+			},
+		},
+		expected: false,
+	}, {
+		description: "Setting running-in-environment-with-injected-sidecars to true with no sidecars present results in false",
+		sidecars:    []v1beta1.Sidecar{},
+		configMap: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()},
+			Data: map[string]string{
+				featureInjectedSidecar: "true",
+			},
+		},
+		expected: false,
+	}, {
+		description: "Setting running-in-environment-with-injected-sidecars to false with sidecars present results in false",
+		sidecars:    []v1beta1.Sidecar{sd},
+		configMap: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()},
+			Data: map[string]string{
+				featureInjectedSidecar: "false",
+			},
+		},
+		expected: false,
+	}, {
+		description: "Setting running-in-environment-with-injected-sidecars to false with no sidecars present results in true",
+		sidecars:    []v1beta1.Sidecar{},
+		configMap: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()},
+			Data: map[string]string{
+				featureInjectedSidecar: "false",
+			},
+		},
+		expected: true,
+	}}
+
+	for _, tc := range tcs {
+		t.Run(tc.description, func(t *testing.T) {
+			kubclient := fakek8s.NewSimpleClientset(tc.configMap)
+			if result := shouldAddReadyAnnotationOnPodCreate(tc.sidecars, kubclient); result != tc.expected {
+				t.Errorf("expected: %t Recieved: %t", tc.expected, result)
+			}
+		})
+	}
+}