diff --git a/pkg/controller/controller_utils.go b/pkg/controller/controller_utils.go index 371835ebdad..8a60fe35361 100644 --- a/pkg/controller/controller_utils.go +++ b/pkg/controller/controller_utils.go @@ -427,22 +427,31 @@ func (s ActivePods) Less(i, j int) bool { } // TODO: take availability into account when we push minReadySeconds information from deployment into pods, // see https://github.com/kubernetes/kubernetes/issues/22065 - // 4. Been ready for less time < more time + // 4. Been ready for empty time < less time < more time // If both pods are ready, the latest ready one is smaller if api.IsPodReady(s[i]) && api.IsPodReady(s[j]) && !podReadyTime(s[i]).Equal(podReadyTime(s[j])) { - return podReadyTime(s[i]).After(podReadyTime(s[j]).Time) + return afterOrZero(podReadyTime(s[i]), podReadyTime(s[j])) } // 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]) } - // 6. Newer pods < older pods + // 6. Empty creation time pods < newer pods < older pods if !s[i].CreationTimestamp.Equal(s[j].CreationTimestamp) { - return s[i].CreationTimestamp.After(s[j].CreationTimestamp.Time) + return afterOrZero(s[i].CreationTimestamp, s[j].CreationTimestamp) } return false } +// afterOrZero checks if time t1 is after time t2; if one of them +// is zero, the zero time is seen as after non-zero time. +func afterOrZero(t1, t2 unversioned.Time) bool { + if t1.Time.IsZero() || t2.Time.IsZero() { + return t1.Time.IsZero() + } + return t1.After(t2.Time) +} + func podReadyTime(pod *api.Pod) unversioned.Time { if api.IsPodReady(pod) { for _, c := range pod.Status.Conditions { diff --git a/pkg/controller/controller_utils_test.go b/pkg/controller/controller_utils_test.go index bac720313fb..7799985d9d7 100644 --- a/pkg/controller/controller_utils_test.go +++ b/pkg/controller/controller_utils_test.go @@ -245,7 +245,7 @@ func TestActivePodFiltering(t *testing.T) { } func TestSortingActivePods(t *testing.T) { - numPods := 8 + numPods := 9 // This rc is not needed by the test, only the newPodList to give the pods labels/a namespace. rc := newReplicationController(0) podList := newPodList(nil, numPods, api.PodRunning, rc) @@ -266,30 +266,35 @@ func TestSortingActivePods(t *testing.T) { // pods[3] is running but not ready. pods[3].Spec.NodeName = "foo" pods[3].Status.Phase = api.PodRunning - // pods[4] is running and ready. + // pods[4] is running and ready but without LastTransitionTime. now := unversioned.Now() pods[4].Spec.NodeName = "foo" pods[4].Status.Phase = api.PodRunning - pods[4].Status.Conditions = []api.PodCondition{{Type: api.PodReady, Status: api.ConditionTrue, LastTransitionTime: now}} + pods[4].Status.Conditions = []api.PodCondition{{Type: api.PodReady, Status: api.ConditionTrue}} pods[4].Status.ContainerStatuses = []api.ContainerStatus{{RestartCount: 3}, {RestartCount: 0}} - // pods[5] is running ready for a longer time than pods[4]. - then := unversioned.Time{Time: now.AddDate(0, -1, 0)} + // pods[5] is running and ready and with LastTransitionTime. pods[5].Spec.NodeName = "foo" pods[5].Status.Phase = api.PodRunning - pods[5].Status.Conditions = []api.PodCondition{{Type: api.PodReady, Status: api.ConditionTrue, LastTransitionTime: then}} + pods[5].Status.Conditions = []api.PodCondition{{Type: api.PodReady, Status: api.ConditionTrue, LastTransitionTime: now}} pods[5].Status.ContainerStatuses = []api.ContainerStatus{{RestartCount: 3}, {RestartCount: 0}} - // pods[6] has lower container restart count than pods[5]. + // pods[6] is running ready for a longer time than pods[5]. + then := unversioned.Time{Time: now.AddDate(0, -1, 0)} pods[6].Spec.NodeName = "foo" pods[6].Status.Phase = api.PodRunning pods[6].Status.Conditions = []api.PodCondition{{Type: api.PodReady, Status: api.ConditionTrue, LastTransitionTime: then}} - pods[6].Status.ContainerStatuses = []api.ContainerStatus{{RestartCount: 2}, {RestartCount: 1}} - pods[6].CreationTimestamp = now - // pods[7] is older than pods[6]. + pods[6].Status.ContainerStatuses = []api.ContainerStatus{{RestartCount: 3}, {RestartCount: 0}} + // pods[7] has lower container restart count than pods[6]. pods[7].Spec.NodeName = "foo" pods[7].Status.Phase = api.PodRunning pods[7].Status.Conditions = []api.PodCondition{{Type: api.PodReady, Status: api.ConditionTrue, LastTransitionTime: then}} pods[7].Status.ContainerStatuses = []api.ContainerStatus{{RestartCount: 2}, {RestartCount: 1}} - pods[7].CreationTimestamp = then + pods[7].CreationTimestamp = now + // pods[8] is older than pods[7]. + pods[8].Spec.NodeName = "foo" + pods[8].Status.Phase = api.PodRunning + pods[8].Status.Conditions = []api.PodCondition{{Type: api.PodReady, Status: api.ConditionTrue, LastTransitionTime: then}} + pods[8].Status.ContainerStatuses = []api.ContainerStatus{{RestartCount: 2}, {RestartCount: 1}} + pods[8].CreationTimestamp = then getOrder := func(pods []*api.Pod) []string { names := make([]string, len(pods))