From 809ac834a0817291aff3a064128e9174927236f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Stachowski?= Date: Thu, 19 Oct 2017 15:24:56 +0200 Subject: [PATCH] Cpu manager file state tests --- pkg/kubelet/cm/cpumanager/BUILD | 1 + pkg/kubelet/cm/cpumanager/cpu_manager_test.go | 126 ++++++++++++++++++ .../cm/cpumanager/policy_static_test.go | 76 +++++++++-- pkg/kubelet/cm/cpumanager/state/state_file.go | 2 +- .../cm/cpumanager/state/state_file_test.go | 62 +++++---- 5 files changed, 230 insertions(+), 37 deletions(-) diff --git a/pkg/kubelet/cm/cpumanager/BUILD b/pkg/kubelet/cm/cpumanager/BUILD index 64a5f7e1bbc..ded9f90b5b1 100644 --- a/pkg/kubelet/cm/cpumanager/BUILD +++ b/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", diff --git a/pkg/kubelet/cm/cpumanager/cpu_manager_test.go b/pkg/kubelet/cm/cpumanager/cpu_manager_test.go index e4df638c3db..2a3b1201a41 100644 --- a/pkg/kubelet/cm/cpumanager/cpu_manager_test.go +++ b/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{ diff --git a/pkg/kubelet/cm/cpumanager/policy_static_test.go b/pkg/kubelet/cm/cpumanager/policy_static_test.go index e6626f44675..f0e592ffd11 100644 --- a/pkg/kubelet/cm/cpumanager/policy_static_test.go +++ b/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) + } + + }) } } diff --git a/pkg/kubelet/cm/cpumanager/state/state_file.go b/pkg/kubelet/cm/cpumanager/state/state_file.go index b9c54f4bce1..20c7763350d 100644 --- a/pkg/kubelet/cm/cpumanager/state/state_file.go +++ b/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() } diff --git a/pkg/kubelet/cm/cpumanager/state/state_file_test.go b/pkg/kubelet/cm/cpumanager/state/state_file_test.go index c04860ac3a9..cc653f9d9e2 100644 --- a/pkg/kubelet/cm/cpumanager/state/state_file_test.go +++ b/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)