Add "await-sidecar-readiness" feature flag

True by default, setting this to false will prevent the first Step in a
TaskRun from waiting on the "Ready" annotation before commencing. This
should allow tasks to be run in environments that don't support
DownwardAPI volumes.

config/config-feature-flags.yaml

@@ -44,6 +44,14 @@ data:
   # See https://github.com/tektoncd/pipeline/issues/2791 for more
   # info.
   disable-creds-init: "false"
+  # Setting this flag to "false" will stop Tekton from waiting for a
+  # TaskRun's sidecar containers to be running before starting the first
+  # step. This will allow Tasks to be run in environments that don't
+  # support the DownwardAPI volume type, but may lead to unintended
+  # behaviour if sidecars are used.
+  #
+  # See https://github.com/tektoncd/pipeline/issues/4937 for more info.
+  await-sidecar-readiness: "true"
   # 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

docs/install.md

@@ -355,11 +355,18 @@ To customize the behavior of the Pipelines Controller, modify the ConfigMap `fea
   node in the cluster must have an appropriate label matching `topologyKey`. If some or all nodes
   are missing the specified `topologyKey` label, it can lead to unintended behavior.
 
-- `running-in-environment-with-injected-sidecars`: set this flag to `"true"` to allow the
-Tekton controller to set the `tekton.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.
+- `await-sidecar-readiness`: set this flag to `"false"` to allow the Tekton controller to start a
+TasksRun's first step immediately without waiting for sidecar containers to be running first. Using
+this option should decrease the time it takes for a TaskRun to start running, and will allow TaskRun
+pods to be scheduled in environments that don't support [Downward API](https://kubernetes.io/docs/tasks/inject-data-application/downward-api-volume-expose-pod-information/)
+volumes (e.g. some virtual kubelet implementations). However, this may lead to unexpected behaviour
+with Tasks that use sidecars, or in clusters that use injected sidecars (e.g. Istio). Setting this flag
+to `"false"` will mean the `running-in-environment-with-injected-sidecars` flag has no effect.
+
+- `running-in-environment-with-injected-sidecars`: set this flag to `"false"` to allow the
+Tekton controller to start a TasksRun's first step immediately if it has no Sidecars specified.
+Using this option should decrease the time it takes for a TaskRun to start running.
+However, for clusters that use injected sidecars (e.g. Istio) this can lead to unexpected behavior.
 
 - `require-git-ssh-secret-known-hosts`: set this flag to `"true"` to require that
 Git SSH Secrets include a `known_hosts` field. This ensures that a git remote server's

pkg/apis/config/feature_flags.go

