Cpu manager file state tests

pkg/kubelet/cm/cpumanager/BUILD

@@ -43,6 +43,7 @@ go_test(
         "//pkg/kubelet/cm/cpumanager/state:go_default_library",
         "//pkg/kubelet/cm/cpumanager/topology:go_default_library",
         "//pkg/kubelet/cm/cpuset:go_default_library",
+        "//vendor/github.com/google/cadvisor/info/v1:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

pkg/kubelet/cm/cpumanager/cpu_manager_test.go

@@ -19,8 +19,12 @@ package cpumanager
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"testing"
+	"time"
 
+	cadvisorapi "github.com/google/cadvisor/info/v1"
+	"io/ioutil"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -29,6 +33,7 @@ import (
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"
 	"k8s.io/kubernetes/pkg/kubelet/cm/cpumanager/state"
 	"k8s.io/kubernetes/pkg/kubelet/cm/cpuset"
+	"os"
 )
 
 type mockState struct {
@@ -223,6 +228,127 @@ func TestCPUManagerAdd(t *testing.T) {
 	}
 }
 
+func TestCPUManagerGenerate(t *testing.T) {
+	testCases := []struct {
+		description                string
+		cpuPolicyName              string
+		nodeAllocatableReservation v1.ResourceList
+		isTopologyBroken           bool
+		panicMsg                   string
+		expectedPolicy             string
+		expectedError              error
+		skipIfPermissionsError     bool
+	}{
+		{
+			description:                "set none policy",
+			cpuPolicyName:              "none",
+			nodeAllocatableReservation: nil,
+			expectedPolicy:             "none",
+		},
+		{
+			description:                "invalid policy name",
+			cpuPolicyName:              "invalid",
+			nodeAllocatableReservation: nil,
+			expectedPolicy:             "none",
+		},
+		{
+			description:                "static policy",
+			cpuPolicyName:              "static",
+			nodeAllocatableReservation: v1.ResourceList{v1.ResourceCPU: *resource.NewQuantity(3, resource.DecimalSI)},
+			expectedPolicy:             "static",
+			skipIfPermissionsError:     true,
+		},
+		{
+			description:                "static policy - broken topology",
+			cpuPolicyName:              "static",
+			nodeAllocatableReservation: v1.ResourceList{},
+			isTopologyBroken:           true,
+			expectedError:              fmt.Errorf("could not detect number of cpus"),
+			skipIfPermissionsError:     true,
+		},
+		{
+			description:                "static policy - broken reservation",
+			cpuPolicyName:              "static",
+			nodeAllocatableReservation: v1.ResourceList{},
+			panicMsg:                   "unable to determine reserved CPU resources for static policy",
+			skipIfPermissionsError:     true,
+		},
+		{
+			description:                "static policy - no CPU resources",
+			cpuPolicyName:              "static",
+			nodeAllocatableReservation: v1.ResourceList{v1.ResourceCPU: *resource.NewQuantity(0, resource.DecimalSI)},
+			panicMsg:                   "the static policy requires systemreserved.cpu + kubereserved.cpu to be greater than zero",
+			skipIfPermissionsError:     true,
+		},
+	}
+
+	mockedMachineInfo := cadvisorapi.MachineInfo{
+		NumCores: 4,
+		Topology: []cadvisorapi.Node{
+			{
+				Cores: []cadvisorapi.Core{
+					{
+						Id:      0,
+						Threads: []int{0},
+					},
+					{
+						Id:      1,
+						Threads: []int{1},
+					},
+					{
+						Id:      2,
+						Threads: []int{2},
+					},
+					{
+						Id:      3,
+						Threads: []int{3},
+					},
+				},
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.description, func(t *testing.T) {
+			machineInfo := &mockedMachineInfo
+			if testCase.isTopologyBroken {
+				machineInfo = &cadvisorapi.MachineInfo{}
+			}
+			sDir, err := ioutil.TempDir("/tmp/", "cpu_manager_test")
+			if err != nil {
+				t.Errorf("cannot create state file: %s", err.Error())
+			}
+			defer os.RemoveAll(sDir)
+			defer func() {
+				if err := recover(); err != nil {
+					if testCase.panicMsg != "" {
+						if !strings.Contains(err.(string), testCase.panicMsg) {
+							t.Errorf("Unexpected panic message. Have: %q wants %q", err, testCase.panicMsg)
+						}
+					} else {
+						t.Errorf("Unexpected panic: %q", err)
+					}
+				} else if testCase.panicMsg != "" {
+					t.Error("Expected panic hasn't been raised")
+				}
+			}()
+
+			mgr, err := NewManager(testCase.cpuPolicyName, 5*time.Second, machineInfo, testCase.nodeAllocatableReservation, sDir)
+			if testCase.expectedError != nil {
+				if !strings.Contains(err.Error(), testCase.expectedError.Error()) {
+					t.Errorf("Unexpected error message. Have: %s wants %s", err.Error(), testCase.expectedError.Error())
+				}
+			} else {
+				rawMgr := mgr.(*manager)
+				if rawMgr.policy.Name() != testCase.expectedPolicy {
+					t.Errorf("Unexpected policy name. Have: %q wants %q", rawMgr.policy.Name(), testCase.expectedPolicy)
+				}
+			}
+		})
+
+	}
+}
+
 func TestCPUManagerRemove(t *testing.T) {
 	mgr := &manager{
 		policy: &mockPolicy{

pkg/kubelet/cm/cpumanager/policy_static_test.go

@@ -38,6 +38,7 @@ type staticPolicyTest struct {
 	expErr          error
 	expCPUAlloc     bool
 	expCSet         cpuset.CPUSet
+	expPanic        bool
 }
 
 func TestStaticPolicyName(t *testing.T) {
@@ -51,18 +52,73 @@ func TestStaticPolicyName(t *testing.T) {
 }
 
 func TestStaticPolicyStart(t *testing.T) {
-	policy := NewStaticPolicy(topoSingleSocketHT, 1).(*staticPolicy)
-
-	st := &mockState{
-		assignments:   state.ContainerCPUAssignments{},
-		defaultCPUSet: cpuset.NewCPUSet(),
+	testCases := []staticPolicyTest{
+		{
+			description: "non-corrupted state",
+			topo:        topoDualSocketHT,
+			stAssignments: state.ContainerCPUAssignments{
+				"0": cpuset.NewCPUSet(0),
+			},
+			stDefaultCPUSet: cpuset.NewCPUSet(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11),
+			expCSet:         cpuset.NewCPUSet(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11),
+		},
+		{
+			description:     "empty cpuset",
+			topo:            topoDualSocketHT,
+			numReservedCPUs: 1,
+			stAssignments:   state.ContainerCPUAssignments{},
+			stDefaultCPUSet: cpuset.NewCPUSet(),
+			expCSet:         cpuset.NewCPUSet(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11),
+		},
+		{
+			description:     "reserved cores 0 & 6 are not present in available cpuset",
+			topo:            topoDualSocketHT,
+			numReservedCPUs: 2,
+			stAssignments:   state.ContainerCPUAssignments{},
+			stDefaultCPUSet: cpuset.NewCPUSet(0, 1),
+			expPanic:        true,
+		},
+		{
+			description: "assigned core 2 is still present in available cpuset",
+			topo:        topoDualSocketHT,
+			stAssignments: state.ContainerCPUAssignments{
+				"0": cpuset.NewCPUSet(0, 1, 2),
+			},
+			stDefaultCPUSet: cpuset.NewCPUSet(2, 3, 4, 5, 6, 7, 8, 9, 10, 11),
+			expPanic:        true,
+		},
 	}
+	for _, testCase := range testCases {
+		t.Run(testCase.description, func(t *testing.T) {
+			defer func() {
+				if err := recover(); err != nil {
+					if !testCase.expPanic {
+						t.Errorf("unexpected panic occured: %q", err)
+					}
+				} else if testCase.expPanic {
+					t.Error("expected panic doesn't occured")
+				}
+			}()
+			policy := NewStaticPolicy(testCase.topo, testCase.numReservedCPUs).(*staticPolicy)
+			st := &mockState{
+				assignments:   testCase.stAssignments,
+				defaultCPUSet: testCase.stDefaultCPUSet,
+			}
+			policy.Start(st)
 
-	policy.Start(st)
-	for cpuid := 1; cpuid < policy.topology.NumCPUs; cpuid++ {
-		if !st.defaultCPUSet.Contains(cpuid) {
-			t.Errorf("StaticPolicy Start() error. expected cpuid %d to be present in defaultCPUSet", cpuid)
-		}
+			if !testCase.stDefaultCPUSet.IsEmpty() {
+				for cpuid := 1; cpuid < policy.topology.NumCPUs; cpuid++ {
+					if !st.defaultCPUSet.Contains(cpuid) {
+						t.Errorf("StaticPolicy Start() error. expected cpuid %d to be present in defaultCPUSet", cpuid)
+					}
+				}
+			}
+			if !st.GetDefaultCPUSet().Equals(testCase.expCSet) {
+				t.Errorf("State CPUSet is different than expected. Have %q wants: %q", st.GetDefaultCPUSet(),
+					testCase.expCSet)
+			}
+
+		})
 	}
 }
 

pkg/kubelet/cm/cpumanager/state/state_file.go

@@ -51,7 +51,7 @@ func NewFileState(filePath string, policyName string) State {
 
 	if err := stateFile.tryRestoreState(); err != nil {
 		// could not restore state, init new state file
-		glog.Infof("[cpumanager] state file: initializing empty state file - reason: \"%s\"")
+		glog.Infof("[cpumanager] state file: initializing empty state file - reason: \"%s\"", err)
 		stateFile.cache.ClearState()
 		stateFile.storeState()
 	}

pkg/kubelet/cm/cpumanager/state/state_file_test.go

@@ -98,7 +98,7 @@ func TestFileStateTryRestore(t *testing.T) {
 		},
 		{
 			"Try restore defaultCPUSet only",
-			"{ \"defaultCpuSet\": \"4-6\"}",
+			`{"policyName": "none", "defaultCpuSet": "4-6"}`,
 			"",
 			&stateMemory{
 				assignments:   ContainerCPUAssignments{},
@@ -107,7 +107,7 @@ func TestFileStateTryRestore(t *testing.T) {
 		},
 		{
 			"Try restore defaultCPUSet only - invalid name",
-			"{ \"defCPUSet\": \"4-6\"}",
+			`{"policyName": "none", "defaultCpuSet" "4-6"}`,
 			"",
 			&stateMemory{
 				assignments:   ContainerCPUAssignments{},
@@ -116,11 +116,13 @@ func TestFileStateTryRestore(t *testing.T) {
 		},
 		{
 			"Try restore assignments only",
-			"{" +
-				"\"entries\": { " +
-				"\"container1\": \"4-6\"," +
-				"\"container2\": \"1-3\"" +
-				"} }",
+			`{
+				"policyName": "none",
+				"entries": {
+					"container1": "4-6",
+					"container2": "1-3"
+				}
+			}`,
 			"",
 			&stateMemory{
 				assignments: ContainerCPUAssignments{
@@ -132,7 +134,7 @@ func TestFileStateTryRestore(t *testing.T) {
 		},
 		{
 			"Try restore invalid assignments",
-			"{ \"entries\": }",
+			`{"entries": }`,
 			"state file: could not unmarshal, corrupted state file",
 			&stateMemory{
 				assignments:   ContainerCPUAssignments{},
@@ -141,12 +143,14 @@ func TestFileStateTryRestore(t *testing.T) {
 		},
 		{
 			"Try restore valid file",
-			"{ " +
-				"\"defaultCpuSet\": \"23-24\", " +
-				"\"entries\": { " +
-				"\"container1\": \"4-6\", " +
-				"\"container2\": \"1-3\"" +
-				" } }",
+			`{
+				"policyName": "none",
+				"defaultCpuSet": "23-24",
+				"entries": {
+					"container1": "4-6",
+					"container2": "1-3"
+				}
+			}`,
 			"",
 			&stateMemory{
 				assignments: ContainerCPUAssignments{
@@ -158,7 +162,10 @@ func TestFileStateTryRestore(t *testing.T) {
 		},
 		{
 			"Try restore un-parsable defaultCPUSet ",
-			"{ \"defaultCpuSet\": \"2-sd\" }",
+			`{
+				"policyName": "none",
+				"defaultCpuSet": "2-sd"
+			}`,
 			"state file: could not parse state file",
 			&stateMemory{
 				assignments:   ContainerCPUAssignments{},
@@ -167,12 +174,14 @@ func TestFileStateTryRestore(t *testing.T) {
 		},
 		{
 			"Try restore un-parsable assignments",
-			"{ " +
-				"\"defaultCpuSet\": \"23-24\", " +
-				"\"entries\": { " +
-				"\"container1\": \"p-6\", " +
-				"\"container2\": \"1-3\"" +
-				" } }",
+			`{
+				"policyName": "none",
+				"defaultCpuSet": "23-24",
+				"entries": {
+					"container1": "p-6",
+					"container2": "1-3"
+				}
+			}`,
 			"state file: could not parse state file",
 			&stateMemory{
 				assignments:   ContainerCPUAssignments{},
@@ -205,7 +214,7 @@ func TestFileStateTryRestore(t *testing.T) {
 			defer os.Remove(sfilePath.Name())
 
 			logData, fileState := stderrCapture(t, func() State {
-				return NewFileState(sfilePath.Name())
+				return NewFileState(sfilePath.Name(), "none")
 			})
 
 			if tc.expErr != "" {
@@ -250,7 +259,7 @@ func TestFileStateTryRestorePanic(t *testing.T) {
 				}
 			}
 		}()
-		NewFileState(sfilePath)
+		NewFileState(sfilePath, "static")
 	})
 }
 
@@ -302,6 +311,7 @@ func TestUpdateStateFile(t *testing.T) {
 			}
 			fileState := stateFile{
 				stateFilePath: sfilePath.Name(),
+				policyName:    "static",
 				cache:         NewMemoryState(),
 			}
 
@@ -331,7 +341,7 @@ func TestUpdateStateFile(t *testing.T) {
 					return
 				}
 			}
-			newFileState := NewFileState(sfilePath.Name())
+			newFileState := NewFileState(sfilePath.Name(), "static")
 			stateEqual(t, newFileState, tc.expectedState)
 		})
 	}
@@ -382,7 +392,7 @@ func TestHelpersStateFile(t *testing.T) {
 				t.Errorf("cannot create temporary test file: %q", err.Error())
 			}
 
-			state := NewFileState(sfFile.Name())
+			state := NewFileState(sfFile.Name(), "static")
 			state.SetDefaultCPUSet(tc.defaultCPUset)
 
 			for containerName, containerCPUs := range tc.containers {
@@ -425,7 +435,7 @@ func TestClearStateStateFile(t *testing.T) {
 				t.Errorf("cannot create temporary test file: %q", err.Error())
 			}
 
-			state := NewFileState(sfFile.Name())
+			state := NewFileState(sfFile.Name(), "static")
 			state.SetDefaultCPUSet(testCase.defaultCPUset)
 			for containerName, containerCPUs := range testCase.containers {
 				state.SetCPUSet(containerName, containerCPUs)