From d0112611cc4170d0fe15a2116302f5b446416b1f Mon Sep 17 00:00:00 2001 From: Nirmal Aagash Chandramohan Date: Tue, 10 Aug 2021 22:15:55 +0000 Subject: [PATCH 1/4] Added Mount Idempotency --- pkg/driver/node.go | 35 ++++++++++++++++++++++++++++++++--- pkg/driver/node_test.go | 8 ++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/pkg/driver/node.go b/pkg/driver/node.go index bb8e728189..798911e740 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -624,10 +624,39 @@ func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeR } klog.V(4).Infof("NodePublishVolume: mounting %s at %s with option %s as fstype %s", source, target, mountOptions, fsType) - if err := d.mounter.Mount(source, target, fsType, mountOptions); err != nil { - if removeErr := os.Remove(target); removeErr != nil { - return status.Errorf(codes.Internal, "Could not remove mount target %q: %v", target, err) + /* + Checking if it's a mount point. There are three cases, + 1. true, err when the directory does not exist or corrupted. + 2. false, nil when the path is already mounted with a device. + 3. true, nil when the path is not mounted with any device. + */ + notMnt, err := d.mounter.IsLikelyNotMountPoint(target) + if err != nil && !os.IsNotExist(err){ + //Checking if the path exists and error is related to Corrupted Mount, in that case, the system could unmount and mount. + _, pathErr := mountutils.PathExists(target) + if pathErr != nil && mountutils.IsCorruptedMnt(pathErr) { + klog.V(4).Infof("Target path %q is a corrupted directory",target) + }else{ + return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) } + } + + if !notMnt { + //Return error when any of the directory inside is not readable which means that a device could be mounted. + _, err = os.ReadDir(target) + if err != nil { + klog.V(4).Infof("Error occurred at reading directory %q. Trying to Unmount.",target) + //Reading the directory failed and trying to unmount and remount + if mntErr := d.mounter.Unmount(target); mntErr != nil { + return status.Errorf(codes.Internal, "Unable to unmount the target %q : %v", target, err) + } + } else { + klog.V(4).Infof("Target path %q is already mounted",target) + return nil + } + } + + if err := d.mounter.Mount(source, target, fsType, mountOptions); err != nil { return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) } diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index 20bf8c48dc..01eb6d7fcb 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -597,6 +597,8 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Eq([]string{"bind"})).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) + mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: devicePath}, @@ -629,6 +631,8 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(FSTypeXfs), gomock.Eq([]string{"bind"})).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) + mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: devicePath}, @@ -670,6 +674,8 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Eq([]string{"bind", "ro"})).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) + mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: devicePath}, @@ -703,6 +709,8 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Eq([]string{"bind", "test-flag"})).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) + mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: "/dev/fake"}, From 482d3bdb73f3927dc7ce86e127e965141c644e0e Mon Sep 17 00:00:00 2001 From: Nirmal Aagash Chandramohan Date: Wed, 11 Aug 2021 18:54:19 +0000 Subject: [PATCH 2/4] fixed:g-format error --- pkg/driver/node.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 798911e740..680bccae2e 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -631,12 +631,12 @@ func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeR 3. true, nil when the path is not mounted with any device. */ notMnt, err := d.mounter.IsLikelyNotMountPoint(target) - if err != nil && !os.IsNotExist(err){ + if err != nil && !os.IsNotExist(err) { //Checking if the path exists and error is related to Corrupted Mount, in that case, the system could unmount and mount. _, pathErr := mountutils.PathExists(target) if pathErr != nil && mountutils.IsCorruptedMnt(pathErr) { - klog.V(4).Infof("Target path %q is a corrupted directory",target) - }else{ + klog.V(4).Infof("Target path %q is a corrupted directory", target) + } else { return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) } } @@ -645,13 +645,13 @@ func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeR //Return error when any of the directory inside is not readable which means that a device could be mounted. _, err = os.ReadDir(target) if err != nil { - klog.V(4).Infof("Error occurred at reading directory %q. Trying to Unmount.",target) + klog.V(4).Infof("Error occurred at reading directory %q. Trying to Unmount.", target) //Reading the directory failed and trying to unmount and remount if mntErr := d.mounter.Unmount(target); mntErr != nil { return status.Errorf(codes.Internal, "Unable to unmount the target %q : %v", target, err) } } else { - klog.V(4).Infof("Target path %q is already mounted",target) + klog.V(4).Infof("Target path %q is already mounted", target) return nil } } @@ -659,7 +659,6 @@ func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeR if err := d.mounter.Mount(source, target, fsType, mountOptions); err != nil { return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) } - return nil } From 25fdec30ee09ed8cd5c956680a7f392c36891a61 Mon Sep 17 00:00:00 2001 From: Nirmal Aagash Chandramohan Date: Wed, 18 Aug 2021 02:31:28 +0000 Subject: [PATCH 3/4] Added Mount Idempotency for block device --- pkg/driver/mocks/mock_mount.go | 14 ++ pkg/driver/mount.go | 9 +- pkg/driver/node.go | 89 +++++++----- pkg/driver/node_test.go | 251 ++++++++++++++++++++++++++++++++- pkg/driver/sanity_test.go | 4 + 5 files changed, 325 insertions(+), 42 deletions(-) diff --git a/pkg/driver/mocks/mock_mount.go b/pkg/driver/mocks/mock_mount.go index eeb81d98c9..138d84f10a 100644 --- a/pkg/driver/mocks/mock_mount.go +++ b/pkg/driver/mocks/mock_mount.go @@ -104,6 +104,20 @@ func (mr *MockMounterMockRecorder) GetDeviceNameFromMount(mountPath interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDeviceNameFromMount", reflect.TypeOf((*MockMounter)(nil).GetDeviceNameFromMount), mountPath) } +// IsCorruptedMnt mocks base method. +func (m *MockMounter) IsCorruptedMnt(err error) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsCorruptedMnt", err) + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsCorruptedMnt indicates an expected call of IsCorruptedMnt. +func (mr *MockMounterMockRecorder) IsCorruptedMnt(err interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsCorruptedMnt", reflect.TypeOf((*MockMounter)(nil).IsCorruptedMnt), err) +} + // GetMountRefs mocks base method. func (m *MockMounter) GetMountRefs(pathname string) ([]string, error) { m.ctrl.T.Helper() diff --git a/pkg/driver/mount.go b/pkg/driver/mount.go index f1ce7b48c8..125384234a 100644 --- a/pkg/driver/mount.go +++ b/pkg/driver/mount.go @@ -18,11 +18,12 @@ package driver import ( "fmt" - "k8s.io/klog" "os" "strconv" "strings" + "k8s.io/klog" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/mounter" mountutils "k8s.io/mount-utils" utilexec "k8s.io/utils/exec" @@ -38,6 +39,7 @@ type Mounter interface { utilexec.Interface // Implemented by NodeMounter below + IsCorruptedMnt(err error) bool GetDeviceNameFromMount(mountPath string) (string, int, error) // TODO this won't make sense on Windows with csi-proxy MakeFile(path string) error @@ -64,6 +66,11 @@ func (m NodeMounter) GetDeviceNameFromMount(mountPath string) (string, int, erro return mountutils.GetDeviceNameFromMount(m, mountPath) } +// IsCorruptedMnt return true if err is about corrupted mount point +func (m NodeMounter) IsCorruptedMnt(err error) bool { + return mountutils.IsCorruptedMnt(err) +} + // This function is mirrored in ./sanity_test.go to make sure sanity test covered this block of code // Please mirror the change to func MakeFile in ./sanity_test.go func (m *NodeMounter) MakeFile(path string) error { diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 680bccae2e..813a8e89c9 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -591,17 +591,57 @@ func (d *nodeService) nodePublishVolumeForBlock(req *csi.NodePublishVolumeReques return status.Errorf(codes.Internal, "Could not create file %q: %v", target, err) } - klog.V(4).Infof("NodePublishVolume [block]: mounting %s at %s", source, target) - if err := d.mounter.Mount(source, target, "", mountOptions); err != nil { - if removeErr := os.Remove(target); removeErr != nil { - return status.Errorf(codes.Internal, "Could not remove mount target %q: %v", target, removeErr) - } + //Checking if the target file is already mounted with a device. + mounted, err := d.isMounted(source, target) + if err != nil { return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) } + if !mounted { + klog.V(4).Infof("NodePublishVolume [block]: mounting %s at %s", source, target) + if err := d.mounter.Mount(source, target, "", mountOptions); err != nil { + if removeErr := os.Remove(target); removeErr != nil { + return status.Errorf(codes.Internal, "Could not remove mount target %q: %v", target, removeErr) + } + return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) + } + } else { + klog.V(4).Infof("NodePublishVolume [block]: Target path %q is already mounted", target) + } + return nil } +func (d *nodeService) isMounted(source string, target string) (bool, error) { + /* + Checking if it's a mount point using IsLikelyNotMountPoint. There are three different return values, + 1. true, err when the directory does not exist or corrupted. + 2. false, nil when the path is already mounted with a device. + 3. true, nil when the path is not mounted with any device. + */ + notMnt, err := d.mounter.IsLikelyNotMountPoint(target) + if err != nil && !os.IsNotExist(err) { + //Checking if the path exists and error is related to Corrupted Mount, in that case, the system could unmount and mount. + _, pathErr := d.mounter.PathExists(target) + if pathErr != nil && d.mounter.IsCorruptedMnt(pathErr) { + klog.V(4).Infof("NodePublishVolume: Target path %q is a corrupted mount. Trying to unmount.", target) + if mntErr := d.mounter.Unmount(target); mntErr != nil { + return !notMnt, status.Errorf(codes.Internal, "Unable to unmount the target %q : %v", target, mntErr) + } + //After successful unmount, the device is ready to be mounted. + return !notMnt, nil + } + return !notMnt, status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) + } + + if !notMnt { + klog.V(4).Infof("NodePublishVolume: Target path %q is already mounted", target) + return !notMnt, nil + } + + return !notMnt, err +} + func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeRequest, mountOptions []string, mode *csi.VolumeCapability_Mount) error { target := req.GetTargetPath() source := req.GetStagingTargetPath() @@ -624,41 +664,20 @@ func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeR } klog.V(4).Infof("NodePublishVolume: mounting %s at %s with option %s as fstype %s", source, target, mountOptions, fsType) - /* - Checking if it's a mount point. There are three cases, - 1. true, err when the directory does not exist or corrupted. - 2. false, nil when the path is already mounted with a device. - 3. true, nil when the path is not mounted with any device. - */ - notMnt, err := d.mounter.IsLikelyNotMountPoint(target) - if err != nil && !os.IsNotExist(err) { - //Checking if the path exists and error is related to Corrupted Mount, in that case, the system could unmount and mount. - _, pathErr := mountutils.PathExists(target) - if pathErr != nil && mountutils.IsCorruptedMnt(pathErr) { - klog.V(4).Infof("Target path %q is a corrupted directory", target) - } else { - return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) - } + + //Checking if the target directory is already mounted with a device. + mounted, err := d.isMounted(source, target) + + if err != nil { + return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) } - if !notMnt { - //Return error when any of the directory inside is not readable which means that a device could be mounted. - _, err = os.ReadDir(target) - if err != nil { - klog.V(4).Infof("Error occurred at reading directory %q. Trying to Unmount.", target) - //Reading the directory failed and trying to unmount and remount - if mntErr := d.mounter.Unmount(target); mntErr != nil { - return status.Errorf(codes.Internal, "Unable to unmount the target %q : %v", target, err) - } - } else { - klog.V(4).Infof("Target path %q is already mounted", target) - return nil + if !mounted { + if err := d.mounter.Mount(source, target, fsType, mountOptions); err != nil { + return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) } } - if err := d.mounter.Mount(source, target, fsType, mountOptions); err != nil { - return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) - } return nil } diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index 01eb6d7fcb..a1f01a4492 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -597,8 +597,109 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Eq([]string{"bind"})).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, nil) + + req := &csi.NodePublishVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: devicePath}, + StagingTargetPath: stagingTargetPath, + TargetPath: targetPath, + VolumeCapability: stdVolCap, + VolumeId: "vol-test", + } + + _, err := awsDriver.NodePublishVolume(context.TODO(), req) + if err != nil { + t.Fatalf("Expect no error but got: %v", err) + } + }, + }, + { + name: "success filesystem mounted already", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockMetadata := mocks.NewMockMetadataService(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + awsDriver := &nodeService{ + metadata: mockMetadata, + mounter: mockMounter, + inFlight: internal.NewInFlight(), + } + + mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) + + req := &csi.NodePublishVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: devicePath}, + StagingTargetPath: stagingTargetPath, + TargetPath: targetPath, + VolumeCapability: stdVolCap, + VolumeId: "vol-test", + } + + _, err := awsDriver.NodePublishVolume(context.TODO(), req) + if err != nil { + t.Fatalf("Expect no error but got: %v", err) + } + }, + }, + { + name: "success filesystem mountpoint error", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockMetadata := mocks.NewMockMetadataService(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + awsDriver := &nodeService{ + metadata: mockMetadata, + mounter: mockMounter, + inFlight: internal.NewInFlight(), + } + + gomock.InOrder( + mockMounter.EXPECT().PathExists(gomock.Eq(targetPath)).Return(true, nil), + ) + mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, errors.New("Internal system error")) + + req := &csi.NodePublishVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: devicePath}, + StagingTargetPath: stagingTargetPath, + TargetPath: targetPath, + VolumeCapability: stdVolCap, + VolumeId: "vol-test", + } + + _, err := awsDriver.NodePublishVolume(context.TODO(), req) + expectErr(t, err, codes.Internal) + }, + }, + { + name: "success filesystem corrupted mountpoint error", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockMetadata := mocks.NewMockMetadataService(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + awsDriver := &nodeService{ + metadata: mockMetadata, + mounter: mockMounter, + inFlight: internal.NewInFlight(), + } + + mockMounter.EXPECT().PathExists(gomock.Eq(targetPath)).Return(true, errors.New("CorruptedMntError")) + mockMounter.EXPECT().IsCorruptedMnt(gomock.Eq(errors.New("CorruptedMntError"))).Return(true) + + mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, errors.New("internal system error")) mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) + mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Eq([]string{"bind"})).Return(nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: devicePath}, @@ -631,8 +732,7 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(FSTypeXfs), gomock.Eq([]string{"bind"})).Return(nil) - mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) - mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: devicePath}, @@ -674,8 +774,7 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Eq([]string{"bind", "ro"})).Return(nil) - mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) - mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: devicePath}, @@ -709,8 +808,7 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq(targetPath)).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(stagingTargetPath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Eq([]string{"bind", "test-flag"})).Return(nil) - mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) - mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: "/dev/fake"}, @@ -761,6 +859,145 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq("/test")).Return(nil) mockMounter.EXPECT().MakeFile(targetPath).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(""), gomock.Eq([]string{"bind"})).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, nil) + + req := &csi.NodePublishVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: "/dev/fake"}, + StagingTargetPath: stagingTargetPath, + TargetPath: targetPath, + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + VolumeId: "vol-test", + } + + _, err := awsDriver.NodePublishVolume(context.TODO(), req) + if err != nil { + t.Fatalf("Expect no error but got: %v", err) + } + }, + }, + { + name: "success mounted already [raw block]", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockMetadata := mocks.NewMockMetadataService(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + awsDriver := &nodeService{ + metadata: mockMetadata, + mounter: mockMounter, + inFlight: internal.NewInFlight(), + } + + gomock.InOrder( + mockMounter.EXPECT().PathExists(gomock.Eq(devicePath)).Return(true, nil), + mockMounter.EXPECT().PathExists(gomock.Eq("/test")).Return(false, nil), + ) + mockMounter.EXPECT().MakeDir(gomock.Eq("/test")).Return(nil) + mockMounter.EXPECT().MakeFile(targetPath).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(false, nil) + + req := &csi.NodePublishVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: "/dev/fake"}, + StagingTargetPath: stagingTargetPath, + TargetPath: targetPath, + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + VolumeId: "vol-test", + } + + _, err := awsDriver.NodePublishVolume(context.TODO(), req) + if err != nil { + t.Fatalf("Expect no error but got: %v", err) + } + }, + }, + { + name: "success mountpoint error [raw block]", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockMetadata := mocks.NewMockMetadataService(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + awsDriver := &nodeService{ + metadata: mockMetadata, + mounter: mockMounter, + inFlight: internal.NewInFlight(), + } + + gomock.InOrder( + mockMounter.EXPECT().PathExists(gomock.Eq(devicePath)).Return(true, nil), + mockMounter.EXPECT().PathExists(gomock.Eq("/test")).Return(false, nil), + mockMounter.EXPECT().PathExists(gomock.Eq(targetPath)).Return(true, nil), + ) + + mockMounter.EXPECT().MakeDir(gomock.Eq("/test")).Return(nil) + mockMounter.EXPECT().MakeFile(targetPath).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, errors.New("Internal System Error")) + + req := &csi.NodePublishVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: "/dev/fake"}, + StagingTargetPath: stagingTargetPath, + TargetPath: targetPath, + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + VolumeId: "vol-test", + } + + _, err := awsDriver.NodePublishVolume(context.TODO(), req) + expectErr(t, err, codes.Internal) + }, + }, + { + name: "success corrupted mountpoint error [raw block]", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + defer mockCtl.Finish() + + mockMetadata := mocks.NewMockMetadataService(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + awsDriver := &nodeService{ + metadata: mockMetadata, + mounter: mockMounter, + inFlight: internal.NewInFlight(), + } + + gomock.InOrder( + mockMounter.EXPECT().PathExists(gomock.Eq(devicePath)).Return(true, nil), + mockMounter.EXPECT().PathExists(gomock.Eq("/test")).Return(false, nil), + mockMounter.EXPECT().PathExists(gomock.Eq(targetPath)).Return(true, errors.New("CorruptedMntError")), + ) + + mockMounter.EXPECT().IsCorruptedMnt(errors.New("CorruptedMntError")).Return(true) + + mockMounter.EXPECT().MakeDir(gomock.Eq("/test")).Return(nil) + mockMounter.EXPECT().MakeFile(targetPath).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Eq(targetPath)).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, errors.New("Internal System Error")) + mockMounter.EXPECT().Mount(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Any(), gomock.Any()).Return(nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: "/dev/fake"}, @@ -805,6 +1042,7 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq("/test")).Return(nil) mockMounter.EXPECT().MakeFile(targetPath).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(devicePathWithPartition), gomock.Eq(targetPath), gomock.Eq(""), gomock.Eq([]string{"bind"})).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: "/dev/fake"}, @@ -850,6 +1088,7 @@ func TestNodePublishVolume(t *testing.T) { mockMounter.EXPECT().MakeDir(gomock.Eq("/test")).Return(nil) mockMounter.EXPECT().MakeFile(targetPath).Return(nil) mockMounter.EXPECT().Mount(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(""), gomock.Eq([]string{"bind"})).Return(nil) + mockMounter.EXPECT().IsLikelyNotMountPoint(gomock.Eq(targetPath)).Return(true, nil) req := &csi.NodePublishVolumeRequest{ PublishContext: map[string]string{DevicePathKey: "/dev/fake"}, diff --git a/pkg/driver/sanity_test.go b/pkg/driver/sanity_test.go index b8e8a3e2e3..fc28bdf4bc 100644 --- a/pkg/driver/sanity_test.go +++ b/pkg/driver/sanity_test.go @@ -293,6 +293,10 @@ func newFakeMounter() *fakeMounter { } } +func (f *fakeMounter) IsCorruptedMnt(err error) bool { + return false +} + func (f *fakeMounter) Mount(source string, target string, fstype string, options []string) error { return nil } From 76265af6dd7942f3657c3332b5889131882d380b Mon Sep 17 00:00:00 2001 From: Nirmal Aagash Chandramohan Date: Wed, 25 Aug 2021 22:38:09 +0000 Subject: [PATCH 4/4] go formatt error fixed using go v1.17 --- pkg/cloud/aws_metrics.go | 1 + pkg/driver/node_linux.go | 1 + pkg/mounter/safe_mounter_unix.go | 1 + pkg/mounter/safe_mounter_windows.go | 1 + 4 files changed, 4 insertions(+) diff --git a/pkg/cloud/aws_metrics.go b/pkg/cloud/aws_metrics.go index 827f1d2298..5f48369abd 100644 --- a/pkg/cloud/aws_metrics.go +++ b/pkg/cloud/aws_metrics.go @@ -1,3 +1,4 @@ +//go:build !providerless // +build !providerless /* diff --git a/pkg/driver/node_linux.go b/pkg/driver/node_linux.go index 4c4a9d0955..dff94d277c 100644 --- a/pkg/driver/node_linux.go +++ b/pkg/driver/node_linux.go @@ -1,3 +1,4 @@ +//go:build linux // +build linux /* diff --git a/pkg/mounter/safe_mounter_unix.go b/pkg/mounter/safe_mounter_unix.go index a72b544748..215047b001 100644 --- a/pkg/mounter/safe_mounter_unix.go +++ b/pkg/mounter/safe_mounter_unix.go @@ -1,3 +1,4 @@ +//go:build linux || darwin // +build linux darwin /* diff --git a/pkg/mounter/safe_mounter_windows.go b/pkg/mounter/safe_mounter_windows.go index 35a3277d78..fd6b34f66b 100644 --- a/pkg/mounter/safe_mounter_windows.go +++ b/pkg/mounter/safe_mounter_windows.go @@ -1,3 +1,4 @@ +//go:build windows // +build windows /*