From 6750836d6907c09565aa6e592582211df60c5c36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wenkai=20Yin=28=E5=B0=B9=E6=96=87=E5=BC=80=29?= Date: Mon, 31 Oct 2022 16:28:06 +0800 Subject: [PATCH] Enhance the restore priorities list to support specifying the low prioritized resources that need to be restored in the last MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhance the restore priorities list to support specifying the low prioritized resources that need to be r estored in the last Signed-off-by: Wenkai Yin(尹文开) --- changelogs/unreleased/5529-ywk253100 | 1 + pkg/cmd/server/server.go | 57 ++++++++------ pkg/restore/priority.go | 92 ++++++++++++++++++++++ pkg/restore/priority_test.go | 110 +++++++++++++++++++++++++++ pkg/restore/restore.go | 40 ++++++---- pkg/restore/restore_test.go | 33 +++++--- 6 files changed, 286 insertions(+), 47 deletions(-) create mode 100644 changelogs/unreleased/5529-ywk253100 create mode 100644 pkg/restore/priority.go create mode 100644 pkg/restore/priority_test.go diff --git a/changelogs/unreleased/5529-ywk253100 b/changelogs/unreleased/5529-ywk253100 new file mode 100644 index 0000000000..186672ea0c --- /dev/null +++ b/changelogs/unreleased/5529-ywk253100 @@ -0,0 +1 @@ +Enhance the restore priorities list to support specifying the low prioritized resources that need to be restored in the last \ No newline at end of file diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 3086977456..86f41ac791 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -114,7 +114,7 @@ type serverConfig struct { pluginDir, metricsAddress, defaultBackupLocation string backupSyncPeriod, podVolumeOperationTimeout, resourceTerminatingTimeout time.Duration defaultBackupTTL, storeValidationFrequency, defaultCSISnapshotTimeout time.Duration - restoreResourcePriorities []string + restoreResourcePriorities restore.Priorities defaultVolumeSnapshotLocations map[string]string restoreOnly bool disabledControllers []string @@ -208,7 +208,7 @@ func NewCommand(f client.Factory) *cobra.Command { command.Flags().DurationVar(&config.podVolumeOperationTimeout, "restic-timeout", config.podVolumeOperationTimeout, "How long backups/restores of pod volumes should be allowed to run before timing out.") command.Flags().BoolVar(&config.restoreOnly, "restore-only", config.restoreOnly, "Run in a mode where only restores are allowed; backups, schedules, and garbage-collection are all disabled. DEPRECATED: this flag will be removed in v2.0. Use read-only backup storage locations instead.") command.Flags().StringSliceVar(&config.disabledControllers, "disable-controllers", config.disabledControllers, fmt.Sprintf("List of controllers to disable on startup. Valid values are %s", strings.Join(controller.DisableableControllers, ","))) - command.Flags().StringSliceVar(&config.restoreResourcePriorities, "restore-resource-priorities", config.restoreResourcePriorities, "Desired order of resource restores; any resource not in the list will be restored alphabetically after the prioritized resources.") + command.Flags().Var(&config.restoreResourcePriorities, "restore-resource-priorities", "Desired order of resource restores, the priority list contains two parts which are split by \"-\" element. The resources before \"-\" element are restored first as high priorities, the resources after \"-\" element are restored last as low priorities, and any resource not in the list will be restored alphabetically between the high and low priorities.") command.Flags().StringVar(&config.defaultBackupLocation, "default-backup-storage-location", config.defaultBackupLocation, "Name of the default backup storage location. DEPRECATED: this flag will be removed in v2.0. Use \"velero backup-location set --default\" instead.") command.Flags().DurationVar(&config.storeValidationFrequency, "store-validation-frequency", config.storeValidationFrequency, "How often to verify if the storage is valid. Optional. Set this to `0s` to disable sync. Default 1 minute.") command.Flags().Var(&volumeSnapshotLocations, "default-volume-snapshot-locations", "List of unique volume providers and default volume snapshot location (provider1:location-01,provider2:location-02,...)") @@ -467,6 +467,7 @@ func (s *server) veleroResourcesExist() error { return nil } +// High priorities: // - Custom Resource Definitions come before Custom Resource so that they can be // restored with their corresponding CRD. // - Namespaces go second because all namespaced resources depend on them. @@ -489,28 +490,36 @@ func (s *server) veleroResourcesExist() error { // - CAPI Clusters come before ClusterResourceSets because failing to do so means the CAPI controller-manager will panic. // Both Clusters and ClusterResourceSets need to come before ClusterResourceSetBinding in order to properly restore workload clusters. // See https://github.com/kubernetes-sigs/cluster-api/issues/4105 -var defaultRestorePriorities = []string{ - "customresourcedefinitions", - "namespaces", - "storageclasses", - "volumesnapshotclass.snapshot.storage.k8s.io", - "volumesnapshotcontents.snapshot.storage.k8s.io", - "volumesnapshots.snapshot.storage.k8s.io", - "persistentvolumes", - "persistentvolumeclaims", - "secrets", - "configmaps", - "serviceaccounts", - "limitranges", - "pods", - // we fully qualify replicasets.apps because prior to Kubernetes 1.16, replicasets also - // existed in the extensions API group, but we back up replicasets from "apps" so we want - // to ensure that we prioritize restoring from "apps" too, since this is how they're stored - // in the backup. - "replicasets.apps", - "clusterclasses.cluster.x-k8s.io", - "clusters.cluster.x-k8s.io", - "clusterresourcesets.addons.cluster.x-k8s.io", +// +// Low priorities: +// - Tanzu ClusterBootstrap go last as it can reference any other kind of resources +var defaultRestorePriorities = restore.Priorities{ + HighPriorities: []string{ + "customresourcedefinitions", + "namespaces", + "storageclasses", + "volumesnapshotclass.snapshot.storage.k8s.io", + "volumesnapshotcontents.snapshot.storage.k8s.io", + "volumesnapshots.snapshot.storage.k8s.io", + "persistentvolumes", + "persistentvolumeclaims", + "secrets", + "configmaps", + "serviceaccounts", + "limitranges", + "pods", + // we fully qualify replicasets.apps because prior to Kubernetes 1.16, replicasets also + // existed in the extensions API group, but we back up replicasets from "apps" so we want + // to ensure that we prioritize restoring from "apps" too, since this is how they're stored + // in the backup. + "replicasets.apps", + "clusterclasses.cluster.x-k8s.io", + "clusters.cluster.x-k8s.io", + "clusterresourcesets.addons.cluster.x-k8s.io", + }, + LowPriorities: []string{ + "clusterbootstraps.run.tanzu.vmware.com", + }, } func (s *server) initRestic() error { diff --git a/pkg/restore/priority.go b/pkg/restore/priority.go new file mode 100644 index 0000000000..c544532c74 --- /dev/null +++ b/pkg/restore/priority.go @@ -0,0 +1,92 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restore + +import ( + "fmt" + "strings" +) + +const ( + prioritySeparator = "-" +) + +// Priorities defines the desired order of resource operations: +// Resources in the HighPriorities list will be handled first +// Resources in the LowPriorities list will be handled last +// Other resources will be handled alphabetically after the high prioritized resources and before the low prioritized resources +type Priorities struct { + HighPriorities []string + LowPriorities []string +} + +// String returns a string representation of Priority. +func (p *Priorities) String() string { + priorities := p.HighPriorities + if len(p.LowPriorities) > 0 { + priorities = append(priorities, prioritySeparator) + priorities = append(priorities, p.LowPriorities...) + } + return strings.Join(priorities, ",") +} + +// Set parses the provided string to the priority object +func (p *Priorities) Set(s string) error { + if len(s) == 0 { + return nil + } + strs := strings.Split(s, ",") + separatorIndex := -1 + for i, str := range strs { + if str == prioritySeparator { + if separatorIndex > -1 { + return fmt.Errorf("multiple priority separator %q found", prioritySeparator) + } + separatorIndex = i + } + } + // has no separator + if separatorIndex == -1 { + p.HighPriorities = strs + return nil + } + // start with separator + if separatorIndex == 0 { + // contain only separator + if len(strs) == 1 { + return nil + } + p.LowPriorities = strs[1:] + return nil + } + // end with separator + if separatorIndex == len(strs)-1 { + p.HighPriorities = strs[:len(strs)-1] + return nil + } + + // separator in the middle + p.HighPriorities = strs[:separatorIndex] + p.LowPriorities = strs[separatorIndex+1:] + + return nil +} + +// Type specifies the flag type +func (p *Priorities) Type() string { + return "stringArray" +} diff --git a/pkg/restore/priority_test.go b/pkg/restore/priority_test.go new file mode 100644 index 0000000000..4336c6bfac --- /dev/null +++ b/pkg/restore/priority_test.go @@ -0,0 +1,110 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restore + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestStringOfPriorities(t *testing.T) { + priority := Priorities{ + HighPriorities: []string{"high"}, + } + assert.Equal(t, "high", priority.String()) + + priority = Priorities{ + HighPriorities: []string{"high"}, + LowPriorities: []string{"low"}, + } + assert.Equal(t, "high,-,low", priority.String()) +} + +func TestSetOfPriority(t *testing.T) { + cases := []struct { + name string + input string + priorities Priorities + hasErr bool + }{ + { + name: "empty input", + input: "", + priorities: Priorities{}, + hasErr: false, + }, + { + name: "only high priorities", + input: "p0", + priorities: Priorities{ + HighPriorities: []string{"p0"}, + }, + hasErr: false, + }, + { + name: "only low priorities", + input: "-,p9", + priorities: Priorities{ + LowPriorities: []string{"p9"}, + }, + hasErr: false, + }, + { + name: "only separator", + input: "-", + priorities: Priorities{}, + hasErr: false, + }, + { + name: "multiple separators", + input: "-,-", + priorities: Priorities{}, + hasErr: true, + }, + { + name: "contain both high and low priorities", + input: "p0,p1,p2,-,p9", + priorities: Priorities{ + HighPriorities: []string{"p0", "p1", "p2"}, + LowPriorities: []string{"p9"}, + }, + hasErr: false, + }, + { + name: "end with separator", + input: "p0,-", + priorities: Priorities{ + HighPriorities: []string{"p0"}, + }, + hasErr: false, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + p := Priorities{} + err := p.Set(c.input) + if c.hasErr { + require.NotNil(t, err) + } else { + require.Nil(t, err) + } + assert.Equal(t, c.priorities, p) + }) + } +} diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 2b252c24d1..e236552182 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -107,7 +107,7 @@ type kubernetesRestorer struct { resticRestorerFactory restic.RestorerFactory resticTimeout time.Duration resourceTerminatingTimeout time.Duration - resourcePriorities []string + resourcePriorities Priorities fileSystem filesystem.Interface pvRenamer func(string) (string, error) logger logrus.FieldLogger @@ -120,7 +120,7 @@ func NewKubernetesRestorer( restoreClient velerov1client.RestoresGetter, discoveryHelper discovery.Helper, dynamicFactory client.DynamicFactory, - resourcePriorities []string, + resourcePriorities Priorities, namespaceClient corev1.NamespaceInterface, resticRestorerFactory restic.RestorerFactory, resticTimeout time.Duration, @@ -351,7 +351,7 @@ type restoreContext struct { renamedPVs map[string]string pvRenamer func(string) (string, error) discoveryHelper discovery.Helper - resourcePriorities []string + resourcePriorities Priorities hooksWaitGroup sync.WaitGroup hooksErrs chan error resourceRestoreHooks []hook.ResourceRestoreHook @@ -367,19 +367,31 @@ type resourceClientKey struct { // getOrderedResources returns an ordered list of resource identifiers to restore, // based on the provided resource priorities and backup contents. The returned list -// begins with all of the prioritized resources (in order), and appends to that -// an alphabetized list of all resources in the backup. -func getOrderedResources(resourcePriorities []string, backupResources map[string]*archive.ResourceItems) []string { - // alphabetize resources in the backup - orderedBackupResources := make([]string, 0, len(backupResources)) +// begins with all of the high prioritized resources (in order), ends with all of +// the low prioritized resources(in order), and an alphabetized list of resources +// in the backup(pick out the prioritized resources) is put in the middle. +func getOrderedResources(resourcePriorities Priorities, backupResources map[string]*archive.ResourceItems) []string { + priorities := map[string]struct{}{} + for _, priority := range resourcePriorities.HighPriorities { + priorities[priority] = struct{}{} + } + for _, priority := range resourcePriorities.LowPriorities { + priorities[priority] = struct{}{} + } + + // pick the prioritized resources out + var orderedBackupResources []string for resource := range backupResources { + if _, exist := priorities[resource]; exist { + continue + } orderedBackupResources = append(orderedBackupResources, resource) } + // alphabetize resources in the backup sort.Strings(orderedBackupResources) - // Main list: everything in resource priorities, followed by what's in the - // backup (alphabetized). - return append(resourcePriorities, orderedBackupResources...) + list := append(resourcePriorities.HighPriorities, orderedBackupResources...) + return append(list, resourcePriorities.LowPriorities...) } type progressUpdate struct { @@ -466,7 +478,7 @@ func (ctx *restoreContext) execute() (Result, Result) { backupResources, make([]restoreableResource, 0), sets.NewString(), - []string{"customresourcedefinitions"}, + Priorities{HighPriorities: []string{"customresourcedefinitions"}}, false, ) warnings.Merge(&w) @@ -1790,7 +1802,7 @@ func (ctx *restoreContext) getOrderedResourceCollection( backupResources map[string]*archive.ResourceItems, restoreResourceCollection []restoreableResource, processedResources sets.String, - resourcePriorities []string, + resourcePriorities Priorities, includeAllResources bool, ) ([]restoreableResource, sets.String, Result, Result) { var warnings, errs Result @@ -1812,7 +1824,7 @@ func (ctx *restoreContext) getOrderedResourceCollection( if includeAllResources { resourceList = getOrderedResources(resourcePriorities, backupResources) } else { - resourceList = resourcePriorities + resourceList = resourcePriorities.HighPriorities } for _, resource := range resourceList { // try to resolve the resource via discovery to a complete group/version/resource diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index f2d5d2ecd2..029ab6f7f1 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -679,7 +679,7 @@ func TestRestoreResourcePriorities(t *testing.T) { backup *velerov1api.Backup apiResources []*test.APIResource tarball io.Reader - resourcePriorities []string + resourcePriorities Priorities }{ { name: "resources are restored according to the specified resource priorities", @@ -713,7 +713,10 @@ func TestRestoreResourcePriorities(t *testing.T) { test.Deployments(), test.ServiceAccounts(), }, - resourcePriorities: []string{"persistentvolumes", "serviceaccounts", "pods", "deployments.apps"}, + resourcePriorities: Priorities{ + HighPriorities: []string{"persistentvolumes", "persistentvolumeclaims", "serviceaccounts"}, + LowPriorities: []string{"deployments.apps"}, + }, }, } @@ -745,7 +748,7 @@ func TestRestoreResourcePriorities(t *testing.T) { ) assertEmptyResults(t, warnings, errs) - assertResourceCreationOrder(t, tc.resourcePriorities, recorder.resources) + assertResourceCreationOrder(t, []string{"persistentvolumes", "persistentvolumeclaims", "serviceaccounts", "pods", "deployments.apps"}, recorder.resources) } } @@ -2622,7 +2625,7 @@ func TestRestorePersistentVolumes(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { h := newHarness(t) - h.restorer.resourcePriorities = []string{"persistentvolumes", "persistentvolumeclaims"} + h.restorer.resourcePriorities = Priorities{HighPriorities: []string{"persistentvolumes", "persistentvolumeclaims"}} h.restorer.pvRenamer = func(oldName string) (string, error) { renamed := "renamed-" + oldName return renamed, nil @@ -2940,19 +2943,19 @@ func TestIsCompleted(t *testing.T) { func Test_getOrderedResources(t *testing.T) { tests := []struct { name string - resourcePriorities []string + resourcePriorities Priorities backupResources map[string]*archive.ResourceItems want []string }{ { name: "when only priorities are specified, they're returned in order", - resourcePriorities: []string{"prio-3", "prio-2", "prio-1"}, + resourcePriorities: Priorities{HighPriorities: []string{"prio-3", "prio-2", "prio-1"}}, backupResources: nil, want: []string{"prio-3", "prio-2", "prio-1"}, }, { name: "when only backup resources are specified, they're returned in alphabetical order", - resourcePriorities: nil, + resourcePriorities: Priorities{}, backupResources: map[string]*archive.ResourceItems{ "backup-resource-3": nil, "backup-resource-2": nil, @@ -2962,14 +2965,26 @@ func Test_getOrderedResources(t *testing.T) { }, { name: "when priorities and backup resources are specified, they're returned in the correct order", - resourcePriorities: []string{"prio-3", "prio-2", "prio-1"}, + resourcePriorities: Priorities{HighPriorities: []string{"prio-3", "prio-2", "prio-1"}}, + backupResources: map[string]*archive.ResourceItems{ + "prio-3": nil, + "backup-resource-3": nil, + "backup-resource-2": nil, + "backup-resource-1": nil, + }, + want: []string{"prio-3", "prio-2", "prio-1", "backup-resource-1", "backup-resource-2", "backup-resource-3"}, + }, + { + name: "when priorities and backup resources are specified, they're returned in the correct order", + resourcePriorities: Priorities{HighPriorities: []string{"prio-3", "prio-2", "prio-1"}, LowPriorities: []string{"prio-0"}}, backupResources: map[string]*archive.ResourceItems{ "prio-3": nil, + "prio-0": nil, "backup-resource-3": nil, "backup-resource-2": nil, "backup-resource-1": nil, }, - want: []string{"prio-3", "prio-2", "prio-1", "backup-resource-1", "backup-resource-2", "backup-resource-3", "prio-3"}, + want: []string{"prio-3", "prio-2", "prio-1", "backup-resource-1", "backup-resource-2", "backup-resource-3", "prio-0"}, }, }