From 86a84e5ad6c7b2c79f896db3ac1a3475715c75a3 Mon Sep 17 00:00:00 2001 From: CSM Bot <105446864+csmbot@users.noreply.github.com> Date: Tue, 12 May 2026 15:23:22 -0400 Subject: [PATCH] Mirror internal repository with cleaned references --- README.md | 2 + base.go | 10 +- fc.go | 34 +++---- go.mod | 8 +- go.sum | 12 +-- iscsi.go | 50 +++++----- iscsi_test.go | 239 +++++++++++++++++++++++++++++++++++++++++++++-- nvme.go | 26 +++--- pkg/scsi/scsi.go | 14 +-- rdm.go | 6 +- 10 files changed, 315 insertions(+), 86 deletions(-) diff --git a/README.md b/README.md index 3fdb5f0..c2b4142 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,4 @@ + # GOBRICK **Library for iSCSI/FC/NVMe volume connection** @@ -15,3 +16,4 @@ dev, err := connector.ConnectVolume(context.Background(), err = connector.DisconnectVolumeByDeviceName(context.Background(), "dm-1") ``` + diff --git a/base.go b/base.go index 7fc291a..67d0d41 100644 --- a/base.go +++ b/base.go @@ -238,7 +238,7 @@ func (bc *baseConnector) cleanNVMeDevices(ctx context.Context, err := bc.cleanMultipathDevice(ctx, dm, wwn) if err != nil { msg := fmt.Sprintf("failed to flush multipath device: %s", err.Error()) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) if !force { return err } @@ -266,7 +266,7 @@ func (bc *baseConnector) cleanDevicesByMpathInfo(ctx context.Context, force bool err := bc.cleanMultipathDeviceByName(ctx, req.mpathName) if err != nil { msg := fmt.Sprintf("failed to flush multipath device: %s", err.Error()) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) if !force { return err } @@ -315,7 +315,7 @@ func (bc *baseConnector) cleanDevices(ctx context.Context, err := bc.cleanMultipathDevice(ctx, dm, wwn) if err != nil { msg := fmt.Sprintf("failed to flush multipath device: %s", err.Error()) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) if !force { return err } @@ -412,7 +412,7 @@ func (bc *baseConnector) getDMWWN(ctx context.Context, dm string) (string, error wwn, err := bc.multipath.GetDMWWID(ctx, dm) if err != nil { msg := fmt.Sprintf("failed to resolve DM %s WWN: %s", dm, err.Error()) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return "", errors.New(msg) } logger.Info(ctx, "WWN for DM %s is: %s", dm, wwn) @@ -438,7 +438,7 @@ func (bc *baseConnector) getNVMEDMWWN(ctx context.Context, dm string) (string, e wwn, err := bc.multipath.GetDMWWID(ctx, dm) if err != nil { msg := fmt.Sprintf("failed to resolve DM %s WWN: %s", dm, err.Error()) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return "", errors.New(msg) } logger.Info(ctx, "WWN for DM %s is: %s", dm, wwn) diff --git a/fc.go b/fc.go index 81399ee..7d107e0 100644 --- a/fc.go +++ b/fc.go @@ -160,7 +160,7 @@ func (fc *FCConnector) ConnectVolume(ctx context.Context, info FCVolumeInfo) (De } if len(hbas) == 0 { msg := "FC HBAs not found. FC is not supported on this host" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } device, err := fc.connectDevice(ctx, hbas, info) @@ -229,7 +229,7 @@ func (fc *FCConnector) cleanConnection(ctx context.Context, force bool, info FCV for _, hba := range hbas { _, hctls, err := fc.findHCTLsForFCHBA(ctx, hba, info) if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) } if len(hctls) == 0 { continue @@ -238,7 +238,7 @@ func (fc *FCConnector) cleanConnection(ctx context.Context, force bool, info FCV if hctl.IsFullInfo() { device, err := fc.scsi.GetDeviceNameByHCTL(ctx, hctl) if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) continue } devices = append(devices, device) @@ -301,7 +301,7 @@ func (fc *FCConnector) connectDevice( devices, err := getDevicesByWWNFunc(ctx, fc)(ctx, wwn) if err != nil || len(devices) == 0 { msg := "failed to get devices by WWN: " + wwn - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } @@ -311,7 +311,7 @@ func (fc *FCConnector) connectDevice( device, err = waitPowerpathDeviceFunc(ctx, fc)(ctx, wwn, devices) if err != nil { msg := "failed to find powerpath device" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } } else if !isMultipathDaemonRunningFunc(ctx, fc)(ctx) { @@ -324,13 +324,13 @@ func (fc *FCConnector) connectDevice( device, err = waitMultipathDeviceFunc(ctx, fc)(ctx, wwn, devices) if err != nil { msg := "failed to find multipath device" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } } if !checkDeviceIsValidFunc(ctx, fc)(ctx, path.Join("/dev/", device)) { msg := "multipath device was found but failed to read data from it" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } d := Device{WWN: wwn, Name: device} @@ -355,7 +355,7 @@ func (fc *FCConnector) waitSingleDevice(ctx context.Context, wwn string, devices time.Sleep(time.Second) } msg := fmt.Sprintf("timeout waiting device for wwn %s", wwn) - logger.Info(ctx, msg) + logger.Info(ctx, "%s", msg) return "", errors.New(msg) } @@ -383,7 +383,7 @@ func (fc *FCConnector) waitMultipathDevice( for _, d := range devices { devPath := path.Join("/dev/", d) if err := addPathFunc(ctx, fc)(ctx, devPath); err != nil { - logger.Info(ctx, err.Error()) + logger.Info(ctx, "%s", err.Error()) } } @@ -405,7 +405,7 @@ func (fc *FCConnector) waitMultipathDevice( } if mpath == "" { msg := fmt.Sprintf("multipath device for WWN %s not found", wwn) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return "", errors.New(msg) } logger.Info(ctx, "multipath device for WWN %s found: %s", wwn, mpath) @@ -434,7 +434,7 @@ func (fc *FCConnector) waitPowerpathDevice( } if ppath == "" { msg := fmt.Sprintf("powerpath device for WWN %s not found", wwn) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return "", errors.New(msg) } logger.Info(ctx, "powerpath device for WWN %s found: %s", wwn, ppath) @@ -467,7 +467,7 @@ func (fc *FCConnector) waitForDeviceWWN( for _, hctl := range hctlsToRescan { err := fc.scsi.RescanSCSIHostByHCTL(ctx, hctl) if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) continue } } @@ -491,7 +491,7 @@ func (fc *FCConnector) waitForDeviceWWN( "try to refresh device information", d) err := fc.scsi.RescanSCSIDeviceByHCTL(ctx, hctl) if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) } } devicesToValidate = append(devicesToValidate, d) @@ -525,7 +525,7 @@ func (fc *FCConnector) waitForDeviceWWN( } } msg := "wwn for FC device not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return "", errors.New(msg) } @@ -541,7 +541,7 @@ func (fc *FCConnector) findHCTLsForFCHBA( matches, err := fc.filePath.Glob(pattern) if err != nil { msg := fmt.Sprintf("HBA: %s failed to match FC target path: %s", hba, err.Error()) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return nil, nil, errors.New(msg) } targetMap := make(map[string]string) @@ -549,7 +549,7 @@ func (fc *FCConnector) findHCTLsForFCHBA( data, err := fc.os.ReadFile(path.Join(m, "port_name")) if err != nil { msg := fmt.Sprintf("HBA: %s failed to read port_name for FC target: %s", hba, err.Error()) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return nil, nil, errors.New(msg) } targetMap[m] = strings.Replace( @@ -609,7 +609,7 @@ func (fc *FCConnector) getFCHBASInfo(ctx context.Context) ([]FCHBA, error) { } match, err := fc.filePath.Glob("/sys/class/fc_host/host*") if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) return nil, err } var hbas []FCHBA diff --git a/go.mod b/go.mod index 9759580..690368b 100644 --- a/go.mod +++ b/go.mod @@ -1,14 +1,14 @@ module github.com/dell/gobrick -go 1.25 +go 1.26 require ( - github.com/dell/goiscsi v1.14.0 - github.com/dell/gonvme v1.13.0 + github.com/dell/goiscsi v1.15.0 + github.com/dell/gonvme v1.14.0 github.com/golang/mock v1.6.0 github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.11.0 - golang.org/x/sync v0.19.0 + golang.org/x/sync v0.20.0 ) require ( diff --git a/go.sum b/go.sum index 4c308bc..2913dc0 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ -github.com/dell/goiscsi v1.14.0 h1:kNDqOlpJ3cLSJh7Hfyn/Kz/FMCKHzV0s/xx4EqnelFw= -github.com/dell/goiscsi v1.14.0/go.mod h1:SCSC8dJCqTosU7SspaoLv6ICTKNEz08rt/I8nZ3+ptc= -github.com/dell/gonvme v1.13.0 h1:j8A1BzYA48gelih3xWd/J6LQ71CbC8Lbdyv0jG8uUNU= -github.com/dell/gonvme v1.13.0/go.mod h1:L5K7V4JZTf12m3k2wdwKwP+/eA6pr8DvlCsJU1QTGOQ= +github.com/dell/goiscsi v1.15.0 h1:71QzLLm4X8XrEkGLnZshpGEDdkgbFuZ8NiwARFwaCtY= +github.com/dell/goiscsi v1.15.0/go.mod h1:jlkRplXgeJHMZZ/dLUkWAnNcOrkIXxuibi9vDbPKYk4= +github.com/dell/gonvme v1.14.0 h1:dRyS0o+3B+cnnncgblb/H0qUJkNzjkPAq/82oqt/eMc= +github.com/dell/gonvme v1.14.0/go.mod h1:bx/tqYBKuY8SHxEpw9b8SiD/98+4TQdMYkYWES39Dgw= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -24,8 +24,8 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/iscsi.go b/iscsi.go index 1e2216d..87ecb22 100644 --- a/iscsi.go +++ b/iscsi.go @@ -226,7 +226,7 @@ func (c *ISCSIConnector) ConnectVolume(ctx context.Context, info ISCSIVolumeInfo return d, nil } msg := fmt.Sprintf("device %s found but failed to read data from it", d.Name) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) err = errors.New(msg) } logger.Error(ctx, "failed to connect volume, try to cleanup: %s", err.Error()) @@ -354,7 +354,7 @@ func (c *ISCSIConnector) connectSingleDevice( } if discoveryComplete && len(devices) == 0 { msg := "discovery complete but devices not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } if wwn == "" && len(devices) != 0 { @@ -380,7 +380,7 @@ func (c *ISCSIConnector) connectSingleDevice( } if lastTry && time.Now().After(endTime) { msg := "registered device not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } time.Sleep(time.Second) @@ -442,7 +442,7 @@ func (c *ISCSIConnector) connectPowerpathDevice( } if discoveryComplete && len(devices) == 0 { msg := "discovery complete but devices not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } if wwn == "" && len(devices) != 0 { @@ -474,7 +474,7 @@ func (c *ISCSIConnector) connectPowerpathDevice( } if lastTry && time.Now().After(endTime) { msg := "registered device not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } time.Sleep(time.Second) @@ -525,7 +525,7 @@ func (c *ISCSIConnector) connectMultipathDevice( } if discoveryComplete && len(devices) == 0 { msg := "discovery complete but devices not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } if wwn == "" && len(devices) != 0 { @@ -545,7 +545,7 @@ func (c *ISCSIConnector) connectMultipathDevice( if err := c.multipath.AddWWID(ctx, wwn); err == nil { wwnAdded = true } else { - logger.Info(ctx, err.Error()) + logger.Info(ctx, "%s", err.Error()) } } } @@ -561,14 +561,14 @@ func (c *ISCSIConnector) connectMultipathDevice( lastTry = true for _, d := range devices { if err := c.multipath.AddPath(ctx, path.Join("/dev/", d)); err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) } } endTime = time.Now().Add(c.waitDeviceRegisterTimeout) } if lastTry && time.Now().After(endTime) { msg := "registered multipath device not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } time.Sleep(time.Second) @@ -612,7 +612,7 @@ func (c *ISCSIConnector) discoverDevice( hctlFound = true hctl = resp } else { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) } } if hctlFound { @@ -620,7 +620,7 @@ func (c *ISCSIConnector) discoverDevice( numRescans++ err := c.scsi.RescanSCSIHostByHCTL(ctx, hctl) if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) } secondsNextScan = int(math.Pow(float64(numRescans+2), 2)) } @@ -633,14 +633,14 @@ func (c *ISCSIConnector) discoverDevice( "try to refresh device information", dev) err := c.scsi.RescanSCSIDeviceByHCTL(ctx, hctl) if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) } } logger.Info(ctx, "device found: %s", dev) result <- dev return } - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) } } @@ -708,13 +708,13 @@ func (c *ISCSIConnector) checkISCSISessions( if len(targetsToLogin) != 0 { newSession, err := c.tryISCSILogin(ctx, targetsToLogin, len(activeSessions) == 0) if err != nil && len(activeSessions) == 0 { - logger.Error(ctx, errMsg) + logger.Error(ctx, "%s", errMsg) return nil, errors.New(errMsg) } activeSessions = append(activeSessions, newSession...) } if len(activeSessions) == 0 { - logger.Error(ctx, errMsg) + logger.Error(ctx, "%s", errMsg) return nil, errors.New(errMsg) } logger.Info(ctx, "found active iSCSI session") @@ -729,12 +729,12 @@ func (c *ISCSIConnector) tryISCSILogin( for _, t := range targets { logPrefix := fmt.Sprintf("Portal: %s, Target: %s :", t.Portal, t.Target) tgt := goiscsi.ISCSITarget{Portal: t.Portal, Target: t.Target} - logger.Info(ctx, logPrefix+"trying login to iSCSI target") + logger.Info(ctx, "%s", logPrefix+"trying login to iSCSI target") mutexKey := strings.Join([]string{t.Portal, t.Target}, ":") if !(c.loginLock.RateCheck( mutexKey, c.failedSessionMinimumLoginRetryInterval) || force) { - logger.Error(ctx, logPrefix+"rate limit - skip login") + logger.Error(ctx, "%s", logPrefix+"rate limit - skip login") continue } err := c.iscsiLib.PerformLogin(tgt) @@ -745,15 +745,15 @@ func (c *ISCSIConnector) tryISCSILogin( s, found, err := c.getSessionByTargetInfo(ctx, t) if err != nil || !found { - logger.Error(ctx, logPrefix+"can't read session info after login") + logger.Error(ctx, "%s", logPrefix+"can't read session info after login") continue } - logger.Info(ctx, logPrefix+"successfully login to iSCSI target") + logger.Info(ctx, "%s", logPrefix+"successfully login to iSCSI target") sessions = append(sessions, s) } if len(targets) != 0 && len(sessions) == 0 { msg := "can't login to all Targets" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return nil, errors.New(msg) } return sessions, nil @@ -787,7 +787,7 @@ func (c *ISCSIConnector) tryEnableManualISCSISessionMGMT(ctx context.Context, ta } } if !c.manualSessionManagement { - logger.Error(ctx, logPrefix+"manual session management not supported") + logger.Error(ctx, "%s", logPrefix+"manual session management not supported") } return nil } @@ -820,9 +820,9 @@ func (c *ISCSIConnector) getSessionByTargetInfo(ctx context.Context, } } if found { - logger.Info(ctx, logPrefix+"iSCSI session found") + logger.Info(ctx, "%s", logPrefix+"iSCSI session found") } else { - logger.Info(ctx, logPrefix+"iSCSI session not found") + logger.Info(ctx, "%s", logPrefix+"iSCSI session not found") } return r, found, nil } @@ -843,7 +843,7 @@ func (c *ISCSIConnector) findHCTLByISCSISessionID( targetData := strings.Split(fileName, ":") if len(targetData) != 3 { msg := "can't parse values from filename" - logger.Info(ctx, msg) + logger.Info(ctx, "%s", msg) return result, errors.New(msg) } result.Host = targetData[0][6:] @@ -861,7 +861,7 @@ func (c *ISCSIConnector) findHCTLByISCSISessionID( result.Host = strings.Split(matches[0], "/")[4][4:] result.Channel = "-" result.Target = "-" - result.Lun = "-" + result.Lun = lun return result, nil } diff --git a/iscsi_test.go b/iscsi_test.go index 7df4001..b8e69eb 100644 --- a/iscsi_test.go +++ b/iscsi_test.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "reflect" + "strconv" "testing" "time" @@ -43,8 +44,8 @@ var ( validISCSITarget1 = "iqn.2015-10.com.dell:dellemc-foobar123" validISCSIPortal2 = "1.1.1.1:3260" validISCSITarget2 = "iqn.2015-10.com.dell:dellemc-spam789" - validHostOnlyIscsiHCTL1 = scsi.HCTL{Host: validSCSIHost1, Channel: "-", Target: "-", Lun: "-"} - validHostOnlyIscsiHCTL2 = scsi.HCTL{Host: validSCSIHost2, Channel: "-", Target: "-", Lun: "-"} + validHostOnlyIscsiHCTL1 = scsi.HCTL{Host: validSCSIHost1, Channel: "-", Target: "-", Lun: strconv.FormatInt(int64(validLunNumber), 10)} + validHostOnlyIscsiHCTL2 = scsi.HCTL{Host: validSCSIHost2, Channel: "-", Target: "-", Lun: strconv.FormatInt(int64(validLunNumber), 10)} validISCSITargetInfo1 = ISCSITargetInfo{ Portal: validISCSIPortal1, Target: validISCSITarget1, @@ -834,7 +835,7 @@ func TestISCSIConnector_findHCTLByISCSISessionID(t *testing.T) { wantErr bool }{ { - name: "can not parse values from filename", + name: "can not parse values from filename - insufficient parts", fields: getDefaultISCSIFields(ctrl), stateSetter: func(fields iscsiFields) { fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{"/sys/class/iscsi_host/host*/device/session1/target1"}, nil) @@ -848,7 +849,7 @@ func TestISCSIConnector_findHCTLByISCSISessionID(t *testing.T) { wantErr: true, }, { - name: "Failed to resolve glob pattern", + name: "Failed to resolve glob pattern - target path", fields: getDefaultISCSIFields(ctrl), stateSetter: func(fields iscsiFields) { fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{}, errors.New("failed to resolve HCTL")) @@ -861,6 +862,186 @@ func TestISCSIConnector_findHCTLByISCSISessionID(t *testing.T) { want: scsi.HCTL{}, wantErr: true, }, + { + name: "empty sessionID", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{}, nil) + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{}, nil) + }, + args: args{ + ctx: ctx, + sessionID: "", + lun: "1", + }, + want: scsi.HCTL{}, + wantErr: true, + }, + { + name: "empty lun parameter", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host34/device/session12/target34:0:0", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "12", + lun: "", + }, + want: scsi.HCTL{Host: "34", Channel: "0", Target: "0", Lun: ""}, + wantErr: false, + }, + { + name: "successful target resolution - full HCTL", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host34/device/session12/target34:0:0", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "12", + lun: "9", + }, + want: scsi.HCTL{Host: "34", Channel: "0", Target: "0", Lun: "9"}, + wantErr: false, + }, + { + name: "successful target resolution - multi-digit host and target", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host128/device/session256/target128:15:1024", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "256", + lun: "512", + }, + want: scsi.HCTL{Host: "128", Channel: "15", Target: "1024", Lun: "512"}, + wantErr: false, + }, + { + name: "host-only fallback - target pattern fails", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + // First call (target pattern) returns empty + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{}, nil) + // Second call (session pattern) succeeds + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host42/device/session99", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "99", + lun: "7", + }, + want: scsi.HCTL{Host: "42", Channel: "-", Target: "-", Lun: "7"}, + wantErr: false, + }, + { + name: "host-only fallback - session pattern fails", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + // First call (target pattern) returns empty + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{}, nil) + // Second call (session pattern) fails + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{}, errors.New("session pattern failed")) + }, + args: args{ + ctx: ctx, + sessionID: "99", + lun: "7", + }, + want: scsi.HCTL{}, + wantErr: true, + }, + { + name: "malformed target filename - too many parts", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host*/device/session1/target34:0:0:extra", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "1", + lun: "1", + }, + want: scsi.HCTL{}, + wantErr: true, + }, + { + name: "multiple target matches - should use first one", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host34/device/session12/target34:0:0", + "/sys/class/iscsi_host/host34/device/session12/target34:1:1", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "12", + lun: "9", + }, + want: scsi.HCTL{Host: "34", Channel: "0", Target: "0", Lun: "9"}, + wantErr: false, + }, + { + name: "very long sessionID and lun", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host99999/device/session123456789/target99999:255:65535", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "123456789", + lun: "999999", + }, + want: scsi.HCTL{Host: "99999", Channel: "255", Target: "65535", Lun: "999999"}, + wantErr: false, + }, + { + name: "special characters in sessionID (should still work)", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host1/device/sessionabc-def/target1:0:0", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "abc-def", + lun: "1", + }, + want: scsi.HCTL{Host: "1", Channel: "0", Target: "0", Lun: "1"}, + wantErr: false, + }, + { + name: "zero values in HCTL fields", + fields: getDefaultISCSIFields(ctrl), + stateSetter: func(fields iscsiFields) { + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host0/device/session0/target0:0:0", + }, nil) + }, + args: args{ + ctx: ctx, + sessionID: "0", + lun: "0", + }, + want: scsi.HCTL{Host: "0", Channel: "0", Target: "0", Lun: "0"}, + wantErr: false, + }, } for _, tt := range tests { @@ -888,13 +1069,59 @@ func TestISCSIConnector_findHCTLByISCSISessionID(t *testing.T) { t.Errorf("findHCTLByISCSISessionID() error = %v, wantErr %v", err, tt.wantErr) return } - if got != tt.want { - t.Errorf("findHCTLByISCSISessionID() got = %v, want %v", got, tt.want) + if !tt.wantErr { + assertHCTLEqual(t, got, tt.want) } }) } } +// Additional test for context cancellation behavior +func TestISCSIConnector_findHCTLByISCSISessionID_ContextCancellation(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Create a cancelled context + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately + + fields := getDefaultISCSIFields(ctrl) + + // Setup mock to return a valid result (but context is already cancelled) + fields.filePath.EXPECT().Glob(gomock.Any()).Return([]string{ + "/sys/class/iscsi_host/host34/device/session12/target34:0:0", + }, nil) + + c := &ISCSIConnector{ + filePath: fields.filePath, + } + + // The function should still work even with cancelled context since it doesn't check context + got, err := c.findHCTLByISCSISessionID(ctx, "12", "9") + if err != nil { + t.Errorf("Unexpected error with cancelled context: %v", err) + } + want := scsi.HCTL{Host: "34", Channel: "0", Target: "0", Lun: "9"} + assertHCTLEqual(t, got, want) +} + +// Test helper function to validate HCTL equality with better error messages +func assertHCTLEqual(t *testing.T, got, want scsi.HCTL) { + t.Helper() + if got.Host != want.Host { + t.Errorf("Host field mismatch: got = %v, want = %v", got.Host, want.Host) + } + if got.Channel != want.Channel { + t.Errorf("Channel field mismatch: got = %v, want = %v", got.Channel, want.Channel) + } + if got.Target != want.Target { + t.Errorf("Target field mismatch: got = %v, want = %v", got.Target, want.Target) + } + if got.Lun != want.Lun { + t.Errorf("Lun field mismatch: got = %v, want = %v", got.Lun, want.Lun) + } +} + func TestAddDefaultISCSIPortToVolumeInfoPortals(t *testing.T) { // Test case 1: When the portal doesn't contain a port t.Run("Portal without port", func(t *testing.T) { diff --git a/nvme.go b/nvme.go index 8b9b5f6..3f75fff 100644 --- a/nvme.go +++ b/nvme.go @@ -226,7 +226,7 @@ func (c *NVMeConnector) ConnectVolume(ctx context.Context, info NVMeVolumeInfo, return d, nil } msg := fmt.Sprintf("device %s found but failed to read data from it", d.Name) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) err = errors.New(msg) } logger.Error(ctx, "failed to connect volume, try to cleanup: %s", err.Error()) @@ -343,12 +343,12 @@ func (c *NVMeConnector) connectSingleDevice(ctx context.Context, info NVMeVolume } if discoveryComplete && len(devices) == 0 { msg := "discovery complete but devices not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } if wwn == "" && len(devices) != 0 { msg := "invalid WWN provided" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } if wwn != "" && nguid != "" { @@ -365,7 +365,7 @@ func (c *NVMeConnector) connectSingleDevice(ctx context.Context, info NVMeVolume } if lastTry && time.Now().After(endTime) { msg := "registered device not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } time.Sleep(time.Second) @@ -419,7 +419,7 @@ func (c *NVMeConnector) connectMultipathDevice( } if discoveryComplete && len(devices) == 0 { msg := "discover complete but devices not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } if wwn == "" && len(devices) != 0 { @@ -436,7 +436,7 @@ func (c *NVMeConnector) connectMultipathDevice( if err := c.multipath.AddWWID(ctx, wwn); err == nil { wwnAdded = true } else { - logger.Info(ctx, err.Error()) + logger.Info(ctx, "%s", err.Error()) } } } @@ -454,14 +454,14 @@ func (c *NVMeConnector) connectMultipathDevice( lastTry = true for _, d := range devices { if err := c.multipath.AddPath(ctx, path.Join("/dev/", d)); err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) } } endTime = time.Now().Add(c.waitDeviceRegisterTimeout) } if lastTry && time.Now().After(endTime) { msg := "registered multipath device not found" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } time.Sleep(time.Second) @@ -519,7 +519,7 @@ func (c *NVMeConnector) discoverDevice(ctx context.Context, wg *sync.WaitGroup, } } devicePathResult = DevicePathResult{devicePaths: devicePaths, nguid: nguidResult} - logger.Debug(ctx, "devicePathResult", devicePathResult, "nguidResult", nguidResult, "retryCount", retryCount) + logger.Debug(ctx, "devicePathResult: %v nguidResult: %v retryCount: %v", devicePathResult, nguidResult, retryCount) if nguidResult != "" || retryCount == 1 { break } @@ -651,7 +651,7 @@ func (c *NVMeConnector) checkNVMeSessions( errMsg := "can't find active nvme session" if len(activeSessions) == 0 { - logger.Error(ctx, errMsg) + logger.Error(ctx, "%s", errMsg) return nil, errors.New(errMsg) } logger.Info(ctx, "found active nvme sessions") @@ -679,9 +679,9 @@ func (c *NVMeConnector) getSessionByTargetInfo(ctx context.Context, } } if found { - logger.Info(ctx, logPrefix+"nvme session found") + logger.Info(ctx, "%s", logPrefix+"nvme session found") } else { - logger.Info(ctx, logPrefix+"nvme session not found") + logger.Info(ctx, "%s", logPrefix+"nvme session not found") } return r, found, nil } @@ -691,7 +691,7 @@ func (c *NVMeConnector) getFCHostInfo(ctx context.Context) ([]FCHBAInfo, error) logger.Info(ctx, "get FC hbas info") match, err := c.filePath.Glob("/sys/class/fc_host/host*") if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) return nil, err } diff --git a/pkg/scsi/scsi.go b/pkg/scsi/scsi.go index 5b53f1a..2105411 100644 --- a/pkg/scsi/scsi.go +++ b/pkg/scsi/scsi.go @@ -321,7 +321,7 @@ func (s *Scsi) deleteSCSIDeviceByPath(ctx context.Context, devPath string) error logger.Info(ctx, "device state is: %s", deviceState) if deviceState == "blocked" { msg := "device is in blocked state" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return errors.New(msg) } deleteFile, err := s.os.OpenFile(deletePath, os.O_APPEND|os.O_WRONLY, 0o200) @@ -462,7 +462,7 @@ func (s *Scsi) getDevicesByWWN(ctx context.Context, wwn string) ([]string, error return result, nil }) if err != nil { - logger.Error(ctx, err.Error()) + logger.Error(ctx, "%s", err.Error()) return nil, err } devs := ret.(map[string][]string) @@ -517,7 +517,7 @@ func (s *Scsi) getDeviceNameByHCTL(ctx context.Context, h HCTL) (string, error) msg := fmt.Sprintf("can't match block device with provided HCTL, "+ "%s %s %s %s", h.Host, h.Channel, h.Target, h.Lun) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return "", errors.New(msg) } // Sort devices and return the first so we don't return a partition @@ -558,13 +558,13 @@ func (s *Scsi) waitUdevSymlink(ctx context.Context, deviceName string, wwn strin symlink, err := s.filePath.EvalSymlinks(checkPath) if err != nil { msg := fmt.Sprintf("symlink for path %s not found: %s", checkPath, err.Error()) - logger.Info(ctx, msg) + logger.Info(ctx, "%s", msg) return errors.New(msg) } log.Debugf("check path: %s, symlink: %s for wwn: %s", checkPath, symlink, wwn) if d := strings.Replace(symlink, "/dev/", "", 1); d != deviceName { msg := fmt.Sprintf("udev symlink point to unexpected device: %s", d) - logger.Info(ctx, msg) + logger.Info(ctx, "%s", msg) return errors.New(msg) } logger.Info(ctx, "udev symlink for %s with WWN %s found", deviceName, wwn) @@ -594,12 +594,12 @@ func (s *Scsi) waitUdevSymlinkNVMe(ctx context.Context, deviceName string, wwn s symlink, err := s.GetNVMESymlink(checkPath) if err != nil { msg := fmt.Sprintf("symlink for path %s not found: %s", checkPath, err.Error()) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return errors.New(msg) } if d := strings.Replace(symlink, "/dev/", "", 1); d != deviceName { msg := fmt.Sprintf("udev symlink point to unexpected device: %s", d) - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return errors.New(msg) } logger.Info(ctx, "udev symlink for %s with WWN %s found", deviceName, wwn) diff --git a/rdm.go b/rdm.go index 3109583..9b5f035 100644 --- a/rdm.go +++ b/rdm.go @@ -63,7 +63,7 @@ func (fc *FCConnector) cleanRDMConnection(ctx context.Context, force bool, info devices, err := fc.scsi.GetDevicesByWWN(ctx, wwn) if err != nil || len(devices) == 0 { msg := "failed to get devices by WWN: " + wwn - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return errors.New(msg) } logger.Info(ctx, "devices found: %s", devices) @@ -91,7 +91,7 @@ func (fc *FCConnector) connectRDM(ctx context.Context, info RDMVolumeInfo) (Devi devices, err := fc.scsi.GetDevicesByWWN(ctx, wwn) if err != nil || len(devices) == 0 { msg := "failed to get devices by WWN: " + wwn - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } var device string @@ -102,7 +102,7 @@ func (fc *FCConnector) connectRDM(ctx context.Context, info RDMVolumeInfo) (Devi if !fc.scsi.CheckDeviceIsValid(ctx, path.Join("/dev/", device)) { msg := "device invalid" - logger.Error(ctx, msg) + logger.Error(ctx, "%s", msg) return Device{}, errors.New(msg) } d := Device{WWN: wwn, Name: device}