From 0fbd19bc063e874b6c571d62101e819afc387475 Mon Sep 17 00:00:00 2001 From: Klaudiusz Dembler Date: Fri, 9 Feb 2018 10:05:13 +0100 Subject: [PATCH] Tweaks --- pkg/kubelet/cm/cpumanager/cpu_manager.go | 1 - .../cm/cpumanager/state/state_checkpoint.go | 10 +++++----- .../cpumanager/state/state_checkpoint_test.go | 20 +++++++++---------- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/pkg/kubelet/cm/cpumanager/cpu_manager.go b/pkg/kubelet/cm/cpumanager/cpu_manager.go index ed185bc2cc5..c7ad7d8e55f 100644 --- a/pkg/kubelet/cm/cpumanager/cpu_manager.go +++ b/pkg/kubelet/cm/cpumanager/cpu_manager.go @@ -33,7 +33,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/status" - "path" ) // ActivePodsFunc is a function that returns a list of pods to reconcile. diff --git a/pkg/kubelet/cm/cpumanager/state/state_checkpoint.go b/pkg/kubelet/cm/cpumanager/state/state_checkpoint.go index ee0ead3eadb..f55ed4f1253 100644 --- a/pkg/kubelet/cm/cpumanager/state/state_checkpoint.go +++ b/pkg/kubelet/cm/cpumanager/state/state_checkpoint.go @@ -27,8 +27,8 @@ import ( utilstore "k8s.io/kubernetes/pkg/kubelet/util/store" ) -// CPUManagerCheckpointName is the name of checkpoint file -const CPUManagerCheckpointName = "cpu_manager_state" +// cpuManagerCheckpointName is the name of checkpoint file +const cpuManagerCheckpointName = "cpu_manager_state" var _ State = &stateCheckpoint{} @@ -54,7 +54,7 @@ func NewCheckpointState(stateDir string, policyName string) (State, error) { if err := stateCheckpoint.restoreState(); err != nil { return nil, fmt.Errorf("could not restore state from checkpoint: %v\n"+ "Please drain this node and delete the CPU manager checkpoint file %q before restarting Kubelet.", - err, path.Join(stateDir, CPUManagerCheckpointName)) + err, path.Join(stateDir, cpuManagerCheckpointName)) } return stateCheckpoint, nil @@ -72,7 +72,7 @@ func (sc *stateCheckpoint) restoreState() error { tmpContainerCPUSet := cpuset.NewCPUSet() checkpoint := NewCPUManagerCheckpoint() - if err = sc.checkpointManager.GetCheckpoint(CPUManagerCheckpointName, checkpoint); err != nil { + if err = sc.checkpointManager.GetCheckpoint(cpuManagerCheckpointName, checkpoint); err != nil { // TODO: change to errors.ErrCheckpointNotFound may be required after issue in checkpointing PR is resolved if err == utilstore.ErrKeyNotFound { sc.storeState() @@ -115,7 +115,7 @@ func (sc *stateCheckpoint) storeState() { checkpoint.Entries[containerID] = cset.String() } - err := sc.checkpointManager.CreateCheckpoint(CPUManagerCheckpointName, checkpoint) + err := sc.checkpointManager.CreateCheckpoint(cpuManagerCheckpointName, checkpoint) if err != nil { panic("[cpumanager] could not save checkpoint: " + err.Error()) diff --git a/pkg/kubelet/cm/cpumanager/state/state_checkpoint_test.go b/pkg/kubelet/cm/cpumanager/state/state_checkpoint_test.go index 01cb7ed6afb..9d30d5dca05 100644 --- a/pkg/kubelet/cm/cpumanager/state/state_checkpoint_test.go +++ b/pkg/kubelet/cm/cpumanager/state/state_checkpoint_test.go @@ -146,15 +146,12 @@ func TestCheckpointStateRestore(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { // ensure there is no previous checkpoint - cpm.RemoveCheckpoint(CPUManagerCheckpointName) + cpm.RemoveCheckpoint(cpuManagerCheckpointName) // prepare checkpoint for testing - var checkpoint *testutil.MockCheckpoint = nil if strings.TrimSpace(tc.checkpointContent) != "" { - checkpoint = &testutil.MockCheckpoint{Content: tc.checkpointContent} - } - if checkpoint != nil { - if err := cpm.CreateCheckpoint(CPUManagerCheckpointName, checkpoint); err != nil { + checkpoint := &testutil.MockCheckpoint{Content: tc.checkpointContent} + if err := cpm.CreateCheckpoint(cpuManagerCheckpointName, checkpoint); err != nil { t.Fatalf("could not create testing checkpoint: %v", err) } } @@ -204,16 +201,18 @@ func TestCheckpointStateStore(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { // ensure there is no previous checkpoint - cpm.RemoveCheckpoint(CPUManagerCheckpointName) + cpm.RemoveCheckpoint(cpuManagerCheckpointName) cs1, err := NewCheckpointState(testingDir, "none") if err != nil { t.Fatalf("could not create testing checkpointState instance: %v", err) } + // set values of cs1 instance so they are stored in checkpoint and can be read by cs2 cs1.SetDefaultCPUSet(tc.expectedState.defaultCPUSet) cs1.SetCPUAssignments(tc.expectedState.assignments) + // restore checkpoint with previously stored values cs2, err := NewCheckpointState(testingDir, "none") if err != nil { t.Fatalf("could not create testing checkpointState instance: %v", err) @@ -262,7 +261,7 @@ func TestCheckpointStateHelpers(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { // ensure there is no previous checkpoint - cpm.RemoveCheckpoint(CPUManagerCheckpointName) + cpm.RemoveCheckpoint(cpuManagerCheckpointName) state, err := NewCheckpointState(testingDir, "none") if err != nil { @@ -277,11 +276,10 @@ func TestCheckpointStateHelpers(t *testing.T) { } state.Delete(container) - if cpus := state.GetCPUSetOrDefault(container); !cpus.Equals(tc.defaultCPUset) { + if _, ok := state.GetCPUSet(container); ok { t.Fatal("deleted container still existing in state") } } - }) } } @@ -316,7 +314,7 @@ func TestCheckpointStateClear(t *testing.T) { t.Fatal("cleared state with non-empty default cpu set") } for container := range tc.containers { - if !cpuset.NewCPUSet().Equals(state.GetCPUSetOrDefault(container)) { + if _, ok := state.GetCPUSet(container); ok { t.Fatalf("container %q with non-default cpu set in cleared state", container) } }