From c5ea9cfd66d97c9da0f8aebb139ad85963e38778 Mon Sep 17 00:00:00 2001 From: Kevin Carr Date: Mon, 3 Jun 2024 10:31:49 -0500 Subject: [PATCH] #7617 support k8s native sidecar --- config/config-feature-flags.yaml | 2 + docs/additional-configs.md | 2 + docs/pipeline-api.md | 34 +++++ docs/tasks.md | 5 + .../v1/taskruns/sidecar-ready-script.yaml | 1 + examples/v1/taskruns/sidecar-ready.yaml | 9 ++ go.mod | 1 + pkg/apis/config/feature_flags.go | 8 + pkg/apis/config/feature_flags_test.go | 4 + .../testdata/feature-flags-all-flags-set.yaml | 1 + ...ags-invalid-enable-kubernetes-sidecar.yaml | 21 +++ pkg/apis/pipeline/v1/container_types.go | 35 +++++ pkg/apis/pipeline/v1/container_types_test.go | 32 ++++ pkg/apis/pipeline/v1/openapi_generated.go | 7 + pkg/apis/pipeline/v1/swagger.json | 4 + pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 5 + pkg/apis/pipeline/v1beta1/container_types.go | 35 +++++ .../pipeline/v1beta1/openapi_generated.go | 7 + pkg/apis/pipeline/v1beta1/swagger.json | 4 + .../pipeline/v1beta1/zz_generated.deepcopy.go | 5 + pkg/pod/pod.go | 47 +++++- pkg/pod/pod_test.go | 140 ++++++++++++++++++ pkg/pod/status.go | 5 + pkg/reconciler/taskrun/taskrun.go | 25 +++- pkg/reconciler/taskrun/taskrun_test.go | 2 +- test/e2e-tests-kind-prow-alpha.env | 1 + test/e2e-tests.sh | 14 ++ test/featureflags.go | 1 + test/sidecar_test.go | 6 + 29 files changed, 454 insertions(+), 9 deletions(-) create mode 100644 pkg/apis/config/testdata/feature-flags-invalid-enable-kubernetes-sidecar.yaml diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index dcc52f95c7d..398f37d1880 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -138,3 +138,5 @@ data: disable-inline-spec: "" # Setting this flag to "true" will enable the use of concise resolver syntax enable-concise-resolver-syntax: "false" + # Setthing this flag to "true" will enable native Kubernetes Sidecar support + enable-kubernetes-sidecar: "false" diff --git a/docs/additional-configs.md b/docs/additional-configs.md index 08ed079979d..159fa5506b4 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -333,6 +333,8 @@ enables [beta features](#beta-features). When using v1 APIs, setting this field allows only stable features, and setting it to "beta" allows only beta features. Set this field to "alpha" to allow [alpha features](#alpha-features) to be used. +- `enable-kubernetes-sidecar`: Set this flag to `"true"` to enable native kubernetes sidecar support. This will allow Tekton sidecars to run as Kubernetes sidecars. Must be using Kubernetes v1.29 or greater. + For example: ```yaml diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index a2656ffaef2..044799e5a90 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -4135,6 +4135,23 @@ other Step or Sidecar that does not also request this Workspace will not have access to it.

+ + +restartPolicy
+ + +Kubernetes core/v1.ContainerRestartPolicy + + + + +(Optional) +

RestartPolicy refers to kubernetes RestartPolicy. It can only be set for an +initContainer and must have it’s policy set to “Always”. It is currently +left optional to help support Kubernetes versions prior to 1.29 when this feature +was introduced.

+ +

SidecarState @@ -13459,6 +13476,23 @@ other Step or Sidecar that does not also request this Workspace will not have access to it.

+ + +restartPolicy
+ + +Kubernetes core/v1.ContainerRestartPolicy + + + + +(Optional) +

RestartPolicy refers to kubernetes RestartPolicy. It can only be set for an +initContainer and must have it’s policy set to “Always”. It is currently +left optional to help support Kubernetes versions prior to 1.29 when this feature +was introduced.

