Bugfix 1447 clean wwids file (#61)

* Add method to remove wwid from wwids file * Call RemoveDeviceFromWWIDSFile after successful FlushDevice * Add mock methods for RemoveDeviceFromWWIDSFile * Add UT for RemoveDeviceFromWWIDSFile * Fix formatting issue
dell · Oct 14, 2024 · 06cfe99 · 06cfe99
1 parent 55d831f
commit 06cfe99
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 11 deletions.
diff --git a/base.go b/base.go
@@ -100,7 +100,7 @@ func (bc *baseConnector) disconnectDevicesByDeviceName(ctx context.Context, name
 		logger.Error(ctx, "failed to find devices by wwn: %s", err.Error())
 		return err
 	}
-	return bc.cleanDevices(ctx, false, devices)
+	return bc.cleanDevices(ctx, false, devices, wwn)
 }
 
 func (bc *baseConnector) disconnectNVMEDevicesByDeviceName(ctx context.Context, name string) error {
@@ -126,11 +126,11 @@ func (bc *baseConnector) disconnectNVMEDevicesByDeviceName(ctx context.Context,
 		logger.Error(ctx, "failed to find devices by wwn: %s", err.Error())
 		return err
 	}
-	return bc.cleanNVMeDevices(ctx, false, devices)
+	return bc.cleanNVMeDevices(ctx, false, devices, wwn)
 }
 
 func (bc *baseConnector) cleanNVMeDevices(ctx context.Context,
-	force bool, devices []string,
+	force bool, devices []string, wwn string,
 ) error {
 	defer tracer.TraceFuncCall(ctx, "baseConnector.cleanNVMeDevices")()
 	var newDevices []string
@@ -142,7 +142,7 @@ func (bc *baseConnector) cleanNVMeDevices(ctx context.Context,
 	if err != nil {
 		logger.Info(ctx, "multipath device not found: %s", err.Error())
 	} else {
-		err := bc.cleanMultipathDevice(ctx, dm)
+		err := bc.cleanMultipathDevice(ctx, dm, wwn)
 		if err != nil {
 			msg := fmt.Sprintf("failed to flush multipath device: %s", err.Error())
 			logger.Error(ctx, msg)
@@ -167,14 +167,14 @@ func (bc *baseConnector) cleanNVMeDevices(ctx context.Context,
 }
 
 func (bc *baseConnector) cleanDevices(ctx context.Context,
-	force bool, devices []string,
+	force bool, devices []string, wwn string,
 ) error {
 	defer tracer.TraceFuncCall(ctx, "baseConnector.cleanDevices")()
 	dm, err := bc.scsi.GetDMDeviceByChildren(ctx, devices)
 	if err != nil {
 		logger.Info(ctx, "multipath device not found: %s", err.Error())
 	} else {
-		err := bc.cleanMultipathDevice(ctx, dm)
+		err := bc.cleanMultipathDevice(ctx, dm, wwn)
 		if err != nil {
 			msg := fmt.Sprintf("failed to flush multipath device: %s", err.Error())
 			logger.Error(ctx, msg)
@@ -204,15 +204,23 @@ func (bc *baseConnector) cleanDevices(ctx context.Context,
 	return nil
 }
 
-func (bc *baseConnector) cleanMultipathDevice(ctx context.Context, dm string) error {
+func (bc *baseConnector) cleanMultipathDevice(ctx context.Context, dm, wwid string) error {
 	defer tracer.TraceFuncCall(ctx, "baseConnector.cleanMultipathDevice")()
 	ctx, cancelFunc := context.WithTimeout(ctx, bc.multipathFlushTimeout)
 	defer cancelFunc()
 
+	if len(wwid) == 0 {
+		wwid, _ = bc.multipath.GetDMWWID(ctx, dm)
+	}
+
 	for i := 0; i < bc.multipathFlushRetries; i++ {
 		logger.Info(ctx, "trying to flush multipath device with retries: retry %d", i)
 		err := bc.retryFlushMultipathDevice(ctx, dm)
 		if err == nil {
+			err := bc.multipath.RemoveDeviceFromWWIDSFile(ctx, wwid)
+			if err != nil {
+				logger.Error(ctx, "failed to remove wwid %s from wwids file: %s", wwid, err.Error())
+			}
 			return nil
 		}
 	}

diff --git a/fc.go b/fc.go
@@ -223,7 +223,7 @@ func (fc *FCConnector) cleanConnection(ctx context.Context, force bool, info FCV
 		}
 	}
 	logger.Info(ctx, "devices found: %s", devices)
-	return fc.baseConnector.cleanDevices(ctx, force, devices)
+	return fc.baseConnector.cleanDevices(ctx, force, devices, "")
 }
 
 func (fc *FCConnector) validateFCVolumeInfo(ctx context.Context, info FCVolumeInfo) error {

diff --git a/internal/multipath/interface.go b/internal/multipath/interface.go
@@ -27,6 +27,7 @@ type Multipath interface {
 	AddPath(ctx context.Context, path string) error
 	DelPath(ctx context.Context, path string) error
 	FlushDevice(ctx context.Context, deviceMapName string) error
+	RemoveDeviceFromWWIDSFile(ctx context.Context, wwid string) error
 	IsDaemonRunning(ctx context.Context) bool
 	GetDMWWID(ctx context.Context, deviceMapName string) (string, error)
 }
diff --git a/internal/multipath/multipath_mock.go b/internal/multipath/multipath_mock.go
diff --git a/iscsi.go b/iscsi.go
@@ -304,7 +304,7 @@ func (c *ISCSIConnector) cleanConnection(ctx context.Context, force bool, info I
 	if len(devices) == 0 {
 		return nil
 	}
-	return c.baseConnector.cleanDevices(ctx, force, devices)
+	return c.baseConnector.cleanDevices(ctx, force, devices, "")
 }
 
 func (c *ISCSIConnector) connectSingleDevice(

diff --git a/nvme.go b/nvme.go
@@ -298,7 +298,7 @@ func (c *NVMeConnector) cleanConnection(ctx context.Context, force bool, info NV
 	if len(devices) == 0 {
 		return nil
 	}
-	return c.baseConnector.cleanNVMeDevices(ctx, force, devices)
+	return c.baseConnector.cleanNVMeDevices(ctx, force, devices, wwn)
 }
 
 func (c *NVMeConnector) connectSingleDevice(ctx context.Context, info NVMeVolumeInfo, useFC bool) (Device, error) {

diff --git a/pkg/multipath/multipath.go b/pkg/multipath/multipath.go
@@ -87,6 +87,12 @@ func (mp *Multipath) FlushDevice(ctx context.Context, deviceMapName string) erro
 	return mp.flushDevice(ctx, deviceMapName)
 }
 
+// RemoveDeviceFromWWIDSFile removes multipath device from /etc/multipath/wwids file.
+func (mp *Multipath) RemoveDeviceFromWWIDSFile(ctx context.Context, wwid string) error {
+	defer tracer.TraceFuncCall(ctx, "multipath.RemoveDeviceFromWWIDSFile")()
+	return mp.removeDeviceFromWWIDSFile(ctx, wwid)
+}
+
 // IsDaemonRunning check if multipath daemon running
 func (mp *Multipath) IsDaemonRunning(ctx context.Context) bool {
 	defer tracer.TraceFuncCall(ctx, "multipath.IsDaemonRunning")()
@@ -149,6 +155,12 @@ func (mp *Multipath) flushDevice(ctx context.Context, deviceMapName string) erro
 	return err
 }
 
+func (mp *Multipath) removeDeviceFromWWIDSFile(ctx context.Context, wwid string) error {
+	logger.Info(ctx, "multipath - remove from wwids file wwid: %s", wwid)
+	_, err := mp.runCommand(ctx, multipathTool, []string{"-w", wwid})
+	return err
+}
+
 func (mp *Multipath) getDMWWID(ctx context.Context, deviceMapName string) (string, error) {
 	logger.Info(ctx, "multipath - resolve WWID: %s", deviceMapName)
 	var dataRead bool

diff --git a/pkg/multipath/multipath_test.go b/pkg/multipath/multipath_test.go
@@ -575,3 +575,67 @@ func Test_multipath_runCommand(t *testing.T) {
 		})
 	}
 }
+
+func TestMultipath_RemoveDeviceFromWWIDSFile(t *testing.T) {
+	type args struct {
+		ctx  context.Context
+		wwid string
+	}
+	ctx := context.Background()
+
+	defaultArgs := args{ctx: ctx, wwid: mh.ValidWWID}
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mocks := mh.MockHelper{
+		Ctrl:                     ctrl,
+		OSEXECCommandContextName: multipathDaemon,
+		OSEXECCommandContextArgs: []string{"-w", mh.ValidWWID},
+		OSEXECCmdOKReturn:        "ok",
+	}
+
+	tests := []struct {
+		name        string
+		fields      mpFields
+		stateSetter func(fields mpFields)
+		args        args
+		wantErr     bool
+	}{
+		{
+			name:   "ok",
+			fields: getDefaultMPFields(ctrl),
+			stateSetter: func(fields mpFields) {
+				_, cmdMock := mocks.OSExecCommandContextOK(fields.osexec)
+				mocks.OSEXECCmdOKReturn = "ok"
+				mocks.OSExecCmdOK(cmdMock)
+			},
+			args:    defaultArgs,
+			wantErr: false,
+		},
+		{
+			name:   "exec error",
+			fields: getDefaultMPFields(ctrl),
+			stateSetter: func(fields mpFields) {
+				_, cmdMock := mocks.OSExecCommandContextOK(fields.osexec)
+				mocks.OSExecCmdErr(cmdMock)
+			},
+			args:    defaultArgs,
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mp := &Multipath{
+				chroot:   tt.fields.chroot,
+				osexec:   tt.fields.osexec,
+				filePath: tt.fields.filePath,
+			}
+			tt.stateSetter(tt.fields)
+			if err := mp.RemoveDeviceFromWWIDSFile(tt.args.ctx, tt.args.wwid); (err != nil) != tt.wantErr {
+				t.Errorf("Multipath.RemoveDeviceFromWWIDSFile() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
diff --git a/rdm.go b/rdm.go
@@ -67,7 +67,7 @@ func (fc *FCConnector) cleanRDMConnection(ctx context.Context, force bool, info
 		return errors.New(msg)
 	}
 	logger.Info(ctx, "devices found: %s", devices)
-	return fc.baseConnector.cleanDevices(ctx, force, devices)
+	return fc.baseConnector.cleanDevices(ctx, force, devices, wwn)
 }
 
 func (fc *FCConnector) validateRDMVolumeInfo(ctx context.Context, info RDMVolumeInfo) error {