From 98e8f42f330f4708009df04c270ce3799baec5c5 Mon Sep 17 00:00:00 2001 From: vinay kulkarni Date: Tue, 7 Mar 2023 05:59:34 +0000 Subject: [PATCH] panic on pod resources alloc checkpoint failure --- pkg/kubelet/status/fake_status_manager.go | 5 --- pkg/kubelet/status/status_manager.go | 38 ++++--------------- .../testing/mock_pod_status_provider.go | 15 -------- 3 files changed, 7 insertions(+), 51 deletions(-) diff --git a/pkg/kubelet/status/fake_status_manager.go b/pkg/kubelet/status/fake_status_manager.go index 18b61bf7c7a..42cd611984e 100644 --- a/pkg/kubelet/status/fake_status_manager.go +++ b/pkg/kubelet/status/fake_status_manager.go @@ -63,11 +63,6 @@ func (m *fakeManager) RemoveOrphanedStatuses(podUIDs map[types.UID]bool) { return } -func (m *fakeManager) State() state.Reader { - klog.InfoS("State()") - return m.state -} - func (m *fakeManager) GetContainerResourceAllocation(podUID string, containerName string) (v1.ResourceList, bool) { klog.InfoS("GetContainerResourceAllocation()") return m.state.GetContainerResourceAllocation(podUID, containerName) diff --git a/pkg/kubelet/status/status_manager.go b/pkg/kubelet/status/status_manager.go index e35ad681e1e..7ee44059cb4 100644 --- a/pkg/kubelet/status/status_manager.go +++ b/pkg/kubelet/status/status_manager.go @@ -137,9 +137,6 @@ type Manager interface { // the provided podUIDs. RemoveOrphanedStatuses(podUIDs map[types.UID]bool) - // State returns a read-only interface to the internal status manager state. - State() state.Reader - // GetContainerResourceAllocation returns checkpointed ResourcesAllocated value for the container GetContainerResourceAllocation(podUID string, containerName string) (v1.ResourceList, bool) @@ -189,13 +186,13 @@ func isPodStatusByKubeletEqual(oldStatus, status *v1.PodStatus) bool { } func (m *manager) Start() { - // Create pod allocation checkpoint manager even if we don't have a client to allow local get/set of ResourcesAllocated & Resize + // Create pod allocation checkpoint manager even if client is nil so as to allow local get/set of ResourcesAllocated & Resize if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { stateImpl, err := state.NewStateCheckpoint(m.stateFileDirectory, podStatusManagerStateFile) if err != nil { - //TODO(vinaykul,InPlacePodVerticalScaling): Check if this error should be a critical (stop kubelet) failure + // This is a crictical, non-recoverable failure. klog.ErrorS(err, "Could not initialize pod allocation checkpoint manager, please drain node and remove policy state file") - return + panic(err) } m.state = stateImpl } @@ -235,20 +232,12 @@ func (m *manager) Start() { }, 0) } -// State returns the pod resources checkpoint state of the pod status manager -func (m *manager) State() state.Reader { - return m.state -} - // GetContainerResourceAllocation returns the last checkpointed ResourcesAllocated values // If checkpoint manager has not been initialized, it returns nil, false func (m *manager) GetContainerResourceAllocation(podUID string, containerName string) (v1.ResourceList, bool) { m.podStatusesLock.RLock() defer m.podStatusesLock.RUnlock() - if m.state != nil { - return m.state.GetContainerResourceAllocation(podUID, containerName) - } - return nil, false + return m.state.GetContainerResourceAllocation(podUID, containerName) } // GetPodResizeStatus returns the last checkpointed ResizeStaus value @@ -256,17 +245,11 @@ func (m *manager) GetContainerResourceAllocation(podUID string, containerName st func (m *manager) GetPodResizeStatus(podUID string) (v1.PodResizeStatus, bool) { m.podStatusesLock.RLock() defer m.podStatusesLock.RUnlock() - if m.state != nil { - return m.state.GetPodResizeStatus(podUID) - } - return "", false + return m.state.GetPodResizeStatus(podUID) } // SetPodAllocation checkpoints the resources allocated to a pod's containers func (m *manager) SetPodAllocation(pod *v1.Pod) error { - if m.state == nil { - return fmt.Errorf("pod allocation checkpoint manager is not initialized") - } m.podStatusesLock.RLock() defer m.podStatusesLock.RUnlock() for _, container := range pod.Spec.Containers { @@ -283,9 +266,6 @@ func (m *manager) SetPodAllocation(pod *v1.Pod) error { // SetPodResizeStatus checkpoints the last resizing decision for the pod. func (m *manager) SetPodResizeStatus(podUID types.UID, resizeStatus v1.PodResizeStatus) error { - if m.state == nil { - return fmt.Errorf("pod allocation checkpoint manager is not initialized") - } m.podStatusesLock.RLock() defer m.podStatusesLock.RUnlock() return m.state.SetPodResizeStatus(string(podUID), resizeStatus) @@ -708,9 +688,7 @@ func (m *manager) deletePodStatus(uid types.UID) { delete(m.podStatuses, uid) m.podStartupLatencyHelper.DeletePodStartupState(uid) if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { - if m.state != nil { - m.state.Delete(string(uid), "") - } + m.state.Delete(string(uid), "") } } @@ -723,9 +701,7 @@ func (m *manager) RemoveOrphanedStatuses(podUIDs map[types.UID]bool) { klog.V(5).InfoS("Removing pod from status map.", "podUID", key) delete(m.podStatuses, key) if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { - if m.state != nil { - m.state.Delete(string(key), "") - } + m.state.Delete(string(key), "") } } } diff --git a/pkg/kubelet/status/testing/mock_pod_status_provider.go b/pkg/kubelet/status/testing/mock_pod_status_provider.go index 457c79d9ba1..2de1bee1691 100644 --- a/pkg/kubelet/status/testing/mock_pod_status_provider.go +++ b/pkg/kubelet/status/testing/mock_pod_status_provider.go @@ -27,7 +27,6 @@ import ( v1 "k8s.io/api/core/v1" types "k8s.io/apimachinery/pkg/types" container "k8s.io/kubernetes/pkg/kubelet/container" - state "k8s.io/kubernetes/pkg/kubelet/status/state" ) // MockPodStatusProvider is a mock of PodStatusProvider interface. @@ -322,20 +321,6 @@ func (mr *MockManagerMockRecorder) Start() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Start", reflect.TypeOf((*MockManager)(nil).Start)) } -// State mocks base method. -func (m *MockManager) State() state.Reader { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "State") - ret0, _ := ret[0].(state.Reader) - return ret0 -} - -// State indicates an expected call of State. -func (mr *MockManagerMockRecorder) State() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "State", reflect.TypeOf((*MockManager)(nil).State)) -} - // TerminatePod mocks base method. func (m *MockManager) TerminatePod(pod *v1.Pod) { m.ctrl.T.Helper()