Merge pull request #132064 from natasha41575/unexportIsPodResizeInProgress

unexport the allocation manager's IsPodResizeInProgress method

pkg/kubelet/allocation/allocation_manager.go

@@ -89,13 +89,6 @@ type Manager interface {
 	// calculations after this function is called. It also updates the cached ResizeStatus according to
 	// the allocation decision and pod status.
 	HandlePodResourcesResize(runtime kubecontainer.Runtime, allocatedPods []*v1.Pod, pod *v1.Pod, podStatus *kubecontainer.PodStatus) (*v1.Pod, error)
-
-	// IsPodResizingInProgress checks whether the actuated resizable resources differ from the allocated resources
-	// for any running containers. Specifically, the following differences are ignored:
-	// - Non-resizable containers: non-restartable init containers, ephemeral containers
-	// - Non-resizable resources: only CPU & memory are resizable
-	// - Non-running containers: they will be sized correctly when (re)started
-	IsPodResizeInProgress(allocatedPod *v1.Pod, podStatus *kubecontainer.PodStatus) bool
 }
 
 type manager struct {
@@ -295,7 +288,7 @@ func (m *manager) HandlePodResourcesResize(
 		if err != nil {
 			return
 		}
-		if m.IsPodResizeInProgress(allocatedPod, podStatus) {
+		if m.isPodResizeInProgress(allocatedPod, podStatus) {
 			// If a resize is in progress, make sure the cache has the correct state in case the Kubelet restarted.
 			m.statusManager.SetPodResizeInProgressCondition(pod.UID, "", "", false)
 		} else {
@@ -452,7 +445,12 @@ func (m *manager) canResizePod(allocatedPods []*v1.Pod, pod *v1.Pod) (bool, stri
 	return true, "", ""
 }
 
-func (m *manager) IsPodResizeInProgress(allocatedPod *v1.Pod, podStatus *kubecontainer.PodStatus) bool {
+// IsPodResizingInProgress checks whether the actuated resizable resources differ from the allocated resources
+// for any running containers. Specifically, the following differences are ignored:
+// - Non-resizable containers: non-restartable init containers, ephemeral containers
+// - Non-resizable resources: only CPU & memory are resizable
+// - Non-running containers: they will be sized correctly when (re)started
+func (m *manager) isPodResizeInProgress(allocatedPod *v1.Pod, podStatus *kubecontainer.PodStatus) bool {
 	return !podutil.VisitContainers(&allocatedPod.Spec, podutil.InitContainers|podutil.Containers,
 		func(allocatedContainer *v1.Container, containerType podutil.ContainerType) (shouldContinue bool) {
 			if !isResizableContainer(allocatedContainer, containerType) {

pkg/kubelet/allocation/allocation_manager_test.go

@@ -17,14 +17,24 @@ limitations under the License.
 package allocation
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
+	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/kubernetes/pkg/kubelet/allocation/state"
+	"k8s.io/kubernetes/pkg/kubelet/cm"
+	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
+	kubepod "k8s.io/kubernetes/pkg/kubelet/pod"
+	"k8s.io/kubernetes/pkg/kubelet/status"
+	statustest "k8s.io/kubernetes/pkg/kubelet/status/testing"
+	kubeletutil "k8s.io/kubernetes/pkg/kubelet/util"
+	_ "k8s.io/kubernetes/pkg/volume/hostpath"
+	"k8s.io/utils/ptr"
 )
 
 func TestUpdatePodFromAllocation(t *testing.T) {
@@ -169,3 +179,256 @@ func TestUpdatePodFromAllocation(t *testing.T) {
 		})
 	}
 }
+
+func TestIsPodResizeInProgress(t *testing.T) {
+	type testResources struct {
+		cpuReq, cpuLim, memReq, memLim int64
+	}
+	type testContainer struct {
+		allocated               testResources
+		actuated                *testResources
+		nonSidecarInit, sidecar bool
+		isRunning               bool
+		unstarted               bool // Whether the container is missing from the pod status
+	}
+
+	tests := []struct {
+		name            string
+		containers      []testContainer
+		expectHasResize bool
+	}{{
+		name: "simple running container",
+		containers: []testContainer{{
+			allocated: testResources{100, 100, 100, 100},
+			actuated:  &testResources{100, 100, 100, 100},
+			isRunning: true,
+		}},
+		expectHasResize: false,
+	}, {
+		name: "simple unstarted container",
+		containers: []testContainer{{
+			allocated: testResources{100, 100, 100, 100},
+			unstarted: true,
+		}},
+		expectHasResize: false,
+	}, {
+		name: "simple resized container/cpu req",
+		containers: []testContainer{{
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{150, 200, 100, 200},
+			isRunning: true,
+		}},
+		expectHasResize: true,
+	}, {
+		name: "simple resized container/cpu limit",
+		containers: []testContainer{{
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{100, 300, 100, 200},
+			isRunning: true,
+		}},
+		expectHasResize: true,
+	}, {
+		name: "simple resized container/mem req",
+		containers: []testContainer{{
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{100, 200, 150, 200},
+			isRunning: true,
+		}},
+		expectHasResize: true,
+	}, {
+		name: "simple resized container/cpu+mem req",
+		containers: []testContainer{{
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{150, 200, 150, 200},
+			isRunning: true,
+		}},
+		expectHasResize: true,
+	}, {
+		name: "simple resized container/mem limit",
+		containers: []testContainer{{
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{100, 200, 100, 300},
+			isRunning: true,
+		}},
+		expectHasResize: true,
+	}, {
+		name: "terminated resized container",
+		containers: []testContainer{{
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{200, 200, 100, 200},
+			isRunning: false,
+		}},
+		expectHasResize: false,
+	}, {
+		name: "non-sidecar init container",
+		containers: []testContainer{{
+			allocated:      testResources{100, 200, 100, 200},
+			nonSidecarInit: true,
+			isRunning:      true,
+		}, {
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{100, 200, 100, 200},
+			isRunning: true,
+		}},
+		expectHasResize: false,
+	}, {
+		name: "non-resized sidecar",
+		containers: []testContainer{{
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{100, 200, 100, 200},
+			sidecar:   true,
+			isRunning: true,
+		}, {
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{100, 200, 100, 200},
+			isRunning: true,
+		}},
+		expectHasResize: false,
+	}, {
+		name: "resized sidecar",
+		containers: []testContainer{{
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{200, 200, 100, 200},
+			sidecar:   true,
+			isRunning: true,
+		}, {
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{100, 200, 100, 200},
+			isRunning: true,
+		}},
+		expectHasResize: true,
+	}, {
+		name: "several containers and a resize",
+		containers: []testContainer{{
+			allocated:      testResources{100, 200, 100, 200},
+			nonSidecarInit: true,
+			isRunning:      true,
+		}, {
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{100, 200, 100, 200},
+			isRunning: true,
+		}, {
+			allocated: testResources{100, 200, 100, 200},
+			unstarted: true,
+		}, {
+			allocated: testResources{100, 200, 100, 200},
+			actuated:  &testResources{200, 200, 100, 200}, // Resized
+			isRunning: true,
+		}},
+		expectHasResize: true,
+	}, {
+		name: "best-effort pod",
+		containers: []testContainer{{
+			allocated: testResources{},
+			actuated:  &testResources{},
+			isRunning: true,
+		}},
+		expectHasResize: false,
+	}, {
+		name: "burstable pod/not resizing",
+		containers: []testContainer{{
+			allocated: testResources{cpuReq: 100},
+			actuated:  &testResources{cpuReq: 100},
+			isRunning: true,
+		}},
+		expectHasResize: false,
+	}, {
+		name: "burstable pod/resized",
+		containers: []testContainer{{
+			allocated: testResources{cpuReq: 100},
+			actuated:  &testResources{cpuReq: 500},
+			isRunning: true,
+		}},
+		expectHasResize: true,
+	}}
+
+	mkRequirements := func(r testResources) v1.ResourceRequirements {
+		res := v1.ResourceRequirements{
+			Requests: v1.ResourceList{},
+			Limits:   v1.ResourceList{},
+		}
+		if r.cpuReq != 0 {
+			res.Requests[v1.ResourceCPU] = *resource.NewMilliQuantity(r.cpuReq, resource.DecimalSI)
+		}
+		if r.cpuLim != 0 {
+			res.Limits[v1.ResourceCPU] = *resource.NewMilliQuantity(r.cpuLim, resource.DecimalSI)
+		}
+		if r.memReq != 0 {
+			res.Requests[v1.ResourceMemory] = *resource.NewQuantity(r.memReq, resource.DecimalSI)
+		}
+		if r.memLim != 0 {
+			res.Limits[v1.ResourceMemory] = *resource.NewQuantity(r.memLim, resource.DecimalSI)
+		}
+		return res
+	}
+	mkContainer := func(index int, c testContainer) v1.Container {
+		container := v1.Container{
+			Name:      fmt.Sprintf("c%d", index),
+			Resources: mkRequirements(c.allocated),
+		}
+		if c.sidecar {
+			container.RestartPolicy = ptr.To(v1.ContainerRestartPolicyAlways)
+		}
+		return container
+	}
+
+	statusManager := status.NewManager(&fake.Clientset{}, kubepod.NewBasicPodManager(), &statustest.FakePodDeletionSafetyProvider{}, kubeletutil.NewPodStartupLatencyTracker())
+	containerManager := cm.NewFakeContainerManager()
+	am := NewInMemoryManager(containerManager, statusManager)
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			pod := &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-pod",
+					UID:  "12345",
+				},
+			}
+			t.Cleanup(func() { am.RemovePod(pod.UID) })
+			podStatus := &kubecontainer.PodStatus{
+				ID:   pod.UID,
+				Name: pod.Name,
+			}
+			for i, c := range test.containers {
+				// Add the container to the pod
+				container := mkContainer(i, c)
+				if c.nonSidecarInit || c.sidecar {
+					pod.Spec.InitContainers = append(pod.Spec.InitContainers, container)
+				} else {
+					pod.Spec.Containers = append(pod.Spec.Containers, container)
+				}
+
+				// Add the container to the pod status, if it's started.
+				if !test.containers[i].unstarted {
+					cs := kubecontainer.Status{
+						Name: container.Name,
+					}
+					if test.containers[i].isRunning {
+						cs.State = kubecontainer.ContainerStateRunning
+					} else {
+						cs.State = kubecontainer.ContainerStateExited
+					}
+					podStatus.ContainerStatuses = append(podStatus.ContainerStatuses, &cs)
+				}
+
+				// Register the actuated container (if needed)
+				if c.actuated != nil {
+					actuatedContainer := container.DeepCopy()
+					actuatedContainer.Resources = mkRequirements(*c.actuated)
+					require.NoError(t, am.SetActuatedResources(pod, actuatedContainer))
+
+					fetched, found := am.GetActuatedResources(pod.UID, container.Name)
+					require.True(t, found)
+					assert.Equal(t, actuatedContainer.Resources, fetched)
+				} else {
+					_, found := am.GetActuatedResources(pod.UID, container.Name)
+					require.False(t, found)
+				}
+			}
+			require.NoError(t, am.SetAllocatedResources(pod))
+
+			hasResizedResources := am.(*manager).isPodResizeInProgress(pod, podStatus)
+			require.Equal(t, test.expectHasResize, hasResizedResources, "hasResizedResources")
+		})
+	}
+}

pkg/kubelet/kubelet_test.go

@@ -3991,260 +3991,6 @@ func TestRecordAdmissionRejection(t *testing.T) {
 	}
 }
 
