Skip to content

Add Volume deletion validation #68

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+275 −13
Open

Add Volume deletion validation #68

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
39 changes: 36 additions & 3 deletions pkg/cloud/cloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ import (
"context"
"errors"
"fmt"
"os"
"strings"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/fsx"
"github.com/aws/aws-sdk-go-v2/service/fsx/types"
"github.com/kubernetes-sigs/aws-fsx-openzfs-csi-driver/pkg/util"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/klog/v2"
"os"
"strings"
"time"
)

// Polling
Expand Down Expand Up @@ -102,11 +103,13 @@ type Cloud interface {
DeleteFileSystem(ctx context.Context, parameters map[string]string) error
DescribeFileSystem(ctx context.Context, fileSystemId string) (*FileSystem, error)
WaitForFileSystemAvailable(ctx context.Context, fileSystemId string) error
WaitForFileSystemDeletion(ctx context.Context, fileSystemId string) error
WaitForFileSystemResize(ctx context.Context, fileSystemId string, resizeGiB int32) error
CreateVolume(ctx context.Context, parameters map[string]string) (*Volume, error)
DeleteVolume(ctx context.Context, parameters map[string]string) error
DescribeVolume(ctx context.Context, volumeId string) (*Volume, error)
WaitForVolumeAvailable(ctx context.Context, volumeId string) error
WaitForVolumeDeletion(ctx context.Context, volumeId string) error
WaitForVolumeResize(ctx context.Context, volumeId string, resizeGiB int32) error
CreateSnapshot(ctx context.Context, options map[string]string) (*Snapshot, error)
DeleteSnapshot(ctx context.Context, parameters map[string]string) error
Expand Down Expand Up @@ -242,6 +245,21 @@ func (c *cloud) WaitForFileSystemAvailable(ctx context.Context, fileSystemId str
return err
}

func (c *cloud) WaitForFileSystemDeletion(ctx context.Context, fileSystemId string) error {
err := wait.PollUntilContextTimeout(ctx, PollCheckInterval, PollCheckTimeout, true, func(ctx context.Context) (done bool, err error) {
_, err = c.getFileSystem(ctx, fileSystemId)
if err == ErrNotFound {
return true, nil
} else if err != nil {
return true, err
}
klog.V(2).InfoS("WaitForFileSystemDeletion", "filesystem", fileSystemId)
return false, nil
})

return err
}

func (c *cloud) WaitForFileSystemResize(ctx context.Context, fileSystemId string, resizeGiB int32) error {
err := wait.Poll(PollCheckInterval, PollCheckTimeout, func() (done bool, err error) {
updateAction, err := c.getUpdateResizeFilesystemAdministrativeAction(ctx, fileSystemId, resizeGiB)
Expand Down Expand Up @@ -340,6 +358,21 @@ func (c *cloud) WaitForVolumeAvailable(ctx context.Context, volumeId string) err
return err
}

func (c *cloud) WaitForVolumeDeletion(ctx context.Context, volumeId string) error {
err := wait.PollUntilContextTimeout(ctx, PollCheckInterval, PollCheckTimeout, true, func(ctx context.Context) (done bool, err error) {
_, err = c.getVolume(ctx, volumeId)
if err == ErrNotFound {
return true, nil
} else if err != nil {
return true, err
}
klog.V(2).InfoS("WaitForVolumeDeletion", "volume", volumeId)
return false, nil
})

return err
}

// WaitForVolumeResize TODO: Remove this function and its associated tests.
func (c *cloud) WaitForVolumeResize(ctx context.Context, volumeId string, resizeGiB int32) error {
err := wait.Poll(PollCheckInterval, PollCheckTimeout, func() (done bool, err error) {
Expand Down
112 changes: 108 additions & 4 deletions pkg/cloud/cloud_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ package cloud
import (
"context"
"errors"
"reflect"
"strconv"
"testing"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/fsx"
"github.com/aws/aws-sdk-go-v2/service/fsx/types"
"github.com/golang/mock/gomock"
"github.com/kubernetes-sigs/aws-fsx-openzfs-csi-driver/pkg/cloud/mocks"
"github.com/kubernetes-sigs/aws-fsx-openzfs-csi-driver/pkg/util"
"reflect"
"strconv"
"testing"
"time"
)

func TestCreateFileSystem(t *testing.T) {
Expand Down Expand Up @@ -594,6 +595,57 @@ func TestWaitForFileSystemAvailable(t *testing.T) {
}
}

func TestWaitForFileSystemDeletion(t *testing.T) {
fileSystemId := "fs-123456789abcdefgh"
testCases := []struct {
name string
testFunc func(t *testing.T)
}{
{
name: "success: DescribeFileSystems return not found",
testFunc: func(t *testing.T) {
mockCtl := gomock.NewController(t)
mockFSx := mocks.NewMockFSx(mockCtl)
c := &cloud{
fsx: mockFSx,
}

ctx := context.Background()
mockFSx.EXPECT().DescribeFileSystems(gomock.Any(), gomock.Any()).Return(nil, ErrNotFound)
err := c.WaitForFileSystemDeletion(ctx, fileSystemId)
if err != nil {
t.Fatalf("WaitForFileSystemDeletion is failed: %v", err)
}

mockCtl.Finish()
},
},
{
name: "fail: DescribeFileSystems return other error",
testFunc: func(t *testing.T) {
mockCtl := gomock.NewController(t)
mockFSx := mocks.NewMockFSx(mockCtl)
c := &cloud{
fsx: mockFSx,
}

ctx := context.Background()
mockFSx.EXPECT().DescribeFileSystems(gomock.Any(), gomock.Any()).Return(nil, errors.New(""))
err := c.WaitForFileSystemDeletion(ctx, fileSystemId)
if err == nil {
t.Fatalf("WaitForFileSystemDeletion is not failed: %v", err)
}

mockCtl.Finish()
},
},
}

for _, tc := range testCases {
t.Run(tc.name, tc.testFunc)
}
}

func TestWaitForFileSystemResize(t *testing.T) {
var (
filesystemId = "fs-1234"
Expand Down Expand Up @@ -1149,6 +1201,58 @@ func TestWaitForVolumeAvailable(t *testing.T) {
}
}

func TestWaitForVolumeDeletion(t *testing.T) {
volumeId := "fsvol-0987654321abcdefg"

testCases := []struct {
name string
testFunc func(t *testing.T)
}{
{
name: "success: DescribeVolumes return empty",
testFunc: func(t *testing.T) {
mockCtl := gomock.NewController(t)
mockFSx := mocks.NewMockFSx(mockCtl)
c := &cloud{
fsx: mockFSx,
}

ctx := context.Background()
mockFSx.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(nil, ErrNotFound)
err := c.WaitForVolumeDeletion(ctx, volumeId)
if err != nil {
t.Fatalf("WaitForVolumeDeletion is failed: %v", err)
}

mockCtl.Finish()
},
},
{
name: "fail: DescribeVolumes return other error",
testFunc: func(t *testing.T) {
mockCtl := gomock.NewController(t)
mockFSx := mocks.NewMockFSx(mockCtl)
c := &cloud{
fsx: mockFSx,
}

ctx := context.Background()
mockFSx.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(nil, errors.New(""))
err := c.WaitForVolumeDeletion(ctx, volumeId)
if err == nil {
t.Fatalf("WaitForVolumeDeletion is not failed: %v", err)
}

mockCtl.Finish()
},
},
}

for _, tc := range testCases {
t.Run(tc.name, tc.testFunc)
}
}

func TestWaitForVolumeResize(t *testing.T) {
var (
volumeId = "fsvol-1234"
Expand Down
23 changes: 22 additions & 1 deletion pkg/cloud/fakes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ package cloud
import (
"context"
"fmt"
"github.com/aws/aws-sdk-go-v2/service/fsx/types"
"math/rand"
"reflect"
"strconv"
"strings"
"time"

"github.com/aws/aws-sdk-go-v2/service/fsx/types"
)

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

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

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

func (c *FakeCloudProvider) WaitForFileSystemDeletion(ctx context.Context, fileSystemId string) error {
return nil
}

func (c *FakeCloudProvider) WaitForFileSystemResize(ctx context.Context, fileSystemId string, newSizeGiB int32) error {
return nil
}
Expand Down Expand Up @@ -171,8 +182,14 @@ func (c *FakeCloudProvider) CreateVolume(ctx context.Context, parameters map[str
}

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

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

func (c *FakeCloudProvider) WaitForVolumeDeletion(ctx context.Context, volumeId string) error {
return nil
}

func (c *FakeCloudProvider) WaitForVolumeResize(ctx context.Context, volumeId string, newSizeGiB int32) error {
return nil
}
Expand Down
19 changes: 16 additions & 3 deletions pkg/driver/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import (
"context"
"errors"
"fmt"
"os"
"strconv"
"strings"

"github.com/aws/aws-sdk-go-v2/service/fsx/types"
"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/kubernetes-sigs/aws-fsx-openzfs-csi-driver/pkg/cloud"
Expand All @@ -28,9 +32,6 @@ import (
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/timestamppb"
"k8s.io/klog/v2"
"os"
"strconv"
"strings"
)

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

switch splitVolumeId[0] {
case cloud.FilesystemPrefix:
err = d.cloud.WaitForFileSystemDeletion(ctx, volumeID)
case cloud.VolumePrefix:
err = d.cloud.WaitForVolumeDeletion(ctx, volumeID)
}

if err != nil {
return nil, status.Errorf(codes.Internal, "Could not delete volume ID %q: %v", volumeID, err)
}

klog.V(4).InfoS("DeleteVolume: volume not found, returning with success", "volumeId", volumeID)
return &csi.DeleteVolumeResponse{}, nil
}

Expand Down
Loading