github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-30 06:54:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #124952 from AxeZhan/maxContainerRestarts

Browse Source 
[Sidecar Containers] Pods comparison by maxContainerRestarts should account for sidecar containers
This commit is contained in:
Kubernetes Prow Robot 2024-11-07 11:13:30 +00:00 committed by GitHub
commit 9a9331afd6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 201 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
pkg/controller/controller_utils.go
View File

@ -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.

69
pkg/controller/controller_utils_test.go
View File

@ -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}},

46
staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go
View File

@ -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

76
staging/src/k8s.io/kubectl/pkg/util/podutils/podutils_test.go
View File

@ -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) {