Merge pull request #125066 from TommyStarK/unit-tests/kubelet-eviction

kubelet/eviction: improve unit test coverage

pkg/kubelet/eviction/helpers_test.go

@@ -26,6 +26,7 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -46,7 +47,10 @@ func quantityMustParse(value string) *resource.Quantity {
 
 func TestGetReclaimableThreshold(t *testing.T) {
 	testCases := map[string]struct {
-		thresholds []evictionapi.Threshold
+		thresholds                   []evictionapi.Threshold
+		expectedThreshold            evictionapi.Threshold
+		expectedResourceName         v1.ResourceName
+		expectedReclaimableToBeFound bool
 	}{
 		"": {
 			thresholds: []evictionapi.Threshold{
@@ -101,14 +105,40 @@ func TestGetReclaimableThreshold(t *testing.T) {
 					},
 				},
 			},
+			expectedThreshold: evictionapi.Threshold{
+				Signal:   evictionapi.Signal("memory.available"),
+				Operator: evictionapi.OpLessThan,
+				Value: evictionapi.ThresholdValue{
+					Quantity: quantityMustParse("150Mi"),
+				},
+			},
+			expectedResourceName:         v1.ResourceName("memory"),
+			expectedReclaimableToBeFound: true,
+		},
+		"no thresholds": {
+			thresholds:           []evictionapi.Threshold{},
+			expectedThreshold:    evictionapi.Threshold{},
+			expectedResourceName: v1.ResourceName(""),
+		},
+		"threshold was crossed but reclaim not implemented (invalid signal)": {
+			thresholds: []evictionapi.Threshold{
+				{
+					Signal: "mem.available",
+				},
+			},
+			expectedThreshold:    evictionapi.Threshold{},
+			expectedResourceName: v1.ResourceName(""),
 		},
 	}
-	for testName, testCase := range testCases {
+	for _, testCase := range testCases {
 		sort.Sort(byEvictionPriority(testCase.thresholds))
-		_, _, ok := getReclaimableThreshold(testCase.thresholds)
+		threshold, ressourceName, found := getReclaimableThreshold(testCase.thresholds)
-		if !ok {
+		assert.Equal(t, testCase.expectedReclaimableToBeFound, found)
-			t.Errorf("Didn't find reclaimable threshold, test: %v", testName)
+		assert.Equal(t, testCase.expectedResourceName, ressourceName)
-		}
+		assert.Equal(t, testCase.expectedThreshold.Signal, threshold.Signal)
+		assert.Equal(t, testCase.expectedThreshold.Operator, threshold.Operator)
+		assert.Equal(t, testCase.expectedThreshold.Value.Quantity.String(), threshold.Value.Quantity.String())
+		assert.Equal(t, testCase.expectedThreshold.GracePeriod, threshold.GracePeriod)
 	}
 }
 
@@ -455,6 +485,78 @@ func TestParseThresholdConfig(t *testing.T) {
 			expectErr:               true,
 			expectThresholds:        []evictionapi.Threshold{},
 		},
+		"min-reclaim-invalid-signal": {
+			allocatableConfig:       []string{},
+			evictionHard:            map[string]string{},
+			evictionSoft:            map[string]string{},
+			evictionSoftGracePeriod: map[string]string{},
+			evictionMinReclaim:      map[string]string{"mem.available": "300Mi"},
+			expectErr:               true,
+			expectThresholds:        []evictionapi.Threshold{},
+		},
+		"min-reclaim-empty-value": {
+			allocatableConfig:       []string{},
+			evictionHard:            map[string]string{},
+			evictionSoft:            map[string]string{},
+			evictionSoftGracePeriod: map[string]string{},
+			evictionMinReclaim:      map[string]string{"memory.available": ""},
+			expectErr:               true,
+			expectThresholds:        []evictionapi.Threshold{},
+		},
+		"min-reclaim-negative-percentage": {
+			allocatableConfig:       []string{},
+			evictionHard:            map[string]string{},
+			evictionSoft:            map[string]string{},
+			evictionSoftGracePeriod: map[string]string{},
+			evictionMinReclaim:      map[string]string{"memory.available": "-15%"},
+			expectErr:               true,
+			expectThresholds:        []evictionapi.Threshold{},
+		},
+		"min-reclaim-invalid-percentage": {
+			allocatableConfig:       []string{},
+			evictionHard:            map[string]string{},
+			evictionSoft:            map[string]string{},
+			evictionSoftGracePeriod: map[string]string{},
+			evictionMinReclaim:      map[string]string{"memory.available": "10..5%"},
+			expectErr:               true,
+			expectThresholds:        []evictionapi.Threshold{},
+		},
+		"hard-signal-empty-eviction-value": {
+			allocatableConfig:       []string{},
+			evictionHard:            map[string]string{"memory.available": ""},
+			evictionSoft:            map[string]string{},
+			evictionSoftGracePeriod: map[string]string{},
+			evictionMinReclaim:      map[string]string{},
+			expectErr:               true,
+			expectThresholds:        []evictionapi.Threshold{},
+		},
+		"hard-signal-invalid-float-percentage": {
+			allocatableConfig:       []string{},
+			evictionHard:            map[string]string{"memory.available": "10..5%"},
+			evictionSoft:            map[string]string{},
+			evictionSoftGracePeriod: map[string]string{},
+			evictionMinReclaim:      map[string]string{},
+			expectErr:               true,
+			expectThresholds:        []evictionapi.Threshold{},
+		},
+		"soft-grace-period-invalid-signal": {
+			allocatableConfig:       []string{},
+			evictionHard:            map[string]string{},
+			evictionSoft:            map[string]string{"memory.available": "150Mi"},
+			evictionSoftGracePeriod: map[string]string{"mem.available": "30s"},
+			evictionMinReclaim:      map[string]string{},
+			expectErr:               true,
+			expectThresholds:        []evictionapi.Threshold{},
+		},
+		"soft-invalid-grace-period": {
+			allocatableConfig:       []string{},
+			evictionHard:            map[string]string{},
+			evictionSoft:            map[string]string{"memory.available": "150Mi"},
+			evictionSoftGracePeriod: map[string]string{"memory.available": "30mins"},
+			evictionMinReclaim:      map[string]string{},
+			expectErr:               true,
+			expectThresholds:        []evictionapi.Threshold{},
+		},
 	}
 	for testName, testCase := range testCases {
 		thresholds, err := ParseThresholdConfig(testCase.allocatableConfig, testCase.evictionHard, testCase.evictionSoft, testCase.evictionSoftGracePeriod, testCase.evictionMinReclaim)
@@ -648,6 +750,47 @@ func TestAddAllocatableThresholds(t *testing.T) {
 	}
 }
 
