Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add disabled PV re-provisioning by StorageClasses option on restore #8287

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/8287-clcondorcet
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add disabled PV re-provisioning by StorageClasses option on restore
9 changes: 9 additions & 0 deletions config/crd/v1/bases/velero.io_restores.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ spec:
BackupName is the unique name of the Velero backup to restore
from.
type: string
disabledPVReprovisioningStorageClasses:
description: |-
DisabledPVReprovisioningStorageClasses is a slice of StorageClasses names.
PV without snaptshot and having one of these StorageClass will not be
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PV without snaptshot and having one of these StorageClass will not be
PV without snapshot and having one of these StorageClass will not be

re-provisionned (even when ReclaimPolicy is Delete).
items:
type: string
nullable: true
type: array
excludedNamespaces:
description: |-
ExcludedNamespaces contains a list of namespaces that are not
Expand Down
2 changes: 1 addition & 1 deletion config/crd/v1/crds/crds.go

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions pkg/apis/velero/v1/restore_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,13 @@ type RestoreSpec struct {
// +optional
// +nullable
UploaderConfig *UploaderConfigForRestore `json:"uploaderConfig,omitempty"`

// DisabledPVReprovisioningStorageClasses is a slice of StorageClasses names.
// PV without snaptshot and having one of these StorageClass will not be
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// PV without snaptshot and having one of these StorageClass will not be
// PV without snapshot and having one of these StorageClass will not be

// re-provisionned (even when ReclaimPolicy is Delete).
// +optional
// +nullable
DisabledPVReprovisioningStorageClasses []string `json:"disabledPVReprovisioningStorageClasses,omitempty"`
}

// UploaderConfigForRestore defines the configuration for the restore.
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/velero/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions pkg/builder/restore_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,9 @@ func (b *RestoreBuilder) ItemOperationTimeout(timeout time.Duration) *RestoreBui
b.object.Spec.ItemOperationTimeout.Duration = timeout
return b
}

// DisabledPVReprovisioningStorageClasses appends to the Restore's disabled PV re-rpovisioning StorageClasses.
func (b *RestoreBuilder) DisabledPVReprovisioningStorageClasses(storageClasses ...string) *RestoreBuilder {
b.object.Spec.DisabledPVReprovisioningStorageClasses = append(b.object.Spec.DisabledPVReprovisioningStorageClasses, storageClasses...)
return b
}
54 changes: 29 additions & 25 deletions pkg/cmd/cli/restore/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,31 +78,32 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command {
}

type CreateOptions struct {
BackupName string
ScheduleName string
RestoreName string
RestoreVolumes flag.OptionalBool
PreserveNodePorts flag.OptionalBool
Labels flag.Map
Annotations flag.Map
IncludeNamespaces flag.StringArray
ExcludeNamespaces flag.StringArray
ExistingResourcePolicy string
IncludeResources flag.StringArray
ExcludeResources flag.StringArray
StatusIncludeResources flag.StringArray
StatusExcludeResources flag.StringArray
NamespaceMappings flag.Map
Selector flag.LabelSelector
OrSelector flag.OrLabelSelector
IncludeClusterResources flag.OptionalBool
Wait bool
AllowPartiallyFailed flag.OptionalBool
ItemOperationTimeout time.Duration
ResourceModifierConfigMap string
WriteSparseFiles flag.OptionalBool
ParallelFilesDownload int
client kbclient.WithWatch
BackupName string
ScheduleName string
RestoreName string
RestoreVolumes flag.OptionalBool
PreserveNodePorts flag.OptionalBool
Labels flag.Map
Annotations flag.Map
IncludeNamespaces flag.StringArray
ExcludeNamespaces flag.StringArray
ExistingResourcePolicy string
IncludeResources flag.StringArray
ExcludeResources flag.StringArray
StatusIncludeResources flag.StringArray
StatusExcludeResources flag.StringArray
NamespaceMappings flag.Map
Selector flag.LabelSelector
OrSelector flag.OrLabelSelector
IncludeClusterResources flag.OptionalBool
Wait bool
AllowPartiallyFailed flag.OptionalBool
ItemOperationTimeout time.Duration
ResourceModifierConfigMap string
WriteSparseFiles flag.OptionalBool
ParallelFilesDownload int
client kbclient.WithWatch
DisabledPVReprovisioningStorageClasses flag.StringArray
}

func NewCreateOptions() *CreateOptions {
Expand Down Expand Up @@ -158,6 +159,8 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) {
f.NoOptDefVal = cmd.TRUE

flags.IntVar(&o.ParallelFilesDownload, "parallel-files-download", 0, "The number of restore operations to run in parallel. If set to 0, the default parallelism will be the number of CPUs for the node that node agent pod is running.")

flags.Var(&o.DisabledPVReprovisioningStorageClasses, "disable-pv-reprovisioning-storageclasses", "PV with no snaptshot that has a StorageClass specified by this flag will not be re-provisionned.")
}