-func TestIsPodResizeInProgress(t *testing.T) {
-	type testResources struct {
-		cpuReq, cpuLim, memReq, memLim int64
-	}
-	type testContainer struct {
-		allocated               testResources
-		actuated                *testResources
-		nonSidecarInit, sidecar bool
-		isRunning               bool
-		unstarted               bool // Whether the container is missing from the pod status
-	}
-
-	tests := []struct {
-		name            string
-		containers      []testContainer
-		expectHasResize bool
-	}{{
-		name: "simple running container",
-		containers: []testContainer{{
-			allocated: testResources{100, 100, 100, 100},
-			actuated:  &testResources{100, 100, 100, 100},
-			isRunning: true,
-		}},
-		expectHasResize: false,
-	}, {
-		name: "simple unstarted container",
-		containers: []testContainer{{
-			allocated: testResources{100, 100, 100, 100},
-			unstarted: true,
-		}},
-		expectHasResize: false,
-	}, {
-		name: "simple resized container/cpu req",
-		containers: []testContainer{{
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{150, 200, 100, 200},
-			isRunning: true,
-		}},
-		expectHasResize: true,
-	}, {
-		name: "simple resized container/cpu limit",
-		containers: []testContainer{{
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{100, 300, 100, 200},
-			isRunning: true,
-		}},
-		expectHasResize: true,
-	}, {
-		name: "simple resized container/mem req",
-		containers: []testContainer{{
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{100, 200, 150, 200},
-			isRunning: true,
-		}},
-		expectHasResize: true,
-	}, {
-		name: "simple resized container/cpu+mem req",
-		containers: []testContainer{{
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{150, 200, 150, 200},
-			isRunning: true,
-		}},
-		expectHasResize: true,
-	}, {
-		name: "simple resized container/mem limit",
-		containers: []testContainer{{
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{100, 200, 100, 300},
-			isRunning: true,
-		}},
-		expectHasResize: true,
-	}, {
-		name: "terminated resized container",
-		containers: []testContainer{{
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{200, 200, 100, 200},
-			isRunning: false,
-		}},
-		expectHasResize: false,
-	}, {
-		name: "non-sidecar init container",
-		containers: []testContainer{{
-			allocated:      testResources{100, 200, 100, 200},
-			nonSidecarInit: true,
-			isRunning:      true,
-		}, {
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{100, 200, 100, 200},
-			isRunning: true,
-		}},
-		expectHasResize: false,
-	}, {
-		name: "non-resized sidecar",
-		containers: []testContainer{{
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{100, 200, 100, 200},
-			sidecar:   true,
-			isRunning: true,
-		}, {
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{100, 200, 100, 200},
-			isRunning: true,
-		}},
-		expectHasResize: false,
-	}, {
-		name: "resized sidecar",
-		containers: []testContainer{{
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{200, 200, 100, 200},
-			sidecar:   true,
-			isRunning: true,
-		}, {
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{100, 200, 100, 200},
-			isRunning: true,
-		}},
-		expectHasResize: true,
-	}, {
-		name: "several containers and a resize",
-		containers: []testContainer{{
-			allocated:      testResources{100, 200, 100, 200},
-			nonSidecarInit: true,
-			isRunning:      true,
-		}, {
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{100, 200, 100, 200},
-			isRunning: true,
-		}, {
-			allocated: testResources{100, 200, 100, 200},
-			unstarted: true,
-		}, {
-			allocated: testResources{100, 200, 100, 200},
-			actuated:  &testResources{200, 200, 100, 200}, // Resized
-			isRunning: true,
-		}},
-		expectHasResize: true,
-	}, {
-		name: "best-effort pod",
-		containers: []testContainer{{
-			allocated: testResources{},
-			actuated:  &testResources{},
-			isRunning: true,
-		}},
-		expectHasResize: false,
-	}, {
-		name: "burstable pod/not resizing",
-		containers: []testContainer{{
-			allocated: testResources{cpuReq: 100},
-			actuated:  &testResources{cpuReq: 100},
-			isRunning: true,
-		}},
-		expectHasResize: false,
-	}, {
-		name: "burstable pod/resized",
-		containers: []testContainer{{
-			allocated: testResources{cpuReq: 100},
-			actuated:  &testResources{cpuReq: 500},
-			isRunning: true,
-		}},
-		expectHasResize: true,
-	}}
-
-	mkRequirements := func(r testResources) v1.ResourceRequirements {
-		res := v1.ResourceRequirements{
-			Requests: v1.ResourceList{},
-			Limits:   v1.ResourceList{},
-		}
-		if r.cpuReq != 0 {
-			res.Requests[v1.ResourceCPU] = *resource.NewMilliQuantity(r.cpuReq, resource.DecimalSI)
-		}
-		if r.cpuLim != 0 {
-			res.Limits[v1.ResourceCPU] = *resource.NewMilliQuantity(r.cpuLim, resource.DecimalSI)
-		}
-		if r.memReq != 0 {
-			res.Requests[v1.ResourceMemory] = *resource.NewQuantity(r.memReq, resource.DecimalSI)
-		}
-		if r.memLim != 0 {
-			res.Limits[v1.ResourceMemory] = *resource.NewQuantity(r.memLim, resource.DecimalSI)
-		}
-		return res
-	}
-	mkContainer := func(index int, c testContainer) v1.Container {
-		container := v1.Container{
-			Name:      fmt.Sprintf("c%d", index),
-			Resources: mkRequirements(c.allocated),
-		}
-		if c.sidecar {
-			container.RestartPolicy = ptr.To(v1.ContainerRestartPolicyAlways)
-		}
-		return container
-	}
-
-	testKubelet := newTestKubelet(t, false)
-	defer testKubelet.Cleanup()
-	kl := testKubelet.kubelet
-	am := kl.allocationManager
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			pod := &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "test-pod",
-					UID:  "12345",
-				},
-			}
-			t.Cleanup(func() { am.RemovePod(pod.UID) })
-			podStatus := &kubecontainer.PodStatus{
-				ID:   pod.UID,
-				Name: pod.Name,
-			}
-			for i, c := range test.containers {
-				// Add the container to the pod
-				container := mkContainer(i, c)
-				if c.nonSidecarInit || c.sidecar {
-					pod.Spec.InitContainers = append(pod.Spec.InitContainers, container)
-				} else {
-					pod.Spec.Containers = append(pod.Spec.Containers, container)
-				}
-
-				// Add the container to the pod status, if it's started.
-				if !test.containers[i].unstarted {
-					cs := kubecontainer.Status{
-						Name: container.Name,
-					}
-					if test.containers[i].isRunning {
-						cs.State = kubecontainer.ContainerStateRunning
-					} else {
-						cs.State = kubecontainer.ContainerStateExited
-					}
-					podStatus.ContainerStatuses = append(podStatus.ContainerStatuses, &cs)
-				}
-
-				// Register the actuated container (if needed)
-				if c.actuated != nil {
-					actuatedContainer := container.DeepCopy()
-					actuatedContainer.Resources = mkRequirements(*c.actuated)
-					require.NoError(t, am.SetActuatedResources(pod, actuatedContainer))
-
-					fetched, found := am.GetActuatedResources(pod.UID, container.Name)
-					require.True(t, found)
-					assert.Equal(t, actuatedContainer.Resources, fetched)
-				} else {
-					_, found := am.GetActuatedResources(pod.UID, container.Name)
-					require.False(t, found)
-				}
-			}
-			require.NoError(t, am.SetAllocatedResources(pod))
-
-			hasResizedResources := kl.allocationManager.IsPodResizeInProgress(pod, podStatus)
-			require.Equal(t, test.expectHasResize, hasResizedResources, "hasResizedResources")
-		})
-	}
-}
-
 func TestCrashLoopBackOffConfiguration(t *testing.T) {
 	testCases := []struct {
 		name            string