Add back code and tests

Signed-off-by: Carlisia <carlisiac@vmware.com>

pkg/controller/backup_controller.go

@@ -158,19 +158,22 @@ func (c *backupController) processBackup(key string) error {
 	log.Debug("Running processBackup")
 	ns, name, err := cache.SplitMetaNamespaceKey(key)
 	if err != nil {
-		return errors.Wrap(err, "error splitting queue key")
+		log.WithError(err).Errorf("error splitting key")
+		return nil
 	}
 
 	log.Debug("Getting backup")
 	original, err := c.lister.Backups(ns).Get(name)
 	if apierrors.IsNotFound(err) {
-		log.Debug("backup not found")
+		log.Debugf("backup %s not found", name)
 		return nil
 	}
 	if err != nil {
 		return errors.Wrap(err, "error getting backup")
 	}
 
+	fmt.Println("original name is.....---- ", original.Name)
+
 	// Double-check we have the correct phase. In the unlikely event that multiple controller
 	// instances are running, it's possible for controller A to succeed in changing the phase to
 	// InProgress, while controller B's attempt to patch the phase fails. When controller B
@@ -196,6 +199,9 @@ func (c *backupController) processBackup(key string) error {
 		request.Status.StartTimestamp.Time = c.clock.Now()
 	}
 
+	fmt.Println("request.Backup.Name name is.....---- ", request.Backup.Name)
+	fmt.Println("original name is.....---- ", original.Name)
+
 	// update status
 	updatedBackup, err := patchBackup(original, request.Backup, c.client)
 	if err != nil {
@@ -284,11 +290,12 @@ func (c *backupController) prepareBackupRequest(backup *velerov1api.Backup) *pkg
 	}
 	request.Labels[velerov1api.StorageLocationLabel] = request.Spec.StorageLocation
 
-	// validate the included/excluded resources and namespaces
+	// validate the included/excluded resources
 	for _, err := range collections.ValidateIncludesExcludes(request.Spec.IncludedResources, request.Spec.ExcludedResources) {
 		request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded resource lists: %v", err))
 	}
 
+	// validate the included/excluded namespaces
 	for _, err := range collections.ValidateIncludesExcludes(request.Spec.IncludedNamespaces, request.Spec.ExcludedNamespaces) {
 		request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err))
 	}
@@ -410,6 +417,8 @@ func (c *backupController) validateAndGetSnapshotLocations(backup *velerov1api.B
 }
 
 func (c *backupController) runBackup(backup *pkgbackup.Request) error {
+	fmt.Println("runbackup name is.....---- ", backup.Name)
+
 	log := c.logger.WithField("backup", kubeutil.NamespaceAndName(backup))
 	log.Info("Starting backup")
 
@@ -451,6 +460,12 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 	}
 
 	var errs []error
+	errs = append(errs, validateUniqueness(backupStore, backup.StorageLocation.Spec.StorageType.ObjectStorage.Bucket, backup.Name)...)
+	if len(errs) > 0 {
+		backup.Status.Phase = velerov1api.BackupPhaseFailed
+		backup.Status.CompletionTimestamp.Time = c.clock.Now()
+		return kerrors.NewAggregate(errs)
+	}
 
 	// Do the actual backup
 	if err := c.backupper.Backup(log, backup, backupFile, actions, pluginManager); err != nil {
@@ -483,6 +498,18 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 	return kerrors.NewAggregate(errs)
 }
 
