diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index 11a85e60aa9..87e04c9c6a0 100644 --- a/config/config-feature-flags.yaml +++ b/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" diff --git a/docs/install.md b/docs/install.md index d0c12c45b0e..82937d14d7f 100644 --- a/docs/install.md +++ b/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 diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 98ffae2c62a..2af57b26482 100644 --- a/pkg/pod/pod.go +++ b/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 +} diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 1a41b6ac942..b601d9b7eb9 100644 --- a/pkg/pod/pod_test.go +++ b/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) + } + }) + } +}