Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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/9700-priyansh17
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue #9700, add a 2-second gap between temporary CSI VolumeSnapshotContent create and delete operations
Comment thread
priyansh17 marked this conversation as resolved.
Outdated
8 changes: 8 additions & 0 deletions internal/delete/actions/csi/volumesnapshotcontent_action.go
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of sleeping, should it be watching VSC for status updates/bound/avail phase etc which would be more indicative of storage driver recognizing the operation?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess that discussion was had.. alas better than what we had before.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes Thanks. Shubham had same point which was discussed

Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package csi

import (
"context"
"time"

"github.com/google/uuid"
snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
Expand All @@ -40,6 +41,10 @@ type volumeSnapshotContentDeleteItemAction struct {
crClient crclient.Client
}

const tempVSCCreateDeleteGap = 2 * time.Second

var sleepBetweenTempVSCCreateAndDelete = time.Sleep

// AppliesTo returns information indicating
// VolumeSnapshotContentRestoreItemAction action should be invoked
// while restoring VolumeSnapshotContent.snapshot.storage.k8s.io resources
Expand Down Expand Up @@ -123,6 +128,9 @@ func (p *volumeSnapshotContentDeleteItemAction) Execute(
}
p.log.Infof("Created temp VolumeSnapshotContent %s with DeletionPolicy=Delete to trigger cloud snapshot cleanup", snapCont.Name)

// Add a small delay before delete to avoid create/delete race conditions in CSI controllers.
sleepBetweenTempVSCCreateAndDelete(tempVSCCreateDeleteGap)

// Delete the temp VSC immediately to trigger cloud snapshot removal.
// The CSI driver will handle the actual cloud snapshot deletion.
if err := p.crClient.Delete(
Expand Down
49 changes: 49 additions & 0 deletions internal/delete/actions/csi/volumesnapshotcontent_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"testing"
"time"

snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
"github.com/sirupsen/logrus"
Expand All @@ -46,6 +47,21 @@ type fakeClientWithErrors struct {
deleteError error
}

type fakeClientWithCallTracking struct {
crclient.Client
events *[]string
}

func (c *fakeClientWithCallTracking) Create(ctx context.Context, obj crclient.Object, opts ...crclient.CreateOption) error {
*c.events = append(*c.events, "create")
return c.Client.Create(ctx, obj, opts...)
}

func (c *fakeClientWithCallTracking) Delete(ctx context.Context, obj crclient.Object, opts ...crclient.DeleteOption) error {
*c.events = append(*c.events, "delete")
return c.Client.Delete(ctx, obj, opts...)
}

func (c *fakeClientWithErrors) Get(ctx context.Context, key crclient.ObjectKey, obj crclient.Object, opts ...crclient.GetOption) error {
if c.getError != nil {
return c.getError
Expand Down Expand Up @@ -325,6 +341,39 @@ func TestTryDeleteOriginalVSC(t *testing.T) {
})
}

func TestVSCExecute_CreateSleepDeleteOrder(t *testing.T) {
snapshotHandleStr := "test"
vsc := builder.ForVolumeSnapshotContent("bar").
ObjectMeta(builder.WithLabelsMap(map[string]string{velerov1api.BackupNameLabel: "backup"})).
Status(&snapshotv1api.VolumeSnapshotContentStatus{SnapshotHandle: &snapshotHandleStr}).
Result()

vscMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(vsc)
require.NoError(t, err)

events := make([]string, 0, 3)
realClient := velerotest.NewFakeControllerRuntimeClient(t)
trackingClient := &fakeClientWithCallTracking{Client: realClient, events: &events}

originalSleep := sleepBetweenTempVSCCreateAndDelete
t.Cleanup(func() {
sleepBetweenTempVSCCreateAndDelete = originalSleep
})

sleepBetweenTempVSCCreateAndDelete = func(d time.Duration) {
require.Equal(t, tempVSCCreateDeleteGap, d)
events = append(events, "sleep")
}

p := volumeSnapshotContentDeleteItemAction{log: logrus.StandardLogger(), crClient: trackingClient}
err = p.Execute(&velero.DeleteItemActionExecuteInput{
Item: &unstructured.Unstructured{Object: vscMap},
Backup: builder.ForBackup("velero", "backup").Result(),
})
require.NoError(t, err)
require.Equal(t, []string{"create", "sleep", "delete"}, events)
}

func boolPtr(b bool) *bool {
return &b
}
Expand Down
Loading