+func validateUniqueness(backupStore persistence.BackupStore, bucket, name string) []error {
+	var errs []error
+	exists, err := backupStore.BackupExists(bucket, name)
+	if err != nil {
+		errs = append(errs, errors.Errorf("Error checking if backup already exists in object storage: %v", err))
+	}
+	if exists {
+		errs = append(errs, errors.Errorf("Backup already exists in object storage"))
+	}
+	return errs
+}
+
 func recordBackupMetrics(backup *velerov1api.Backup, backupFile *os.File, serverMetrics *metrics.ServerMetrics) error {
 	backupScheduleName := backup.GetLabels()[velerov1api.ScheduleNameLabel]
 

pkg/controller/backup_controller_test.go

@@ -56,23 +56,24 @@ func (b *fakeBackupper) Backup(logger logrus.FieldLogger, backup *pkgbackup.Requ
 	return args.Error(0)
 }
 
-func TestProcessBackupNonProcessedItems(t *testing.T) {
+func TestProcessBackupProcessing(t *testing.T) {
 	tests := []struct {
 		name        string
 		key         string
 		backup      *v1.Backup
 		expectedErr string
 	}{
+		// processed successfully
 		{
-			name:        "bad key returns error",
-			key:         "bad/key/here",
-			expectedErr: "error splitting queue key: unexpected key format: \"bad/key/here\"",
+			name: "bad key does not return error",
+			key:  "bad/key/here",
 		},
 		{
-			name:        "backup not found in lister returns error",
-			key:         "nonexistent/backup",
-			expectedErr: "error getting backup: backup.velero.io \"backup\" not found",
+			name: "backup not found in lister does not return error",
+			key:  "nonexistent/backup",
 		},
+
+		// skipped
 		{
 			name:   "FailedValidation backup is not processed",
 			key:    "velero/backup-1",
@@ -255,18 +256,21 @@ func TestDefaultBackupTTL(t *testing.T) {
 }
 
 func TestProcessBackupCompletions(t *testing.T) {
-	defaultBackupLocation := velerotest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation
+	defaultBackupLocation := velerotest.NewTestBackupStorageLocation().WithName("loc-1").WithObjectStorage("store-1").BackupStorageLocation
 
 	now, err := time.Parse(time.RFC1123Z, time.RFC1123Z)
 	require.NoError(t, err)
 	now = now.Local()
 
 	tests := []struct {
-		name           string
-		backup         *v1.Backup
-		backupLocation *v1.BackupStorageLocation
-		expectedResult *v1.Backup
+		name                string
+		backup              *v1.Backup
+		backupLocation      *v1.BackupStorageLocation
+		expectedResult      *v1.Backup
+		backupExists        bool
+		existenceCheckError error
 	}{
+		// Completed
 		{
 			name:           "backup with no backup location gets the default",
 			backup:         velerotest.NewTestBackup().WithName("backup-1").Backup,
@@ -294,7 +298,7 @@ func TestProcessBackupCompletions(t *testing.T) {
 		{
 			name:           "backup with a specific backup location keeps it",
 			backup:         velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("alt-loc").Backup,
-			backupLocation: velerotest.NewTestBackupStorageLocation().WithName("alt-loc").BackupStorageLocation,
+			backupLocation: velerotest.NewTestBackupStorageLocation().WithName("alt-loc").WithObjectStorage("store-1").BackupStorageLocation,
 			expectedResult: &v1.Backup{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: v1.DefaultNamespace,
@@ -340,6 +344,83 @@ func TestProcessBackupCompletions(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:           "backup with existing backup will fail",
+			backupExists:   false,
+			backup:         velerotest.NewTestBackup().WithName("backup-1").Backup,
+			backupLocation: defaultBackupLocation,
+			expectedResult: &v1.Backup{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: v1.DefaultNamespace,
+					Name:      "backup-1",
+					Labels: map[string]string{
+						"velero.io/storage-location": "loc-1",
+					},
+				},
+				Spec: v1.BackupSpec{
+					StorageLocation: defaultBackupLocation.Name,
+				},
+				Status: v1.BackupStatus{
+					Phase:               v1.BackupPhaseCompleted,
+					Version:             1,
+					StartTimestamp:      metav1.NewTime(now),
+					CompletionTimestamp: metav1.NewTime(now),
+					Expiration:          metav1.NewTime(now),
+				},
+			},
+		},
+
+		// Failed
+		{
+			name:           "backup with existing backup will fail",
+			backupExists:   true,
+			backup:         velerotest.NewTestBackup().WithName("backup-1").Backup,
+			backupLocation: defaultBackupLocation,
+			expectedResult: &v1.Backup{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: v1.DefaultNamespace,
+					Name:      "backup-1",
+					Labels: map[string]string{
+						"velero.io/storage-location": "loc-1",
+					},
+				},
+				Spec: v1.BackupSpec{
+					StorageLocation: defaultBackupLocation.Name,
+				},
+				Status: v1.BackupStatus{
+					Phase:               v1.BackupPhaseFailed,
+					Version:             1,
+					StartTimestamp:      metav1.NewTime(now),
+					CompletionTimestamp: metav1.NewTime(now),
+					Expiration:          metav1.NewTime(now),
+				},
+			},
+		},
+		{
+			name:                "error when checking if backup exists will cause backup to fail",
+			backup:              velerotest.NewTestBackup().WithName("backup-1").Backup,
+			existenceCheckError: errors.New("Backup already exists in object storage"),
+			backupLocation:      defaultBackupLocation,
+			expectedResult: &v1.Backup{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: v1.DefaultNamespace,
+					Name:      "backup-1",
+					Labels: map[string]string{
+						"velero.io/storage-location": "loc-1",
+					},
+				},
+				Spec: v1.BackupSpec{
+					StorageLocation: defaultBackupLocation.Name,
+				},
+				Status: v1.BackupStatus{
+					Phase:               v1.BackupPhaseFailed,
+					Version:             1,
+					StartTimestamp:      metav1.NewTime(now),
+					CompletionTimestamp: metav1.NewTime(now),
+					Expiration:          metav1.NewTime(now),
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -380,6 +461,7 @@ func TestProcessBackupCompletions(t *testing.T) {
 			completionTimestampIsPresent := func(buf *bytes.Buffer) bool {
 				return strings.Contains(buf.String(), `"completionTimestamp": "2006-01-02T22:04:05Z"`)
 			}
+			backupStore.On("BackupExists", test.backupLocation.Spec.StorageType.ObjectStorage.Bucket, test.backup.Name).Return(test.backupExists, test.existenceCheckError)
 			backupStore.On("PutBackup", test.backup.Name, mock.MatchedBy(completionTimestampIsPresent), mock.Anything, mock.Anything, mock.Anything).Return(nil)
 
 			// add the test's backup to the informer/lister store
@@ -406,6 +488,8 @@ func TestProcessBackupCompletions(t *testing.T) {
 			res, err := clientset.VeleroV1().Backups(test.backup.Namespace).Get(test.backup.Name, metav1.GetOptions{})
 			require.NoError(t, err)
 
+			// failed tests for failed backup should have a phase of failed
+
 			assert.Equal(t, test.expectedResult, res)
 		})
 	}