switch to using .status.startTimestamp for sorting backups

Signed-off-by: Steve Kriss <steve@heptio.com>

pkg/controller/restore_controller.go

@@ -384,19 +384,11 @@ func backupXorScheduleProvided(restore *api.Restore) bool {
 }
 
 // mostRecentCompletedBackup returns the most recent backup that's
-// completed from a list of backups. Since the backups are expected
-// to be from a single schedule, "most recent" is defined as first
-// when sorted in reverse alphabetical order by name.
+// completed from a list of backups.
 func mostRecentCompletedBackup(backups []*api.Backup) *api.Backup {
 	sort.Slice(backups, func(i, j int) bool {
-		// Use '>' because we want descending sort.
-		// Using Name rather than CreationTimestamp because in the case of
-		// backups synced into a new cluster, the CreationTimestamp value is
-		// time of creation in the new cluster rather than time of backup.
-		// TODO would be useful to have a new API field in backup.status
-		// that captures the time of backup as a time value (particularly
-		// for non-scheduled backups).
-		return backups[i].Name > backups[j].Name
+		// Use .After() because we want descending sort.
+		return backups[i].Status.StartTimestamp.After(backups[j].Status.StartTimestamp.Time)
 	})
 
 	for _, backup := range backups {

pkg/controller/restore_controller_test.go

@@ -23,6 +23,7 @@ import (
 	"io"
 	"io/ioutil"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -543,25 +544,28 @@ func TestCompleteAndValidateWhenScheduleNameSpecified(t *testing.T) {
 	assert.Empty(t, restore.Spec.BackupName)
 
 	// multiple completed backups created from the schedule: use most recent
-	// (defined as last in alphabetical order)
+	now := time.Now()
+
 	require.NoError(t, sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(arktest.
 		NewTestBackup().
-		WithName("a").
+		WithName("foo").
 		WithLabel("ark-schedule", "schedule-1").
 		WithPhase(api.BackupPhaseCompleted).
+		WithStartTimestamp(now).
 		Backup,
 	))
 	require.NoError(t, sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(arktest.
 		NewTestBackup().
-		WithName("b").
+		WithName("bar").
 		WithLabel("ark-schedule", "schedule-1").
 		WithPhase(api.BackupPhaseCompleted).
+		WithStartTimestamp(now.Add(time.Second)).
 		Backup,
 	))
 
 	errs = c.completeAndValidate(restore)
 	assert.Nil(t, errs)
-	assert.Equal(t, "b", restore.Spec.BackupName)
+	assert.Equal(t, "bar", restore.Spec.BackupName)
 }
 
 func TestBackupXorScheduleProvided(t *testing.T) {
@@ -628,21 +632,25 @@ func TestMostRecentCompletedBackup(t *testing.T) {
 
 	assert.Nil(t, mostRecentCompletedBackup(backups))
 
+	now := time.Now()
+
 	backups = append(backups, &api.Backup{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "a1",
+			Name: "foo",
 		},
 		Status: api.BackupStatus{
-			Phase: api.BackupPhaseCompleted,
+			Phase:          api.BackupPhaseCompleted,
+			StartTimestamp: metav1.Time{Time: now},
 		},
 	})
 
 	expected := &api.Backup{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "b1",
+			Name: "bar",
 		},
 		Status: api.BackupStatus{
-			Phase: api.BackupPhaseCompleted,
+			Phase:          api.BackupPhaseCompleted,
+			StartTimestamp: metav1.Time{Time: now.Add(time.Second)},
 		},
 	}
 	backups = append(backups, expected)

pkg/util/test/test_backup.go

@@ -130,3 +130,8 @@ func (b *TestBackup) WithFinalizers(finalizers ...string) *TestBackup {
 
 	return b
 }
+
+func (b *TestBackup) WithStartTimestamp(startTime time.Time) *TestBackup {
+	b.Status.StartTimestamp = metav1.Time{Time: startTime}
+	return b
+}