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.

Sign up for GitHub

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

Issue 6964: use preparingTimeout for snapshot readiness wait #7011

Merged
merged 1 commit into from
Oct 25, 2023
+55 −40
Merged

Issue 6964: use preparingTimeout for snapshot readiness wait #7011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/7011-Lyndon-Li
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix #6964. Don't use csiSnapshotTimeout (10 min) for waiting snapshot to readyToUse for data mover, so as to make the behavior complied with CSI snapshot backup
3 changes: 2 additions & 1 deletion pkg/controller/data_upload_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,8 @@ func (r *DataUploadReconciler) setupExposeParam(du *velerov2alpha1api.DataUpload
StorageClass: du.Spec.CSISnapshot.StorageClass,
HostingPodLabels: map[string]string{velerov1api.DataUploadLabel: du.Name},
AccessMode: accessMode,
Timeout: du.Spec.OperationTimeout.Duration,
OperationTimeout: du.Spec.OperationTimeout.Duration,
ExposeTimeout: r.preparingTimeout,
VolumeSize: pvc.Spec.Resources.Requests[corev1.ResourceStorage],
}, nil
}
Expand Down
13 changes: 8 additions & 5 deletions pkg/exposer/csi_snapshot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ type CSISnapshotExposeParam struct {
// HostingPodLabels is the labels that are going to apply to the hosting pod
HostingPodLabels map[string]string

// Timeout specifies the time wait for resources operations in Expose
Timeout time.Duration
// OperationTimeout specifies the time wait for resources operations in Expose
OperationTimeout time.Duration

// ExposeTimeout specifies the timeout for the entire expose process
ExposeTimeout time.Duration

// VolumeSize specifies the size of the source volume
VolumeSize resource.Quantity
Expand Down Expand Up @@ -97,7 +100,7 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1.Obje

curLog.Info("Exposing CSI snapshot")

volumeSnapshot, err := csi.WaitVolumeSnapshotReady(ctx, e.csiSnapshotClient, csiExposeParam.SnapshotName, csiExposeParam.SourceNamespace, csiExposeParam.Timeout, curLog)
volumeSnapshot, err := csi.WaitVolumeSnapshotReady(ctx, e.csiSnapshotClient, csiExposeParam.SnapshotName, csiExposeParam.SourceNamespace, csiExposeParam.ExposeTimeout, curLog)
if err != nil {
return errors.Wrapf(err, "error wait volume snapshot ready")
}
Expand All @@ -124,14 +127,14 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1.Obje
}
}()

err = csi.EnsureDeleteVS(ctx, e.csiSnapshotClient, volumeSnapshot.Name, volumeSnapshot.Namespace, csiExposeParam.Timeout)
err = csi.EnsureDeleteVS(ctx, e.csiSnapshotClient, volumeSnapshot.Name, volumeSnapshot.Namespace, csiExposeParam.OperationTimeout)
if err != nil {
return errors.Wrap(err, "error to delete volume snapshot")
}

curLog.WithField("vs name", volumeSnapshot.Name).Infof("VS is deleted in namespace %s", volumeSnapshot.Namespace)

err = csi.EnsureDeleteVSC(ctx, e.csiSnapshotClient, vsc.Name, csiExposeParam.Timeout)
err = csi.EnsureDeleteVSC(ctx, e.csiSnapshotClient, vsc.Name, csiExposeParam.OperationTimeout)
if err != nil {
return errors.Wrap(err, "error to delete volume snapshot content")
}
Expand Down
78 changes: 44 additions & 34 deletions pkg/exposer/csi_snapshot_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,20 @@ func TestExpose(t *testing.T) {
name: "wait vs ready fail",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
Timeout: time.Millisecond,
SnapshotName: "fake-vs",
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
},
err: "error wait volume snapshot ready: error to get volumesnapshot /fake-vs: volumesnapshots.snapshot.storage.k8s.io \"fake-vs\" not found",
},
{
name: "get vsc fail",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
Timeout: time.Millisecond,
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
},
snapshotClientObj: []runtime.Object{
vsObject,
Expand All @@ -174,9 +176,10 @@ func TestExpose(t *testing.T) {
name: "delete vs fail",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
Timeout: time.Millisecond,
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
},
snapshotClientObj: []runtime.Object{
vsObject,
Expand All @@ -197,9 +200,10 @@ func TestExpose(t *testing.T) {
name: "delete vsc fail",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
Timeout: time.Millisecond,
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
},
snapshotClientObj: []runtime.Object{
vsObject,
Expand All @@ -220,9 +224,10 @@ func TestExpose(t *testing.T) {
name: "create backup vs fail",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
Timeout: time.Millisecond,
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
},
snapshotClientObj: []runtime.Object{
vsObject,
Expand All @@ -243,9 +248,10 @@ func TestExpose(t *testing.T) {
name: "create backup vsc fail",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
Timeout: time.Millisecond,
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
},
snapshotClientObj: []runtime.Object{
vsObject,
Expand Down Expand Up @@ -280,10 +286,11 @@ func TestExpose(t *testing.T) {
name: "create backup pvc fail",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
Timeout: time.Millisecond,
AccessMode: AccessModeFileSystem,
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
AccessMode: AccessModeFileSystem,
},
snapshotClientObj: []runtime.Object{
vsObject,
Expand All @@ -304,10 +311,11 @@ func TestExpose(t *testing.T) {
name: "create backup pod fail",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
AccessMode: AccessModeFileSystem,
Timeout: time.Millisecond,
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
AccessMode: AccessModeFileSystem,
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
},
snapshotClientObj: []runtime.Object{
vsObject,
Expand All @@ -331,10 +339,11 @@ func TestExpose(t *testing.T) {
name: "success",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
AccessMode: AccessModeFileSystem,
Timeout: time.Millisecond,
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
AccessMode: AccessModeFileSystem,
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
},
snapshotClientObj: []runtime.Object{
vsObject,
Expand All @@ -348,11 +357,12 @@ func TestExpose(t *testing.T) {
name: "restore size from exposeParam",
ownerBackup: backup,
exposeParam: CSISnapshotExposeParam{
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
AccessMode: AccessModeFileSystem,
Timeout: time.Millisecond,
VolumeSize: *resource.NewQuantity(567890, ""),
SnapshotName: "fake-vs",
SourceNamespace: "fake-ns",
AccessMode: AccessModeFileSystem,
OperationTimeout: time.Millisecond,
ExposeTimeout: time.Millisecond,
VolumeSize: *resource.NewQuantity(567890, ""),
},
snapshotClientObj: []runtime.Object{
vsObjectWithoutRestoreSize,
Expand Down
Loading