func (o *CreateOptions) Complete(args []string, f client.Factory) error {
Expand Down Expand Up @@ -339,6 +342,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error {
WriteSparseFiles: o.WriteSparseFiles.Value,
ParallelFilesDownload: o.ParallelFilesDownload,
},
DisabledPVReprovisioningStorageClasses: o.DisabledPVReprovisioningStorageClasses,
},
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/cli/restore/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func TestCreateCommand(t *testing.T) {
itemOperationTimeout := "10m0s"
writeSparseFiles := "true"
parallel := 2
disabledPVReprovisioningStorageClasses := "storageclass1,storageclass2"
flags := new(pflag.FlagSet)
o := NewCreateOptions()
o.BindFlags(flags)
Expand All @@ -104,6 +105,7 @@ func TestCreateCommand(t *testing.T) {
flags.Parse([]string{"--item-operation-timeout", itemOperationTimeout})
flags.Parse([]string{"--write-sparse-files", writeSparseFiles})
flags.Parse([]string{"--parallel-files-download", "2"})
flags.Parse([]string{"--disable-pv-reprovisioning-storageclasses", disabledPVReprovisioningStorageClasses})
client := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch)

f.On("Namespace").Return(mock.Anything)
Expand Down Expand Up @@ -142,6 +144,7 @@ func TestCreateCommand(t *testing.T) {
require.Equal(t, itemOperationTimeout, o.ItemOperationTimeout.String())
require.Equal(t, writeSparseFiles, o.WriteSparseFiles.String())
require.Equal(t, parallel, o.ParallelFilesDownload)
require.Equal(t, disabledPVReprovisioningStorageClasses, o.DisabledPVReprovisioningStorageClasses.String())
})

t.Run("create a restore from schedule", func(t *testing.T) {
Expand Down
7 changes: 7 additions & 0 deletions pkg/cmd/util/output/restore_describer.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,13 @@ func DescribeRestore(

d.Printf("\tCluster-scoped:\t%s\n", BoolPointerString(restore.Spec.IncludeClusterResources, "excluded", "included", "auto"))

d.Println()
s = emptyDisplay
if restore.Spec.DisabledPVReprovisioningStorageClasses != nil {
s = strings.Join(restore.Spec.IncludedNamespaces, ", ")
}
d.Printf("Disabled PV Re-provisioning StorageClasses:\t%s\n", s)

d.Println()
d.DescribeMap("Namespace mappings", restore.Spec.NamespaceMapping)

Expand Down
47 changes: 46 additions & 1 deletion pkg/restore/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,19 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
// When the PV data is skipped from backup, it's BackupVolumeInfo BackupMethod
// is not set, and it will fall into the default case.
default:
if hasDeleteReclaimPolicy(obj.Object) {
hasDisabledStorageClass, err := hasDisabledPVReprovisioningStorageClass(obj, ctx)
if err != nil {
errs.Add(namespace, err)
return warnings, errs, itemExists
}

if hasDisabledStorageClass {
obj, err = ctx.handleSkippedPVHasDisabledReprovisioningStorageClass(obj, restoreLogger)
if err != nil {
errs.Add(namespace, err)
return warnings, errs, itemExists
}
} else if hasDeleteReclaimPolicy(obj.Object) {
restoreLogger.Infof("Dynamically re-provisioning persistent volume because it doesn't have a snapshot and its reclaim policy is Delete.")
ctx.pvsToProvision.Insert(name)

Expand Down Expand Up @@ -2527,3 +2539,36 @@ func (ctx *restoreContext) handleSkippedPVHasRetainPolicy(
obj = resetVolumeBindingInfo(obj)
return obj, nil
}

func hasDisabledPVReprovisioningStorageClass(unstructuredPV *unstructured.Unstructured, ctx *restoreContext) (bool, error) {
disabledStorageClasses := ctx.restore.Spec.DisabledPVReprovisioningStorageClasses

// Converting Unstructured to PV object.
pv := new(v1.PersistentVolume)
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredPV.Object, pv); err != nil {
return false, errors.Wrapf(err, "error converting persistent volume to structured")
}

// Checking if PV StorageClass is in the DisabledPVReprovisioningStorageClasses list.
for _, disabledStorageClass := range disabledStorageClasses {
if disabledStorageClass == pv.Spec.StorageClassName {
return true, nil
}
}
return false, nil
}

func (ctx *restoreContext) handleSkippedPVHasDisabledReprovisioningStorageClass(
obj *unstructured.Unstructured,
logger logrus.FieldLogger,
) (*unstructured.Unstructured, error) {
logger.Infof("Restoring persistent volume as-is because it doesn't have a snapshot and it's storage class has re-provisionning disabled.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.Infof("Restoring persistent volume as-is because it doesn't have a snapshot and it's storage class has re-provisionning disabled.")
logger.Infof("Restoring persistent volume as-is because it doesn't have a snapshot and restore has storage class re-provisionning disabled.")


// Check to see if the claimRef.namespace field needs to be remapped, and do so if necessary.
if _, err := remapClaimRefNS(ctx, obj); err != nil {
return nil, err
}

obj = resetVolumeBindingInfo(obj)
return obj, nil
}
Loading