[ISSUE-1045] Fake Attach Feature for Non-LVM-Block-Mode Volumes (#1046)

* fix pr validation startup failure Signed-off-by: Shi, Crane <[email protected]> * directly use generated file for fake attach block mode Signed-off-by: Shi, Crane <[email protected]> * still use loopback device wrap Signed-off-by: Shi, Crane <[email protected]> * refine code for creating fake device Signed-off-by: Shi, Crane <[email protected]> * fix go lint error Signed-off-by: Shi, Crane <[email protected]> * add mock func implementation Signed-off-by: Shi, Crane <[email protected]> * fix go lint Signed-off-by: Shi, Crane <[email protected]> * fix UT Signed-off-by: Shi, Crane <[email protected]> * refine func Signed-off-by: Shi, Crane <[email protected]> * support fake attach block mode with fake device, add removeLoopDevice, Signed-off-by: Shi, Crane <[email protected]> * fix go lint Signed-off-by: Shi, Crane <[email protected]> * refine Signed-off-by: Shi, Crane <[email protected]> * support clean fake device in fake-attach block-mode Signed-off-by: Shi, Crane <[email protected]> * support non-existing current fake device case Signed-off-by: Shi, Crane <[email protected]> * change fake device dir on host Signed-off-by: Shi, Crane <[email protected]> * refine log Signed-off-by: Shi, Crane <[email protected]> * support the case of get fake device info failure Signed-off-by: Shi, Crane <[email protected]> * clean fake device also in removal of fake-attach block-mode vol Signed-off-by: Shi, Crane <[email protected]> * if fake-device ann is invalid, re-create the fake device and update ann Signed-off-by: Shi, Crane <[email protected]> * fix Signed-off-by: Shi, Crane <[email protected]> * enhance Signed-off-by: Shi, Crane <[email protected]> * refine Signed-off-by: Shi, Crane <[email protected]> * refine Signed-off-by: Shi, Crane <[email protected]> * add comment Signed-off-by: Shi, Crane <[email protected]> * add UT Signed-off-by: Shi, Crane <[email protected]> * add UT Signed-off-by: Shi, Crane <[email protected]> * add UT Signed-off-by: Shi, Crane <[email protected]> * add UT Signed-off-by: Shi, Crane <[email protected]> * check loop device err shouldn't block subsequent op; clean fake device should also check loop device first Signed-off-by: Shi, Crane <[email protected]> * update fake-attach doc accordingly Signed-off-by: Shi, Crane <[email protected]> * fix typo Signed-off-by: Shi, Crane <[email protected]> * refine doc Signed-off-by: Shi, Crane <[email protected]> --------- Signed-off-by: Shi, Crane <[email protected]>
dell · Aug 17, 2023 · 4c3fd07 · 4c3fd07
1 parent 8085a0a
commit 4c3fd07
Show file tree

Hide file tree

Showing 11 changed files with 634 additions and 71 deletions.
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
@@ -107,21 +107,23 @@ jobs:
         make install-controller-gen
         make generate-deepcopy
 
-    - name: Verify Changed files
-      uses: tj-actions/[email protected]
-      id: changed_files
-      with:
-        files: |
-           api/generated/v1/*.go
-           api/v1/*/*.go
-           '.(go)$'
-           
-    - name: Display changed files
-      if: steps.changed_files.outputs.files_changed == 'true'
-      run: |
-        echo "Changed files: ${{ steps.changed_files.outputs.changed_files }}"
-        
-    - name: Perform action when files change.
-      if: steps.changed_files.outputs.files_changed == 'true'
-      run: |
-        exit 1
+# Temporarily comment out the forbidden 3rd-party action script from our github PR validation workflow
+# to fix our github PR validation startup failure.
+#    - name: Verify Changed files
+#      uses: tj-actions/[email protected]
+#      id: changed_files
+#      with:
+#        files: |
+#           api/generated/v1/*.go
+#           api/v1/*/*.go
+#           '.(go)$'
+#
+#    - name: Display changed files
+#      if: steps.changed_files.outputs.files_changed == 'true'
+#      run: |
+#        echo "Changed files: ${{ steps.changed_files.outputs.changed_files }}"
+#
+#    - name: Perform action when files change.
+#      if: steps.changed_files.outputs.files_changed == 'true'
+#      run: |
+#        exit 1
diff --git a/docs/fake-attach.md b/docs/fake-attach.md
@@ -35,22 +35,25 @@ If fake-attach volume is successfully created, CSI Volume CR will be annotated w
 ### NodeStageVolume
 When `pv.attach.kubernetes.io/ignore-if-inaccessible: yes` annotation is set CSI must:
 - ignore NodeStageVolume errors
-- put `fake-attach: yes` annotation on CSI Volume CR if error is and annotation wasn't set before
-- delete `fake-attach: yes` annotation on CSI Volume CR if error is not and annotation was set before
+- put `fake-attach: yes` annotation on CSI Volume CR if there is stageVolume error and annotation wasn't set before, i.e. the volume turns from healthy to unhealthy
+- if there is stageVolume error on block-mode volume without `fake-device: <device path>` annotation, try to setup a fake loop device mapped to regular file, add `fake-device: <device path>` annotation to volume, and then try to mount this fake device to stagingTargetPath as normal stage volume
+- if there is stageVolume error on block-mode volume with `fake-device: <device path>` annotation, check whether this device is really the fake device of this volume first. if the check passed, also try to mount this fake device to stagingTargetPath as normal stage volume
+- delete `fake-attach: yes` annotation on CSI Volume CR if there is no stageVolume error and annotation was set before, i.e. the volume turns from unhealthy back to healthy
+- In this scenario, for block-mode volume with `fake-device: <device path>` annotation, if this device is really the fake device of this volume, then try to clean this fake device. By the way, the removal of block-mode volume with annotations `fake-attach: yes` and `fake-device: <device path>` will also trigger the clean of fake device.
 
 Event FakeAttachInvolved generated when `fake-attach: yes` annotation is setting.
 
 Event FakeAttachCleared generated when `fake-attach: yes` annotation is deleting.
 
 ### NodePublishVolume 
-CSI must check `fake-attach` annotation and mount tmpfs volume in read-write mode.
+CSI must check `fake-attach` annotation and mount tmpfs volume in read-write mode for fs-mode volume
 
 Command to mount tmpfs volume: `mount -t tmpfs -o size=1M,rw <volumeID> <destination folder>`
 
 rw option is used as workaround for issue-906 (OpenShift 4.8)
 
 ### NodeUnpublishVolume 
-tmpfs volume must be unmounted usually.
+tmpfs volume must be unmounted usually for fs-mode volume.
 
 ### NodeUnstageVolume 
 Do nothing.

diff --git a/pkg/base/linuxutils/fs/fs_helper.go b/pkg/base/linuxutils/fs/fs_helper.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -42,6 +43,8 @@ const (
 
 	// wipefs is a system utility
 	wipefs = "wipefs "
+	// losetupCmd is base cmd to operate loop devices, losetup
+	losetupCmd = "losetup "
 	// CheckSpaceCmdImpl cmd for getting space on the mounted FS, produce output in megabytes (--block-size=M)
 	CheckSpaceCmdImpl = "df %s --output=target,avail --block-size=M" // add mounted fs part
 	// MkFSCmdTmpl mkfs command template
@@ -74,6 +77,17 @@ const (
 	BindOption = "--bind"
 	// MountOptionsFlag flag to set mount options
 	MountOptionsFlag = "-o"
+	// CreateFileByDDCmdTmpl cmd for creating file with specified size by dd command
+	CreateFileByDDCmdTmpl = "dd if=/dev/zero of=%s bs=1M count=%s"
+	// ReadLoopBackDeviceMappingCmd cmd for reading loopback device mapping
+	ReadLoopBackDeviceMappingCmd = losetupCmd + "-O NAME,BACK-FILE %s"
+	// SetupLoopBackDeviceCmdTmpl cmd for loopback device setup
+	SetupLoopBackDeviceCmdTmpl = losetupCmd + "-f --show %s"
+	// DetachLoopBackDeviceCmdTmpl cmd for loopback device detach
+	DetachLoopBackDeviceCmdTmpl = losetupCmd + "-d %s"
+
+	// NoSuchDeviceErrMsg is the err msg in stderr of the cmd output when specified device cannot be found
+	NoSuchDeviceErrMsg = "No such device"
 )
 
 // WrapFS is an interface that encapsulates operation with file systems
@@ -91,6 +105,10 @@ type WrapFS interface {
 	FindMountPoint(target string) (string, error)
 	Mount(src, dst string, opts ...string) error
 	Unmount(src string) error
+	CreateFileWithSizeInMB(filePath string, sizeMB int) error
+	ReadLoopDevice(device string) (string, error)
+	CreateLoopDevice(src string) (string, error)
+	RemoveLoopDevice(device string) error
 }
 
 // WrapFSImpl is a WrapFS implementer
@@ -336,3 +354,79 @@ func (h *WrapFSImpl) GetFSUUID(device string) (string, error) {
 	}
 	return strings.TrimSpace(stdout), err
 }
+
+// CreateFileWithSizeInMB creates file with specified size in MB unit by dd command
+// Receives the file path and size in MB unit
+// Returns error if something went wrong
+func (h *WrapFSImpl) CreateFileWithSizeInMB(filePath string, sizeMB int) error {
+	err := h.MkDir(path.Dir(filePath))
+	if err != nil {
+		return fmt.Errorf("failed to create parent dir of file %s: %w", filePath, err)
+	}
+
+	cmd := fmt.Sprintf(CreateFileByDDCmdTmpl, filePath, strconv.Itoa(sizeMB))
+	_, _, err = h.e.RunCmd(cmd,
+		command.UseMetrics(true),
+		command.CmdName(fmt.Sprintf(CreateFileByDDCmdTmpl, "", "")))
+	if err != nil {
+		return fmt.Errorf("failed to create file %s with size %d MB: %w", filePath, sizeMB, err)
+	}
+	return nil
+}
+
+// ReadLoopDevice get loopback device's mapped backing file
+// Receives the specified device's path
+// Returns the loopback device's mapped backing file or empty string and error if something went wrong
+func (h *WrapFSImpl) ReadLoopDevice(device string) (string, error) {
+	cmd := fmt.Sprintf(ReadLoopBackDeviceMappingCmd, device)
+	returnedErrPrefix := fmt.Sprintf("failed to read loopback device %s", device)
+
+	stdout, stderr, err := h.e.RunCmd(cmd,
+		command.UseMetrics(true),
+		command.CmdName(strings.TrimSpace(fmt.Sprintf(ReadLoopBackDeviceMappingCmd, ""))))
+
+	if err != nil {
+		if strings.Contains(stderr, NoSuchDeviceErrMsg) {
+			return "", nil
+		}
+		return "", fmt.Errorf("%s: %w", returnedErrPrefix, err)
+	}
+
+	lines := strings.Split(stdout, "\n")
+	if len(lines) < 2 || len(lines[1]) == 0 {
+		return "", fmt.Errorf("%s: invalid command stdout", returnedErrPrefix)
+	}
+	dataFields := strings.SplitN(lines[1], " ", 2)
+	if len(dataFields) == 1 {
+		return "", nil
+	}
+	return strings.TrimSpace(dataFields[1]), nil
+}
+
+// CreateLoopDevice create loopback device mapped to specified src
+// Receives the file path of the specified src, whether a regular file or block device
+// Returns the loopback device's file path or empty string and error if something went wrong
+func (h *WrapFSImpl) CreateLoopDevice(src string) (string, error) {
+	cmd := fmt.Sprintf(SetupLoopBackDeviceCmdTmpl, src)
+	stdout, _, err := h.e.RunCmd(cmd,
+		command.UseMetrics(true),
+		command.CmdName(strings.TrimSpace(fmt.Sprintf(SetupLoopBackDeviceCmdTmpl, ""))))
+	if err != nil {
+		return "", fmt.Errorf("failed to create loopback device for %s: %w", src, err)
+	}
+	return strings.TrimSpace(stdout), nil
+}
+
+// RemoveLoopDevice remove the specified loopback device
+// Receives the loop device path
+// Returns error if something went wrong
+func (h *WrapFSImpl) RemoveLoopDevice(device string) error {
+	cmd := fmt.Sprintf(DetachLoopBackDeviceCmdTmpl, device)
+	_, _, err := h.e.RunCmd(cmd,
+		command.UseMetrics(true),
+		command.CmdName(strings.TrimSpace(fmt.Sprintf(DetachLoopBackDeviceCmdTmpl, ""))))
+	if err != nil {
+		return fmt.Errorf("failed to remove loopback device %s: %w", device, err)
+	}
+	return nil
+}
diff --git a/pkg/base/linuxutils/fs/fs_helper_test.go b/pkg/base/linuxutils/fs/fs_helper_test.go
@@ -19,6 +19,9 @@ package fs
 import (
 	"errors"
 	"fmt"
+	"path"
+	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -284,3 +287,92 @@ func Test_GetFSUUID(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, testUUID, uuid)
 }
+
+func TestCreateFileWithSizeInMB(t *testing.T) {
+	var (
+		e        = &mocks.GoMockExecutor{}
+		filePath = "/fake/pvc-a"
+		sizeMB   = 1
+		fh       = NewFSImpl(e)
+		mkdirCmd = fmt.Sprintf(MkDirCmdTmpl, path.Dir(filePath))
+		ddCmd    = fmt.Sprintf(CreateFileByDDCmdTmpl, filePath, strconv.Itoa(sizeMB))
+		err      error
+	)
+
+	e.OnCommand(mkdirCmd).Return("", "", nil).Times(2)
+	e.OnCommand(ddCmd).Return("", "", nil).Times(1)
+	err = fh.CreateFileWithSizeInMB(filePath, sizeMB)
+	assert.Nil(t, err)
+
+	e.OnCommand(ddCmd).Return("", "", testError).Times(1)
+	err = fh.CreateFileWithSizeInMB(filePath, sizeMB)
+	assert.NotNil(t, err)
+
+	e.OnCommand(mkdirCmd).Return("", "", testError).Times(1)
+	err = fh.CreateFileWithSizeInMB(filePath, sizeMB)
+	assert.NotNil(t, err)
+}
+
+func TestReadLoopDevice(t *testing.T) {
+	var (
+		e                 = &mocks.GoMockExecutor{}
+		fh                = NewFSImpl(e)
+		device            = "/dev/loop2"
+		deviceBackFile    = "/fake/pvc-a"
+		cmd               = fmt.Sprintf(ReadLoopBackDeviceMappingCmd, device)
+		stdout            = fmt.Sprintf("NAME BACK-FILE\n%s %s\n", device, deviceBackFile)
+		returnedErrPrefix = fmt.Sprintf("failed to read loopback device %s", device)
+	)
+
+	e.OnCommand(cmd).Return(stdout, "", nil).Times(1)
+	backFile, err := fh.ReadLoopDevice(device)
+	assert.Nil(t, err)
+	assert.Equal(t, backFile, deviceBackFile)
+
+	e.OnCommand(cmd).Return("", NoSuchDeviceErrMsg, testError).Times(1)
+	backFile, err = fh.ReadLoopDevice(device)
+	assert.Nil(t, err)
+	assert.Empty(t, backFile)
+
+	e.OnCommand(cmd).Return("", "os error", testError).Times(1)
+	backFile, err = fh.ReadLoopDevice(device)
+	assert.NotNil(t, err)
+	assert.True(t, strings.HasSuffix(err.Error(), testError.Error()))
+	assert.True(t, strings.HasPrefix(err.Error(), returnedErrPrefix))
+	assert.Empty(t, backFile)
+
+	stdout = "NAME BACK-FILE\n"
+	e.OnCommand(cmd).Return(stdout, "", nil).Times(1)
+	backFile, err = fh.ReadLoopDevice(device)
+	assert.NotNil(t, err)
+	assert.True(t, strings.HasSuffix(err.Error(), "invalid command stdout"))
+	assert.True(t, strings.HasPrefix(err.Error(), returnedErrPrefix))
+	assert.Empty(t, backFile)
+
+	stdout = fmt.Sprintf("NAME BACK-FILE\n%s\n", device)
+	e.OnCommand(cmd).Return(stdout, "", nil).Times(1)
+	backFile, err = fh.ReadLoopDevice(device)
+	assert.Nil(t, err)
+	assert.Empty(t, backFile)
+}
+
+func TestCreateLoopDevice(t *testing.T) {
+	var (
+		e      = &mocks.GoMockExecutor{}
+		fh     = NewFSImpl(e)
+		src    = "/fake/pvc-a"
+		cmd    = fmt.Sprintf(SetupLoopBackDeviceCmdTmpl, src)
+		stdout = "/dev/loop2"
+	)
+
+	e.OnCommand(cmd).Return(stdout, "", nil).Times(1)
+	loopDev, err := fh.CreateLoopDevice(src)
+	assert.Nil(t, err)
+	assert.Equal(t, loopDev, stdout)
+
+	// cmd failed
+	e.OnCommand(cmd).Return("", "", testError).Times(1)
+	loopDev, err = fh.CreateLoopDevice(src)
+	assert.NotNil(t, err)
+	assert.Empty(t, loopDev)
+}
diff --git a/pkg/mocks/linuxutils/fs_helper.go b/pkg/mocks/linuxutils/fs_helper.go
@@ -110,3 +110,31 @@ func (m *MockWrapFS) Unmount(src string) error {
 
 	return args.Error(0)
 }
+
+// CreateFileWithSizeInMB is a mock implementation
+func (m *MockWrapFS) CreateFileWithSizeInMB(filePath string, sizeMB int) error {
+	args := m.Mock.Called(filePath, sizeMB)
+
+	return args.Error(0)
+}
+
+// ReadLoopDevice is a mock implementation
+func (m *MockWrapFS) ReadLoopDevice(device string) (string, error) {
+	args := m.Mock.Called(device)
+
+	return args.String(0), args.Error(1)
+}
+
+// CreateLoopDevice is a mock implementation
+func (m *MockWrapFS) CreateLoopDevice(src string) (string, error) {
+	args := m.Mock.Called(src)
+
+	return args.String(0), args.Error(1)
+}
+
+// RemoveLoopDevice is a mock implementation
+func (m *MockWrapFS) RemoveLoopDevice(device string) error {
+	args := m.Mock.Called(device)
+
+	return args.Error(0)
+}
diff --git a/pkg/mocks/provisioners/fs_operations.go b/pkg/mocks/provisioners/fs_operations.go
@@ -53,3 +53,10 @@ func (m *MockFsOpts) CreateFSIfNotExist(fsType fs.FileSystem, device, uuid strin
 
 	return args.Error(0)
 }
+
+// CreateFakeDevice is a mock implementation
+func (m *MockFsOpts) CreateFakeDevice(src string) (string, error) {
+	args := m.Mock.Called(src)
+
+	return args.String(0), args.Error(1)
+}
diff --git a/pkg/node/common_test_data.go b/pkg/node/common_test_data.go
@@ -60,17 +60,20 @@ var (
 	testCtx    = context.Background()
 	disk1      = api.Drive{UUID: uuid.New().String(), SerialNumber: "hdd1", Size: 1024 * 1024 * 1024 * 500, NodeId: nodeID, Path: "/dev/sda"}
 	disk2      = api.Drive{UUID: uuid.New().String(), SerialNumber: "hdd2", Size: 1024 * 1024 * 1024 * 200, NodeId: nodeID, Path: "/dev/sda"}
+	disk4      = api.Drive{UUID: uuid.New().String(), SerialNumber: "hdd4", Size: 1024 * 1024 * 1024 * 500, NodeId: nodeID, Path: "/dev/sdb"}
 	// volumes
 	testV1ID = "volume-1-id"
 	testV2ID = "volume-2-id"
 	testV3ID = "volume-3-id"
+	testV4ID = "volume-4-id"
 
 	testVolume1 = api.Volume{
 		Id:           testV1ID,
 		NodeId:       nodeID,
 		Location:     disk1.UUID,
 		StorageClass: apiV1.StorageClassHDD,
 		CSIStatus:    apiV1.VolumeReady,
+		Mode:         apiV1.ModeFS,
 	}
 	testVolume2 = api.Volume{
 		Id:           testV2ID,
@@ -80,6 +83,14 @@ var (
 		CSIStatus:    apiV1.Created,
 	}
 	testVolume3 = api.Volume{Id: testV3ID, NodeId: nodeID, Location: ""}
+	testVolume4 = api.Volume{
+		Id:           testV4ID,
+		NodeId:       nodeID,
+		Location:     disk4.UUID,
+		StorageClass: apiV1.StorageClassHDD,
+		CSIStatus:    apiV1.VolumeReady,
+		Mode:         apiV1.ModeRAWPART,
+	}
 
 	testVolumeCR1 = vcrd.Volume{
 		TypeMeta: k8smetav1.TypeMeta{Kind: "Volume", APIVersion: apiV1.APIV1Version},
@@ -108,6 +119,15 @@ var (
 		},
 		Spec: testVolume3,
 	}
+	testVolumeCR4 = vcrd.Volume{
+		TypeMeta: k8smetav1.TypeMeta{Kind: "Volume", APIVersion: apiV1.APIV1Version},
+		ObjectMeta: k8smetav1.ObjectMeta{
+			Name:              testVolume4.Id,
+			Namespace:         testNs,
+			CreationTimestamp: k8smetav1.Time{Time: time.Now()},
+		},
+		Spec: testVolume4,
+	}
 
 	testVolumeCap = &csi.VolumeCapability{
 		AccessType: &csi.VolumeCapability_Mount{