From 58fdc3fe672d9a0ea73e871ea99df204973abc62 Mon Sep 17 00:00:00 2001
From: payall4u <payall4u@qq.com>
Date: Wed, 29 Nov 2023 20:23:44 +0800
Subject: [PATCH] Add nil check and add unit test

---
 .../collector/cadvisor/cadvisor_linux.go      |  2 +-
 pkg/webhooks/pod/mutating.go                  | 21 ++++++-----
 pkg/webhooks/pod/mutating_test.go             | 37 +++++++++++++++----
 3 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/pkg/ensurance/collector/cadvisor/cadvisor_linux.go b/pkg/ensurance/collector/cadvisor/cadvisor_linux.go
index f0850606d..ce0416a57 100644
--- a/pkg/ensurance/collector/cadvisor/cadvisor_linux.go
+++ b/pkg/ensurance/collector/cadvisor/cadvisor_linux.go
@@ -78,7 +78,7 @@ func NewCadvisorManager(cgroupDriver string) Manager {
 	sysfs := csysfs.NewRealSysFs()
 	maxHousekeepingConfig := cmanager.HouskeepingConfig{Interval: &maxHousekeepingInterval, AllowDynamic: &allowDynamic}
 
-	m, err := cmanager.New(memCache, sysfs, maxHousekeepingConfig, includedMetrics, http.DefaultClient, []string{"/" + utils.CgroupKubePods},  nil /* containerEnvMetadataWhiteList */, "" /* perfEventsFile */, time.Duration(0) /*resctrlInterval*/)
+	m, err := cmanager.New(memCache, sysfs, maxHousekeepingConfig, includedMetrics, http.DefaultClient, []string{"/" + utils.CgroupKubePods}, nil /* containerEnvMetadataWhiteList */, "" /* perfEventsFile */, time.Duration(0) /*resctrlInterval*/)
 	if err != nil {
 		klog.Errorf("Failed to create cadvisor manager start: %v", err)
 		return nil
diff --git a/pkg/webhooks/pod/mutating.go b/pkg/webhooks/pod/mutating.go
index 215630308..92d645c66 100644
--- a/pkg/webhooks/pod/mutating.go
+++ b/pkg/webhooks/pod/mutating.go
@@ -4,17 +4,16 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/gocrane/crane/pkg/ensurance/util"
+	"github.com/gocrane/api/ensurance/v1alpha1"
 	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/v2"
 
-	"github.com/gocrane/api/ensurance/v1alpha1"
 	"github.com/gocrane/crane/pkg/ensurance/config"
+	"github.com/gocrane/crane/pkg/ensurance/util"
 )
 
 var (
@@ -46,11 +45,7 @@ func (m *MutatingAdmission) Default(ctx context.Context, obj runtime.Object) err
 		return nil
 	}
 
-	if m.Config == nil || !m.Config.QOSInitializer.Enable {
-		return nil
-	}
-
-	if pod.Labels == nil {
+	if !m.available() {
 		return nil
 	}
 
@@ -84,6 +79,7 @@ func (m *MutatingAdmission) Default(ctx context.Context, obj runtime.Object) err
 		klog.V(2).Infof("Injection skipped: not a low CPUPriority pod, qos %s", qos.Name)
 		return nil
 	}
+
 	for _, container := range pod.Spec.InitContainers {
 		if container.Name == m.Config.QOSInitializer.InitContainerTemplate.Name {
 			klog.V(2).Infof("Injection skipped: pod has initializerContainer already")
@@ -110,3 +106,10 @@ func (m *MutatingAdmission) Default(ctx context.Context, obj runtime.Object) err
 
 	return nil
 }
+
+func (m *MutatingAdmission) available() bool {
+	return m.Config != nil &&
+		m.Config.QOSInitializer.Enable &&
+		m.Config.QOSInitializer.InitContainerTemplate != nil &&
+		m.Config.QOSInitializer.VolumeTemplate != nil
+}
diff --git a/pkg/webhooks/pod/mutating_test.go b/pkg/webhooks/pod/mutating_test.go
index 8d2d2c4cd..bc8792604 100644
--- a/pkg/webhooks/pod/mutating_test.go
+++ b/pkg/webhooks/pod/mutating_test.go
@@ -6,10 +6,9 @@ import (
 
 	"github.com/gocrane/api/ensurance/v1alpha1"
 	"github.com/stretchr/testify/assert"
-	"k8s.io/utils/pointer"
-
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
 	"sigs.k8s.io/yaml"
 
 	"github.com/gocrane/crane/pkg/ensurance/config"
@@ -47,6 +46,21 @@ func TestDefaultingPodQOSInitializer(t *testing.T) {
 	}
 }
 
+func TestPrecheck(t *testing.T) {
+	configYaml := "apiVersion: ensurance.crane.io/v1alpha1\nkind: QOSConfig\nqosInitializer:\n  enable: true\n  selector: \n    matchLabels:\n      app: nginx\n"
+
+	config := &config.QOSConfig{}
+	err := yaml.Unmarshal([]byte(configYaml), config)
+	if err != nil {
+		t.Errorf("unmarshal config failed:%v", err)
+	}
+	m := MutatingAdmission{
+		Config:     config,
+		listPodQOS: MockListPodQOSFunc,
+	}
+	assert.False(t, m.available())
+}
+
 func MockListPodQOSFunc() ([]*v1alpha1.PodQOS, error) {
 	return []*v1alpha1.PodQOS{
 		{
@@ -90,14 +104,21 @@ func MockListPodQOSFunc() ([]*v1alpha1.PodQOS, error) {
 }
 
 func MockPod(name string, labels ...string) *v1.Pod {
-	labelmap := map[string]string{}
-	for i := 0; i < len(labels)-1; i += 2 {
-		labelmap[labels[i]] = labels[i+1]
-	}
-	return &v1.Pod{
+	pod := &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:   name,
-			Labels: labelmap,
+			Labels: nil,
 		},
 	}
+
+	if len(labels) < 2 {
+		return pod
+	}
+
+	labelmap := map[string]string{}
+	for i := 0; i < len(labels)-1; i += 2 {
+		labelmap[labels[i]] = labels[i+1]
+	}
+	pod.Labels = labelmap
+	return pod
 }