store backups & restores in backups/, restores/ subdirs in obj storage

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

docs/storage-layout-reorg-v0.10.md

@@ -0,0 +1,112 @@
+# Object Storage Layout Changes in v0.10
+
+## Overview
+
+Ark v0.10 includes breaking changes to where data is stored in your object storage bucket. You'll need to run a [one-time migration procedure](#upgrading-to-v0.10)
+if you're upgrading from prior versions of Ark.
+
+## Details
+
+Prior to v0.10, Ark stored data in an object storage bucket using the following structure:
+
+```
+<your-bucket>/
+    backup-1/
+        ark-backup.json
+        backup-1.tar.gz
+        backup-1-logs.gz
+        restore-of-backup-1-logs.gz
+        restore-of-backup-1-results.gz
+    backup-2/
+        ark-backup.json
+        backup-2.tar.gz
+        backup-2-logs.gz
+        restore-of-backup-2-logs.gz
+        restore-of-backup-2-results.gz
+    ...
+```
+
+As of v0.10, we've reorganized this layout to provide a cleaner and more extensible directory structure. The new layout looks like:
+
+```
+<your-bucket>[/<your-prefix>]/
+    backups/
+        backup-1/
+            ark-backup.json
+            backup-1.tar.gz
+            backup-1-logs.gz
+        backup-2/
+            ark-backup.json
+            backup-2.tar.gz
+            backup-2-logs.gz
+        ...
+    restores/
+        restore-of-backup-1/
+            restore-of-backup-1-logs.gz
+            restore-of-backup-1-results.gz
+        restore-of-backup-2/
+            restore-of-backup-2-logs.gz
+            restore-of-backup-2-results.gz
+        ...
+    ...
+```
+
+## Upgrading to v0.10
+
+Before upgrading to v0.10, you'll need to run a one-time upgrade script to rearrange the contents of your existing Ark bucket(s) to be compatible with
+the new layout.
+
+Please note that the following scripts **will not** migrate existing restore logs/results into the new `restores/` subdirectory. This means that they
+will not be accessible using `ark restore describe` or `ark restore logs`. They *will* remain in the relevant backup's subdirectory so they are manually
+accessible, and will eventually be garbage-collected along with the backup. We've taken this approach in order to keep the migration scripts simple 
+and less error-prone.
+
+### rclone-Based Script
+
+This script uses [rclone][1], which you can download and install following the instructions [here][2]. 
+Please read through the script carefully before starting and execute it step-by-step.
+
+```bash
+ARK_BUCKET=<your-ark-bucket>
+ARK_TEMP_MIGRATION_BUCKET=<a-temp-bucket-for-migration>
+
+# 1. This is an interactive step that configures rclone to be 
+#    able to access your storage provider. Follow the instructions,
+#    and keep track of the "remote name" for the next step:
+rclone config
+
+# 2. Store the name of the rclone remote that you just set up
+#    in Step #1:
+RCLONE_REMOTE_NAME=<your-remote-name>
+
+# 3. Create a temporary bucket to be used as a backup of your 
+#    current Ark bucket's contents:
+rclone mkdir ${RCLONE_REMOTE_NAME}:${ARK_TEMP_MIGRATION_BUCKET}
+
+# 4. Do a full copy of the contents of your Ark bucket into the 
+#    temporary bucket:
+rclone copy ${RCLONE_REMOTE_NAME}:${ARK_BUCKET} ${RCLONE_REMOTE_NAME}:${ARK_TEMP_MIGRATION_BUCKET}
+
+# 5. Verify that the temporary bucket contains an exact copy of 
+#    your Ark bucket's contents. You should see a short block
+#    of output stating "0 differences found":
+rclone check ${RCLONE_REMOTE_NAME}:${ARK_BUCKET} ${RCLONE_REMOTE_NAME}:${ARK_TEMP_MIGRATION_BUCKET}
+
+# 6. Delete your Ark bucket's contents (this command does not 
+#    delete the bucket itself, only the contents):
+rclone delete ${RCLONE_REMOTE_NAME}:${ARK_BUCKET}
+
+# 7. Copy the contents of the temporary bucket into your Ark bucket, 
+#    under the 'backups/' directory/prefix:
+rclone copy ${RCLONE_REMOTE_NAME}:${ARK_TEMP_MIGRATION_BUCKET} ${RCLONE_REMOTE_NAME}:${ARK_BUCKET}/backups
+
+# 8. Verify that the 'backups/' directory in your Ark bucket now
+#    contains an exact copy of the temporary bucket's contents:
+rclone check ${RCLONE_REMOTE_NAME}:${ARK_BUCKET}/backups ${RCLONE_REMOTE_NAME}:${ARK_TEMP_MIGRATION_BUCKET}
+
+# 9. Once you've confirmed that Ark v0.10 works with your revised Ark
+#    bucket, you can delete the temporary migration bucket.
+```
+
+[1]: https://rclone.org/
+[2]: https://rclone.org/downloads/

