Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: minor version check for sidecar logic #8447

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/version"
"k8s.io/client-go/kubernetes"
"k8s.io/utils/strings/slices"
"knative.dev/pkg/changeset"
Expand Down Expand Up @@ -433,10 +434,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta
mergedPodContainers := stepContainers
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
Expand All @@ -446,9 +443,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta
if err != nil {
return nil, err
}
svMinorInt, _ := strconv.Atoi(sv.Minor)
svMajorInt, _ := strconv.Atoi(sv.Major)
if svMajorInt >= 1 && svMinorInt >= SidecarK8sMinorVersionCheck {
if IsNativeSidecarSupport(sv) {
// Add RestartPolicy and Merge into initContainer
useTektonSidecar = false
for i := range sidecarContainers {
Expand Down Expand Up @@ -735,3 +730,16 @@ func artifactPathReferencedInStep(step v1.Step) bool {
}
return false
}

// isNativeSidecarSupport returns true if k8s api has native sidecar support
// based on the k8s version (1.29+).
// See https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/ for more info.
func IsNativeSidecarSupport(serverVersion *version.Info) bool {
minor := strings.TrimSuffix(serverVersion.Minor, "+") // Remove '+' if present
majorInt, _ := strconv.Atoi(serverVersion.Major)
minorInt, _ := strconv.Atoi(minor)
if (majorInt == 1 && minorInt >= SidecarK8sMinorVersionCheck) || majorInt > 1 {
return true
}
return false
}
72 changes: 72 additions & 0 deletions pkg/pod/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3767,3 +3767,75 @@ func TestPodBuildWithK8s129(t *testing.T) {
t.Errorf("Sidecar does not have RestartPolicy Always: %s", diff.PrintWantGot(d))
}
}
func TestIsNativeSidecarSupport(t *testing.T) {
tests := []struct {
name string
serverVersion *version.Info
want bool
}{
{
name: "Kubernetes version 1.29",
serverVersion: &version.Info{
Major: "1",
Minor: "29",
},
want: true,
},
{
name: "Kubernetes version 1.30",
serverVersion: &version.Info{
Major: "1",
Minor: "30",
},
want: true,
},
{
name: "Kubernetes version 2.0",
serverVersion: &version.Info{
Major: "2",
Minor: "0",
},
want: true,
},
{
name: "Kubernetes version 1.28",
serverVersion: &version.Info{
Major: "1",
Minor: "28",
},
want: false,
},
{
name: "Kubernetes version 1.29+",
serverVersion: &version.Info{
Major: "1",
Minor: "29+",
},
want: true,
},
{
name: "Kubernetes version 1.28+",
serverVersion: &version.Info{
Major: "1",
Minor: "28+",
},
want: false,
},
{
name: "Kubernetes version 0.29",
serverVersion: &version.Info{
Major: "0",
Minor: "29",
},
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := IsNativeSidecarSupport(tt.serverVersion); got != tt.want {
t.Errorf("IsNativeSidecarSupport() = %v, want %v", got, tt.want)
}
})
}
}
9 changes: 1 addition & 8 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"reflect"
"slices"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -153,20 +152,14 @@ 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)

// 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 {
if podconvert.IsNativeSidecarSupport(sv) {
useTektonSidecar = false
logger.Infof("Using Kubernetes Native Sidecars \n")
}
Expand Down
Loading