Skip to content

Commit

Permalink
chore: refactor to use IsNativeSidecarSupport function
Browse files Browse the repository at this point in the history
  • Loading branch information
say5 committed Dec 18, 2024
1 parent bf59db5 commit 3fcc4fa
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 17 deletions.
23 changes: 15 additions & 8 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,10 +443,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta
if err != nil {
return nil, err
}
svMinor := strings.TrimSuffix(sv.Minor, "+") // Remove '+' if present
svMajorInt, _ := strconv.Atoi(sv.Major)
svMinorInt, _ := strconv.Atoi(svMinor)
if svMajorInt >= 1 && svMinorInt >= SidecarK8sMinorVersionCheck {
if IsNativeSidecarSupport(sv) {
// Add RestartPolicy and Merge into initContainer
useTektonSidecar = false
for i := range sidecarContainers {
Expand Down Expand Up @@ -736,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)
}
})
}
}
10 changes: 1 addition & 9 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,21 +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
}
svMinor := strings.TrimSuffix(sv.Minor, "+") // Remove '+' if present
svMajorInt, _ := strconv.Atoi(sv.Major)
svMinorInt, _ := strconv.Atoi(svMinor)
if svMajorInt >= 1 && svMinorInt >= podconvert.SidecarK8sMinorVersionCheck {
if podconvert.IsNativeSidecarSupport(sv) {
useTektonSidecar = false
logger.Infof("Using Kubernetes Native Sidecars \n")
}
Expand Down

0 comments on commit 3fcc4fa

Please sign in to comment.