Improvements for native sidecar support and compatibility.

cmd/webhook/main.go

@@ -25,6 +25,7 @@ import (
 	"github.com/go-logr/logr"
 	wh "github.com/googlecloudplatform/gcs-fuse-csi-driver/pkg/webhook"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/version"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
@@ -52,7 +53,7 @@ var (
 	sidecarImage            = flag.String("sidecar-image", "", "The gcsfuse sidecar container image.")
 
 	// These are set at compile time.
-	version = "unknown"
+	webhookVersion = "unknown"
 )
 
 const (
@@ -67,7 +68,7 @@ func main() {
 	// This line prevents controller-runtime from complaining about log.SetLogger never being called
 	log.SetLogger(logr.New(log.NullLogSink{}))
 
-	klog.Infof("Running Google Cloud Storage FUSE CSI driver admission webhook version %v, sidecar container image %v", version, *sidecarImage)
+	klog.Infof("Running Google Cloud Storage FUSE CSI driver admission webhook version %v, sidecar container image %v", webhookVersion, *sidecarImage)
 
 	// Load webhook config
 	c := wh.LoadConfig(*sidecarImage, *imagePullPolicy, *cpuRequest, *cpuLimit, *memoryRequest, *memoryLimit, *ephemeralStorageRequest, *ephemeralStorageLimit)
@@ -78,7 +79,19 @@ func main() {
 	// Setup client
 	client, err := kubernetes.NewForConfig(kubeConfig)
 	if err != nil {
-		klog.Fatalf("Unable to get clientset: %v", err)
+		klog.Warningf("Unable to get clientset: %v", err)
+	}
+
+	var serverVersion *version.Version
+	// Get and format sever version.
+	v, err := client.DiscoveryClient.ServerVersion()
+	if err != nil || v == nil {
+		klog.Warningf("Unable to get server version : %v", err)
+	} else {
+		serverVersion, err = version.ParseGeneric(v.String())
+		if err != nil {
+			klog.Warningf(`Unable to parse server version "%s": %v`, v.String(), err)
+		}
 	}
 
 	// Setup stop channel
@@ -120,10 +133,11 @@ func main() {
 	klog.Info("Registering webhooks to the webhook server.")
 	hookServer.Register("/inject", &webhook.Admission{
 		Handler: &wh.SidecarInjector{
-			Client:     mgr.GetClient(),
-			Config:     c,
-			Decoder:    admission.NewDecoder(runtime.NewScheme()),
-			NodeLister: nodeLister,
+			Client:        mgr.GetClient(),
+			Config:        c,
+			Decoder:       admission.NewDecoder(runtime.NewScheme()),
+			NodeLister:    nodeLister,
+			ServerVersion: serverVersion,
 		},
 	})
 

pkg/webhook/mutatingwebhook.go

@@ -45,15 +45,16 @@ const (
 	annotationGcsfuseSidecarMemoryRequestKey           = "gke-gcsfuse/memory-request"
 	annotationGcsfuseSidecarEphemeralStorageRequestKey = "gke-gcsfuse/ephemeral-storage-request"
 
-	istioSidecarName = "istio-proxy"
+	IstioSidecarName = "istio-proxy"
 )
 
 type SidecarInjector struct {
 	Client client.Client
 	// default sidecar container config values, can be overwritten by the pod annotations
-	Config     *Config
-	Decoder    *admission.Decoder
-	NodeLister listersv1.NodeLister
+	Config        *Config
+	Decoder       *admission.Decoder
+	NodeLister    listersv1.NodeLister
+	ServerVersion *version.Version
 }
 
 // Handle injects a gcsfuse sidecar container and a emptyDir to incoming qualified pods.
@@ -232,48 +233,54 @@ func (si *SidecarInjector) supportsNativeSidecar() (bool, error) {
 	}
 
 	if len(clusterNodes) == 0 {
-		// TODO(jaimebz): Rely on cluster version in the event there's no nodes to reference.
-		supportsNativeSidecar = false
+		// Rely on cluster version in the event there's no nodes to reference.
+		if si.ServerVersion != nil {
+			supportsNativeSidecar = si.ServerVersion.AtLeast(minimumSupportedVersion)
+		} else {
+			supportsNativeSidecar = false
+		}
 	}
 
 	return supportsNativeSidecar, nil
 }
 
 func injectSidecarContainer(pod *corev1.Pod, config *Config, supportsNativeSidecar bool) {
 	if supportsNativeSidecar {
-		sidecarSpec := GetNativeSidecarContainerSpec(config)
-		if sidecarPresentAtFirstPosition(pod.Spec.InitContainers, istioSidecarName) {
-			pod.Spec.InitContainers = injectAtSecondPosition(pod.Spec.InitContainers, sidecarSpec)
-		} else {
-			pod.Spec.InitContainers = append([]corev1.Container{sidecarSpec}, pod.Spec.InitContainers...)
-		}
+		pod.Spec.InitContainers = insert(pod.Spec.InitContainers, GetNativeSidecarContainerSpec(config), getInjectIndex(pod.Spec.InitContainers))
 	} else {
-		sidecarSpec := GetSidecarContainerSpec(config)
-		if sidecarPresentAtFirstPosition(pod.Spec.Containers, istioSidecarName) {
-			pod.Spec.Containers = injectAtSecondPosition(pod.Spec.Containers, sidecarSpec)
-		} else {
-			pod.Spec.Containers = append([]corev1.Container{sidecarSpec}, pod.Spec.Containers...)
-		}
+		pod.Spec.Containers = insert(pod.Spec.Containers, GetSidecarContainerSpec(config), getInjectIndex(pod.Spec.Containers))
 	}
 }
 
-func injectAtSecondPosition(containers []corev1.Container, sidecar corev1.Container) []corev1.Container {
-	const index = 1
-	if len(containers) == 0 {
-		return []corev1.Container{sidecar}
+func insert(a []corev1.Container, value corev1.Container, index int) []corev1.Container {
+	// For index == len(a)
+	if len(a) == index {
+		return append(a, value)
 	}
-	containers = append(containers, corev1.Container{})
-	copy(containers[index+1:], containers[index:])
-	containers[index] = sidecar
 
-	return containers
+	// For index < len(a)
+	a = append(a[:index+1], a[index:]...)
+	a[index] = value
+
+	return a
+}
+
+func getInjectIndex(containers []corev1.Container) int {
+	idx, present := containerPresent(containers, IstioSidecarName)
+	if present {
+		return idx + 1
+	}
+
+	return 0
 }
 
-// Checks the first index of the container array for the istio container sidecar.
-func sidecarPresentAtFirstPosition(containers []corev1.Container, sidecarName string) bool {
-	if len(containers) == 0 {
-		return false
+// Checks by name matching that the container is present in container list.
+func containerPresent(containers []corev1.Container, container string) (int, bool) {
+	for idx, c := range containers {
+		if c.Name == container {
+			return idx, true
+		}
 	}
 
-	return (containers[0].Name == sidecarName)
+	return -1, false
 }

pkg/webhook/mutatingwebhook_test.go

@@ -40,7 +40,7 @@ import (
 )
 
 var istioContainer = corev1.Container{
-	Name: "istio-proxy",
+	Name: IstioSidecarName,
 }
 
 func TestPrepareConfig(t *testing.T) {
@@ -683,25 +683,27 @@ func skewVersionNodes() []corev1.Node {
 	}
 }
 
-func TestInjectAtSecondPosition(t *testing.T) {
+func TestInsert(t *testing.T) {
 	t.Parallel()
 	testCases := []struct {
 		name         string
 		containers   []corev1.Container
 		sidecar      corev1.Container
 		expectResult []corev1.Container
+		idx          int
 	}{
 		{
-			name:       "successful injection at second position, 0 element initially",
+			name:       "successful injection at 1st position, 0 element initially",
 			containers: []corev1.Container{},
 			sidecar: corev1.Container{
-				Name: "two",
+				Name: "one",
 			},
 			expectResult: []corev1.Container{
 				{
-					Name: "two",
+					Name: "one",
 				},
 			},
+			idx: 0,
 		},
 		{
 			name: "successful injection at second position, 1 element initially",
@@ -721,6 +723,7 @@ func TestInjectAtSecondPosition(t *testing.T) {
 					Name: "two",
 				},
 			},
+			idx: 1,
 		},
 		{
 			name: "successful injection at second position, 3 elements initially",
@@ -752,12 +755,163 @@ func TestInjectAtSecondPosition(t *testing.T) {
 					Name: "four",
 				},
 			},
+			idx: 1,
+		},
+
+		{
+			name: "successful injection at first position, 3 elements initially",
+			containers: []corev1.Container{
+				{
+					Name: "one",
+				},
+				{
+					Name: "two",
+				},
+				{
+					Name: "three",
+				},
+			},
+			sidecar: corev1.Container{
+				Name: "sidecar",
+			},
+			expectResult: []corev1.Container{
+				{
+					Name: "sidecar",
+				},
+				{
+					Name: "one",
+				},
+				{
+					Name: "two",
+				},
+				{
+					Name: "three",
+				},
+			},
+			idx: 0,
+		},
+		{
+			name: "successful injection at last position, 3 elements initially",
+			containers: []corev1.Container{
+				{
+					Name: "one",
+				},
+				{
+					Name: "two",
+				},
+				{
+					Name: "three",
+				},
+			},
+			sidecar: corev1.Container{
+				Name: "four",
+			},
+			expectResult: []corev1.Container{
+				{
+					Name: "one",
+				},
+				{
+					Name: "two",
+				},
+				{
+					Name: "three",
+				},
+				{
+					Name: "four",
+				},
+			},
+			idx: 3,
 		},
 	}
 	for _, tc := range testCases {
-		result := injectAtSecondPosition(tc.containers, tc.sidecar)
+		result := insert(tc.containers, tc.sidecar, tc.idx)
 		if diff := cmp.Diff(tc.expectResult, result); diff != "" {
 			t.Errorf(`for test "%s", got different results (-expect, +got):\n"%s"`, tc.name, diff)
 		}
 	}
 }
+
+func TestGetInjectIndex(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name       string
+		containers []corev1.Container
+		idx        int
+	}{
+		{
+			name:       "injection at first position, 0 element initially",
+			containers: []corev1.Container{},
+			idx:        0,
+		},
+		{
+			name: "injection at first position, 1 element initially",
+			containers: []corev1.Container{
+				{
+					Name: "one",
+				},
+			},
+			idx: 0,
+		},
+		{
+			name: "injection at first position, 3 elements initially",
+			containers: []corev1.Container{
+				{
+					Name: "one",
+				},
+				{
+					Name: "two",
+				},
+				{
+					Name: "three",
+				},
+			},
+			idx: 0,
+		},
+		{
+			name: "injection at second position, 3 elements initially",
+			containers: []corev1.Container{
+				istioContainer,
+				{
+					Name: "two",
+				},
+				{
+					Name: "three",
+				},
+			},
+			idx: 1,
+		},
+		{
+			name: "injection at third position, 3 elements initially",
+			containers: []corev1.Container{
+				{
+					Name: "one",
+				},
+				istioContainer,
+				{
+					Name: "three",
+				},
+			},
+			idx: 2,
+		},
+		{
+			name: "injection at last position, 3 elements initially",
+			containers: []corev1.Container{
+				{
+					Name: "one",
+				},
+				{
+					Name: "two",
+				},
+				istioContainer,
+			},
+			idx: 3,
+		},
+	}
+	for _, tc := range testCases {
+		idx := getInjectIndex(tc.containers)
+		if idx != tc.idx {
+			t.Errorf(`expected injection to be at index "%d" but got "%d"`, tc.idx, idx)
+		}
+	}
+}