Skip to content

Commit

Permalink
Merge pull request #7011 from Lyndon-Li/issue-fix-6964-2
Browse files Browse the repository at this point in the history 
Issue 6964: use preparingTimeout for snapshot readiness wait
  • Loading branch information
@Lyndon-Li
Lyndon-Li authored Oct 25, 2023
2 parents 941dd00 + 0eade6c commit 30bf6bd
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 40 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/7011-Lyndon-Li
View file
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
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
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
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

0 comments on commit 30bf6bd

Please sign in to comment.