+func TestFallbackResourcesUsage(t *testing.T) {
+	for _, test := range []struct {
+		description   string
+		usageFuncName string
+		usageFunc     func() int64
+	}{
+		{
+			description:   "disk usage, fallback value",
+			usageFuncName: "diskUsage",
+			usageFunc: func() int64 {
+				return diskUsage(&statsapi.FsStats{}).Value()
+			},
+		},
+		{
+			description:   "inode usage, fallback value",
+			usageFuncName: "inodeUsage",
+			usageFunc: func() int64 {
+				return inodeUsage(&statsapi.FsStats{}).Value()
+			},
+		},
+		{
+			description:   "memory usage, fallback value",
+			usageFuncName: "memoryUsage",
+			usageFunc: func() int64 {
+				return memoryUsage(&statsapi.MemoryStats{}).Value()
+			},
+		},
+		{
+			description:   "process usage, fallback value",
+			usageFuncName: "processUsage",
+			usageFunc: func() int64 {
+				return int64(processUsage(&statsapi.ProcessStats{}))
+			},
+		},
+	} {
+		t.Run(test.description, func(t *testing.T) {
+			assert.NotEqual(t, 0, test.usageFunc(), fmt.Sprintf("%s: unexpected fallback value", test.usageFuncName))
+		})
+	}
+}
+
 func thresholdsEqual(expected []evictionapi.Threshold, actual []evictionapi.Threshold) bool {
 	if len(expected) != len(actual) {
 		return false
@@ -3259,3 +3402,49 @@ func TestEvictonMessageWithResourceResize(t *testing.T) {
 		})
 	}
 }
+
+func TestStatsNotFoundForPod(t *testing.T) {
+	pod1 := newPod("fake-pod1", defaultPriority, []v1.Container{
+		newContainer("fake-container1", newResourceList("", "", ""), newResourceList("", "", "")),
+	}, nil)
+	pod2 := newPod("fake-pod2", defaultPriority, []v1.Container{
+		newContainer("fake-container2", newResourceList("", "", ""), newResourceList("", "", "")),
+	}, nil)
+	statsFn := func(pod *v1.Pod) (statsapi.PodStats, bool) {
+		return statsapi.PodStats{}, false
+	}
+
+	for _, test := range []struct {
+		description string
+		compFunc    func(stats statsFunc) cmpFunc
+	}{
+		{
+			description: "process",
+			compFunc:    process,
+		},
+		{
+			description: "memory",
+			compFunc:    memory,
+		},
+		{
+			description: "exceedMemoryRequests",
+			compFunc:    exceedMemoryRequests,
+		},
+		{
+			description: "exceedDiskRequests",
+			compFunc: func(stats statsFunc) cmpFunc {
+				return exceedDiskRequests(stats, nil, "")
+			},
+		},
+		{
+			description: "disk",
+			compFunc: func(stats statsFunc) cmpFunc {
+				return disk(stats, nil, "")
+			},
+		},
+	} {
+		t.Run(test.description, func(t *testing.T) {
+			assert.Equal(t, 0, test.compFunc(statsFn)(pod1, pod2), "unexpected default result")
+		})
+	}
+}

pkg/kubelet/eviction/memory_threshold_notifier_test.go

@@ -26,6 +26,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/api/resource"
 	statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
 	evictionapi "k8s.io/kubernetes/pkg/kubelet/eviction/api"
@@ -151,6 +152,46 @@ func TestUpdateThreshold(t *testing.T) {
 	}
 }
 
+func TestUpdateThresholdWithInvalidSummary(t *testing.T) {
+	testCases := []struct {
+		description                  string
+		summary                      *statsapi.Summary
+		allocatableEvictionThreshold bool
+	}{
+		{
+			description: "incomplete summary",
+			summary: &statsapi.Summary{
+				Node: statsapi.NodeStats{
+					Memory: &statsapi.MemoryStats{},
+				},
+			},
+		},
+		{
+			description:                  "system container not found in metrics",
+			allocatableEvictionThreshold: true,
+			summary: &statsapi.Summary{
+				Node: statsapi.NodeStats{
+					SystemContainers: []statsapi.ContainerStats{
+						{
+							Name: "invalid",
+						},
+					},
+					Memory: &statsapi.MemoryStats{},
+				},
+			},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			m := newTestMemoryThresholdNotifier(evictionapi.Threshold{}, nil, nil)
+			if tc.allocatableEvictionThreshold {
+				m.threshold.Signal = evictionapi.SignalAllocatableMemoryAvailable
+			}
+			assert.Error(t, m.UpdateThreshold(tc.summary))
+		})
+	}
+}
+
 func TestStart(t *testing.T) {
 	noResources := resource.MustParse("0")
 	threshold := evictionapi.Threshold{