enable a schedule to be provided as the source for a restore

- ScheduleName is added as an API field to the Restore object - Restore controller validates that exactly one of BackupName or ScheduleName has been provided - If ScheduleName is provided, Restore controller populates BackupName with the name of the most recent successful backup created from the schedule - --from-schedule flag is added to `ark restore create` CLI cmd Signed-off-by: Steve Kriss <steve@heptio.com>
2026-08-15 11:46:06 +00:00 · 2018-07-09 15:07:38 -07:00
parent f349f85b05
commit 706ae07d0d
7 changed files with 369 additions and 53 deletions
@@ -24,6 +24,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"sort"
 	"sync"
 	"time"

@@ -32,6 +33,7 @@ import (
 	"github.com/sirupsen/logrus"

 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -45,6 +47,7 @@ import (
 	listers "github.com/heptio/ark/pkg/generated/listers/ark/v1"
 	"github.com/heptio/ark/pkg/plugin"
 	"github.com/heptio/ark/pkg/restore"
+	"github.com/heptio/ark/pkg/util/boolptr"
 	"github.com/heptio/ark/pkg/util/collections"
 	kubeutil "github.com/heptio/ark/pkg/util/kube"
 )
@@ -252,15 +255,8 @@ func (controller *restoreController) processRestore(key string) error {
 	// don't modify items in the cache
 	restore = restore.DeepCopy()

-	excludedResources := sets.NewString(restore.Spec.ExcludedResources...)
-	for _, nonrestorable := range nonRestorableResources {
-		if !excludedResources.Has(nonrestorable) {
-			restore.Spec.ExcludedResources = append(restore.Spec.ExcludedResources, nonrestorable)
-		}
-	}
-
-	// validation
-	if restore.Status.ValidationErrors = controller.getValidationErrors(restore); len(restore.Status.ValidationErrors) > 0 {
+	// complete & validate restore
+	if restore.Status.ValidationErrors = controller.completeAndValidate(restore); len(restore.Status.ValidationErrors) > 0 {
 		restore.Status.Phase = api.RestorePhaseFailedValidation
 	} else {
 		restore.Status.Phase = api.RestorePhaseInProgress
@@ -304,37 +300,114 @@ func (controller *restoreController) processRestore(key string) error {
 	return nil
 }

-func (controller *restoreController) getValidationErrors(itm *api.Restore) []string {
-	var validationErrors []string
-
-	if itm.Spec.BackupName == "" {
-		validationErrors = append(validationErrors, "BackupName must be non-empty and correspond to the name of a backup in object storage.")
-	} else if _, err := controller.fetchBackup(controller.bucket, itm.Spec.BackupName); err != nil {
-		validationErrors = append(validationErrors, fmt.Sprintf("Error retrieving backup: %v", err))
+func (controller *restoreController) completeAndValidate(restore *api.Restore) []string {
+	// add non-restorable resources to restore's excluded resources
+	excludedResources := sets.NewString(restore.Spec.ExcludedResources...)
+	for _, nonrestorable := range nonRestorableResources {
+		if !excludedResources.Has(nonrestorable) {
+			restore.Spec.ExcludedResources = append(restore.Spec.ExcludedResources, nonrestorable)
+		}
 	}

-	includedResources := sets.NewString(itm.Spec.IncludedResources...)
+	var validationErrors []string
+
+	// validate that included resources don't contain any non-restorable resources
+	includedResources := sets.NewString(restore.Spec.IncludedResources...)
 	for _, nonRestorableResource := range nonRestorableResources {
 		if includedResources.Has(nonRestorableResource) {
 			validationErrors = append(validationErrors, fmt.Sprintf("%v are non-restorable resources", nonRestorableResource))
 		}
 	}

-	for _, err := range collections.ValidateIncludesExcludes(itm.Spec.IncludedNamespaces, itm.Spec.ExcludedNamespaces) {
-		validationErrors = append(validationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err))
-	}
-
-	for _, err := range collections.ValidateIncludesExcludes(itm.Spec.IncludedResources, itm.Spec.ExcludedResources) {
+	// validate included/excluded resources
+	for _, err := range collections.ValidateIncludesExcludes(restore.Spec.IncludedResources, restore.Spec.ExcludedResources) {
 		validationErrors = append(validationErrors, fmt.Sprintf("Invalid included/excluded resource lists: %v", err))
 	}

-	if !controller.pvProviderExists && itm.Spec.RestorePVs != nil && *itm.Spec.RestorePVs {
+	// validate included/excluded namespaces
+	for _, err := range collections.ValidateIncludesExcludes(restore.Spec.IncludedNamespaces, restore.Spec.ExcludedNamespaces) {
+		validationErrors = append(validationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err))
+	}
+
+	// validate that PV provider exists if we're restoring PVs
+	if boolptr.IsSetToTrue(restore.Spec.RestorePVs) && !controller.pvProviderExists {
 		validationErrors = append(validationErrors, "Server is not configured for PV snapshot restores")
 	}

+	// validate that exactly one of BackupName and ScheduleName have been specified
+	if !backupXorScheduleProvided(restore) {
+		return append(validationErrors, "Either a backup or schedule must be specified as a source for the restore, but not both")
+	}
+
+	// if ScheduleName is specified, fill in BackupName with the most recent successful backup from
+	// the schedule
+	if restore.Spec.ScheduleName != "" {
+		selector := labels.SelectorFromSet(labels.Set(map[string]string{
+			"ark-schedule": restore.Spec.ScheduleName,
+		}))
+
+		backups, err := controller.backupLister.Backups(controller.namespace).List(selector)
+		if err != nil {
+			return append(validationErrors, "Unable to list backups for schedule")
+		}
+		if len(backups) == 0 {
+			return append(validationErrors, "No backups found for schedule")
+		}
+
+		if backup := mostRecentCompletedBackup(backups); backup != nil {
+			restore.Spec.BackupName = backup.Name
+		} else {
+			return append(validationErrors, "No completed backups found for schedule")
+		}
+	}
+
+	// validate that we can fetch the source backup
+	if _, err := controller.fetchBackup(controller.bucket, restore.Spec.BackupName); err != nil {
+		return append(validationErrors, fmt.Sprintf("Error retrieving backup: %v", err))
+	}
+
 	return validationErrors
 }

+// backupXorScheduleProvided returns true if exactly one of BackupName and
+// ScheduleName are non-empty for the restore, or false otherwise.
+func backupXorScheduleProvided(restore *api.Restore) bool {
+	if restore.Spec.BackupName != "" && restore.Spec.ScheduleName != "" {
+		return false
+	}
+
+	if restore.Spec.BackupName == "" && restore.Spec.ScheduleName == "" {
+		return false
+	}
+
+	return true
+}
+
+// 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.
+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
+	})
+
+	for _, backup := range backups {
+		if backup.Status.Phase == api.BackupPhaseCompleted {
+			return backup
+		}
+	}
+
+	return nil
+}
+
 func (controller *restoreController) fetchBackup(bucket, name string) (*api.Backup, error) {
 	backup, err := controller.backupLister.Backups(controller.namespace).Get(name)
 	if err == nil {
@@ -28,6 +28,7 @@ import (
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"

+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	core "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
@@ -172,13 +173,32 @@ func TestProcessRestore(t *testing.T) {
 			expectedValidationErrors: []string{"Invalid included/excluded resource lists: excludes list cannot contain an item in the includes list: a-resource"},
 		},
 		{
-			name:                     "new restore with empty backup name fails validation",
+			name:                     "new restore with empty backup and schedule names fails validation",
 			restore:                  NewRestore("foo", "bar", "", "ns-1", "", api.RestorePhaseNew).Restore,
 			expectedErr:              false,
 			expectedPhase:            string(api.RestorePhaseFailedValidation),
-			expectedValidationErrors: []string{"BackupName must be non-empty and correspond to the name of a backup in object storage."},
+			expectedValidationErrors: []string{"Either a backup or schedule must be specified as a source for the restore, but not both"},
+		},
+		{
+			name:                     "new restore with backup and schedule names provided fails validation",
+			restore:                  NewRestore("foo", "bar", "backup-1", "ns-1", "", api.RestorePhaseNew).WithSchedule("sched-1").Restore,
+			expectedErr:              false,
+			expectedPhase:            string(api.RestorePhaseFailedValidation),
+			expectedValidationErrors: []string{"Either a backup or schedule must be specified as a source for the restore, but not both"},
+		},
+		{
+			name:    "valid restore with schedule name gets executed",
+			restore: NewRestore("foo", "bar", "", "ns-1", "", api.RestorePhaseNew).WithSchedule("sched-1").Restore,
+			backup: arktest.
+				NewTestBackup().
+				WithName("backup-1").
+				WithLabel("ark-schedule", "sched-1").
+				WithPhase(api.BackupPhaseCompleted).
+				Backup,
+			expectedErr:          false,
+			expectedPhase:        string(api.RestorePhaseInProgress),
+			expectedRestorerCall: NewRestore("foo", "bar", "backup-1", "ns-1", "", api.RestorePhaseInProgress).WithSchedule("sched-1").Restore,
 		},
-
 		{
 			name:                        "restore with non-existent backup name fails",
 			restore:                     NewRestore("foo", "bar", "backup-1", "ns-1", "*", api.RestorePhaseNew).Restore,
@@ -337,6 +357,10 @@ func TestProcessRestore(t *testing.T) {

 					res.Status.Phase = api.RestorePhase(phase)

+					if backupName, err := collections.GetString(patchMap, "spec.backupName"); err == nil {
+						res.Spec.BackupName = backupName
+					}
+
 					return true, res, nil
 				})
 			}
@@ -356,8 +380,8 @@ func TestProcessRestore(t *testing.T) {
 				downloadedBackup := ioutil.NopCloser(bytes.NewReader([]byte("hello world")))
 				backupSvc.On("DownloadBackup", mock.Anything, mock.Anything).Return(downloadedBackup, nil)
 				restorer.On("Restore", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(warnings, errors)
-				backupSvc.On("UploadRestoreLog", "bucket", test.restore.Spec.BackupName, test.restore.Name, mock.Anything).Return(test.uploadLogError)
-				backupSvc.On("UploadRestoreResults", "bucket", test.restore.Spec.BackupName, test.restore.Name, mock.Anything).Return(nil)
+				backupSvc.On("UploadRestoreLog", "bucket", test.backup.Name, test.restore.Name, mock.Anything).Return(test.uploadLogError)
+				backupSvc.On("UploadRestoreResults", "bucket", test.backup.Name, test.restore.Name, mock.Anything).Return(nil)
 			}

 			var (
@@ -372,7 +396,7 @@ func TestProcessRestore(t *testing.T) {
 			}

 			if test.backupServiceGetBackupError != nil {
-				backupSvc.On("GetBackup", "bucket", test.restore.Spec.BackupName).Return(nil, test.backupServiceGetBackupError)
+				backupSvc.On("GetBackup", "bucket", mock.Anything).Return(nil, test.backupServiceGetBackupError)
 			}

 			if test.restore != nil {
@@ -394,6 +418,10 @@ func TestProcessRestore(t *testing.T) {
 			}

 			// structs and func for decoding patch content
+			type SpecPatch struct {
+				BackupName string `json:"backupName"`
+			}
+
 			type StatusPatch struct {
 				Phase            api.RestorePhase `json:"phase"`
 				ValidationErrors []string         `json:"validationErrors"`
@@ -401,6 +429,7 @@ func TestProcessRestore(t *testing.T) {
 			}

 			type Patch struct {
+				Spec   SpecPatch   `json:"spec,omitempty"`
 				Status StatusPatch `json:"status"`
 			}

@@ -421,6 +450,12 @@ func TestProcessRestore(t *testing.T) {
 				},
 			}

+			if test.restore.Spec.ScheduleName != "" && test.backup != nil {
+				expected.Spec = SpecPatch{
+					BackupName: test.backup.Name,
+				}
+			}
+
 			arktest.ValidatePatch(t, actions[0], expected, decode)

 			// if we don't expect a restore, validate it wasn't called and exit the test
@@ -450,6 +485,171 @@ func TestProcessRestore(t *testing.T) {
 	}
 }

+func TestCompleteAndValidateWhenScheduleNameSpecified(t *testing.T) {
+	var (
+		client          = fake.NewSimpleClientset()
+		sharedInformers = informers.NewSharedInformerFactory(client, 0)
+		logger          = arktest.NewLogger()
+	)
+
+	c := NewRestoreController(
+		api.DefaultNamespace,
+		sharedInformers.Ark().V1().Restores(),
+		client.ArkV1(),
+		client.ArkV1(),
+		nil,
+		nil,
+		"bucket",
+		sharedInformers.Ark().V1().Backups(),
+		false,
+		logger,
+		nil,
+	).(*restoreController)
+
+	restore := &api.Restore{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: api.DefaultNamespace,
+			Name:      "restore-1",
+		},
+		Spec: api.RestoreSpec{
+			ScheduleName: "schedule-1",
+		},
+	}
+
+	// no backups created from the schedule: fail validation
+	require.NoError(t, sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(arktest.
+		NewTestBackup().
+		WithName("backup-1").
+		WithLabel("ark-schedule", "non-matching-schedule").
+		WithPhase(api.BackupPhaseCompleted).
+		Backup,
+	))
+
+	errs := c.completeAndValidate(restore)
+	assert.Equal(t, []string{"No backups found for schedule"}, errs)
+	assert.Empty(t, restore.Spec.BackupName)
+
+	// no completed backups created from the schedule: fail validation
+	require.NoError(t, sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(arktest.
+		NewTestBackup().
+		WithName("backup-2").
+		WithLabel("ark-schedule", "schedule-1").
+		WithPhase(api.BackupPhaseInProgress).
+		Backup,
+	))
+
+	errs = c.completeAndValidate(restore)
+	assert.Equal(t, []string{"No completed backups found for schedule"}, errs)
+	assert.Empty(t, restore.Spec.BackupName)
+
+	// multiple completed backups created from the schedule: use most recent
+	// (defined as last in alphabetical order)
+	require.NoError(t, sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(arktest.
+		NewTestBackup().
+		WithName("a").
+		WithLabel("ark-schedule", "schedule-1").
+		WithPhase(api.BackupPhaseCompleted).
+		Backup,
+	))
+	require.NoError(t, sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(arktest.
+		NewTestBackup().
+		WithName("b").
+		WithLabel("ark-schedule", "schedule-1").
+		WithPhase(api.BackupPhaseCompleted).
+		Backup,
+	))
+
+	errs = c.completeAndValidate(restore)
+	assert.Nil(t, errs)
+	assert.Equal(t, "b", restore.Spec.BackupName)
+}
+
+func TestBackupXorScheduleProvided(t *testing.T) {
+	r := &api.Restore{}
+	assert.False(t, backupXorScheduleProvided(r))
+
+	r.Spec.BackupName = "backup-1"
+	r.Spec.ScheduleName = "schedule-1"
+	assert.False(t, backupXorScheduleProvided(r))
+
+	r.Spec.BackupName = "backup-1"
+	r.Spec.ScheduleName = ""
+	assert.True(t, backupXorScheduleProvided(r))
+
+	r.Spec.BackupName = ""
+	r.Spec.ScheduleName = "schedule-1"
+	assert.True(t, backupXorScheduleProvided(r))
+
+}
+
+func TestMostRecentCompletedBackup(t *testing.T) {
+	backups := []*api.Backup{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "a",
+			},
+			Status: api.BackupStatus{
+				Phase: "",
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "b",
+			},
+			Status: api.BackupStatus{
+				Phase: api.BackupPhaseNew,
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "c",
+			},
+			Status: api.BackupStatus{
+				Phase: api.BackupPhaseInProgress,
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "d",
+			},
+			Status: api.BackupStatus{
+				Phase: api.BackupPhaseFailedValidation,
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "e",
+			},
+			Status: api.BackupStatus{
+				Phase: api.BackupPhaseFailed,
+			},
+		},
+	}
+
+	assert.Nil(t, mostRecentCompletedBackup(backups))
+
+	backups = append(backups, &api.Backup{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "a1",
+		},
+		Status: api.BackupStatus{
+			Phase: api.BackupPhaseCompleted,
+		},
+	})
+
+	expected := &api.Backup{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "b1",
+		},
+		Status: api.BackupStatus{
+			Phase: api.BackupPhaseCompleted,
+		},
+	}
+	backups = append(backups, expected)
+
+	assert.Equal(t, expected, mostRecentCompletedBackup(backups))
+}
+
 func NewRestore(ns, name, backup, includeNS, includeResource string, phase api.RestorePhase) *arktest.TestRestore {
 	restore := arktest.NewTestRestore(ns, name, phase).WithBackup(backup)