Skip to content

Commit 2c42912

Browse files
phuhung273phuhung273
committed
Add Volume deletion validation
Signed-off-by: phuhung273 <phuhung.tranpham@gmail.com>
1 parent c000335 commit 2c42912

File tree

6 files changed

+275
-13
lines changed

6 files changed

+275
-13
lines changed

‎pkg/cloud/cloud.go‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,17 @@ import (
1919
"context"
2020
"errors"
2121
"fmt"
22+
"os"
23+
"strings"
24+
"time"
25+
2226
"github.com/aws/aws-sdk-go-v2/aws"
2327
"github.com/aws/aws-sdk-go-v2/config"
2428
"github.com/aws/aws-sdk-go-v2/service/fsx"
2529
"github.com/aws/aws-sdk-go-v2/service/fsx/types"
2630
"github.com/kubernetes-sigs/aws-fsx-openzfs-csi-driver/pkg/util"
2731
"k8s.io/apimachinery/pkg/util/wait"
2832
"k8s.io/klog/v2"
29-
"os"
30-
"strings"
31-
"time"
3233
)
3334

3435
// Polling
@@ -102,11 +103,13 @@ type Cloud interface {
102103
DeleteFileSystem(ctx context.Context, parameters map[string]string) error
103104
DescribeFileSystem(ctx context.Context, fileSystemId string) (*FileSystem, error)
104105
WaitForFileSystemAvailable(ctx context.Context, fileSystemId string) error
106+
WaitForFileSystemDeletion(ctx context.Context, fileSystemId string) error
105107
WaitForFileSystemResize(ctx context.Context, fileSystemId string, resizeGiB int32) error
106108
CreateVolume(ctx context.Context, parameters map[string]string) (*Volume, error)
107109
DeleteVolume(ctx context.Context, parameters map[string]string) error
108110
DescribeVolume(ctx context.Context, volumeId string) (*Volume, error)
109111
WaitForVolumeAvailable(ctx context.Context, volumeId string) error
112+
WaitForVolumeDeletion(ctx context.Context, volumeId string) error
110113
WaitForVolumeResize(ctx context.Context, volumeId string, resizeGiB int32) error
111114
CreateSnapshot(ctx context.Context, options map[string]string) (*Snapshot, error)
112115
DeleteSnapshot(ctx context.Context, parameters map[string]string) error
@@ -242,6 +245,21 @@ func (c *cloud) WaitForFileSystemAvailable(ctx context.Context, fileSystemId str
242245
return err
243246
}
244247

248+
func (c *cloud) WaitForFileSystemDeletion(ctx context.Context, fileSystemId string) error {
249+
err := wait.PollUntilContextTimeout(ctx, PollCheckInterval, PollCheckTimeout, true, func(ctx context.Context) (done bool, err error) {
250+
_, err = c.getFileSystem(ctx, fileSystemId)
251+
if err == ErrNotFound {
252+
return true, nil
253+
} else if err != nil {
254+
return true, err
255+
}
256+
klog.V(2).InfoS("WaitForFileSystemDeletion", "filesystem", fileSystemId)
257+
return false, nil
258+
})
259+
260+
return err
261+
}
262+
245263
func (c *cloud) WaitForFileSystemResize(ctx context.Context, fileSystemId string, resizeGiB int32) error {
246264
err := wait.Poll(PollCheckInterval, PollCheckTimeout, func() (done bool, err error) {
247265
updateAction, err := c.getUpdateResizeFilesystemAdministrativeAction(ctx, fileSystemId, resizeGiB)
@@ -340,6 +358,21 @@ func (c *cloud) WaitForVolumeAvailable(ctx context.Context, volumeId string) err
340358
return err
341359
}
342360

361+
func (c *cloud) WaitForVolumeDeletion(ctx context.Context, volumeId string) error {
362+
err := wait.PollUntilContextTimeout(ctx, PollCheckInterval, PollCheckTimeout, true, func(ctx context.Context) (done bool, err error) {
363+
_, err = c.getVolume(ctx, volumeId)
364+
if err == ErrNotFound {
365+
return true, nil
366+
} else if err != nil {
367+
return true, err
368+
}
369+
klog.V(2).InfoS("WaitForVolumeDeletion", "volume", volumeId)
370+
return false, nil
371+
})
372+
373+
return err
374+
}
375+
343376
// WaitForVolumeResize TODO: Remove this function and its associated tests.
344377
func (c *cloud) WaitForVolumeResize(ctx context.Context, volumeId string, resizeGiB int32) error {
345378
err := wait.Poll(PollCheckInterval, PollCheckTimeout, func() (done bool, err error) {

‎pkg/cloud/cloud_test.go‎

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,17 @@ package cloud
1818
import (
1919
"context"
2020
"errors"
21+
"reflect"
22+
"strconv"
23+
"testing"
24+
"time"
25+
2126
"github.com/aws/aws-sdk-go-v2/aws"
2227
"github.com/aws/aws-sdk-go-v2/service/fsx"
2328
"github.com/aws/aws-sdk-go-v2/service/fsx/types"
2429
"github.com/golang/mock/gomock"
2530
"github.com/kubernetes-sigs/aws-fsx-openzfs-csi-driver/pkg/cloud/mocks"
2631
"github.com/kubernetes-sigs/aws-fsx-openzfs-csi-driver/pkg/util"
27-
"reflect"
28-
"strconv"
29-
"testing"
30-
"time"
3132
)
3233

3334
func TestCreateFileSystem(t *testing.T) {
@@ -594,6 +595,57 @@ func TestWaitForFileSystemAvailable(t *testing.T) {
594595
}
595596
}
596597

598+
func TestWaitForFileSystemDeletion(t *testing.T) {
599+
fileSystemId := "fs-123456789abcdefgh"
600+
testCases := []struct {
601+
name string
602+
testFunc func(t *testing.T)
603+
}{
604+
{
605+
name: "success: DescribeFileSystems return not found",
606+
testFunc: func(t *testing.T) {
607+
mockCtl := gomock.NewController(t)
608+
mockFSx := mocks.NewMockFSx(mockCtl)
609+
c := &cloud{
610+
fsx: mockFSx,
611+
}
612+
613+
ctx := context.Background()
614+
mockFSx.EXPECT().DescribeFileSystems(gomock.Any(), gomock.Any()).Return(nil, ErrNotFound)
615+
err := c.WaitForFileSystemDeletion(ctx, fileSystemId)
616+
if err != nil {
617+
t.Fatalf("WaitForFileSystemDeletion is failed: %v", err)
618+
}
619+
620+
mockCtl.Finish()
621+
},
622+
},
623+
{
624+
name: "fail: DescribeFileSystems return other error",
625+
testFunc: func(t *testing.T) {
626+
mockCtl := gomock.NewController(t)
627+
mockFSx := mocks.NewMockFSx(mockCtl)
628+
c := &cloud{
629+
fsx: mockFSx,
630+
}
631+
632+
ctx := context.Background()
633+
mockFSx.EXPECT().DescribeFileSystems(gomock.Any(), gomock.Any()).Return(nil, errors.New(""))
634+
err := c.WaitForFileSystemDeletion(ctx, fileSystemId)
635+
if err == nil {
636+
t.Fatalf("WaitForFileSystemDeletion is not failed: %v", err)
637+
}
638+
639+
mockCtl.Finish()
640+
},
641+
},
642+
}
643+
644+
for _, tc := range testCases {
645+
t.Run(tc.name, tc.testFunc)
646+
}
647+
}
648+
597649
func TestWaitForFileSystemResize(t *testing.T) {
598650
var (
599651
filesystemId = "fs-1234"
@@ -1149,6 +1201,58 @@ func TestWaitForVolumeAvailable(t *testing.T) {
11491201
}
11501202
}
11511203

1204+
func TestWaitForVolumeDeletion(t *testing.T) {
1205+
volumeId := "fsvol-0987654321abcdefg"
1206+
1207+
testCases := []struct {
1208+
name string
1209+
testFunc func(t *testing.T)
1210+
}{
1211+
{
1212+
name: "success: DescribeVolumes return empty",
1213+
testFunc: func(t *testing.T) {
1214+
mockCtl := gomock.NewController(t)
1215+
mockFSx := mocks.NewMockFSx(mockCtl)
1216+
c := &cloud{
1217+
fsx: mockFSx,
1218+
}
1219+
1220+
ctx := context.Background()
1221+
mockFSx.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(nil, ErrNotFound)
1222+
err := c.WaitForVolumeDeletion(ctx, volumeId)
1223+
if err != nil {
1224+
t.Fatalf("WaitForVolumeDeletion is failed: %v", err)
1225+
}
1226+
1227+
mockCtl.Finish()
1228+
},
1229+
},
1230+
{
1231+
name: "fail: DescribeVolumes return other error",
1232+
testFunc: func(t *testing.T) {
1233+
mockCtl := gomock.NewController(t)
1234+
mockFSx := mocks.NewMockFSx(mockCtl)
1235+
c := &cloud{
1236+
fsx: mockFSx,
1237+
}
1238+
1239+
ctx := context.Background()
1240+
mockFSx.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(nil, errors.New(""))
1241+
err := c.WaitForVolumeDeletion(ctx, volumeId)
1242+
if err == nil {
1243+
t.Fatalf("WaitForVolumeDeletion is not failed: %v", err)
1244+
}
1245+
1246+
mockCtl.Finish()
1247+
},
1248+
},
1249+
}
1250+
1251+
for _, tc := range testCases {
1252+
t.Run(tc.name, tc.testFunc)
1253+
}
1254+
}
1255+
11521256
func TestWaitForVolumeResize(t *testing.T) {
11531257
var (
11541258
volumeId = "fsvol-1234"

‎pkg/cloud/fakes.go‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ package cloud
1818
import (
1919
"context"
2020
"fmt"
21-
"github.com/aws/aws-sdk-go-v2/service/fsx/types"
2221
"math/rand"
2322
"reflect"
2423
"strconv"
2524
"strings"
2625
"time"
26+
27+
"github.com/aws/aws-sdk-go-v2/service/fsx/types"
2728
)
2829

2930
var random *rand.Rand
@@ -117,8 +118,14 @@ func (c *FakeCloudProvider) ResizeFileSystem(ctx context.Context, fileSystemId s
117118
}
118119

119120
func (c *FakeCloudProvider) DeleteFileSystem(ctx context.Context, parameters map[string]string) error {
121+
// parameters["FileSystemId"] in Sanity test is JSON string, eg: "\"fs-1234\""
122+
// While actual id is "fs-1234"
123+
// For backward compatibility, we can remove both the JSON string and the unquote string
124+
unquoteId := strings.Trim(parameters["FileSystemId"], "\"")
120125
delete(c.fileSystems, parameters["FileSystemId"])
126+
delete(c.fileSystems, unquoteId)
121127
delete(c.fileSystemsParameters, parameters["FileSystemId"])
128+
delete(c.fileSystemsParameters, unquoteId)
122129
return nil
123130
}
124131

@@ -135,6 +142,10 @@ func (c *FakeCloudProvider) WaitForFileSystemAvailable(ctx context.Context, file
135142
return nil
136143
}
137144

145+
func (c *FakeCloudProvider) WaitForFileSystemDeletion(ctx context.Context, fileSystemId string) error {
146+
return nil
147+
}
148+
138149
func (c *FakeCloudProvider) WaitForFileSystemResize(ctx context.Context, fileSystemId string, newSizeGiB int32) error {
139150
return nil
140151
}
@@ -171,8 +182,14 @@ func (c *FakeCloudProvider) CreateVolume(ctx context.Context, parameters map[str
171182
}
172183

173184
func (c *FakeCloudProvider) DeleteVolume(ctx context.Context, parameters map[string]string) (err error) {
185+
// parameters["VolumeId"] in Sanity test is JSON string, eg: "\"fsvol-1234\""
186+
// While actual id is "fsvol-1234"
187+
// For backward compatibility, we can remove both the JSON string and the unquote string
188+
unquoteId := strings.Trim(parameters["VolumeId"], "\"")
174189
delete(c.volumes, parameters["VolumeId"])
190+
delete(c.volumes, unquoteId)
175191
delete(c.volumesParameters, parameters["VolumeId"])
192+
delete(c.volumesParameters, unquoteId)
176193
return nil
177194
}
178195

@@ -189,6 +206,10 @@ func (c *FakeCloudProvider) WaitForVolumeAvailable(ctx context.Context, volumeId
189206
return nil
190207
}
191208

209+
func (c *FakeCloudProvider) WaitForVolumeDeletion(ctx context.Context, volumeId string) error {
210+
return nil
211+
}
212+
192213
func (c *FakeCloudProvider) WaitForVolumeResize(ctx context.Context, volumeId string, newSizeGiB int32) error {
193214
return nil
194215
}

‎pkg/driver/controller.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ import (
1919
"context"
2020
"errors"
2121
"fmt"
22+
"os"
23+
"strconv"
24+
"strings"
25+
2226
"github.com/aws/aws-sdk-go-v2/service/fsx/types"
2327
"github.com/container-storage-interface/spec/lib/go/csi"
2428
"github.com/kubernetes-sigs/aws-fsx-openzfs-csi-driver/pkg/cloud"
@@ -28,9 +32,6 @@ import (
2832
"google.golang.org/grpc/status"
2933
"google.golang.org/protobuf/types/known/timestamppb"
3034
"k8s.io/klog/v2"
31-
"os"
32-
"strconv"
33-
"strings"
3435
)
3536

3637
var (
@@ -369,6 +370,18 @@ func (d *controllerService) DeleteVolume(ctx context.Context, req *csi.DeleteVol
369370
return nil, status.Errorf(codes.Internal, "Could not delete volume ID %q: %v", volumeID, err)
370371
}
371372

373+
switch splitVolumeId[0] {
374+
case cloud.FilesystemPrefix:
375+
err = d.cloud.WaitForFileSystemDeletion(ctx, volumeID)
376+
case cloud.VolumePrefix:
377+
err = d.cloud.WaitForVolumeDeletion(ctx, volumeID)
378+
}
379+
380+
if err != nil {
381+
return nil, status.Errorf(codes.Internal, "Could not delete volume ID %q: %v", volumeID, err)
382+
}
383+
384+
klog.V(4).InfoS("DeleteVolume: volume not found, returning with success", "volumeId", volumeID)
372385
return &csi.DeleteVolumeResponse{}, nil
373386
}
374387

0 commit comments

Comments
 (0)