When comparing timestamp of pods, kill pods with zero timestamps first

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 {

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].
+	// pods[5] is running and ready and with LastTransitionTime.
-	then := unversioned.Time{Time: now.AddDate(0, -1, 0)}
 	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].Status.ContainerStatuses = []api.ContainerStatus{{RestartCount: 3}, {RestartCount: 0}}
-	pods[6].CreationTimestamp = now
+	// pods[7] has lower container restart count than pods[6].
-	// pods[7] is older 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))