[orchestrator] Fix task sorting

manager/orchestrator/task.go

@@ -9,6 +9,7 @@ import (
 	"github.com/docker/swarmkit/identity"
 	"github.com/docker/swarmkit/manager/constraint"
 	"github.com/docker/swarmkit/protobuf/ptypes"
+	google_protobuf "github.com/gogo/protobuf/types"
 )
 
 // NewTask creates a new task.
@@ -143,6 +144,14 @@ func InvalidNode(n *api.Node) bool {
 		n.Spec.Availability == api.NodeAvailabilityDrain
 }
 
+func taskTimestamp(t *api.Task) *google_protobuf.Timestamp {
+	if t.Status.AppliedAt != nil {
+		return t.Status.AppliedAt
+	}
+
+	return t.Status.Timestamp
+}
+
 // TasksByTimestamp sorts tasks by applied timestamp if available, otherwise
 // status timestamp.
 type TasksByTimestamp []*api.Task
@@ -159,15 +168,8 @@ func (t TasksByTimestamp) Swap(i, j int) {
 
 // Less implements the Less method for sorting.
 func (t TasksByTimestamp) Less(i, j int) bool {
-	iTimestamp := t[i].Status.Timestamp
-	if t[i].Status.AppliedAt != nil {
-		iTimestamp = t[i].Status.AppliedAt
-	}
-
-	jTimestamp := t[j].Status.Timestamp
-	if t[j].Status.AppliedAt != nil {
-		iTimestamp = t[j].Status.AppliedAt
-	}
+	iTimestamp := taskTimestamp(t[i])
+	jTimestamp := taskTimestamp(t[j])
 
 	if iTimestamp == nil {
 		return true

manager/orchestrator/task_test.go

@@ -1,7 +1,10 @@
 package orchestrator
 
 import (
+	google_protobuf "github.com/gogo/protobuf/types"
 	"github.com/stretchr/testify/assert"
+	"sort"
+	"strconv"
 	"testing"
 
 	"github.com/docker/swarmkit/api"
@@ -105,3 +108,41 @@ func TestIsTaskDirtyPlacementConstraintsOnly(t *testing.T) {
 	service.Spec.Task.Placement.Constraints = nil
 	assert.False(t, IsTaskDirtyPlacementConstraintsOnly(service.Spec.Task, task))
 }
+
+// Test Task sorting, which is currently based on
+// Status.AppliedAt, and then on Status.Timestamp.
+func TestTaskSort(t *testing.T) {
+	var tasks []*api.Task
+	size := 5
+	seconds := int64(size)
+	for i := 0; i < size; i++ {
+		task := &api.Task{
+			ID: "id_" + strconv.Itoa(i),
+			Status: api.TaskStatus{
+				Timestamp: &google_protobuf.Timestamp{Seconds: seconds},
+			},
+		}
+
+		seconds--
+		tasks = append(tasks, task)
+	}
+
+	sort.Sort(TasksByTimestamp(tasks))
+	for i, task := range tasks {
+		expected := &google_protobuf.Timestamp{Seconds: int64(i + 1)}
+		assert.Equal(t, expected, task.Status.Timestamp)
+		assert.Equal(t, "id_"+strconv.Itoa(size-(i+1)), task.ID)
+	}
+
+	for i, task := range tasks {
+		task.Status.AppliedAt = &google_protobuf.Timestamp{Seconds: int64(size - i)}
+	}
+
+	sort.Sort(TasksByTimestamp(tasks))
+	sort.Sort(TasksByTimestamp(tasks))
+	for i, task := range tasks {
+		expected := &google_protobuf.Timestamp{Seconds: int64(i + 1)}
+		assert.Equal(t, expected, task.Status.AppliedAt)
+		assert.Equal(t, "id_"+strconv.Itoa(i), task.ID)
+	}
+}