From 683f7afc0d525c1fd990ca52d7bbf8f9fca23eee Mon Sep 17 00:00:00 2001
From: Steve Kriss <steve@heptio.com>
Date: Wed, 11 Jul 2018 09:56:19 -0700
Subject: [PATCH] switch to using .status.startTimestamp for sorting backups

Signed-off-by: Steve Kriss <steve@heptio.com>
---
 pkg/controller/restore_controller.go      | 14 +++----------
 pkg/controller/restore_controller_test.go | 24 +++++++++++++++--------
 pkg/util/test/test_backup.go              |  5 +++++
 3 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go
index 0b3519457..7af63bf81 100644
--- a/pkg/controller/restore_controller.go
+++ b/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 {
diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go
index cbeef630a..a9a15c504 100644
--- a/pkg/controller/restore_controller_test.go
+++ b/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)
diff --git a/pkg/util/test/test_backup.go b/pkg/util/test/test_backup.go
index 4dc52409b..d7f877e51 100644
--- a/pkg/util/test/test_backup.go
+++ b/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
+}