From 2457d8f11636634409663d0ceddeecd27a779925 Mon Sep 17 00:00:00 2001 From: Justin Nauman Date: Tue, 5 Sep 2017 17:06:15 -0500 Subject: [PATCH] Removing instead of deprecating Namespace attr - Per discussion, there is no reason to deal with the complexity of backwards compatibility with the Namespace attribute on the Restore domain. - Also noticed there was an error on the validation of the BackupController where the message would actually just be the index of the error and not the contents of the message itself. Signed-off-by: Justin Nauman --- pkg/apis/ark/v1/restore.go | 5 ----- pkg/controller/backup_controller.go | 4 ++-- pkg/controller/restore_controller.go | 21 +++++---------------- pkg/controller/restore_controller_test.go | 8 ++++---- pkg/util/test/test_restore.go | 6 ------ 5 files changed, 11 insertions(+), 33 deletions(-) diff --git a/pkg/apis/ark/v1/restore.go b/pkg/apis/ark/v1/restore.go index 2e4aa8fd1..43b649db9 100644 --- a/pkg/apis/ark/v1/restore.go +++ b/pkg/apis/ark/v1/restore.go @@ -24,11 +24,6 @@ type RestoreSpec struct { // from. BackupName string `json:"backupName"` - // NOTE: This is deprecated. IncludedNamespaces and ExcludedNamespaces - // should be used instead - // Namespaces is a slice of namespaces in the Ark backup to restore. - Namespaces []string `json:"namespaces"` - // IncludedNamespaces is a slice of namespace names to include objects // from. If empty, all namespaces are included. IncludedNamespaces []string `json:"includedNamespaces"` diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 3d7303202..1cfa36355 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -292,11 +292,11 @@ func cloneBackup(in interface{}) (*api.Backup, error) { func (controller *backupController) getValidationErrors(itm *api.Backup) []string { var validationErrors []string - for err := range collections.ValidateIncludesExcludes(itm.Spec.IncludedResources, itm.Spec.ExcludedResources) { + for _, err := range collections.ValidateIncludesExcludes(itm.Spec.IncludedResources, itm.Spec.ExcludedResources) { validationErrors = append(validationErrors, fmt.Sprintf("Invalid included/excluded resource lists: %v", err)) } - for err := range collections.ValidateIncludesExcludes(itm.Spec.IncludedNamespaces, itm.Spec.ExcludedNamespaces) { + for _, err := range collections.ValidateIncludesExcludes(itm.Spec.IncludedNamespaces, itm.Spec.ExcludedNamespaces) { validationErrors = append(validationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err)) } diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index 73f089db3..76a2d52df 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -220,22 +220,15 @@ func (controller *restoreController) processRestore(key string) error { return err } + if len(restore.Spec.IncludedNamespaces) == 0 { + restore.Spec.IncludedNamespaces = []string{"*"} + } + // validation if restore.Status.ValidationErrors = controller.getValidationErrors(restore); len(restore.Status.ValidationErrors) > 0 { restore.Status.Phase = api.RestorePhaseFailedValidation } else { restore.Status.Phase = api.RestorePhaseInProgress - - if len(restore.Spec.Namespaces) != 0 { - glog.V(4).Info("the restore.Spec.Namespaces field has been deprecated. Please use the IncludedNamespaces and ExcludedNamespaces feature instead") - - restore.Spec.IncludedNamespaces = restore.Spec.Namespaces - restore.Spec.Namespaces = nil - } - - if len(restore.Spec.IncludedNamespaces) == 0 { - restore.Spec.IncludedNamespaces = []string{"*"} - } } // update status @@ -286,11 +279,7 @@ func (controller *restoreController) getValidationErrors(itm *api.Restore) []str validationErrors = append(validationErrors, "BackupName must be non-empty and correspond to the name of a backup in object storage.") } - if len(itm.Spec.Namespaces) > 0 && len(itm.Spec.IncludedNamespaces) > 0 { - validationErrors = append(validationErrors, "Namespaces and IncludedNamespaces can not both be defined on the backup spec.") - } - - for err := range collections.ValidateIncludesExcludes(itm.Spec.IncludedNamespaces, itm.Spec.ExcludedNamespaces) { + for _, err := range collections.ValidateIncludesExcludes(itm.Spec.IncludedNamespaces, itm.Spec.ExcludedNamespaces) { validationErrors = append(validationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err)) } diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 648d90d54..73f12b0c9 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -73,16 +73,16 @@ func TestProcessRestore(t *testing.T) { expectedErr: false, }, { - name: "restore with both namespaces and includedNamespaces fails validation", - restore: NewTestRestore("foo", "bar", api.RestorePhaseNew).WithBackup("backup-1").WithNamespace("ns-1").WithIncludedNamespace("another-1").Restore, + name: "restore with both namespace in both includedNamespaces and excludedNamespaces fails validation", + restore: NewTestRestore("foo", "bar", api.RestorePhaseNew).WithBackup("backup-1").WithIncludedNamespace("another-1").WithExcludedNamespace("another-1").Restore, backup: NewTestBackup().WithName("backup-1").Backup, expectedErr: false, expectedRestoreUpdates: []*api.Restore{ NewTestRestore("foo", "bar", api.RestorePhaseFailedValidation). WithBackup("backup-1"). - WithNamespace("ns-1"). WithIncludedNamespace("another-1"). - WithValidationError("Namespace and ItemNamespaces can not both be defined on the backup spec.").Restore, + WithExcludedNamespace("another-1"). + WithValidationError("Invalid included/excluded namespace lists: excludes list cannot contain an item in the includes list: another-1").Restore, }, }, { diff --git a/pkg/util/test/test_restore.go b/pkg/util/test/test_restore.go index d560db15b..57ca9a094 100644 --- a/pkg/util/test/test_restore.go +++ b/pkg/util/test/test_restore.go @@ -45,12 +45,6 @@ func NewDefaultTestRestore() *TestRestore { return NewTestRestore(api.DefaultNamespace, "", api.RestorePhase("")) } - -func (r *TestRestore) WithNamespace(name string) *TestRestore { - r.Spec.Namespaces = append(r.Spec.Namespaces, name) - return r -} - func (r *TestRestore) WithIncludedNamespace(name string) *TestRestore { r.Spec.IncludedNamespaces = append(r.Spec.IncludedNamespaces, name) return r