kubelet: Rename PodManager DeletePod to RemovePod

RemovePod is more consistent within the kubelet to be the opposite
of AddPod, and the pod is not being deleted just "removed" from
tracking.
This commit is contained in:
Clayton Coleman 2023-04-14 15:49:32 -04:00
parent 166256f73e
commit 1f16d71185
No known key found for this signature in database
GPG Key ID: CF7DB7FC943D3E0E
7 changed files with 32 additions and 31 deletions

View File

@ -2624,7 +2624,7 @@ func (kl *Kubelet) HandlePodUpdates(pods []*v1.Pod) {
func (kl *Kubelet) HandlePodRemoves(pods []*v1.Pod) { func (kl *Kubelet) HandlePodRemoves(pods []*v1.Pod) {
start := kl.clock.Now() start := kl.clock.Now()
for _, pod := range pods { for _, pod := range pods {
kl.podManager.DeletePod(pod) kl.podManager.RemovePod(pod)
pod, mirrorPod, wasMirror := kl.podManager.GetPodAndMirrorPod(pod) pod, mirrorPod, wasMirror := kl.podManager.GetPodAndMirrorPod(pod)
if wasMirror { if wasMirror {

View File

@ -2530,9 +2530,9 @@ func TestHandlePodResourcesResize(t *testing.T) {
testPod2.UID: true, testPod2.UID: true,
testPod3.UID: true, testPod3.UID: true,
} }
defer kubelet.podManager.DeletePod(testPod3) defer kubelet.podManager.RemovePod(testPod3)
defer kubelet.podManager.DeletePod(testPod2) defer kubelet.podManager.RemovePod(testPod2)
defer kubelet.podManager.DeletePod(testPod1) defer kubelet.podManager.RemovePod(testPod1)
tests := []struct { tests := []struct {
name string name string

View File

@ -77,10 +77,10 @@ type Manager interface {
AddPod(pod *v1.Pod) AddPod(pod *v1.Pod)
// UpdatePod updates the given pod in the manager. // UpdatePod updates the given pod in the manager.
UpdatePod(pod *v1.Pod) UpdatePod(pod *v1.Pod)
// DeletePod deletes the given pod from the manager. For mirror pods, // RemovePod deletes the given pod from the manager. For mirror pods,
// this means deleting the mappings related to mirror pods. For non- // this means deleting the mappings related to mirror pods. For non-
// mirror pods, this means deleting from indexes for all non-mirror pods. // mirror pods, this means deleting from indexes for all non-mirror pods.
DeletePod(pod *v1.Pod) RemovePod(pod *v1.Pod)
// TranslatePodUID returns the actual UID of a pod. If the UID belongs to // TranslatePodUID returns the actual UID of a pod. If the UID belongs to
// a mirror pod, returns the UID of its static pod. Otherwise, returns the // a mirror pod, returns the UID of its static pod. Otherwise, returns the
@ -98,7 +98,7 @@ type Manager interface {
// basicManager is a functional Manager. // basicManager is a functional Manager.
// //
// All fields in basicManager are read-only and are updated calling SetPods, // All fields in basicManager are read-only and are updated calling SetPods,
// AddPod, UpdatePod, or DeletePod. // AddPod, UpdatePod, or RemovePod.
type basicManager struct { type basicManager struct {
// Protects all internal maps. // Protects all internal maps.
lock sync.RWMutex lock sync.RWMutex
@ -189,7 +189,7 @@ func (pm *basicManager) updatePodsInternal(pods ...*v1.Pod) {
} }
} }
func (pm *basicManager) DeletePod(pod *v1.Pod) { func (pm *basicManager) RemovePod(pod *v1.Pod) {
updateMetrics(pod, nil) updateMetrics(pod, nil)
pm.lock.Lock() pm.lock.Lock()
defer pm.lock.Unlock() defer pm.lock.Unlock()

View File

@ -111,7 +111,7 @@ func TestGetSetPods(t *testing.T) {
} }
func TestDeletePods(t *testing.T) { func TestRemovePods(t *testing.T) {
mirrorPod := &v1.Pod{ mirrorPod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
UID: types.UID("mirror-pod-uid"), UID: types.UID("mirror-pod-uid"),
@ -147,11 +147,11 @@ func TestDeletePods(t *testing.T) {
podManager, _ := newTestManager() podManager, _ := newTestManager()
podManager.SetPods(updates) podManager.SetPods(updates)
podManager.DeletePod(staticPod) podManager.RemovePod(staticPod)
actualPods := podManager.GetPods() actualPods := podManager.GetPods()
if len(actualPods) == len(expectedPods) { if len(actualPods) == len(expectedPods) {
t.Fatalf("Run DeletePod() error, expected %d pods, got %d pods; ", len(expectedPods)-1, len(actualPods)) t.Fatalf("Run RemovePod() error, expected %d pods, got %d pods; ", len(expectedPods)-1, len(actualPods))
} }
_, _, orphanedMirrorPodNames := podManager.GetPodsAndMirrorPods() _, _, orphanedMirrorPodNames := podManager.GetPodsAndMirrorPods()

View File

@ -64,18 +64,6 @@ func (mr *MockManagerMockRecorder) AddPod(arg0 interface{}) *gomock.Call {
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPod", reflect.TypeOf((*MockManager)(nil).AddPod), arg0) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPod", reflect.TypeOf((*MockManager)(nil).AddPod), arg0)
} }
// DeletePod mocks base method.
func (m *MockManager) DeletePod(arg0 *v1.Pod) {
m.ctrl.T.Helper()
m.ctrl.Call(m, "DeletePod", arg0)
}
// DeletePod indicates an expected call of DeletePod.
func (mr *MockManagerMockRecorder) DeletePod(arg0 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeletePod", reflect.TypeOf((*MockManager)(nil).DeletePod), arg0)
}
// GetMirrorPodByPod mocks base method. // GetMirrorPodByPod mocks base method.
func (m *MockManager) GetMirrorPodByPod(arg0 *v1.Pod) (*v1.Pod, bool) { func (m *MockManager) GetMirrorPodByPod(arg0 *v1.Pod) (*v1.Pod, bool) {
m.ctrl.T.Helper() m.ctrl.T.Helper()
@ -212,6 +200,18 @@ func (mr *MockManagerMockRecorder) GetUIDTranslations() *gomock.Call {
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUIDTranslations", reflect.TypeOf((*MockManager)(nil).GetUIDTranslations)) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUIDTranslations", reflect.TypeOf((*MockManager)(nil).GetUIDTranslations))
} }
// RemovePod mocks base method.
func (m *MockManager) RemovePod(arg0 *v1.Pod) {
m.ctrl.T.Helper()
m.ctrl.Call(m, "RemovePod", arg0)
}
// RemovePod indicates an expected call of RemovePod.
func (mr *MockManagerMockRecorder) RemovePod(arg0 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemovePod", reflect.TypeOf((*MockManager)(nil).RemovePod), arg0)
}
// SetPods mocks base method. // SetPods mocks base method.
func (m *MockManager) SetPods(arg0 []*v1.Pod) { func (m *MockManager) SetPods(arg0 []*v1.Pod) {
m.ctrl.T.Helper() m.ctrl.T.Helper()

View File

@ -53,7 +53,7 @@ import (
type mutablePodManager interface { type mutablePodManager interface {
AddPod(*v1.Pod) AddPod(*v1.Pod)
UpdatePod(*v1.Pod) UpdatePod(*v1.Pod)
DeletePod(*v1.Pod) RemovePod(*v1.Pod)
} }
// Generate new instance of test pod with the same initial value. // Generate new instance of test pod with the same initial value.
@ -572,7 +572,7 @@ func TestStaticPod(t *testing.T) {
verifyActions(t, m, []core.Action{}) verifyActions(t, m, []core.Action{})
t.Logf("Change mirror pod identity.") t.Logf("Change mirror pod identity.")
m.podManager.(mutablePodManager).DeletePod(mirrorPod) m.podManager.(mutablePodManager).RemovePod(mirrorPod)
mirrorPod.UID = "new-mirror-pod" mirrorPod.UID = "new-mirror-pod"
mirrorPod.Status = v1.PodStatus{} mirrorPod.Status = v1.PodStatus{}
m.podManager.(mutablePodManager).AddPod(mirrorPod) m.podManager.(mutablePodManager).AddPod(mirrorPod)

View File

@ -17,10 +17,11 @@ limitations under the License.
package populator package populator
import ( import (
"k8s.io/klog/v2/ktesting"
"testing" "testing"
"time" "time"
"k8s.io/klog/v2/ktesting"
"fmt" "fmt"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -287,7 +288,7 @@ func TestFindAndAddNewPods_WithReprocessPodAndVolumeRetrievalError(t *testing.T)
if !dswp.podPreviouslyProcessed(podName) { if !dswp.podPreviouslyProcessed(podName) {
t.Fatalf("Failed to record that the volumes for the specified pod: %s have been processed by the populator", podName) t.Fatalf("Failed to record that the volumes for the specified pod: %s have been processed by the populator", podName)
} }
fakePodManager.DeletePod(pod) fakePodManager.RemovePod(pod)
} }
func TestFindAndAddNewPods_WithVolumeRetrievalError(t *testing.T) { func TestFindAndAddNewPods_WithVolumeRetrievalError(t *testing.T) {
@ -325,7 +326,7 @@ func TestFindAndAddNewPods_WithVolumeRetrievalError(t *testing.T) {
type mutablePodManager interface { type mutablePodManager interface {
GetPodByName(string, string) (*v1.Pod, bool) GetPodByName(string, string) (*v1.Pod, bool)
DeletePod(*v1.Pod) RemovePod(*v1.Pod)
} }
func TestFindAndAddNewPods_FindAndRemoveDeletedPods(t *testing.T) { func TestFindAndAddNewPods_FindAndRemoveDeletedPods(t *testing.T) {
@ -338,7 +339,7 @@ func TestFindAndAddNewPods_FindAndRemoveDeletedPods(t *testing.T) {
t.Fatalf("Failed to get pod by pod name: %s and namespace: %s", pod.Name, pod.Namespace) t.Fatalf("Failed to get pod by pod name: %s and namespace: %s", pod.Name, pod.Namespace)
} }
podGet.Status.Phase = v1.PodFailed podGet.Status.Phase = v1.PodFailed
dswp.podManager.(mutablePodManager).DeletePod(pod) dswp.podManager.(mutablePodManager).RemovePod(pod)
dswp.findAndRemoveDeletedPods() dswp.findAndRemoveDeletedPods()
@ -461,7 +462,7 @@ func TestFindAndRemoveDeletedPodsWithUncertain(t *testing.T) {
t.Fatalf("Failed to get pod by pod name: %s and namespace: %s", pod.Name, pod.Namespace) t.Fatalf("Failed to get pod by pod name: %s and namespace: %s", pod.Name, pod.Namespace)
} }
podGet.Status.Phase = v1.PodFailed podGet.Status.Phase = v1.PodFailed
dswp.podManager.(mutablePodManager).DeletePod(pod) dswp.podManager.(mutablePodManager).RemovePod(pod)
fakePodState.removed = map[kubetypes.UID]struct{}{pod.UID: {}} fakePodState.removed = map[kubetypes.UID]struct{}{pod.UID: {}}
// Add the volume to ASW by reconciling. // Add the volume to ASW by reconciling.
@ -758,7 +759,7 @@ func TestFindAndAddNewPods_FindAndRemoveDeletedPods_Valid_Block_VolumeDevices(t
t.Fatalf("Failed to get pod by pod name: %s and namespace: %s", pod.Name, pod.Namespace) t.Fatalf("Failed to get pod by pod name: %s and namespace: %s", pod.Name, pod.Namespace)
} }
podGet.Status.Phase = v1.PodFailed podGet.Status.Phase = v1.PodFailed
fakePodManager.DeletePod(pod) fakePodManager.RemovePod(pod)
fakePodState.removed = map[kubetypes.UID]struct{}{pod.UID: {}} fakePodState.removed = map[kubetypes.UID]struct{}{pod.UID: {}}
//pod is added to fakePodManager but pod state knows the pod is removed, so here findAndRemoveDeletedPods() will remove the pod and volumes it is mounted //pod is added to fakePodManager but pod state knows the pod is removed, so here findAndRemoveDeletedPods() will remove the pod and volumes it is mounted