Skip to content

Commit

Permalink
addressing the review comment
Browse files Browse the repository at this point in the history
  • Loading branch information
karthikk92 committed Nov 15, 2024
2 parents 6a1fe1b + 9948db8 commit 7c6a63c
Show file tree
Hide file tree
Showing 12 changed files with 82 additions and 154 deletions.
49 changes: 0 additions & 49 deletions .github/workflows/actions.yml

This file was deleted.

17 changes: 17 additions & 0 deletions .github/workflows/common-workflows.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: Common Workflows
on: # yamllint disable-line rule:truthy
push:
branches: [main]
pull_request:
branches: ["**"]

jobs:

# golang static analysis checks
go-static-analysis:
uses: dell/common-github-actions/.github/workflows/go-static-analysis.yaml@main
name: Golang Validation

common:
name: Quality Checks
uses: dell/common-github-actions/.github/workflows/go-common.yml@main
2 changes: 1 addition & 1 deletion .github/workflows/go-version.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# Reusable workflow to perform go version update on Golang based projects
name: Go Version Update

on:
on: # yamllint disable-line rule:truthy
workflow_dispatch:
repository_dispatch:
types: [go-update-workflow]
Expand Down
30 changes: 0 additions & 30 deletions .github/workflows/linters.yaml

This file was deleted.

7 changes: 3 additions & 4 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
name: Release GoNVMe
# Invocable as a reusable workflow
# Can be manually triggered
on:
workflow_call:
on: # yamllint disable-line rule:truthy
workflow_call:
workflow_dispatch:
inputs:
version:
description: 'Version to release (major, minor, patch)'
description: 'Version to release (major, minor, patch) Ex: 1.0.0'
required: true
default: 'none'
jobs:
csm-release:
uses: dell/common-github-actions/.github/workflows/csm-release-libs.yaml@main
Expand Down
30 changes: 0 additions & 30 deletions .golangci.yaml

This file was deleted.

10 changes: 0 additions & 10 deletions gonvme.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
package gonvme

import (
"time"

"github.com/dell/gonvme/internal/logger"
"github.com/dell/gonvme/internal/tracer"
)
Expand Down Expand Up @@ -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
}
Expand Down
17 changes: 8 additions & 9 deletions gonvme_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"errors"
"fmt"
"strconv"

log "github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -64,7 +64,7 @@ type MockCommandRunner struct {
Err error
}

func (m *MockCommandRunner) RunCommand(name string, args ...string) (string, error) {
func (m *MockCommandRunner) RunCommand(_ string, _ ...string) (string, error) {
// Return pre-defined output or error for testing
return m.Output, m.Err
}
Expand Down Expand Up @@ -92,16 +92,15 @@ func NewMockNVMe(opts map[string]string) *MockNVMe {

command := []string{"chroot", "which", "nvme"}
output, err := RunCommandWithRunner(mockRunner, command)

if err != nil{
if err != nil {
log.Errorf("Empty command")
}

// Assertions
expectedPath := "/sbin/nvme"
if output != expectedPath {
log.Errorf("expected %v, got %v", expectedPath, output)
}
// Assertions
expectedPath := "/sbin/nvme"
if output != expectedPath {
log.Errorf("expected %v, got %v", expectedPath, output)
}

return &nvme
}
Expand Down
38 changes: 18 additions & 20 deletions gonvme_tcp_fc.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const (
type NVMe struct {
NVMeType
sessionParser NVMeSessionParser
NVMeCommand string
NVMeCommand string
}

// NewNVMe - returns a new NVMe client
Expand All @@ -74,7 +74,16 @@ func NewNVMe(opts map[string]string) *NVMe {
nvme_path=path
}
}
<<<<<<< HEAD

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)

Check failure on line 77 in gonvme_tcp_fc.go

View workflow job for this annotation

GitHub Actions / Golang Validation / Lint golang code

syntax error: unexpected <<, expected } (typecheck)
nvme.NVMeCommand = nvme_path
=======
log.Infof("NVMe %s", command)
path, err := exec.Command(command[0], command[1:]...).Output() // #nosec G204
if err != nil {
log.Fatalf("Failed to find nvme command: %v", err)
}
nvme.NVMeCommand = strings.TrimSpace(string(path))
>>>>>>> 9948db86815aa2658bd252ddcb05cbcbd96ce9e1
return &nvme
}

Expand Down Expand Up @@ -205,39 +214,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:

Expand Down Expand Up @@ -346,40 +346,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 {
Expand Down Expand Up @@ -477,6 +467,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() {
Expand Down Expand Up @@ -546,6 +540,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() {
Expand Down
2 changes: 1 addition & 1 deletion gonvme_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 17 additions & 0 deletions internal/logger/logger_test.go
Original file line number Diff line number Diff line change
@@ -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
17 changes: 17 additions & 0 deletions internal/tracer/tracer_test.go
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 7c6a63c

Please sign in to comment.