+ +

SidecarState diff --git a/docs/tasks.md b/docs/tasks.md index b8ab3eb7217..d01c29dd915 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -1113,6 +1113,11 @@ to run alongside the `Steps` in your `Task`. You can use `Sidecars` to provide a `Sidecars` spin up before your `Task` executes and are deleted after the `Task` execution completes. For further information, see [`Sidecars` in `TaskRuns`](taskruns.md#specifying-sidecars). +**Note**: Starting in v0.62 you can enable native Kubernetes sidecar support using the `enable-kubernetes-sidecar` feature flag ([see instructions](./additional-configs.md#customizing-the-pipelines-controller-behavior)). If kubernetes does not wait for your sidecar application to be ready, use a `startupProbe` to help kubernetes identify when it is ready. + +Refer to the detailed instructions listed in [additional config](additional-configs.md#enabling-larger-results-using-sidecar-logs) +to learn how to enable this feature. + In the example below, a `Step` uses a Docker-in-Docker `Sidecar` to build a Docker image: ```yaml diff --git a/examples/v1/taskruns/sidecar-ready-script.yaml b/examples/v1/taskruns/sidecar-ready-script.yaml index 50dd03f61d6..9a5ff0cb35d 100644 --- a/examples/v1/taskruns/sidecar-ready-script.yaml +++ b/examples/v1/taskruns/sidecar-ready-script.yaml @@ -9,6 +9,7 @@ spec: image: docker.io/library/ubuntu script: | echo "hello from sidecar" > /shared/message + sleep 2 volumeMounts: - name: shared mountPath: /shared diff --git a/examples/v1/taskruns/sidecar-ready.yaml b/examples/v1/taskruns/sidecar-ready.yaml index 20116680dd9..cbba245b934 100644 --- a/examples/v1/taskruns/sidecar-ready.yaml +++ b/examples/v1/taskruns/sidecar-ready.yaml @@ -18,6 +18,15 @@ spec: - -c - sleep 5 && touch /shared/ready timeoutSeconds: 10 + # Adding startup probe for k8s native sidecar support + # Readiness Probe is not honored for k8s native sidecar support + startupProbe: + exec: + command: + - sh + - -c + - sleep 5 && touch /shared/ready + timeoutSeconds: 10 volumeMounts: - name: shared mountPath: /shared diff --git a/go.mod b/go.mod index d3eec4c37eb..445e464ee3e 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,7 @@ module github.com/tektoncd/pipeline go 1.22 + toolchain go1.22.4 require ( diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index 69b80754297..d0db62052cf 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -108,6 +108,10 @@ const ( EnableParamEnum = "enable-param-enum" // EnableConciseResolverSyntax is the flag to enable concise resolver syntax EnableConciseResolverSyntax = "enable-concise-resolver-syntax" + // EnableKubernetesSidecar is the flag to enable kubernetes sidecar support + EnableKubernetesSidecar = "enable-kubernetes-sidecar" + // DefaultEnableKubernetesSidecar is the default value for EnableKubernetesSidecar + DefaultEnableKubernetesSidecar = false // DisableInlineSpec is the flag to disable embedded spec // in Taskrun or Pipelinerun @@ -208,6 +212,7 @@ type FeatureFlags struct { EnableArtifacts bool DisableInlineSpec string EnableConciseResolverSyntax bool + EnableKubernetesSidecar bool } // GetFeatureFlagsConfigName returns the name of the configmap containing all @@ -312,6 +317,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setPerFeatureFlag(EnableConciseResolverSyntax, DefaultEnableConciseResolverSyntax, &tc.EnableConciseResolverSyntax); err != nil { return nil, err } + if err := setFeature(EnableKubernetesSidecar, DefaultEnableKubernetesSidecar, &tc.EnableKubernetesSidecar); err != nil { + return nil, err + } return &tc, nil } diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index b9e6a588348..c9eca976953 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -83,6 +83,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableParamEnum: true, DisableInlineSpec: "pipeline,pipelinerun,taskrun", EnableConciseResolverSyntax: true, + EnableKubernetesSidecar: true, }, fileName: "feature-flags-all-flags-set", }, @@ -314,6 +315,9 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) { }, { fileName: "feature-flags-invalid-enable-concise-resolver-syntax", want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax for feature enable-concise-resolver-syntax`, + }, { + fileName: "feature-flags-invalid-enable-kubernetes-sidecar", + want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax`, }} { t.Run(tc.fileName, func(t *testing.T) { cm := test.ConfigMapFromTestFile(t, tc.fileName) diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml index 3a798646664..a01101604a1 100644 --- a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -38,3 +38,4 @@ data: enable-artifacts: "true" disable-inline-spec: "pipeline,pipelinerun,taskrun" enable-concise-resolver-syntax: "true" + enable-kubernetes-sidecar: "true" diff --git a/pkg/apis/config/testdata/feature-flags-invalid-enable-kubernetes-sidecar.yaml b/pkg/apis/config/testdata/feature-flags-invalid-enable-kubernetes-sidecar.yaml new file mode 100644 index 00000000000..cc78bf080d8 --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags-invalid-enable-kubernetes-sidecar.yaml @@ -0,0 +1,21 @@ +# Copyright 2024 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 +# +# https://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. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: feature-flags + namespace: tekton-pipelines +data: + enable-kubernetes-sidecar: "invalid" diff --git a/pkg/apis/pipeline/v1/container_types.go b/pkg/apis/pipeline/v1/container_types.go index 51e9f18717d..2dc4a8984f9 100644 --- a/pkg/apis/pipeline/v1/container_types.go +++ b/pkg/apis/pipeline/v1/container_types.go @@ -543,10 +543,43 @@ type Sidecar struct { // +optional // +listType=atomic Workspaces []WorkspaceUsage `json:"workspaces,omitempty"` + + // RestartPolicy refers to kubernetes RestartPolicy. It can only be set for an + // initContainer and must have it's policy set to "Always". It is currently + // left optional to help support Kubernetes versions prior to 1.29 when this feature + // was introduced. + // +optional + RestartPolicy *corev1.ContainerRestartPolicy `json:"restartPolicy,omitempty"` } // ToK8sContainer converts the Sidecar to a Kubernetes Container struct func (s *Sidecar) ToK8sContainer() *corev1.Container { + if s.RestartPolicy == nil { + return &corev1.Container{ + Name: s.Name, + Image: s.Image, + Command: s.Command, + Args: s.Args, + WorkingDir: s.WorkingDir, + Ports: s.Ports, + EnvFrom: s.EnvFrom, + Env: s.Env, + Resources: s.ComputeResources, + VolumeMounts: s.VolumeMounts, + VolumeDevices: s.VolumeDevices, + LivenessProbe: s.LivenessProbe, + ReadinessProbe: s.ReadinessProbe, + StartupProbe: s.StartupProbe, + Lifecycle: s.Lifecycle, + TerminationMessagePath: s.TerminationMessagePath, + TerminationMessagePolicy: s.TerminationMessagePolicy, + ImagePullPolicy: s.ImagePullPolicy, + SecurityContext: s.SecurityContext, + Stdin: s.Stdin, + StdinOnce: s.StdinOnce, + TTY: s.TTY, + } + } return &corev1.Container{ Name: s.Name, Image: s.Image, @@ -561,6 +594,7 @@ func (s *Sidecar) ToK8sContainer() *corev1.Container { VolumeDevices: s.VolumeDevices, LivenessProbe: s.LivenessProbe, ReadinessProbe: s.ReadinessProbe, + RestartPolicy: s.RestartPolicy, StartupProbe: s.StartupProbe, Lifecycle: s.Lifecycle, TerminationMessagePath: s.TerminationMessagePath, @@ -597,6 +631,7 @@ func (s *Sidecar) SetContainerFields(c corev1.Container) { s.Stdin = c.Stdin s.StdinOnce = c.StdinOnce s.TTY = c.TTY + s.RestartPolicy = c.RestartPolicy } // GetVarSubstitutionExpressions walks all the places a substitution reference can be used diff --git a/pkg/apis/pipeline/v1/container_types_test.go b/pkg/apis/pipeline/v1/container_types_test.go index 06948f9acaa..fd7c558eafa 100644 --- a/pkg/apis/pipeline/v1/container_types_test.go +++ b/pkg/apis/pipeline/v1/container_types_test.go @@ -120,3 +120,35 @@ func TestSidecarGetVarSubstitutionExpressions(t *testing.T) { t.Fatalf("Unexpected result (-want, +got): %s", d) } } + +func TestSidecarRestartPolicyToK8sContainer(t *testing.T) { + always := corev1.ContainerRestartPolicyAlways + s := Sidecar{ + Name: "sidecarName", + RestartPolicy: &always, + } + + expectedContainer := corev1.Container{ + Name: "sidecarName", + RestartPolicy: &always, + } + + c := s.ToK8sContainer() + + if !(c.RestartPolicy == expectedContainer.RestartPolicy) { + t.Fatalf("Unexpected result with RestartPolicy") + } + + s = Sidecar{ + Name: "sidecarName", + } + + expectedContainer = corev1.Container{ + Name: "sidecarName", + } + + c = s.ToK8sContainer() + if !(c.RestartPolicy == expectedContainer.RestartPolicy) { + t.Fatalf("Unexpected result without RestartPolicy") + } +} diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index 928f446c294..212bbe88928 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -2745,6 +2745,13 @@ func schema_pkg_apis_pipeline_v1_Sidecar(ref common.ReferenceCallback) common.Op }, }, }, + "restartPolicy": { + SchemaProps: spec.SchemaProps{ + Description: "RestartPolicy refers to kubernetes RestartPolicy. It can only be set for an initContainer and must have it's policy set to \"Always\". It is currently left optional to help support Kubernetes versions prior to 1.29 when this feature was introduced.", + Type: []string{"string"}, + Format: "", + }, + }, }, Required: []string{"name"}, }, diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 4ece3c37804..b5ee9828ec9 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1325,6 +1325,10 @@ "description": "Periodic probe of Sidecar service readiness. Container will be removed from service endpoints if the probe fails. Cannot be updated. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#container-probes", "$ref": "#/definitions/v1.Probe" }, + "restartPolicy": { + "description": "RestartPolicy refers to kubernetes RestartPolicy. It can only be set for an initContainer and must have it's policy set to \"Always\". It is currently left optional to help support Kubernetes versions prior to 1.29 when this feature was introduced.", + "type": "string" + }, "script": { "description": "Script is the contents of an executable file to execute.\n\nIf Script is not empty, the Step cannot have an Command or Args.", "type": "string" diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 5b8ca4bb07c..80430b4de73 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1248,6 +1248,11 @@ func (in *Sidecar) DeepCopyInto(out *Sidecar) { *out = make([]WorkspaceUsage, len(*in)) copy(*out, *in) } + if in.RestartPolicy != nil { + in, out := &in.RestartPolicy, &out.RestartPolicy + *out = new(corev1.ContainerRestartPolicy) + **out = **in + } return } diff --git a/pkg/apis/pipeline/v1beta1/container_types.go b/pkg/apis/pipeline/v1beta1/container_types.go index 26e08e69fe9..95f852bf496 100644 --- a/pkg/apis/pipeline/v1beta1/container_types.go +++ b/pkg/apis/pipeline/v1beta1/container_types.go @@ -747,10 +747,43 @@ type Sidecar struct { // +optional // +listType=atomic Workspaces []WorkspaceUsage `json:"workspaces,omitempty"` + + // RestartPolicy refers to kubernetes RestartPolicy. It can only be set for an + // initContainer and must have it's policy set to "Always". It is currently + // left optional to help support Kubernetes versions prior to 1.29 when this feature + // was introduced. + // +optional + RestartPolicy *corev1.ContainerRestartPolicy `json:"restartPolicy,omitempty"` } // ToK8sContainer converts the Sidecar to a Kubernetes Container struct func (s *Sidecar) ToK8sContainer() *corev1.Container { + if s.RestartPolicy == nil { + return &corev1.Container{ + Name: s.Name, + Image: s.Image, + Command: s.Command, + Args: s.Args, + WorkingDir: s.WorkingDir, + Ports: s.Ports, + EnvFrom: s.EnvFrom, + Env: s.Env, + Resources: s.Resources, + VolumeMounts: s.VolumeMounts, + VolumeDevices: s.VolumeDevices, + LivenessProbe: s.LivenessProbe, + ReadinessProbe: s.ReadinessProbe, + StartupProbe: s.StartupProbe, + Lifecycle: s.Lifecycle, + TerminationMessagePath: s.TerminationMessagePath, + TerminationMessagePolicy: s.TerminationMessagePolicy, + ImagePullPolicy: s.ImagePullPolicy, + SecurityContext: s.SecurityContext, + Stdin: s.Stdin, + StdinOnce: s.StdinOnce, + TTY: s.TTY, + } + } return &corev1.Container{ Name: s.Name, Image: s.Image, @@ -765,6 +798,7 @@ func (s *Sidecar) ToK8sContainer() *corev1.Container { VolumeDevices: s.VolumeDevices, LivenessProbe: s.LivenessProbe, ReadinessProbe: s.ReadinessProbe, + RestartPolicy: s.RestartPolicy, StartupProbe: s.StartupProbe, Lifecycle: s.Lifecycle, TerminationMessagePath: s.TerminationMessagePath, @@ -801,4 +835,5 @@ func (s *Sidecar) SetContainerFields(c corev1.Container) { s.Stdin = c.Stdin s.StdinOnce = c.StdinOnce s.TTY = c.TTY + s.RestartPolicy = c.RestartPolicy } diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index dde2863832c..65a8e49823a 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -3616,6 +3616,13 @@ func schema_pkg_apis_pipeline_v1beta1_Sidecar(ref common.ReferenceCallback) comm }, }, }, + "restartPolicy": { + SchemaProps: spec.SchemaProps{ + Description: "RestartPolicy refers to kubernetes RestartPolicy. It can only be set for an initContainer and must have it's policy set to \"Always\". It is currently left optional to help support Kubernetes versions prior to 1.29 when this feature was introduced.", + Type: []string{"string"}, + Format: "", + }, + }, }, Required: []string{"name"}, }, diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index e17ab8e0f33..87df8750801 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -1932,6 +1932,10 @@ "default": {}, "$ref": "#/definitions/v1.ResourceRequirements" }, + "restartPolicy": { + "description": "RestartPolicy refers to kubernetes RestartPolicy. It can only be set for an initContainer and must have it's policy set to \"Always\". It is currently left optional to help support Kubernetes versions prior to 1.29 when this feature was introduced.", + "type": "string" + }, "script": { "description": "Script is the contents of an executable file to execute.\n\nIf Script is not empty, the Step cannot have an Command or Args.", "type": "string" diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index e2e55e8f8d9..bb9fbaa35ec 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1696,6 +1696,11 @@ func (in *Sidecar) DeepCopyInto(out *Sidecar) { *out = make([]WorkspaceUsage, len(*in)) copy(*out, *in) } + if in.RestartPolicy != nil { + in, out := &in.RestartPolicy, &out.RestartPolicy + *out = new(corev1.ContainerRestartPolicy) + **out = **in + } return } diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 792e2143e11..fc0db922d8a 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -74,6 +74,11 @@ const ( // TerminationReasonCancelled indicates a step was cancelled. TerminationReasonCancelled = "Cancelled" + + StepArtifactPathPattern = "step.artifacts.path" + + // K8s version to determine if to use native k8s sidecar or Tekton sidecar + SidecarK8sMinorVersionCheck = 29 ) // These are effectively const, but Go doesn't have such an annotation. @@ -426,11 +431,41 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta } mergedPodContainers := stepContainers - - // Merge sidecar containers with step containers. - for _, sc := range sidecarContainers { - sc.Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", sidecarPrefix, sc.Name)) - mergedPodContainers = append(mergedPodContainers, sc) + mergedPodInitContainers := initContainers + + // Check if current k8s version is less than 1.29 + // Since Kubernetes Major version cannot be 0 and if it's 2 then sidecar will be in + // we are only concerned about major version 1 and if the minor is less than 29 then + // we need to do the current logic + useTektonSidecar := true + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableKubernetesSidecar { + // Go through the logic for enable-kubernetes feature flag + // Kubernetes Version + dc := b.KubeClient.Discovery() + sv, err := dc.ServerVersion() + if err != nil { + return nil, err + } + svMinorInt, _ := strconv.Atoi(sv.Minor) + svMajorInt, _ := strconv.Atoi(sv.Major) + if svMajorInt >= 1 && svMinorInt >= SidecarK8sMinorVersionCheck { + // Add RestartPolicy and Merge into initContainer + useTektonSidecar = false + for i := range sidecarContainers { + sc := &sidecarContainers[i] + always := corev1.ContainerRestartPolicyAlways + sc.RestartPolicy = &always + sc.Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", sidecarPrefix, sc.Name)) + mergedPodInitContainers = append(mergedPodInitContainers, *sc) + } + } + } + if useTektonSidecar { + // Merge sidecar containers with step containers. + for _, sc := range sidecarContainers { + sc.Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", sidecarPrefix, sc.Name)) + mergedPodContainers = append(mergedPodContainers, sc) + } } var dnsPolicy corev1.DNSPolicy @@ -479,7 +514,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta }, Spec: corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: initContainers, + InitContainers: mergedPodInitContainers, Containers: mergedPodContainers, ServiceAccountName: taskRun.Spec.ServiceAccountName, Volumes: volumes, diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 2917333a4f1..8eb65ae5e60 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -37,6 +37,8 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/version" + fakediscovery "k8s.io/client-go/discovery/fake" fakek8s "k8s.io/client-go/kubernetes/fake" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -3998,3 +4000,141 @@ func Test_artifactsPathReferenced(t *testing.T) { }) } } + +func TestPodBuildWithK8s129(t *testing.T) { + always := corev1.ContainerRestartPolicyAlways + ts := v1.TaskSpec{ + Steps: []v1.Step{{ + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. + }}, + Sidecars: []v1.Sidecar{{ + Name: "name", + Image: "image", + Command: []string{"cmd"}, + }}, + } + want := &corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{ + entrypointInitContainer( + images.EntrypointImage, + []v1.Step{{Name: "name"}}, + false, /* setSecurityContext */ + false /* windows */), + { + Name: "sidecar-name", + Image: "image", + Command: []string{"/tekton/bin/entrypoint"}, + Args: []string{ + "-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/run/0/out", + "-termination_path", + "/tekton/termination", + "-step_metadata_dir", + "/tekton/run/0/status", + "-entrypoint", + "cmd", + "--", + }, + RestartPolicy: &always, + }, + }, + Containers: []corev1.Container{{ + Name: "step-name", + Image: "image", + Command: []string{"/tekton/bin/entrypoint"}, + Args: []string{ + "-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/run/0/out", + "-termination_path", + "/tekton/termination", + "-step_metadata_dir", + "/tekton/run/0/status", + "-entrypoint", + "cmd", + "--", + }, + }}, + } + featureFlags := map[string]string{ + "enable-kubernetes-sidecar": "true", + } + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: featureFlags, + }, + ) + kubeclient := fakek8s.NewSimpleClientset( + &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}, + &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "service-account", Namespace: "default"}, + Secrets: []corev1.ObjectReference{{ + Name: "multi-creds", + }}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multi-creds", + Namespace: "default", + Annotations: map[string]string{ + "tekton.dev/docker-0": "https://us.gcr.io", + "tekton.dev/docker-1": "https://docker.io", + "tekton.dev/git-0": "github.com", + "tekton.dev/git-1": "gitlab.com", + }, + }, + Type: "kubernetes.io/basic-auth", + Data: map[string][]byte{ + "username": []byte("foo"), + "password": []byte("BestEver"), + }, + }, + ) + fakeDisc, _ := kubeclient.Discovery().(*fakediscovery.FakeDiscovery) + fakeDisc.FakedServerVersion = &version.Info{ + Major: "1", + Minor: "29", + } + + trs := v1.TaskRunSpec{ + TaskSpec: &ts, + } + + tr := &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "taskrunName", + Namespace: "default", + }, + Spec: trs, + } + + // No entrypoints should be looked up. + entrypointCache := fakeCache{} + + builder := Builder{ + Images: images, + KubeClient: kubeclient, + EntrypointCache: entrypointCache, + } + got, err := builder.Build(store.ToContext(context.Background()), tr, ts) + if err != nil { + t.Errorf("Pod build failed: %s", err) + } + if d := cmp.Diff(want.InitContainers[1].Name, got.Spec.InitContainers[1].Name); d != "" { + t.Errorf("Pod does not have sidecar in init list: %s", diff.PrintWantGot(d)) + } + + if d := cmp.Diff(want.InitContainers[1].RestartPolicy, got.Spec.InitContainers[1].RestartPolicy); d != "" { + t.Errorf("Sidecar does not have RestartPolicy Always: %s", diff.PrintWantGot(d)) + } +} diff --git a/pkg/pod/status.go b/pkg/pod/status.go index bee46ebcf90..c0db895fe43 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -151,6 +151,11 @@ func MakeTaskRunStatus(ctx context.Context, logger *zap.SugaredLogger, tr v1.Tas sidecarStatuses = append(sidecarStatuses, s) } } + for _, s := range pod.Status.InitContainerStatuses { + if IsContainerSidecar(s.Name) { + sidecarStatuses = append(sidecarStatuses, s) + } + } var merr *multierror.Error if err := setTaskRunStatusBasedOnStepStatus(ctx, logger, stepStatuses, &tr, pod.Status.Phase, kubeclient, ts); err != nil { diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index b51daad6e11..ece5e891bf8 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -22,6 +22,7 @@ import ( "fmt" "reflect" "slices" + "strconv" "strings" "time" @@ -152,8 +153,28 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgrecon // and may not have had all of the assumed default specified. tr.SetDefaults(ctx) - if err := c.stopSidecars(ctx, tr); err != nil { - return err + // Check if current k8s version is less than 1.29 + // Since Kubernetes Major version cannot be 0 and if it's 2 then sidecar will be in + // we are only concerned about major version 1 and if the minor is less than 29 then + // we need to do the current logic + useTektonSidecar := true + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableKubernetesSidecar { + dc := c.KubeClientSet.Discovery() + sv, err := dc.ServerVersion() + if err != nil { + return err + } + svMajorInt, _ := strconv.Atoi(sv.Major) + svMinorInt, _ := strconv.Atoi(sv.Minor) + if svMajorInt >= 1 && svMinorInt >= 29 { + useTektonSidecar = false + logger.Infof("Using Kubernetes Native Sidecars \n") + } + } + if useTektonSidecar { + if err := c.stopSidecars(ctx, tr); err != nil { + return err + } } return c.finishReconcileUpdateEmitEvents(ctx, tr, before, nil) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 282ac982151..ea4b9c1ce6c 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -2317,7 +2317,7 @@ status: // Check actions actions := clients.Kube.Actions() if len(actions) != 2 || !actions[0].Matches("list", "configmaps") || !actions[1].Matches("watch", "configmaps") { - t.Errorf("expected 2 actions (list configmaps, and watch configmaps) created by the reconciler,"+ + t.Errorf("expected 3 actions (list configmaps, and watch configmaps) created by the reconciler,"+ " got %d. Actions: %#v", len(actions), actions) } diff --git a/test/e2e-tests-kind-prow-alpha.env b/test/e2e-tests-kind-prow-alpha.env index dfcc2e76444..5ab894c2ac7 100644 --- a/test/e2e-tests-kind-prow-alpha.env +++ b/test/e2e-tests-kind-prow-alpha.env @@ -9,3 +9,4 @@ ENABLE_CEL_IN_WHENEXPRESSION=true ENABLE_PARAM_ENUM=true ENABLE_ARTIFACTS=true ENABLE_CONCISE_RESOLVER_SYNTAX=true +ENABLE_KUBERNETES_SIDECAR=true diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 8a8b63a48eb..9d78515fdb5 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -33,6 +33,7 @@ ENABLE_CEL_IN_WHENEXPRESSION=${ENABLE_CEL_IN_WHENEXPRESSION:="false"} ENABLE_PARAM_ENUM=${ENABLE_PARAM_ENUM:="false"} ENABLE_ARTIFACTS=${ENABLE_ARTIFACTS:="false"} ENABLE_CONCISE_RESOLVER_SYNTAX=${ENABLE_CONCISE_RESOLVER_SYNTAX:="false"} +ENABLE_KUBERNETES_SIDECAR=${ENABLE_KUBERNETES_SIDECAR:="false"} failed=0 # Script entry point. @@ -143,6 +144,18 @@ function set_enable_concise_resolver_syntax() { kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" } +function set_enable_kubernetes_sidecar() { + local method="$1" + if [ "$method" != "false" ] && [ "$method" != "true" ]; then + printf "Invalid value for enable-kubernetes-sidecar %s\n" ${method} + exit 255 + fi + printf "Setting enable-kubernetes-sidecar to %s\n", ${method} + jsonpatch=$(printf "{\"data\": {\"enable-kubernetes-sidecar\": \"%s\"}}" $1) + echo "feature-flags ConfigMap patch: ${jsonpatch}" + kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" +} + function run_e2e() { # Run the integration tests header "Running Go e2e tests" @@ -171,6 +184,7 @@ set_cel_in_whenexpression "$ENABLE_CEL_IN_WHENEXPRESSION" set_enable_param_enum "$ENABLE_PARAM_ENUM" set_enable_artifacts "$ENABLE_ARTIFACTS" set_enable_concise_resolver_syntax "$ENABLE_CONCISE_RESOLVER_SYNTAX" +set_enable_kubernetes_sidecar "$ENABLE_KUBERNETES_SIDECAR" run_e2e (( failed )) && fail_test diff --git a/test/featureflags.go b/test/featureflags.go index 823d19b998f..a6b6db071f6 100644 --- a/test/featureflags.go +++ b/test/featureflags.go @@ -119,6 +119,7 @@ func getFeatureFlagsBaseOnAPIFlag(t *testing.T) *config.FeatureFlags { "enable-param-enum": "true", "enable-artifacts": "true", "enable-concise-resolver-syntax": "true", + "enable-kubernetes-sidecar": "true", }) if err != nil { t.Fatalf("error creating alpha feature flags configmap: %v", err) diff --git a/test/sidecar_test.go b/test/sidecar_test.go index 68c492d2607..33660030bd9 100644 --- a/test/sidecar_test.go +++ b/test/sidecar_test.go @@ -62,6 +62,12 @@ func TestSidecarTaskSupport(t *testing.T) { for _, test := range tests { t.Run(test.desc, func(t *testing.T) { + // If Kubernetes Sidecar support is enabled the Pod will terminate and it gets caught as an error though it's expected + ff := getFeatureFlagsBaseOnAPIFlag(t) + + if ff.EnableKubernetesSidecar { + t.SkipNow() + } t.Parallel() ctx, cancel := context.WithCancel(ctx)