Skip to content

Commit cdf03a6

Browse files
say5tekton-robot
authored andcommitted
chore: refactor to use IsNativeSidecarSupport function
1 parent 03d929a commit cdf03a6

File tree

3 files changed

+88
-17
lines changed

3 files changed

+88
-17
lines changed

pkg/pod/pod.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
corev1 "k8s.io/api/core/v1"
3838
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3939
"k8s.io/apimachinery/pkg/runtime/schema"
40+
"k8s.io/apimachinery/pkg/version"
4041
"k8s.io/client-go/kubernetes"
4142
"k8s.io/utils/strings/slices"
4243
"knative.dev/pkg/changeset"
@@ -433,10 +434,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta
433434
mergedPodContainers := stepContainers
434435
mergedPodInitContainers := initContainers
435436

436-
// Check if current k8s version is less than 1.29
437-
// Since Kubernetes Major version cannot be 0 and if it's 2 then sidecar will be in
438-
// we are only concerned about major version 1 and if the minor is less than 29 then
439-
// we need to do the current logic
440437
useTektonSidecar := true
441438
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableKubernetesSidecar {
442439
// Go through the logic for enable-kubernetes feature flag
@@ -446,10 +443,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta
446443
if err != nil {
447444
return nil, err
448445
}
449-
svMinor := strings.TrimSuffix(sv.Minor, "+") // Remove '+' if present
450-
svMajorInt, _ := strconv.Atoi(sv.Major)
451-
svMinorInt, _ := strconv.Atoi(svMinor)
452-
if svMajorInt >= 1 && svMinorInt >= SidecarK8sMinorVersionCheck {
446+
if IsNativeSidecarSupport(sv) {
453447
// Add RestartPolicy and Merge into initContainer
454448
useTektonSidecar = false
455449
for i := range sidecarContainers {
@@ -736,3 +730,16 @@ func artifactPathReferencedInStep(step v1.Step) bool {
736730
}
737731
return false
738732
}
733+
734+
// isNativeSidecarSupport returns true if k8s api has native sidecar support
735+
// based on the k8s version (1.29+).
736+
// See https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/ for more info.
737+
func IsNativeSidecarSupport(serverVersion *version.Info) bool {
738+
minor := strings.TrimSuffix(serverVersion.Minor, "+") // Remove '+' if present
739+
majorInt, _ := strconv.Atoi(serverVersion.Major)
740+
minorInt, _ := strconv.Atoi(minor)
741+
if (majorInt == 1 && minorInt >= SidecarK8sMinorVersionCheck) || majorInt > 1 {
742+
return true
743+
}
744+
return false
745+
}

pkg/pod/pod_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3767,3 +3767,75 @@ func TestPodBuildWithK8s129(t *testing.T) {
37673767
t.Errorf("Sidecar does not have RestartPolicy Always: %s", diff.PrintWantGot(d))
37683768
}
37693769
}
3770+
func TestIsNativeSidecarSupport(t *testing.T) {
3771+
tests := []struct {
3772+
name string
3773+
serverVersion *version.Info
3774+
want bool
3775+
}{
3776+
{
3777+
name: "Kubernetes version 1.29",
3778+
serverVersion: &version.Info{
3779+
Major: "1",
3780+
Minor: "29",
3781+
},
3782+
want: true,
3783+
},
3784+
{
3785+
name: "Kubernetes version 1.30",
3786+
serverVersion: &version.Info{
3787+
Major: "1",
3788+
Minor: "30",
3789+
},
3790+
want: true,
3791+
},
3792+
{
3793+
name: "Kubernetes version 2.0",
3794+
serverVersion: &version.Info{
3795+
Major: "2",
3796+
Minor: "0",
3797+
},
3798+
want: true,
3799+
},
3800+
{
3801+
name: "Kubernetes version 1.28",
3802+
serverVersion: &version.Info{
3803+
Major: "1",
3804+
Minor: "28",
3805+
},
3806+
want: false,
3807+
},
3808+
{
3809+
name: "Kubernetes version 1.29+",
3810+
serverVersion: &version.Info{
3811+
Major: "1",
3812+
Minor: "29+",
3813+
},
3814+
want: true,
3815+
},
3816+
{
3817+
name: "Kubernetes version 1.28+",
3818+
serverVersion: &version.Info{
3819+
Major: "1",
3820+
Minor: "28+",
3821+
},
3822+
want: false,
3823+
},
3824+
{
3825+
name: "Kubernetes version 0.29",
3826+
serverVersion: &version.Info{
3827+
Major: "0",
3828+
Minor: "29",
3829+
},
3830+
want: false,
3831+
},
3832+
}
3833+
3834+
for _, tt := range tests {
3835+
t.Run(tt.name, func(t *testing.T) {
3836+
if got := IsNativeSidecarSupport(tt.serverVersion); got != tt.want {
3837+
t.Errorf("IsNativeSidecarSupport() = %v, want %v", got, tt.want)
3838+
}
3839+
})
3840+
}
3841+
}

pkg/reconciler/taskrun/taskrun.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"fmt"
2323
"reflect"
2424
"slices"
25-
"strconv"
2625
"strings"
2726
"time"
2827

@@ -153,21 +152,14 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgrecon
153152
// and may not have had all of the assumed default specified.
154153
tr.SetDefaults(ctx)
155154

156-
// Check if current k8s version is less than 1.29
157-
// Since Kubernetes Major version cannot be 0 and if it's 2 then sidecar will be in
158-
// we are only concerned about major version 1 and if the minor is less than 29 then
159-
// we need to do the current logic
160155
useTektonSidecar := true
161156
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableKubernetesSidecar {
162157
dc := c.KubeClientSet.Discovery()
163158
sv, err := dc.ServerVersion()
164159
if err != nil {
165160
return err
166161
}
167-
svMinor := strings.TrimSuffix(sv.Minor, "+") // Remove '+' if present
168-
svMajorInt, _ := strconv.Atoi(sv.Major)
169-
svMinorInt, _ := strconv.Atoi(svMinor)
170-
if svMajorInt >= 1 && svMinorInt >= podconvert.SidecarK8sMinorVersionCheck {
162+
if podconvert.IsNativeSidecarSupport(sv) {
171163
useTektonSidecar = false
172164
logger.Infof("Using Kubernetes Native Sidecars \n")
173165
}

0 commit comments

Comments
 (0)