Add nil check and add unit test

gocrane · Nov 29, 2023 · 74132af · 74132af
1 parent 3e8bbbe
commit 74132af
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 20 deletions.
diff --git a/pkg/ensurance/analyzer/analyzer.go b/pkg/ensurance/analyzer/analyzer.go
@@ -6,8 +6,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/gocrane/crane/pkg/ensurance/util"
-
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -29,6 +27,7 @@ import (
 	ecache "github.com/gocrane/crane/pkg/ensurance/cache"
 	"github.com/gocrane/crane/pkg/ensurance/executor"
 	"github.com/gocrane/crane/pkg/ensurance/executor/podinfo"
+	"github.com/gocrane/crane/pkg/ensurance/util"
 	"github.com/gocrane/crane/pkg/known"
 	"github.com/gocrane/crane/pkg/metrics"
 	"github.com/gocrane/crane/pkg/utils"

diff --git 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
@@ -4,17 +4,17 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/gocrane/crane/pkg/ensurance/util"
 	"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 +46,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 +80,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 +107,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
@@ -4,14 +4,14 @@ import (
 	"context"
 	"testing"
 
-	"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/api/ensurance/v1alpha1"
+
 	"github.com/gocrane/crane/pkg/ensurance/config"
 )
 
@@ -47,6 +47,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 +105,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
 }