diff --git a/pkg/controller/controller_utils.go b/pkg/controller/controller_utils.go index 143a3d361fe..ffd99ed1236 100644 --- a/pkg/controller/controller_utils.go +++ b/pkg/controller/controller_utils.go @@ -713,8 +713,8 @@ func (s ByLogging) Less(i, j int) bool { } } // 5. Pods with containers with higher restart counts < lower restart counts - if maxContainerRestarts(s[i]) != maxContainerRestarts(s[j]) { - return maxContainerRestarts(s[i]) > maxContainerRestarts(s[j]) + if res := compareMaxContainerRestarts(s[i], s[j]); res != nil { + return *res } // 6. older pods < newer pods < empty timestamp pods if !s[i].CreationTimestamp.Equal(&s[j].CreationTimestamp) { @@ -756,8 +756,8 @@ func (s ActivePods) Less(i, j int) bool { } } // 5. Pods with containers with higher restart counts < lower restart counts - if maxContainerRestarts(s[i]) != maxContainerRestarts(s[j]) { - return maxContainerRestarts(s[i]) > maxContainerRestarts(s[j]) + if res := compareMaxContainerRestarts(s[i], s[j]); res != nil { + return *res } // 6. Empty creation time pods < newer pods < older pods if !s[i].CreationTimestamp.Equal(&s[j].CreationTimestamp) { @@ -878,8 +878,8 @@ func (s ActivePodsWithRanks) Less(i, j int) bool { } } // 7. Pods with containers with higher restart counts < lower restart counts - if maxContainerRestarts(s.Pods[i]) != maxContainerRestarts(s.Pods[j]) { - return maxContainerRestarts(s.Pods[i]) > maxContainerRestarts(s.Pods[j]) + if res := compareMaxContainerRestarts(s.Pods[i], s.Pods[j]); res != nil { + return *res } // 8. Empty creation time pods < newer pods < older pods if !s.Pods[i].CreationTimestamp.Equal(&s.Pods[j].CreationTimestamp) { @@ -936,12 +936,41 @@ func podReadyTime(pod *v1.Pod) *metav1.Time { return &metav1.Time{} } -func maxContainerRestarts(pod *v1.Pod) int { - maxRestarts := 0 +func maxContainerRestarts(pod *v1.Pod) (regularRestarts, sidecarRestarts int) { for _, c := range pod.Status.ContainerStatuses { - maxRestarts = max(maxRestarts, int(c.RestartCount)) + regularRestarts = max(regularRestarts, int(c.RestartCount)) } - return maxRestarts + names := sets.New[string]() + for _, c := range pod.Spec.InitContainers { + if c.RestartPolicy != nil && *c.RestartPolicy == v1.ContainerRestartPolicyAlways { + names.Insert(c.Name) + } + } + for _, c := range pod.Status.InitContainerStatuses { + if names.Has(c.Name) { + sidecarRestarts = max(sidecarRestarts, int(c.RestartCount)) + } + } + return +} + +// We use *bool here to determine equality: +// true: pi has a higher container restart count. +// false: pj has a higher container restart count. +// nil: Both have the same container restart count. +func compareMaxContainerRestarts(pi *v1.Pod, pj *v1.Pod) *bool { + regularRestartsI, sidecarRestartsI := maxContainerRestarts(pi) + regularRestartsJ, sidecarRestartsJ := maxContainerRestarts(pj) + if regularRestartsI != regularRestartsJ { + res := regularRestartsI > regularRestartsJ + return &res + } + // If pods have the same restart count, an attempt is made to compare the restart counts of sidecar containers. + if sidecarRestartsI != sidecarRestartsJ { + res := sidecarRestartsI > sidecarRestartsJ + return &res + } + return nil } // FilterActivePods returns pods that have not terminated. diff --git a/pkg/controller/controller_utils_test.go b/pkg/controller/controller_utils_test.go index ef6f74127af..d14daced9f1 100644 --- a/pkg/controller/controller_utils_test.go +++ b/pkg/controller/controller_utils_test.go @@ -491,6 +491,7 @@ func TestSortingActivePods(t *testing.T) { now := metav1.Now() then := metav1.Time{Time: now.AddDate(0, -1, 0)} + restartAlways := v1.ContainerRestartPolicyAlways tests := []struct { name string pods []v1.Pod @@ -546,6 +547,22 @@ func TestSortingActivePods(t *testing.T) { ContainerStatuses: []v1.ContainerStatus{{RestartCount: 3}, {RestartCount: 0}}, }, }, + { + ObjectMeta: metav1.ObjectMeta{Name: "lowerSidecarContainerRestartCount", CreationTimestamp: now}, + Spec: v1.PodSpec{ + NodeName: "foo", + InitContainers: []v1.Container{{ + Name: "sidecar", + RestartPolicy: &restartAlways, + }}, + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + Conditions: []v1.PodCondition{{Type: v1.PodReady, Status: v1.ConditionTrue, LastTransitionTime: then}}, + ContainerStatuses: []v1.ContainerStatus{{RestartCount: 2}, {RestartCount: 1}}, + InitContainerStatuses: []v1.ContainerStatus{{Name: "sidecar", RestartCount: 2}}, + }, + }, { ObjectMeta: metav1.ObjectMeta{Name: "lowerContainerRestartCount", CreationTimestamp: now}, Spec: v1.PodSpec{NodeName: "foo"}, @@ -573,6 +590,7 @@ func TestSortingActivePods(t *testing.T) { "runningNoLastTransitionTime", "runningWithLastTransitionTime", "runningLongerTime", + "lowerSidecarContainerRestartCount", "lowerContainerRestartCount", "oldest", }, @@ -611,12 +629,15 @@ func TestSortingActivePodsWithRanks(t *testing.T) { then5Hours := metav1.Time{Time: now.Add(-5 * time.Hour)} then8Hours := metav1.Time{Time: now.Add(-8 * time.Hour)} zeroTime := metav1.Time{} - pod := func(podName, nodeName string, phase v1.PodPhase, ready bool, restarts int32, readySince metav1.Time, created metav1.Time, annotations map[string]string) *v1.Pod { + restartAlways := v1.ContainerRestartPolicyAlways + pod := func(podName, nodeName string, phase v1.PodPhase, ready bool, restarts int32, sideRestarts int32, readySince metav1.Time, created metav1.Time, annotations map[string]string) *v1.Pod { var conditions []v1.PodCondition var containerStatuses []v1.ContainerStatus + var initContainerStatuses []v1.ContainerStatus if ready { conditions = []v1.PodCondition{{Type: v1.PodReady, Status: v1.ConditionTrue, LastTransitionTime: readySince}} containerStatuses = []v1.ContainerStatus{{RestartCount: restarts}} + initContainerStatuses = []v1.ContainerStatus{{Name: "sidecar", RestartCount: sideRestarts}} } return &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -624,31 +645,36 @@ func TestSortingActivePodsWithRanks(t *testing.T) { Name: podName, Annotations: annotations, }, - Spec: v1.PodSpec{NodeName: nodeName}, + Spec: v1.PodSpec{ + NodeName: nodeName, + InitContainers: []v1.Container{{Name: "sidecar", RestartPolicy: &restartAlways}}, + }, Status: v1.PodStatus{ - Conditions: conditions, - ContainerStatuses: containerStatuses, - Phase: phase, + Conditions: conditions, + ContainerStatuses: containerStatuses, + InitContainerStatuses: initContainerStatuses, + Phase: phase, }, } } var ( - unscheduledPod = pod("unscheduled", "", v1.PodPending, false, 0, zeroTime, zeroTime, nil) - scheduledPendingPod = pod("pending", "node", v1.PodPending, false, 0, zeroTime, zeroTime, nil) - unknownPhasePod = pod("unknown-phase", "node", v1.PodUnknown, false, 0, zeroTime, zeroTime, nil) - runningNotReadyPod = pod("not-ready", "node", v1.PodRunning, false, 0, zeroTime, zeroTime, nil) - runningReadyNoLastTransitionTimePod = pod("ready-no-last-transition-time", "node", v1.PodRunning, true, 0, zeroTime, zeroTime, nil) - runningReadyNow = pod("ready-now", "node", v1.PodRunning, true, 0, now, now, nil) - runningReadyThen = pod("ready-then", "node", v1.PodRunning, true, 0, then1Month, then1Month, nil) - runningReadyNowHighRestarts = pod("ready-high-restarts", "node", v1.PodRunning, true, 9001, now, now, nil) - runningReadyNowCreatedThen = pod("ready-now-created-then", "node", v1.PodRunning, true, 0, now, then1Month, nil) - lowPodDeletionCost = pod("low-deletion-cost", "node", v1.PodRunning, true, 0, now, then1Month, map[string]string{core.PodDeletionCost: "10"}) - highPodDeletionCost = pod("high-deletion-cost", "node", v1.PodRunning, true, 0, now, then1Month, map[string]string{core.PodDeletionCost: "100"}) - unscheduled5Hours = pod("unscheduled-5-hours", "", v1.PodPending, false, 0, then5Hours, then5Hours, nil) - unscheduled8Hours = pod("unscheduled-10-hours", "", v1.PodPending, false, 0, then8Hours, then8Hours, nil) - ready2Hours = pod("ready-2-hours", "", v1.PodRunning, true, 0, then2Hours, then1Month, nil) - ready5Hours = pod("ready-5-hours", "", v1.PodRunning, true, 0, then5Hours, then1Month, nil) - ready10Hours = pod("ready-10-hours", "", v1.PodRunning, true, 0, then8Hours, then1Month, nil) + unscheduledPod = pod("unscheduled", "", v1.PodPending, false, 0, 0, zeroTime, zeroTime, nil) + scheduledPendingPod = pod("pending", "node", v1.PodPending, false, 0, 0, zeroTime, zeroTime, nil) + unknownPhasePod = pod("unknown-phase", "node", v1.PodUnknown, false, 0, 0, zeroTime, zeroTime, nil) + runningNotReadyPod = pod("not-ready", "node", v1.PodRunning, false, 0, 0, zeroTime, zeroTime, nil) + runningReadyNoLastTransitionTimePod = pod("ready-no-last-transition-time", "node", v1.PodRunning, true, 0, 0, zeroTime, zeroTime, nil) + runningReadyNow = pod("ready-now", "node", v1.PodRunning, true, 0, 0, now, now, nil) + runningReadyThen = pod("ready-then", "node", v1.PodRunning, true, 0, 0, then1Month, then1Month, nil) + runningReadyNowHighRestarts = pod("ready-high-restarts", "node", v1.PodRunning, true, 9001, 0, now, now, nil) + runningReadyNowHighSideRestarts = pod("ready-high-restarts", "node", v1.PodRunning, true, 9001, 9001, now, now, nil) + runningReadyNowCreatedThen = pod("ready-now-created-then", "node", v1.PodRunning, true, 0, 0, now, then1Month, nil) + lowPodDeletionCost = pod("low-deletion-cost", "node", v1.PodRunning, true, 0, 0, now, then1Month, map[string]string{core.PodDeletionCost: "10"}) + highPodDeletionCost = pod("high-deletion-cost", "node", v1.PodRunning, true, 0, 0, now, then1Month, map[string]string{core.PodDeletionCost: "100"}) + unscheduled5Hours = pod("unscheduled-5-hours", "", v1.PodPending, false, 0, 0, then5Hours, then5Hours, nil) + unscheduled8Hours = pod("unscheduled-10-hours", "", v1.PodPending, false, 0, 0, then8Hours, then8Hours, nil) + ready2Hours = pod("ready-2-hours", "", v1.PodRunning, true, 0, 0, then2Hours, then1Month, nil) + ready5Hours = pod("ready-5-hours", "", v1.PodRunning, true, 0, 0, then5Hours, then1Month, nil) + ready10Hours = pod("ready-10-hours", "", v1.PodRunning, true, 0, 0, then8Hours, then1Month, nil) ) equalityTests := []struct { p1 *v1.Pod @@ -703,6 +729,7 @@ func TestSortingActivePodsWithRanks(t *testing.T) { {lesser: podWithRank{runningReadyNow, 1}, greater: podWithRank{runningReadyThen, 1}}, {lesser: podWithRank{runningReadyNow, 2}, greater: podWithRank{runningReadyThen, 1}}, {lesser: podWithRank{runningReadyNowHighRestarts, 1}, greater: podWithRank{runningReadyNow, 1}}, + {lesser: podWithRank{runningReadyNowHighSideRestarts, 1}, greater: podWithRank{runningReadyNowHighRestarts, 1}}, {lesser: podWithRank{runningReadyNow, 2}, greater: podWithRank{runningReadyNowHighRestarts, 1}}, {lesser: podWithRank{runningReadyNow, 1}, greater: podWithRank{runningReadyNowCreatedThen, 1}}, {lesser: podWithRank{runningReadyNowCreatedThen, 2}, greater: podWithRank{runningReadyNow, 1}}, diff --git a/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go b/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go index 642a6d47a7a..dfefdef4567 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go +++ b/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go @@ -21,6 +21,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" ) // IsPodAvailable returns true if a pod is available; false otherwise. @@ -113,8 +114,8 @@ func (s ByLogging) Less(i, j int) bool { return afterOrZero(podReadyTime(s[j]), podReadyTime(s[i])) } // 5. Pods with containers with higher restart counts < lower restart counts - if maxContainerRestarts(s[i]) != maxContainerRestarts(s[j]) { - return maxContainerRestarts(s[i]) > maxContainerRestarts(s[j]) + if res := compareMaxContainerRestarts(s[i], s[j]); res != nil { + return *res } // 6. older pods < newer pods < empty timestamp pods if !s[i].CreationTimestamp.Equal(&s[j].CreationTimestamp) { @@ -161,8 +162,8 @@ func (s ActivePods) Less(i, j int) bool { return afterOrZero(podReadyTime(s[i]), podReadyTime(s[j])) } // 7. Pods with containers with higher restart counts < lower restart counts - if maxContainerRestarts(s[i]) != maxContainerRestarts(s[j]) { - return maxContainerRestarts(s[i]) > maxContainerRestarts(s[j]) + if res := compareMaxContainerRestarts(s[i], s[j]); res != nil { + return *res } // 8. Empty creation time pods < newer pods < older pods if !s[i].CreationTimestamp.Equal(&s[j].CreationTimestamp) { @@ -190,12 +191,41 @@ func podReadyTime(pod *corev1.Pod) *metav1.Time { return &metav1.Time{} } -func maxContainerRestarts(pod *corev1.Pod) int { - maxRestarts := 0 +func maxContainerRestarts(pod *corev1.Pod) (regularRestarts, sidecarRestarts int) { for _, c := range pod.Status.ContainerStatuses { - maxRestarts = max(maxRestarts, int(c.RestartCount)) + regularRestarts = max(regularRestarts, int(c.RestartCount)) } - return maxRestarts + names := sets.New[string]() + for _, c := range pod.Spec.InitContainers { + if c.RestartPolicy != nil && *c.RestartPolicy == corev1.ContainerRestartPolicyAlways { + names.Insert(c.Name) + } + } + for _, c := range pod.Status.InitContainerStatuses { + if names.Has(c.Name) { + sidecarRestarts = max(sidecarRestarts, int(c.RestartCount)) + } + } + return +} + +// We use *bool here to determine equality: +// true: pi has a higher container restart count. +// false: pj has a higher container restart count. +// nil: Both have the same container restart count. +func compareMaxContainerRestarts(pi *corev1.Pod, pj *corev1.Pod) *bool { + regularRestartsI, sidecarRestartsI := maxContainerRestarts(pi) + regularRestartsJ, sidecarRestartsJ := maxContainerRestarts(pj) + if regularRestartsI != regularRestartsJ { + res := regularRestartsI > regularRestartsJ + return &res + } + // If pods have the same restart count, an attempt is made to compare the restart counts of sidecar containers. + if sidecarRestartsI != sidecarRestartsJ { + res := sidecarRestartsI > sidecarRestartsJ + return &res + } + return nil } // ContainerType and VisitContainers are taken from diff --git a/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils_test.go b/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils_test.go index b514a40342a..1515823d6bf 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils_test.go +++ b/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils_test.go @@ -29,6 +29,7 @@ func TestActivePods(t *testing.T) { time1 := metav1.Now() time2 := metav1.NewTime(time1.Add(1 * time.Second)) time3 := metav1.NewTime(time1.Add(2 * time.Second)) + restartAlways := corev1.ContainerRestartPolicyAlways tests := []struct { name string @@ -364,6 +365,81 @@ func TestActivePods(t *testing.T) { }, }, }, + { + name: "higher sidecar restart count should sort before lower restart count", + pod1: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podWithMoreRestarts", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "node1", + InitContainers: []corev1.Container{ + { + Name: "sidecar", + RestartPolicy: &restartAlways, + }, + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + LastTransitionTime: time1, + }, + }, + ContainerStatuses: []corev1.ContainerStatus{ + { + RestartCount: 3, + }, + }, + InitContainerStatuses: []corev1.ContainerStatus{ + { + Name: "sidecar", + RestartCount: 3, + }, + }, + }, + }, + pod2: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podWithLessRestarts", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + NodeName: "node1", + InitContainers: []corev1.Container{ + { + Name: "sidecar", + RestartPolicy: &restartAlways, + }, + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + LastTransitionTime: time1, + }, + }, + ContainerStatuses: []corev1.ContainerStatus{ + { + RestartCount: 3, + }, + }, + InitContainerStatuses: []corev1.ContainerStatus{ + { + Name: "sidecar", + RestartCount: 2, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {