Commit ae13b88a authored by Tom Wanielista's avatar Tom Wanielista Committed by Vallery Lancey

Avoid panic in cronjob sorting

This change handles the case where the ith cronjob may have its start time set to nil. Previously, the Less method could cause a panic in case the ith cronjob had its start time set to nil, but the jth cronjob did not. It would panic when calling Before on a nil StartTime.
parent a9f35a67
......@@ -195,13 +195,14 @@ func (o byJobStartTime) Len() int { return len(o) }
func (o byJobStartTime) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
func (o byJobStartTime) Less(i, j int) bool {
if o[j].Status.StartTime == nil {
return o[i].Status.StartTime != nil
if o[i].Status.StartTime == nil && o[j].Status.StartTime != nil {
return false
}
if o[i].Status.StartTime != nil && o[j].Status.StartTime == nil {
return true
}
if o[i].Status.StartTime.Equal(o[j].Status.StartTime) {
return o[i].Name < o[j].Name
}
return o[i].Status.StartTime.Before(o[j].Status.StartTime)
}
......@@ -17,6 +17,8 @@ limitations under the License.
package cronjob
import (
"reflect"
"sort"
"strings"
"testing"
"time"
......@@ -376,3 +378,66 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) {
}
}
}
func TestByJobStartTime(t *testing.T) {
now := metav1.NewTime(time.Date(2018, time.January, 1, 2, 3, 4, 5, time.UTC))
later := metav1.NewTime(time.Date(2019, time.January, 1, 2, 3, 4, 5, time.UTC))
aNil := batchv1.Job{
ObjectMeta: metav1.ObjectMeta{Name: "a"},
Status: batchv1.JobStatus{},
}
bNil := batchv1.Job{
ObjectMeta: metav1.ObjectMeta{Name: "b"},
Status: batchv1.JobStatus{},
}
aSet := batchv1.Job{
ObjectMeta: metav1.ObjectMeta{Name: "a"},
Status: batchv1.JobStatus{StartTime: &now},
}
bSet := batchv1.Job{
ObjectMeta: metav1.ObjectMeta{Name: "b"},
Status: batchv1.JobStatus{StartTime: &now},
}
aSetLater := batchv1.Job{
ObjectMeta: metav1.ObjectMeta{Name: "a"},
Status: batchv1.JobStatus{StartTime: &later},
}
testCases := []struct {
name string
input, expected []batchv1.Job
}{
{
name: "both have nil start times",
input: []batchv1.Job{bNil, aNil},
expected: []batchv1.Job{aNil, bNil},
},
{
name: "only the first has a nil start time",
input: []batchv1.Job{aNil, bSet},
expected: []batchv1.Job{bSet, aNil},
},
{
name: "only the second has a nil start time",
input: []batchv1.Job{aSet, bNil},
expected: []batchv1.Job{aSet, bNil},
},
{
name: "both have non-nil, equal start time",
input: []batchv1.Job{bSet, aSet},
expected: []batchv1.Job{aSet, bSet},
},
{
name: "both have non-nil, different start time",
input: []batchv1.Job{aSetLater, bSet},
expected: []batchv1.Job{bSet, aSetLater},
},
}
for _, testCase := range testCases {
sort.Sort(byJobStartTime(testCase.input))
if !reflect.DeepEqual(testCase.input, testCase.expected) {
t.Errorf("case: '%s', jobs not sorted as expected", testCase.name)
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment