From be0fe3df9b9ab7cf84c4f7055eeaf0ac7e8bc476 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 7 Apr 2020 13:24:48 +0200 Subject: [PATCH 1/2] cpumanager: drop old custom file backend The cpumanager file-based state backend was obsoleted since few releases, aving the cpumanager moved to the checkpointmanager common infrastructure. The old test checking compatibility to/from the old format is also no longer needed, because the checkpoint format is stable (see https://github.com/kubernetes/kubernetes/tree/master/pkg/kubelet/checkpointmanager). Signed-off-by: Francesco Romani --- pkg/kubelet/cm/cpumanager/state/BUILD | 8 +- .../state/state_compatibility_test.go | 88 --- pkg/kubelet/cm/cpumanager/state/state_file.go | 278 --------- .../cm/cpumanager/state/state_file_test.go | 534 ------------------ 4 files changed, 1 insertion(+), 907 deletions(-) delete mode 100644 pkg/kubelet/cm/cpumanager/state/state_compatibility_test.go delete mode 100644 pkg/kubelet/cm/cpumanager/state/state_file.go delete mode 100644 pkg/kubelet/cm/cpumanager/state/state_file_test.go diff --git a/pkg/kubelet/cm/cpumanager/state/BUILD b/pkg/kubelet/cm/cpumanager/state/BUILD index 4033b194a3c..b3a8cdf8b17 100644 --- a/pkg/kubelet/cm/cpumanager/state/BUILD +++ b/pkg/kubelet/cm/cpumanager/state/BUILD @@ -6,7 +6,6 @@ go_library( "checkpoint.go", "state.go", "state_checkpoint.go", - "state_file.go", "state_mem.go", ], importpath = "k8s.io/kubernetes/pkg/kubelet/cm/cpumanager/state", @@ -24,18 +23,13 @@ go_library( go_test( name = "go_default_test", - srcs = [ - "state_checkpoint_test.go", - "state_compatibility_test.go", - "state_file_test.go", - ], + srcs = ["state_checkpoint_test.go"], embed = [":go_default_library"], deps = [ "//pkg/kubelet/checkpointmanager:go_default_library", "//pkg/kubelet/cm/containermap:go_default_library", "//pkg/kubelet/cm/cpumanager/state/testing:go_default_library", "//pkg/kubelet/cm/cpuset:go_default_library", - "//vendor/github.com/stretchr/testify/require:go_default_library", ], ) diff --git a/pkg/kubelet/cm/cpumanager/state/state_compatibility_test.go b/pkg/kubelet/cm/cpumanager/state/state_compatibility_test.go deleted file mode 100644 index 0493d4bb7aa..00000000000 --- a/pkg/kubelet/cm/cpumanager/state/state_compatibility_test.go +++ /dev/null @@ -1,88 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package state - -import ( - "os" - "path" - "testing" - - "github.com/stretchr/testify/require" - "k8s.io/kubernetes/pkg/kubelet/checkpointmanager" - "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" -) - -const compatibilityTestingCheckpoint = "cpumanager_state_compatibility_test" - -var state = &stateMemory{ - assignments: ContainerCPUAssignments{ - "pod": map[string]cpuset.CPUSet{ - "container1": cpuset.NewCPUSet(4, 5, 6), - "container2": cpuset.NewCPUSet(1, 2, 3), - }, - }, - defaultCPUSet: cpuset.NewCPUSet(1, 2, 3), -} - -func TestFileToCheckpointCompatibility(t *testing.T) { - statePath := path.Join(testingDir, compatibilityTestingCheckpoint) - - // ensure there is no previous state saved at testing path - os.Remove(statePath) - // ensure testing state is removed after testing - defer os.Remove(statePath) - - fileState, err := NewFileState(statePath, "none", nil) - if err != nil { - t.Fatalf("could not create new file state: %v", err) - } - - fileState.SetDefaultCPUSet(state.defaultCPUSet) - fileState.SetCPUAssignments(state.assignments) - - restoredState, err := NewCheckpointState(testingDir, compatibilityTestingCheckpoint, "none", nil) - if err != nil { - t.Fatalf("could not restore file state: %v", err) - } - - AssertStateEqual(t, restoredState, state) -} - -func TestCheckpointToFileCompatibility(t *testing.T) { - cpm, err := checkpointmanager.NewCheckpointManager(testingDir) - if err != nil { - t.Fatalf("could not create testing checkpoint manager: %v", err) - } - - // ensure there is no previous checkpoint - cpm.RemoveCheckpoint(compatibilityTestingCheckpoint) - // ensure testing checkpoint is removed after testing - defer cpm.RemoveCheckpoint(compatibilityTestingCheckpoint) - - checkpointState, err := NewCheckpointState(testingDir, compatibilityTestingCheckpoint, "none", nil) - require.NoError(t, err) - - checkpointState.SetDefaultCPUSet(state.defaultCPUSet) - checkpointState.SetCPUAssignments(state.assignments) - - restoredState, err := NewFileState(path.Join(testingDir, compatibilityTestingCheckpoint), "none", nil) - if err != nil { - t.Fatalf("could not create new file state: %v", err) - } - - AssertStateEqual(t, restoredState, state) -} diff --git a/pkg/kubelet/cm/cpumanager/state/state_file.go b/pkg/kubelet/cm/cpumanager/state/state_file.go deleted file mode 100644 index aa944ffa9f9..00000000000 --- a/pkg/kubelet/cm/cpumanager/state/state_file.go +++ /dev/null @@ -1,278 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package state - -import ( - "encoding/json" - "fmt" - "io/ioutil" - "os" - "sync" - - "k8s.io/klog" - "k8s.io/kubernetes/pkg/kubelet/cm/containermap" - "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" -) - -type stateFileDataV1 struct { - PolicyName string `json:"policyName"` - DefaultCPUSet string `json:"defaultCpuSet"` - Entries map[string]string `json:"entries,omitempty"` -} - -type stateFileDataV2 struct { - PolicyName string `json:"policyName"` - DefaultCPUSet string `json:"defaultCpuSet"` - Entries map[string]map[string]string `json:"entries,omitempty"` -} - -var _ State = &stateFile{} - -type stateFile struct { - sync.RWMutex - stateFilePath string - policyName string - cache State - initialContainers containermap.ContainerMap -} - -// NewFileState creates new State for keeping track of cpu/pod assignment with file backend -func NewFileState(filePath string, policyName string, initialContainers containermap.ContainerMap) (State, error) { - stateFile := &stateFile{ - stateFilePath: filePath, - cache: NewMemoryState(), - policyName: policyName, - initialContainers: initialContainers, - } - - if err := stateFile.tryRestoreState(); err != nil { - // could not restore state, init new state file - klog.Errorf("[cpumanager] state file: unable to restore state from disk (%v)"+ - " We cannot guarantee sane CPU affinity for existing containers."+ - " Please drain this node and delete the CPU manager state file \"%s\" before restarting Kubelet.", - err, stateFile.stateFilePath) - return nil, err - } - - return stateFile, nil -} - -// migrateV1StateToV2State() converts state from the v1 format to the v2 format -func (sf *stateFile) migrateV1StateToV2State(src *stateFileDataV1, dst *stateFileDataV2) error { - if src.PolicyName != "" { - dst.PolicyName = src.PolicyName - } - if src.DefaultCPUSet != "" { - dst.DefaultCPUSet = src.DefaultCPUSet - } - for containerID, cset := range src.Entries { - podUID, containerName, err := sf.initialContainers.GetContainerRef(containerID) - if err != nil { - return fmt.Errorf("containerID '%v' not found in initial containers list", containerID) - } - if dst.Entries == nil { - dst.Entries = make(map[string]map[string]string) - } - if _, exists := dst.Entries[podUID]; !exists { - dst.Entries[podUID] = make(map[string]string) - } - dst.Entries[podUID][containerName] = cset - } - return nil -} - -// tryRestoreState tries to read state file, upon any error, -// err message is logged and state is left clean. un-initialized -func (sf *stateFile) tryRestoreState() error { - sf.Lock() - defer sf.Unlock() - var err error - var content []byte - - content, err = ioutil.ReadFile(sf.stateFilePath) - - // If the state file does not exist or has zero length, write a new file. - if os.IsNotExist(err) || len(content) == 0 { - err := sf.storeState() - if err != nil { - return err - } - klog.Infof("[cpumanager] state file: created new state file \"%s\"", sf.stateFilePath) - return nil - } - - // Fail on any other file read error. - if err != nil { - return err - } - - // File exists; try to read it. - var readStateV1 stateFileDataV1 - var readStateV2 stateFileDataV2 - - if err = json.Unmarshal(content, &readStateV1); err != nil { - readStateV1 = stateFileDataV1{} // reset it back to 0 - if err = json.Unmarshal(content, &readStateV2); err != nil { - klog.Errorf("[cpumanager] state file: could not unmarshal, corrupted state file - \"%s\"", sf.stateFilePath) - return err - } - } - - if err = sf.migrateV1StateToV2State(&readStateV1, &readStateV2); err != nil { - klog.Errorf("[cpumanager] state file: could not migrate v1 state to v2 state - \"%s\"", sf.stateFilePath) - return err - } - - if sf.policyName != readStateV2.PolicyName { - return fmt.Errorf("policy configured \"%s\" != policy from state file \"%s\"", sf.policyName, readStateV2.PolicyName) - } - - var tmpDefaultCPUSet cpuset.CPUSet - if tmpDefaultCPUSet, err = cpuset.Parse(readStateV2.DefaultCPUSet); err != nil { - klog.Errorf("[cpumanager] state file: could not parse state file - [defaultCpuSet:\"%s\"]", readStateV2.DefaultCPUSet) - return err - } - - var tmpContainerCPUSet cpuset.CPUSet - tmpAssignments := ContainerCPUAssignments{} - for pod := range readStateV2.Entries { - tmpAssignments[pod] = make(map[string]cpuset.CPUSet) - for container, cpuString := range readStateV2.Entries[pod] { - if tmpContainerCPUSet, err = cpuset.Parse(cpuString); err != nil { - klog.Errorf("[cpumanager] state file: could not parse state file - pod: %s, container: %s, cpuset: \"%s\"", pod, container, cpuString) - return err - } - tmpAssignments[pod][container] = tmpContainerCPUSet - } - } - - sf.cache.SetDefaultCPUSet(tmpDefaultCPUSet) - sf.cache.SetCPUAssignments(tmpAssignments) - - klog.V(2).Infof("[cpumanager] state file: restored state from state file \"%s\"", sf.stateFilePath) - klog.V(2).Infof("[cpumanager] state file: defaultCPUSet: %s", tmpDefaultCPUSet.String()) - - return nil -} - -// saves state to a file, caller is responsible for locking -func (sf *stateFile) storeState() error { - var content []byte - var err error - - data := stateFileDataV2{ - PolicyName: sf.policyName, - DefaultCPUSet: sf.cache.GetDefaultCPUSet().String(), - Entries: map[string]map[string]string{}, - } - - assignments := sf.cache.GetCPUAssignments() - for pod := range assignments { - data.Entries[pod] = map[string]string{} - for container, cset := range assignments[pod] { - data.Entries[pod][container] = cset.String() - } - } - - if content, err = json.Marshal(data); err != nil { - return fmt.Errorf("[cpumanager] state file: could not serialize state to json") - } - - if err = ioutil.WriteFile(sf.stateFilePath, content, 0644); err != nil { - return fmt.Errorf("[cpumanager] state file not written") - } - - return nil -} - -func (sf *stateFile) GetCPUSet(podUID string, containerName string) (cpuset.CPUSet, bool) { - sf.RLock() - defer sf.RUnlock() - - res, ok := sf.cache.GetCPUSet(podUID, containerName) - return res, ok -} - -func (sf *stateFile) GetDefaultCPUSet() cpuset.CPUSet { - sf.RLock() - defer sf.RUnlock() - - return sf.cache.GetDefaultCPUSet() -} - -func (sf *stateFile) GetCPUSetOrDefault(podUID string, containerName string) cpuset.CPUSet { - sf.RLock() - defer sf.RUnlock() - - return sf.cache.GetCPUSetOrDefault(podUID, containerName) -} - -func (sf *stateFile) GetCPUAssignments() ContainerCPUAssignments { - sf.RLock() - defer sf.RUnlock() - return sf.cache.GetCPUAssignments() -} - -func (sf *stateFile) SetCPUSet(podUID string, containerName string, cset cpuset.CPUSet) { - sf.Lock() - defer sf.Unlock() - sf.cache.SetCPUSet(podUID, containerName, cset) - err := sf.storeState() - if err != nil { - klog.Warningf("store state to checkpoint error: %v", err) - } -} - -func (sf *stateFile) SetDefaultCPUSet(cset cpuset.CPUSet) { - sf.Lock() - defer sf.Unlock() - sf.cache.SetDefaultCPUSet(cset) - err := sf.storeState() - if err != nil { - klog.Warningf("store state to checkpoint error: %v", err) - } -} - -func (sf *stateFile) SetCPUAssignments(a ContainerCPUAssignments) { - sf.Lock() - defer sf.Unlock() - sf.cache.SetCPUAssignments(a) - err := sf.storeState() - if err != nil { - klog.Warningf("store state to checkpoint error: %v", err) - } -} - -func (sf *stateFile) Delete(podUID string, containerName string) { - sf.Lock() - defer sf.Unlock() - sf.cache.Delete(podUID, containerName) - err := sf.storeState() - if err != nil { - klog.Warningf("store state to checkpoint error: %v", err) - } -} - -func (sf *stateFile) ClearState() { - sf.Lock() - defer sf.Unlock() - sf.cache.ClearState() - err := sf.storeState() - if err != nil { - klog.Warningf("store state to checkpoint error: %v", err) - } -} diff --git a/pkg/kubelet/cm/cpumanager/state/state_file_test.go b/pkg/kubelet/cm/cpumanager/state/state_file_test.go deleted file mode 100644 index 0f48450a104..00000000000 --- a/pkg/kubelet/cm/cpumanager/state/state_file_test.go +++ /dev/null @@ -1,534 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package state - -import ( - "bytes" - "fmt" - "io" - "io/ioutil" - "os" - "path" - "reflect" - "strings" - "testing" - - "k8s.io/kubernetes/pkg/kubelet/cm/containermap" - "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" -) - -func writeToStateFile(statefile string, content string) { - ioutil.WriteFile(statefile, []byte(content), 0644) -} - -// AssertStateEqual marks provided test as failed if provided states differ -func AssertStateEqual(t *testing.T, sf State, sm State) { - cpusetSf := sf.GetDefaultCPUSet() - cpusetSm := sm.GetDefaultCPUSet() - if !cpusetSf.Equals(cpusetSm) { - t.Errorf("State CPUSet mismatch. Have %v, want %v", cpusetSf, cpusetSm) - } - - cpuassignmentSf := sf.GetCPUAssignments() - cpuassignmentSm := sm.GetCPUAssignments() - if !reflect.DeepEqual(cpuassignmentSf, cpuassignmentSm) { - t.Errorf("State CPU assignments mismatch. Have %s, want %s", cpuassignmentSf, cpuassignmentSm) - } -} - -func stderrCapture(t *testing.T, f func() State) (bytes.Buffer, State) { - stderr := os.Stderr - - readBuffer, writeBuffer, err := os.Pipe() - if err != nil { - t.Errorf("cannot create pipe: %v", err.Error()) - } - - os.Stderr = writeBuffer - var outputBuffer bytes.Buffer - - state := f() - writeBuffer.Close() - io.Copy(&outputBuffer, readBuffer) - os.Stderr = stderr - - return outputBuffer, state -} - -func TestFileStateTryRestore(t *testing.T) { - testCases := []struct { - description string - stateFileContent string - policyName string - initialContainers containermap.ContainerMap - expErr string - expectedState *stateMemory - }{ - { - "Invalid JSON - one byte file", - "\n", - "none", - containermap.ContainerMap{}, - "[cpumanager] state file: unable to restore state from disk (unexpected end of JSON input)", - &stateMemory{}, - }, - { - "Invalid JSON - invalid content", - "{", - "none", - containermap.ContainerMap{}, - "[cpumanager] state file: unable to restore state from disk (unexpected end of JSON input)", - &stateMemory{}, - }, - { - "Try restore defaultCPUSet only", - `{"policyName": "none", "defaultCpuSet": "4-6"}`, - "none", - containermap.ContainerMap{}, - "", - &stateMemory{ - assignments: ContainerCPUAssignments{}, - defaultCPUSet: cpuset.NewCPUSet(4, 5, 6), - }, - }, - { - "Try restore defaultCPUSet only - invalid name", - `{"policyName": "none", "defaultCpuSet" "4-6"}`, - "none", - containermap.ContainerMap{}, - `[cpumanager] state file: unable to restore state from disk (invalid character '"' after object key)`, - &stateMemory{}, - }, - { - "Try restore assignments only", - `{ - "policyName": "none", - "entries": { - "pod": { - "container1": "4-6", - "container2": "1-3" - } - } - }`, - "none", - containermap.ContainerMap{}, - "", - &stateMemory{ - assignments: ContainerCPUAssignments{ - "pod": map[string]cpuset.CPUSet{ - "container1": cpuset.NewCPUSet(4, 5, 6), - "container2": cpuset.NewCPUSet(1, 2, 3), - }, - }, - defaultCPUSet: cpuset.NewCPUSet(), - }, - }, - { - "Try restore invalid policy name", - `{ - "policyName": "A", - "defaultCpuSet": "0-7", - "entries": {} - }`, - "B", - containermap.ContainerMap{}, - `[cpumanager] state file: unable to restore state from disk (policy configured "B" != policy from state file "A")`, - &stateMemory{}, - }, - { - "Try restore invalid assignments", - `{"entries": }`, - "none", - containermap.ContainerMap{}, - "[cpumanager] state file: unable to restore state from disk (invalid character '}' looking for beginning of value)", - &stateMemory{}, - }, - { - "Try restore valid file", - `{ - "policyName": "none", - "defaultCpuSet": "23-24", - "entries": { - "pod": { - "container1": "4-6", - "container2": "1-3" - } - } - }`, - "none", - containermap.ContainerMap{}, - "", - &stateMemory{ - assignments: ContainerCPUAssignments{ - "pod": map[string]cpuset.CPUSet{ - "container1": cpuset.NewCPUSet(4, 5, 6), - "container2": cpuset.NewCPUSet(1, 2, 3), - }, - }, - defaultCPUSet: cpuset.NewCPUSet(23, 24), - }, - }, - { - "Try restore un-parsable defaultCPUSet ", - `{ - "policyName": "none", - "defaultCpuSet": "2-sd" - }`, - "none", - containermap.ContainerMap{}, - `[cpumanager] state file: unable to restore state from disk (strconv.Atoi: parsing "sd": invalid syntax)`, - &stateMemory{}, - }, - { - "Try restore un-parsable assignments", - `{ - "policyName": "none", - "defaultCpuSet": "23-24", - "entries": { - "pod": { - "container1": "p-6", - "container2": "1-3" - } - } - }`, - "none", - containermap.ContainerMap{}, - `[cpumanager] state file: unable to restore state from disk (strconv.Atoi: parsing "p": invalid syntax)`, - &stateMemory{}, - }, - { - "tryRestoreState creates empty state file", - "", - "none", - containermap.ContainerMap{}, - "", - &stateMemory{ - assignments: ContainerCPUAssignments{}, - defaultCPUSet: cpuset.NewCPUSet(), - }, - }, - { - "Try restore with migration", - `{ - "policyName": "none", - "defaultCpuSet": "23-24", - "entries": { - "containerID1": "4-6", - "containerID2": "1-3" - } - }`, - "none", - func() containermap.ContainerMap { - cm := containermap.NewContainerMap() - cm.Add("pod", "container1", "containerID1") - cm.Add("pod", "container2", "containerID2") - return cm - }(), - "", - &stateMemory{ - assignments: ContainerCPUAssignments{ - "pod": map[string]cpuset.CPUSet{ - "container1": cpuset.NewCPUSet(4, 5, 6), - "container2": cpuset.NewCPUSet(1, 2, 3), - }, - }, - defaultCPUSet: cpuset.NewCPUSet(23, 24), - }, - }, - } - - for idx, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - sfilePath, err := ioutil.TempFile("/tmp", fmt.Sprintf("cpumanager_state_file_test_%d", idx)) - if err != nil { - t.Errorf("cannot create temporary file: %q", err.Error()) - } - // Don't create state file, let tryRestoreState figure out that is should create - if tc.stateFileContent != "" { - writeToStateFile(sfilePath.Name(), tc.stateFileContent) - } - - // Always remove file - regardless of who created - defer os.Remove(sfilePath.Name()) - - logData, fileState := stderrCapture(t, func() State { - newFileState, _ := NewFileState(sfilePath.Name(), tc.policyName, tc.initialContainers) - return newFileState - }) - - if tc.expErr != "" { - if logData.String() != "" { - if !strings.Contains(logData.String(), tc.expErr) { - t.Errorf("tryRestoreState() error = %v, wantErr %v", logData.String(), tc.expErr) - return - } - } else { - t.Errorf("tryRestoreState() error = nil, wantErr %v", tc.expErr) - return - } - } - - if fileState == nil { - return - } - - AssertStateEqual(t, fileState, tc.expectedState) - }) - } -} - -func TestFileStateTryRestoreError(t *testing.T) { - - testCases := []struct { - description string - expErr error - }{ - { - " create file error", - fmt.Errorf("[cpumanager] state file not written"), - }, - } - - for _, testCase := range testCases { - t.Run(testCase.description, func(t *testing.T) { - sfilePath := path.Join("/invalid_path/to_some_dir", "cpumanager_state_file_test") - _, err := NewFileState(sfilePath, "static", nil) - if !reflect.DeepEqual(err, testCase.expErr) { - t.Errorf("unexpected error, expected: %s, got: %s", testCase.expErr, err) - } - }) - } -} - -func TestUpdateStateFile(t *testing.T) { - testCases := []struct { - description string - expErr string - expectedState *stateMemory - }{ - { - "Save empty state", - "", - &stateMemory{ - assignments: ContainerCPUAssignments{}, - defaultCPUSet: cpuset.NewCPUSet(), - }, - }, - { - "Save defaultCPUSet only", - "", - &stateMemory{ - assignments: ContainerCPUAssignments{}, - defaultCPUSet: cpuset.NewCPUSet(1, 6), - }, - }, - { - "Save assignments only", - "", - &stateMemory{ - assignments: ContainerCPUAssignments{ - "pod": map[string]cpuset.CPUSet{ - "container1": cpuset.NewCPUSet(4, 5, 6), - "container2": cpuset.NewCPUSet(1, 2, 3), - }, - }, - defaultCPUSet: cpuset.NewCPUSet(), - }, - }, - } - - for idx, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - - sfilePath, err := ioutil.TempFile("/tmp", fmt.Sprintf("cpumanager_state_file_test_%d", idx)) - defer os.Remove(sfilePath.Name()) - if err != nil { - t.Errorf("cannot create temporary file: %q", err.Error()) - } - fileState := stateFile{ - stateFilePath: sfilePath.Name(), - policyName: "static", - cache: NewMemoryState(), - } - - fileState.SetDefaultCPUSet(tc.expectedState.defaultCPUSet) - fileState.SetCPUAssignments(tc.expectedState.assignments) - - logData, _ := stderrCapture(t, func() State { - fileState.storeState() - return &stateFile{} - }) - - errMsg := logData.String() - - if tc.expErr != "" { - if errMsg != "" { - if errMsg != tc.expErr { - t.Errorf("UpdateStateFile() error = %v, wantErr %v", errMsg, tc.expErr) - return - } - } else { - t.Errorf("UpdateStateFile() error = nil, wantErr %v", tc.expErr) - return - } - } else { - if errMsg != "" { - t.Errorf("UpdateStateFile() error = %v, wantErr nil", errMsg) - return - } - } - newFileState, err := NewFileState(sfilePath.Name(), "static", nil) - if err != nil { - t.Errorf("NewFileState() error: %v", err) - return - } - AssertStateEqual(t, newFileState, tc.expectedState) - }) - } -} - -func TestHelpersStateFile(t *testing.T) { - testCases := []struct { - description string - defaultCPUset cpuset.CPUSet - assignments map[string]map[string]cpuset.CPUSet - }{ - { - description: "one container", - defaultCPUset: cpuset.NewCPUSet(0, 1, 2, 3, 4, 5, 6, 7, 8), - assignments: map[string]map[string]cpuset.CPUSet{ - "pod": { - "c1": cpuset.NewCPUSet(0, 1), - }, - }, - }, - { - description: "two containers", - defaultCPUset: cpuset.NewCPUSet(0, 1, 2, 3, 4, 5, 6, 7, 8), - assignments: map[string]map[string]cpuset.CPUSet{ - "pod": { - "c1": cpuset.NewCPUSet(0, 1), - "c2": cpuset.NewCPUSet(2, 3, 4, 5), - }, - }, - }, - { - description: "container with more cpus than is possible", - defaultCPUset: cpuset.NewCPUSet(0, 1, 2, 3, 4, 5, 6, 7, 8), - assignments: map[string]map[string]cpuset.CPUSet{ - "pod": { - "c1": cpuset.NewCPUSet(0, 10), - }, - }, - }, - { - description: "container without assigned cpus", - defaultCPUset: cpuset.NewCPUSet(0, 1, 2, 3, 4, 5, 6, 7, 8), - assignments: map[string]map[string]cpuset.CPUSet{ - "pod": { - "c1": cpuset.NewCPUSet(), - }, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - sfFile, err := ioutil.TempFile("/tmp", "testHelpersStateFile") - defer os.Remove(sfFile.Name()) - if err != nil { - t.Errorf("cannot create temporary test file: %q", err.Error()) - } - - state, err := NewFileState(sfFile.Name(), "static", nil) - if err != nil { - t.Errorf("new file state error: %v", err) - return - } - - state.SetDefaultCPUSet(tc.defaultCPUset) - - for podUID := range tc.assignments { - for containerName, containerCPUs := range tc.assignments[podUID] { - state.SetCPUSet(podUID, containerName, containerCPUs) - if cpus, _ := state.GetCPUSet(podUID, containerName); !cpus.Equals(containerCPUs) { - t.Errorf("state is inconsistent. Wants = %q Have = %q", containerCPUs, cpus) - } - state.Delete(podUID, containerName) - if cpus := state.GetCPUSetOrDefault(podUID, containerName); !cpus.Equals(tc.defaultCPUset) { - t.Error("deleted container still existing in state") - } - - } - } - - }) - } -} - -func TestClearStateStateFile(t *testing.T) { - testCases := []struct { - description string - defaultCPUset cpuset.CPUSet - assignments map[string]map[string]cpuset.CPUSet - }{ - { - description: "valid file", - defaultCPUset: cpuset.NewCPUSet(0, 1, 2, 3, 4, 5, 6, 7, 8), - assignments: map[string]map[string]cpuset.CPUSet{ - "pod": { - "c1": cpuset.NewCPUSet(0, 1), - "c2": cpuset.NewCPUSet(2, 3), - "c3": cpuset.NewCPUSet(4, 5), - }, - }, - }, - } - for _, testCase := range testCases { - t.Run(testCase.description, func(t *testing.T) { - sfFile, err := ioutil.TempFile("/tmp", "testHelpersStateFile") - defer os.Remove(sfFile.Name()) - if err != nil { - t.Errorf("cannot create temporary test file: %q", err.Error()) - } - - state, err := NewFileState(sfFile.Name(), "static", nil) - if err != nil { - t.Errorf("new file state error: %v", err) - return - } - state.SetDefaultCPUSet(testCase.defaultCPUset) - for podUID := range testCase.assignments { - for containerName, containerCPUs := range testCase.assignments[podUID] { - state.SetCPUSet(podUID, containerName, containerCPUs) - } - } - - state.ClearState() - if !cpuset.NewCPUSet().Equals(state.GetDefaultCPUSet()) { - t.Error("cleared state shouldn't has got information about available cpuset") - } - for podUID := range testCase.assignments { - for containerName := range testCase.assignments[podUID] { - if !cpuset.NewCPUSet().Equals(state.GetCPUSetOrDefault(podUID, containerName)) { - t.Error("cleared state shouldn't has got information about containers") - } - } - } - }) - } -} From 623587ec8b5bf1cf34813bcf8126cfee92f2cb0d Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 7 Apr 2020 16:59:07 +0200 Subject: [PATCH 2/2] cpumanager: test: add missing helper add back the missing AssertStateEqual helper; it is needed by some tests we still want to run. Signed-off-by: Francesco Romani --- .../cm/cpumanager/state/state_checkpoint_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/kubelet/cm/cpumanager/state/state_checkpoint_test.go b/pkg/kubelet/cm/cpumanager/state/state_checkpoint_test.go index 789f9674c2b..7fbaace165d 100644 --- a/pkg/kubelet/cm/cpumanager/state/state_checkpoint_test.go +++ b/pkg/kubelet/cm/cpumanager/state/state_checkpoint_test.go @@ -18,6 +18,7 @@ package state import ( "os" + "reflect" "strings" "testing" @@ -397,3 +398,17 @@ func TestCheckpointStateClear(t *testing.T) { }) } } + +func AssertStateEqual(t *testing.T, sf State, sm State) { + cpusetSf := sf.GetDefaultCPUSet() + cpusetSm := sm.GetDefaultCPUSet() + if !cpusetSf.Equals(cpusetSm) { + t.Errorf("State CPUSet mismatch. Have %v, want %v", cpusetSf, cpusetSm) + } + + cpuassignmentSf := sf.GetCPUAssignments() + cpuassignmentSm := sm.GetCPUAssignments() + if !reflect.DeepEqual(cpuassignmentSf, cpuassignmentSm) { + t.Errorf("State CPU assignments mismatch. Have %s, want %s", cpuassignmentSf, cpuassignmentSm) + } +}