addressing the review comment

gonvme.go

@@ -19,8 +19,6 @@
 package gonvme
 
 import (
-	"time"
-
 	"github.com/dell/gonvme/internal/logger"
 	"github.com/dell/gonvme/internal/tracer"
 )
@@ -90,14 +88,6 @@ func SetTracer(customTracer Tracer) {
 	tracer.SetTracer(customTracer)
 }
 
-func setTimeouts(prop *time.Duration, value time.Duration, defaultVal time.Duration) {
-	if value == 0 {
-		*prop = defaultVal
-	} else {
-		*prop = value
-	}
-}
-
 func (i *NVMeType) isMock() bool {
 	return i.mock
 }

gonvme_mock.go

@@ -20,6 +20,8 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
+
+	log "github.com/sirupsen/logrus"
 )
 
 const (
@@ -53,6 +55,27 @@ type MockNVMe struct {
 	NVMeType
 }
 
+type CommandRunner interface {
+	RunCommand(name string, args ...string) (string, error)
+}
+
+type MockCommandRunner struct {
+	Output string
+	Err    error
+}
+
+func (m *MockCommandRunner) RunCommand(_ string, _ ...string) (string, error) {
+	// Return pre-defined output or error for testing
+	return m.Output, m.Err
+}
+
+func RunCommandWithRunner(runner CommandRunner, command []string) (string, error) {
+	if len(command) == 0 {
+		return "", errors.New("command is empty")
+	}
+	return runner.RunCommand(command[0], command[1:]...)
+}
+
 // NewMockNVMe - returns a mock NVMe client
 func NewMockNVMe(opts map[string]string) *MockNVMe {
 	nvme := MockNVMe{
@@ -62,6 +85,23 @@ func NewMockNVMe(opts map[string]string) *MockNVMe {
 		},
 	}
 
+	mockRunner := &MockCommandRunner{
+		Output: string("/sbin/nvme"),
+		Err:    nil,
+	}
+
+	command := []string{"chroot", "which", "nvme"}
+	output, err := RunCommandWithRunner(mockRunner, command)
+	if err != nil {
+		log.Errorf("Empty command")
+	}
+
+	// Assertions
+	expectedPath := "/sbin/nvme"
+	if output != expectedPath {
+		log.Errorf("expected %v, got %v", expectedPath, output)
+	}
+
 	return &nvme
 }
 

gonvme_tcp_fc.go

@@ -36,9 +36,6 @@ const (
 	// DefaultInitiatorNameFile is the default file which contains the initiator nqn
 	DefaultInitiatorNameFile = "/etc/nvme/hostnqn"
 
-	// NVMeCommand - nvme command
-	NVMeCommand = "nvme"
-
 	// NVMePort - port number
 	NVMePort = "4420"
 
@@ -54,6 +51,7 @@ const (
 type NVMe struct {
 	NVMeType
 	sessionParser NVMeSessionParser
+	NVMeCommand   string
 }
 
 // NewNVMe - returns a new NVMe client
@@ -65,6 +63,24 @@ func NewNVMe(opts map[string]string) *NVMe {
 		},
 	}
 	nvme.sessionParser = &sessionParser{}
+	nvme_path := ""
+	paths := []string{"/usr/sbin/nvme", "/usr/bin/nvme", "/bin/nvme"}
+	for _, path := range paths {
+		info, err := os.Stat(path)
+			if os.IsNotExist(err) {
+				log.Errorf("Error: Path %s does not exist\n", path)
+			} else if err != nil {
+				log.Errorf("Error: Unable to access path %s: %v\n", path, err)
+			} else if info.IsDir() {
+				log.Errorf("Error: Path %s is a directory, not an executable\n", path)
+			} else {
+				log.Infof("Success: Path %s exists and is an executable\n", path)
+				nvme_path=path
+				break
+			}
+	}
+	nvme.NVMeCommand = nvme_path
+
 	return &nvme
 }
 
@@ -133,7 +149,7 @@ func (nvme *NVMe) discoverNVMeTCPTargets(address string, login bool) ([]NVMeTarg
 	// TODO: add injection check on address
 	// nvme discovery is done via nvme cli
 	// nvme discover -t tcp -a <NVMe interface IP> -s <port>
-	exe := nvme.buildNVMeCommand([]string{NVMeCommand, "discover", "-t", "tcp", "-a", address, "-s", NVMePort})
+	exe := nvme.buildNVMeCommand([]string{nvme.NVMeCommand, "discover", "-t", "tcp", "-a", address, "-s", NVMePort})
 	cmd := exec.Command(exe[0], exe[1:]...) // #nosec G204
 
 	out, err := cmd.Output()
@@ -195,39 +211,30 @@ func (nvme *NVMe) discoverNVMeTCPTargets(address string, login bool) ([]NVMeTarg
 			if value != NVMeTransportTypeTCP {
 				skipIteration = true
 			}
-			break
 
 		case "traddr:":
 			nvmeTarget.Portal = value
-			break
 
 		case "subnqn:":
 			nvmeTarget.TargetNqn = value
-			break
 
 		case "adrfam:":
 			nvmeTarget.AdrFam = value
-			break
 
 		case "subtype:":
 			nvmeTarget.SubType = value
-			break
 
 		case "treq:":
 			nvmeTarget.Treq = value
-			break
 
 		case "portid:":
 			nvmeTarget.PortID = value
-			break
 
 		case "trsvcid:":
 			nvmeTarget.TrsvcID = value
-			break
 
 		case "sectype:":
 			nvmeTarget.SecType = value
-			break
 
 		default:
 
@@ -274,7 +281,7 @@ func (nvme *NVMe) discoverNVMeFCTargets(targetAddress string, login bool) ([]NVM
 
 		// host_traddr = nn-<Initiator_WWNN>:pn-<Initiator_WWPN>
 		initiatorAddress := strings.Replace(fmt.Sprintf("nn-%s:pn-%s", FCHostInfo.NodeName, FCHostInfo.PortName), "\n", "", -1)
-		exe := nvme.buildNVMeCommand([]string{NVMeCommand, "discover", "-t", "fc", "-a", targetAddress, "-w", initiatorAddress})
+		exe := nvme.buildNVMeCommand([]string{nvme.NVMeCommand, "discover", "-t", "fc", "-a", targetAddress, "-w", initiatorAddress})
 		cmd := exec.Command(exe[0], exe[1:]...) // #nosec G204
 
 		out, err = cmd.Output()
@@ -336,40 +343,30 @@ func (nvme *NVMe) discoverNVMeFCTargets(targetAddress string, login bool) ([]NVM
 				if value != NVMeTransportTypeFC {
 					skipIteration = true
 				}
-				break
 
 			case "traddr:":
 				nvmeTarget.Portal = value
-				break
 
 			case "subnqn:":
 				nvmeTarget.TargetNqn = value
-				break
 
 			case "adrfam:":
 				nvmeTarget.AdrFam = value
-				break
 
 			case "subtype:":
 				nvmeTarget.SubType = value
-				break
 
 			case "treq:":
 				nvmeTarget.Treq = value
-				break
 
 			case "portid:":
 				nvmeTarget.PortID = value
-				break
 
 			case "trsvcid:":
 				nvmeTarget.TrsvcID = value
-				break
 
 			case "sectype:":
 				nvmeTarget.SecType = value
-				break
-
 			}
 		}
 		if !skipIteration && nvmeTarget.TargetNqn != "" && nvmeTarget.Portal == targetAddress {
@@ -467,6 +464,10 @@ func (nvme *NVMe) nvmeTCPConnect(target NVMeTarget, duplicateConnect bool) error
 	var Output string
 	stderr, _ := cmd.StderrPipe()
 	err := cmd.Start()
+	if err != nil {
+		log.Errorf("Error during nvme TCP connect %s at %s: %v", target.TargetNqn, target.Portal, err)
+		return err
+	}
 
 	scanner := bufio.NewScanner(stderr)
 	for scanner.Scan() {
@@ -536,6 +537,10 @@ func (nvme *NVMe) nvmeFCConnect(target NVMeTarget, duplicateConnect bool) error
 	var Output string
 	stderr, _ := cmd.StderrPipe()
 	err := cmd.Start()
+	if err != nil {
+		log.Errorf("Error during NVMe FC connect %s at %s: %v", target.TargetNqn, target.Portal, err)
+		return err
+	}
 
 	scanner := bufio.NewScanner(stderr)
 	for scanner.Scan() {
@@ -592,7 +597,7 @@ func (nvme *NVMe) NVMeDisconnect(target NVMeTarget) error {
 func (nvme *NVMe) nvmeDisconnect(target NVMeTarget) error {
 	// nvme disconnect is done via the nvme cli
 	// nvme disconnect -n <target NQN>
-	exe := nvme.buildNVMeCommand([]string{NVMeCommand, "disconnect", "-n", target.TargetNqn})
+	exe := nvme.buildNVMeCommand([]string{nvme.NVMeCommand, "disconnect", "-n", target.TargetNqn})
 	cmd := exec.Command(exe[0], exe[1:]...) // #nosec G204
 
 	_, err := cmd.Output()

gonvme_test.go

@@ -104,7 +104,7 @@ func TestDiscoverNVMeTCPTargets(t *testing.T) {
 func TestDiscoverNVMeFCTargets(t *testing.T) {
 	reset()
 	c := NewNVMe(map[string]string{})
-	_, err := c.DiscoverNVMeFCTargets(fcTestPortal, false)
+	_, _ = c.DiscoverNVMeFCTargets(fcTestPortal, false)
 	FCHostsInfo, err := c.getFCHostInfo()
 	if err == nil && len(FCHostsInfo) != 0 {
 		_, err := c.DiscoverNVMeFCTargets(fcTestPortal, false)

internal/logger/logger_test.go

@@ -0,0 +1,17 @@
+/*
+ *
+ * Copyright © 2024 Dell Inc. or its subsidiaries. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package logger

internal/tracer/tracer_test.go

@@ -0,0 +1,17 @@
+/*
+ *
+ * Copyright © 2024 Dell Inc. or its subsidiaries. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package tracer