pkg/cmd/server/server.go

@@ -62,6 +62,7 @@ import (
 	clientset "github.com/heptio/ark/pkg/generated/clientset/versioned"
 	informers "github.com/heptio/ark/pkg/generated/informers/externalversions"
 	"github.com/heptio/ark/pkg/metrics"
+	"github.com/heptio/ark/pkg/persistence"
 	"github.com/heptio/ark/pkg/plugin"
 	"github.com/heptio/ark/pkg/podexec"
 	"github.com/heptio/ark/pkg/restic"
@@ -252,11 +253,14 @@ func (s *server) run() error {
 		return err
 	}
 
-	// check to ensure all Ark CRDs exist
 	if err := s.arkResourcesExist(); err != nil {
 		return err
 	}
 
+	if err := s.validateBackupStorageLocations(); err != nil {
+		return err
+	}
+
 	originalConfig, err := s.loadConfig()
 	if err != nil {
 		return err
@@ -391,6 +395,39 @@ func (s *server) arkResourcesExist() error {
 	return nil
 }
 
+// validateBackupStorageLocations checks to ensure all backup storage locations exist
+// and have a compatible layout, and returns an error if not.
+func (s *server) validateBackupStorageLocations() error {
+	s.logger.Info("Checking that all backup storage locations are valid")
+
+	locations, err := s.arkClient.ArkV1().BackupStorageLocations(s.namespace).List(metav1.ListOptions{})
+	if err != nil {
+		return errors.WithStack(err)
+	}
+
+	var invalid []string
+	for _, location := range locations.Items {
+		backupStore, err := persistence.NewObjectBackupStore(&location, s.pluginManager, s.logger)
+		if err != nil {
+			invalid = append(invalid, errors.Wrapf(err, "error getting backup store for location %q", location.Name).Error())
+			continue
+		}
+
+		if err := backupStore.IsValid(); err != nil {
+			invalid = append(invalid, errors.Wrapf(err,
+				"backup store for location %q is invalid (if upgrading from a pre-v0.10 version of Ark, please refer to https://heptio.github.io/ark/v0.10.0/storage-layout-reorg-v0.10 for instructions)",
+				location.Name,
+			).Error())
+		}
+	}
+
+	if len(invalid) > 0 {
+		return errors.Errorf("some backup storage locations are invalid: %s", strings.Join(invalid, "; "))
+	}
+
+	return nil
+}
+
 func (s *server) loadConfig() (*api.Config, error) {
 	s.logger.Info("Retrieving Ark configuration")
 	var (

pkg/controller/backup_deletion_controller.go

@@ -261,7 +261,15 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
 	}
 
 	log.Info("Removing backup from backup storage")
-	if err := c.deleteBackupFromStorage(backup, log); err != nil {
+	pluginManager := c.newPluginManager(log)
+	defer pluginManager.CleanupClients()
+
+	backupStore, backupStoreErr := c.backupStoreForBackup(backup, pluginManager, log)
+	if backupStoreErr != nil {
+		errs = append(errs, backupStoreErr.Error())
+	}
+
+	if err := backupStore.DeleteBackup(backup.Name); err != nil {
 		errs = append(errs, err.Error())
 	}
 
@@ -276,6 +284,13 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
 
 			restoreLog := log.WithField("restore", kube.NamespaceAndName(restore))
 
+			restoreLog.Info("Deleting restore log/results from backup storage")
+			if err := backupStore.DeleteRestore(restore.Name); err != nil {
+				errs = append(errs, err.Error())
+				// if we couldn't delete the restore files, don't delete the API object
+				continue
+			}
+
 			restoreLog.Info("Deleting restore referencing backup")
 			if err := c.restoreClient.Restores(restore.Namespace).Delete(restore.Name, &metav1.DeleteOptions{}); err != nil {
 				errs = append(errs, errors.Wrapf(err, "error deleting restore %s", kube.NamespaceAndName(restore)).Error())
@@ -313,27 +328,19 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
 	return nil
 }
 
-func (c *backupDeletionController) deleteBackupFromStorage(backup *v1.Backup, log logrus.FieldLogger) error {
-	pluginManager := c.newPluginManager(log)
-	defer pluginManager.CleanupClients()
-
+func (c *backupDeletionController) backupStoreForBackup(backup *v1.Backup, pluginManager plugin.Manager, log logrus.FieldLogger) (persistence.BackupStore, error) {
 	backupLocation, err := c.backupLocationLister.BackupStorageLocations(backup.Namespace).Get(backup.Spec.StorageLocation)
 	if err != nil {
-		return errors.WithStack(err)
+		return nil, errors.WithStack(err)
 	}
 
 	backupStore, err := c.newBackupStore(backupLocation, pluginManager, log)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	if err := backupStore.DeleteBackup(backup.Name); err != nil {
-		return errors.Wrap(err, "error deleting backup from backup storage")
-	}
-
-	return nil
+	return backupStore, nil
 }
-
 func (c *backupDeletionController) deleteExistingDeletionRequests(req *v1.DeleteBackupRequest, log logrus.FieldLogger) []error {
 	log.Info("Removing existing deletion requests for backup")
 	selector := labels.SelectorFromSet(labels.Set(map[string]string{

pkg/controller/backup_deletion_controller_test.go

@@ -410,6 +410,8 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) {
 		})
 
 		td.backupStore.On("DeleteBackup", td.req.Spec.BackupName).Return(nil)
+		td.backupStore.On("DeleteRestore", "restore-1").Return(nil)
+		td.backupStore.On("DeleteRestore", "restore-2").Return(nil)
 
 		err := td.controller.processRequest(td.req)
 		require.NoError(t, err)

pkg/controller/download_request_controller.go

@@ -180,7 +180,7 @@ func (c *downloadRequestController) generatePreSignedURL(downloadRequest *v1.Dow
 		return errors.WithStack(err)
 	}
 
-	if update.Status.DownloadURL, err = backupStore.GetDownloadURL(backupName, downloadRequest.Spec.Target); err != nil {
+	if update.Status.DownloadURL, err = backupStore.GetDownloadURL(downloadRequest.Spec.Target); err != nil {
 		return err
 	}
 

pkg/controller/download_request_controller_test.go

@@ -274,7 +274,7 @@ func TestProcessDownloadRequest(t *testing.T) {
 			}
 
 			if tc.expectGetsURL {
-				harness.backupStore.On("GetDownloadURL", tc.backup.Name, tc.downloadRequest.Spec.Target).Return("a-url", nil)
+				harness.backupStore.On("GetDownloadURL", tc.downloadRequest.Spec.Target).Return("a-url", nil)
 			}
 
 			// exercise method under test