diff --git a/pkg/kubelet/allocation/allocation_manager.go b/pkg/kubelet/allocation/allocation_manager.go index 7a977b7d4d9..c25e43bfe99 100644 --- a/pkg/kubelet/allocation/allocation_manager.go +++ b/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) { diff --git a/pkg/kubelet/allocation/allocation_manager_test.go b/pkg/kubelet/allocation/allocation_manager_test.go index c9cbc177ec2..60a23326872 100644 --- a/pkg/kubelet/allocation/allocation_manager_test.go +++ b/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") + }) + } +} diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 0e923277dca..a8ab9768319 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -3994,260 +3994,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