@@ -45,6 +45,8 @@ const (
 	DefaultDisableCredsInit = false
 	// DefaultRunningInEnvWithInjectedSidecars is the default value for "running-in-environment-with-injected-sidecars".
 	DefaultRunningInEnvWithInjectedSidecars = true
+	// DefaultAwaitSidecarReadiness is the default value for "await-sidecar-readiness".
+	DefaultAwaitSidecarReadiness = true
 	// DefaultRequireGitSSHSecretKnownHosts is the default value for "require-git-ssh-secret-known-hosts".
 	DefaultRequireGitSSHSecretKnownHosts = false
 	// DefaultEnableTektonOciBundles is the default value for "enable-tekton-oci-bundles".
@@ -61,6 +63,7 @@ const (
 	disableAffinityAssistantKey         = "disable-affinity-assistant"
 	disableCredsInitKey                 = "disable-creds-init"
 	runningInEnvWithInjectedSidecarsKey = "running-in-environment-with-injected-sidecars"
+	awaitSidecarReadinessKey            = "await-sidecar-readiness"
 	requireGitSSHSecretKnownHostsKey    = "require-git-ssh-secret-known-hosts" // nolint: gosec
 	enableTektonOCIBundles              = "enable-tekton-oci-bundles"
 	enableCustomTasks                   = "enable-custom-tasks"
@@ -81,6 +84,7 @@ type FeatureFlags struct {
 	ScopeWhenExpressionsToTask       bool
 	EnableAPIFields                  string
 	SendCloudEventsForRuns           bool
+	AwaitSidecarReadiness            bool
 	EmbeddedStatus                   string
 }
 
@@ -118,6 +122,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
 	if err := setFeature(runningInEnvWithInjectedSidecarsKey, DefaultRunningInEnvWithInjectedSidecars, &tc.RunningInEnvWithInjectedSidecars); err != nil {
 		return nil, err
 	}
+	if err := setFeature(awaitSidecarReadinessKey, DefaultAwaitSidecarReadiness, &tc.AwaitSidecarReadiness); err != nil {
+		return nil, err
+	}
 	if err := setFeature(requireGitSSHSecretKnownHostsKey, DefaultRequireGitSSHSecretKnownHosts, &tc.RequireGitSSHSecretKnownHosts); err != nil {
 		return nil, err
 	}

pkg/apis/config/feature_flags_test.go

@@ -35,16 +35,25 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 	testCases := []testCase{
 		{
 			expectedConfig: &config.FeatureFlags{
-				RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
-				EnableAPIFields:                  "stable",
-				EmbeddedStatus:                   config.DefaultEmbeddedStatus,
+				DisableAffinityAssistant:         false,
+				RunningInEnvWithInjectedSidecars: true,
+				RequireGitSSHSecretKnownHosts:    false,
+
+				DisableCredsInit:       config.DefaultDisableCredsInit,
+				AwaitSidecarReadiness:  config.DefaultAwaitSidecarReadiness,
+				EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles,
+				EnableCustomTasks:      config.DefaultEnableCustomTasks,
+				EnableAPIFields:        config.DefaultEnableAPIFields,
+				SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
+				EmbeddedStatus:         config.DefaultEmbeddedStatus,
 			},
 			fileName: config.GetFeatureFlagsConfigName(),
 		},
 		{
 			expectedConfig: &config.FeatureFlags{
 				DisableAffinityAssistant:         true,
 				RunningInEnvWithInjectedSidecars: false,
+				AwaitSidecarReadiness:            false,
 				RequireGitSSHSecretKnownHosts:    true,
 				EnableTektonOCIBundles:           true,
 				EnableCustomTasks:                true,
@@ -62,7 +71,12 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				EnableTektonOCIBundles: true,
 				EnableCustomTasks:      true,
 
+				DisableAffinityAssistant:         config.DefaultDisableAffinityAssistant,
+				DisableCredsInit:                 config.DefaultDisableCredsInit,
 				RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
+				AwaitSidecarReadiness:            config.DefaultAwaitSidecarReadiness,
+				RequireGitSSHSecretKnownHosts:    config.DefaultRequireGitSSHSecretKnownHosts,
+				SendCloudEventsForRuns:           config.DefaultSendCloudEventsForRuns,
 				EmbeddedStatus:                   config.DefaultEmbeddedStatus,
 			},
 			fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks",
@@ -73,7 +87,12 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				EnableTektonOCIBundles: true,
 				EnableCustomTasks:      true,
 
+				DisableAffinityAssistant:         config.DefaultDisableAffinityAssistant,
+				DisableCredsInit:                 config.DefaultDisableCredsInit,
 				RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
+				AwaitSidecarReadiness:            config.DefaultAwaitSidecarReadiness,
+				RequireGitSSHSecretKnownHosts:    config.DefaultRequireGitSSHSecretKnownHosts,
+				SendCloudEventsForRuns:           config.DefaultSendCloudEventsForRuns,
 				EmbeddedStatus:                   config.DefaultEmbeddedStatus,
 			},
 			fileName: "feature-flags-bundles-and-custom-tasks",
@@ -92,8 +111,15 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) {
 	FeatureFlagsConfigEmptyName := "feature-flags-empty"
 	expectedConfig := &config.FeatureFlags{
-		RunningInEnvWithInjectedSidecars: true,
-		EnableAPIFields:                  "stable",
+		DisableAffinityAssistant:         config.DefaultDisableAffinityAssistant,
+		DisableCredsInit:                 config.DefaultDisableCredsInit,
+		RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
+		AwaitSidecarReadiness:            config.DefaultAwaitSidecarReadiness,
+		RequireGitSSHSecretKnownHosts:    config.DefaultRequireGitSSHSecretKnownHosts,
+		EnableTektonOCIBundles:           config.DefaultEnableTektonOciBundles,
+		EnableCustomTasks:                config.DefaultEnableCustomTasks,
+		EnableAPIFields:                  config.DefaultEnableAPIFields,
+		SendCloudEventsForRuns:           config.DefaultSendCloudEventsForRuns,
 		EmbeddedStatus:                   config.DefaultEmbeddedStatus,
 	}
 	verifyConfigFileWithExpectedFeatureFlagsConfig(t, FeatureFlagsConfigEmptyName, expectedConfig)

pkg/apis/config/testdata/feature-flags-all-flags-set.yaml

@@ -20,6 +20,7 @@ metadata:
 data:
   disable-affinity-assistant: "true"
   running-in-environment-with-injected-sidecars: "false"
+  await-sidecar-readiness: "false"
   require-git-ssh-secret-known-hosts: "true"
   enable-tekton-oci-bundles: "true"
   enable-custom-tasks: "true"

pkg/pod/pod.go

@@ -115,7 +115,8 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	)
 	volumeMounts := []corev1.VolumeMount{binROMount}
 	implicitEnvVars := []corev1.EnvVar{}
-	alphaAPIEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields
+	featureFlags := config.FromContextOrDefaults(ctx).FeatureFlags
+	alphaAPIEnabled := featureFlags.EnableAPIFields == config.AlphaAPIFields
 
 	// Add our implicit volumes first, so they can be overridden by the user if they prefer.
 	volumes = append(volumes, implicitVolumes...)
@@ -183,7 +184,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 		return nil, err
 	}
 
-	readyImmediately := isPodReadyImmediately(ctx, taskSpec.Sidecars)
+	readyImmediately := isPodReadyImmediately(*featureFlags, taskSpec.Sidecars)
 
 	if alphaAPIEnabled {
 		stepContainers, err = orderContainers(credEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug, !readyImmediately)
@@ -377,18 +378,13 @@ func makeLabels(s *v1beta1.TaskRun) map[string]string {
 }
 
 // isPodReadyImmediately 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 isPodReadyImmediately(ctx context.Context, sidecars []v1beta1.Sidecar) 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.
-	cfg := config.FromContextOrDefaults(ctx)
-	return !cfg.FeatureFlags.RunningInEnvWithInjectedSidecars
+// controller should consider the Pod "Ready" as soon as it's deployed.
+// This will add the `Ready` annotation when creating the Pod,
+// and prevent the first step from waiting for the annotation to appear before starting.
+func isPodReadyImmediately(featureFlags config.FeatureFlags, sidecars []v1beta1.Sidecar) bool {
+	return !featureFlags.AwaitSidecarReadiness ||
+		// If the TaskRun has sidecars, we must wait for them
+		(len(sidecars) == 0 && !featureFlags.RunningInEnvWithInjectedSidecars)
 }
 
 func runMount(i int, ro bool) corev1.VolumeMount {

pkg/pod/pod_test.go

@@ -58,6 +58,7 @@ var (
 		return k == ReleaseAnnotation
 	}
 	featureInjectedSidecar                   = "running-in-environment-with-injected-sidecars"
+	featureAwaitSidecarReadiness             = "await-sidecar-readiness"
 	featureFlagSetReadyAnnotationOnPodCreate = "enable-ready-annotation-on-pod-create"
 
 	defaultActiveDeadlineSeconds = int64(config.DefaultTimeoutMinutes * 60 * deadlineFactor)
@@ -2036,74 +2037,99 @@ func TestIsPodReadyImmediately(t *testing.T) {
 	sd := v1beta1.Sidecar{
 		Name: "a-sidecar",
 	}
+
+	getFeatureFlags := func(cfg map[string]string) *config.FeatureFlags {
+		flags, err := config.NewFeatureFlagsFromMap(cfg)
+		if err != nil {
+			t.Fatalf("Error building feature flags for testingL %s", err)
+		}
+		return flags
+	}
+
 	tcs := []struct {
-		description string
-		sidecars    []v1beta1.Sidecar
-		configMap   *corev1.ConfigMap
-		expected    bool
+		description  string
+		sidecars     []v1beta1.Sidecar
+		featureFlags *config.FeatureFlags
+		expected     bool
 	}{{
-		description: "Default behavior with sidecars present: Pod is not ready on create",
+		description:  "Default behavior with sidecars present: Pod is not ready on create",
+		sidecars:     []v1beta1.Sidecar{sd},
+		featureFlags: getFeatureFlags(map[string]string{}),
+		expected:     false,
+	}, {
+		description:  "Default behavior with no sidecars present: Pod is not ready on create",
+		sidecars:     []v1beta1.Sidecar{},
+		featureFlags: getFeatureFlags(map[string]string{}),
+		expected:     false,
+	}, {
+		description: "Setting await-sidecar-readiness to true and 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: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
-			Data:       map[string]string{},
-		},
+		featureFlags: getFeatureFlags(map[string]string{
+			featureAwaitSidecarReadiness: "true",
+			featureInjectedSidecar:       "true",
+		}),
 		expected: false,
 	}, {
-		description: "Default behavior with no sidecars present: Pod is not ready on create",
+		description: "Setting await-sidecar-readiness to true and 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: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
-			Data:       map[string]string{},
-		},
+		featureFlags: getFeatureFlags(map[string]string{
+			featureAwaitSidecarReadiness: "true",
+			featureInjectedSidecar:       "true",
+		}),
 		expected: false,
 	}, {
-		description: "Setting running-in-environment-with-injected-sidecars to true with sidecars present results in false",
+		description: "Setting await-sidecar-readiness to true and 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: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
-			Data: map[string]string{
-				featureInjectedSidecar: "true",
-			},
-		},
+		featureFlags: getFeatureFlags(map[string]string{
+			featureAwaitSidecarReadiness: "true",
+			featureInjectedSidecar:       "false",
+		}),
 		expected: false,
 	}, {
-		description: "Setting running-in-environment-with-injected-sidecars to true with no sidecars present results in false",
+		description: "Setting await-sidecar-readiness to true and 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: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
-			Data: map[string]string{
-				featureInjectedSidecar: "true",
-			},
-		},
-		expected: false,
+		featureFlags: getFeatureFlags(map[string]string{
+			featureAwaitSidecarReadiness: "true",
+			featureInjectedSidecar:       "false",
+		}),
+		expected: true,
 	}, {
-		description: "Setting running-in-environment-with-injected-sidecars to false with sidecars present results in false",
+		description: "Setting await-sidecar-readiness to false and running-in-environment-with-injected-sidecars to true with sidecars present results in true",
 		sidecars:    []v1beta1.Sidecar{sd},
-		configMap: &corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
-			Data: map[string]string{
-				featureInjectedSidecar: "false",
-			},
-		},
-		expected: false,
+		featureFlags: getFeatureFlags(map[string]string{
+			featureAwaitSidecarReadiness: "false",
+			featureInjectedSidecar:       "true",
+		}),
+		expected: true,
 	}, {
-		description: "Setting running-in-environment-with-injected-sidecars to false with no sidecars present results in true",
+		description: "Setting await-sidecar-readiness to false and running-in-environment-with-injected-sidecars to true with no sidecars present results in true",
 		sidecars:    []v1beta1.Sidecar{},
-		configMap: &corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
-			Data: map[string]string{
-				featureInjectedSidecar: "false",
-			},
-		},
+		featureFlags: getFeatureFlags(map[string]string{
+			featureAwaitSidecarReadiness: "false",
+			featureInjectedSidecar:       "true",
+		}),
+		expected: true,
+	}, {
+		description: "Setting await-sidecar-readiness to false and running-in-environment-with-injected-sidecars to false with sidecars present results in true",
+		sidecars:    []v1beta1.Sidecar{sd},
+		featureFlags: getFeatureFlags(map[string]string{
+			featureAwaitSidecarReadiness: "false",
+			featureInjectedSidecar:       "false",
+		}),
+		expected: true,
+	}, {
+		description: "Setting await-sidecar-readiness to false and running-in-environment-with-injected-sidecars to false with no sidecars present results in true",
+		sidecars:    []v1beta1.Sidecar{},
+		featureFlags: getFeatureFlags(map[string]string{
+			featureAwaitSidecarReadiness: "false",
+			featureInjectedSidecar:       "false",
+		}),
 		expected: true,
 	}}
 
 	for _, tc := range tcs {
 		t.Run(tc.description, func(t *testing.T) {
-			store := config.NewStore(logtesting.TestLogger(t))
-			store.OnConfigChanged(tc.configMap)
-			if result := isPodReadyImmediately(store.ToContext(context.Background()), tc.sidecars); result != tc.expected {
+			if result := isPodReadyImmediately(*tc.featureFlags, tc.sidecars); result != tc.expected {
 				t.Errorf("expected: %t Received: %t", tc.expected, result)
 			}
 		})