Skip to content

Commit

Permalink
Merge pull request #569 from uhthomas/466
Browse files Browse the repository at this point in the history 
fix(collector): show correct nvme capacity
  • Loading branch information
@AnalogJ
AnalogJ authored Feb 24, 2024
2 parents 19bd59d + db86bac commit 44275c6
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 52 deletions.
10 changes: 6 additions & 4 deletions collector/pkg/detect/detect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ package detect
import (
"encoding/json"
"fmt"
"os"
"strings"

"github.com/analogj/scrutiny/collector/pkg/common/shell"
"github.com/analogj/scrutiny/collector/pkg/config"
"github.com/analogj/scrutiny/collector/pkg/models"
"github.com/analogj/scrutiny/webapp/backend/pkg/models/collector"
"github.com/sirupsen/logrus"
"os"
"strings"
)

type Detect struct {
Expand Down Expand Up @@ -47,7 +48,7 @@ func (d *Detect) SmartctlScan() ([]models.Device, error) {
return detectedDevices, nil
}

//updates a device model with information from smartctl --scan
// updates a device model with information from smartctl --scan
// It has a couple of issues however:
// - WWN is provided as component data, rather than a "string". We'll have to generate the WWN value ourselves
// - WWN from smartctl only provided for ATA protocol drives, NVMe and SCSI drives do not include WWN.
Expand Down Expand Up @@ -81,8 +82,9 @@ func (d *Detect) SmartCtlInfo(device *models.Device) error {
device.SerialNumber = availableDeviceInfo.SerialNumber
device.Firmware = availableDeviceInfo.FirmwareVersion
device.RotationSpeed = availableDeviceInfo.RotationRate
device.Capacity = availableDeviceInfo.UserCapacity.Bytes
device.Capacity = availableDeviceInfo.Capacity()
device.FormFactor = availableDeviceInfo.FormFactor.Name
device.DeviceType = availableDeviceInfo.Device.Type
device.DeviceProtocol = availableDeviceInfo.Device.Protocol
if len(availableDeviceInfo.Vendor) > 0 {
device.Manufacturer = availableDeviceInfo.Vendor
Expand Down
135 changes: 99 additions & 36 deletions collector/pkg/detect/detect_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
package detect_test

import (
"os"
"strings"
"testing"

mock_shell "github.com/analogj/scrutiny/collector/pkg/common/shell/mock"
mock_config "github.com/analogj/scrutiny/collector/pkg/config/mock"
"github.com/analogj/scrutiny/collector/pkg/detect"
"github.com/analogj/scrutiny/collector/pkg/models"
"github.com/golang/mock/gomock"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"io/ioutil"
"testing"
)

func TestDetect_SmartctlScan(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -23,7 +26,7 @@ func TestDetect_SmartctlScan(t *testing.T) {
fakeConfig.EXPECT().GetString("commands.metrics_scan_args").AnyTimes().Return("--scan --json")

fakeShell := mock_shell.NewMockInterface(mockCtrl)
testScanResults, err := ioutil.ReadFile("testdata/smartctl_scan_simple.json")
testScanResults, err := os.ReadFile("testdata/smartctl_scan_simple.json")
fakeShell.EXPECT().Command(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(string(testScanResults), err)

d := detect.Detect{
Expand All @@ -32,17 +35,17 @@ func TestDetect_SmartctlScan(t *testing.T) {
Config: fakeConfig,
}

//test
// test
scannedDevices, err := d.SmartctlScan()

//assert
// assert
require.NoError(t, err)
require.Equal(t, 7, len(scannedDevices))
require.Equal(t, "scsi", scannedDevices[0].DeviceType)
}

func TestDetect_SmartctlScan_Megaraid(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -52,7 +55,7 @@ func TestDetect_SmartctlScan_Megaraid(t *testing.T) {
fakeConfig.EXPECT().GetString("commands.metrics_scan_args").AnyTimes().Return("--scan --json")

fakeShell := mock_shell.NewMockInterface(mockCtrl)
testScanResults, err := ioutil.ReadFile("testdata/smartctl_scan_megaraid.json")
testScanResults, err := os.ReadFile("testdata/smartctl_scan_megaraid.json")
fakeShell.EXPECT().Command(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(string(testScanResults), err)

d := detect.Detect{
Expand All @@ -61,20 +64,20 @@ func TestDetect_SmartctlScan_Megaraid(t *testing.T) {
Config: fakeConfig,
}

//test
// test
scannedDevices, err := d.SmartctlScan()

//assert
// assert
require.NoError(t, err)
require.Equal(t, 2, len(scannedDevices))
require.Equal(t, []models.Device{
models.Device{DeviceName: "bus/0", DeviceType: "megaraid,0"},
models.Device{DeviceName: "bus/0", DeviceType: "megaraid,1"},
{DeviceName: "bus/0", DeviceType: "megaraid,0"},
{DeviceName: "bus/0", DeviceType: "megaraid,1"},
}, scannedDevices)
}

func TestDetect_SmartctlScan_Nvme(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -84,7 +87,7 @@ func TestDetect_SmartctlScan_Nvme(t *testing.T) {
fakeConfig.EXPECT().GetString("commands.metrics_scan_args").AnyTimes().Return("--scan --json")

fakeShell := mock_shell.NewMockInterface(mockCtrl)
testScanResults, err := ioutil.ReadFile("testdata/smartctl_scan_nvme.json")
testScanResults, err := os.ReadFile("testdata/smartctl_scan_nvme.json")
fakeShell.EXPECT().Command(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(string(testScanResults), err)

d := detect.Detect{
Expand All @@ -93,19 +96,19 @@ func TestDetect_SmartctlScan_Nvme(t *testing.T) {
Config: fakeConfig,
}

//test
// test
scannedDevices, err := d.SmartctlScan()

//assert
// assert
require.NoError(t, err)
require.Equal(t, 1, len(scannedDevices))
require.Equal(t, []models.Device{
models.Device{DeviceName: "nvme0", DeviceType: "nvme"},
{DeviceName: "nvme0", DeviceType: "nvme"},
}, scannedDevices)
}

func TestDetect_TransformDetectedDevices_Empty(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -129,16 +132,16 @@ func TestDetect_TransformDetectedDevices_Empty(t *testing.T) {
Config: fakeConfig,
}

//test
// test
transformedDevices := d.TransformDetectedDevices(detectedDevices)

//assert
// assert
require.Equal(t, "sda", transformedDevices[0].DeviceName)
require.Equal(t, "scsi", transformedDevices[0].DeviceType)
}

func TestDetect_TransformDetectedDevices_Ignore(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -162,15 +165,15 @@ func TestDetect_TransformDetectedDevices_Ignore(t *testing.T) {
Config: fakeConfig,
}

//test
// test
transformedDevices := d.TransformDetectedDevices(detectedDevices)

//assert
// assert
require.Equal(t, []models.Device{}, transformedDevices)
}

func TestDetect_TransformDetectedDevices_Raid(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -187,7 +190,8 @@ func TestDetect_TransformDetectedDevices_Raid(t *testing.T) {
Device: "/dev/twa0",
DeviceType: []string{"3ware,0", "3ware,1", "3ware,2", "3ware,3", "3ware,4", "3ware,5"},
Ignore: false,
}})
},
})
detectedDevices := models.Scan{
Devices: []models.ScanDevice{
{
Expand All @@ -203,15 +207,15 @@ func TestDetect_TransformDetectedDevices_Raid(t *testing.T) {
Config: fakeConfig,
}

//test
// test
transformedDevices := d.TransformDetectedDevices(detectedDevices)

//assert
// assert
require.Equal(t, 12, len(transformedDevices))
}

func TestDetect_TransformDetectedDevices_Simple(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -234,17 +238,17 @@ func TestDetect_TransformDetectedDevices_Simple(t *testing.T) {
Config: fakeConfig,
}

//test
// test
transformedDevices := d.TransformDetectedDevices(detectedDevices)

//assert
// assert
require.Equal(t, 1, len(transformedDevices))
require.Equal(t, "sat+megaraid", transformedDevices[0].DeviceType)
}

// test https://github.com/AnalogJ/scrutiny/issues/255#issuecomment-1164024126
func TestDetect_TransformDetectedDevices_WithoutDeviceTypeOverride(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -267,16 +271,16 @@ func TestDetect_TransformDetectedDevices_WithoutDeviceTypeOverride(t *testing.T)
Config: fakeConfig,
}

//test
// test
transformedDevices := d.TransformDetectedDevices(detectedDevices)

//assert
// assert
require.Equal(t, 1, len(transformedDevices))
require.Equal(t, "scsi", transformedDevices[0].DeviceType)
}

func TestDetect_TransformDetectedDevices_WhenDeviceNotDetected(t *testing.T) {
//setup
// setup
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
fakeConfig := mock_config.NewMockInterface(mockCtrl)
Expand All @@ -290,10 +294,69 @@ func TestDetect_TransformDetectedDevices_WhenDeviceNotDetected(t *testing.T) {
Config: fakeConfig,
}

//test
// test
transformedDevices := d.TransformDetectedDevices(detectedDevices)

//assert
// assert
require.Equal(t, 1, len(transformedDevices))
require.Equal(t, "ata", transformedDevices[0].DeviceType)
}

func TestDetect_SmartCtlInfo(t *testing.T) {
t.Run("should report nvme info", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

const (
someArgs = "--info --json"

// device info
someDeviceName = "some-device-name"
someModelName = "KCD61LUL3T84"
someSerialNumber = "61Q0A05UT7B8"
someFirmware = "8002"
someDeviceProtocol = "NVMe"
someDeviceType = "nvme"
someCapacity int64 = 3840755982336
)

fakeConfig := mock_config.NewMockInterface(ctrl)
fakeConfig.EXPECT().
GetCommandMetricsInfoArgs("/dev/" + someDeviceName).
Return(someArgs)
fakeConfig.EXPECT().
GetString("commands.metrics_smartctl_bin").
Return("smartctl")

someLogger := logrus.WithFields(logrus.Fields{})

smartctlInfoResults, err := os.ReadFile("testdata/smartctl_info_nvme.json")
require.NoError(t, err)

fakeShell := mock_shell.NewMockInterface(ctrl)
fakeShell.EXPECT().
Command(someLogger, "smartctl", append(strings.Split(someArgs, " "), "/dev/"+someDeviceName), "", gomock.Any()).
Return(string(smartctlInfoResults), err)

d := detect.Detect{
Logger: someLogger,
Shell: fakeShell,
Config: fakeConfig,
}

someDevice := &models.Device{
WWN: "some wwn",
DeviceName: someDeviceName,
}

require.NoError(t, d.SmartCtlInfo(someDevice))

assert.Equal(t, someDeviceName, someDevice.DeviceName)
assert.Equal(t, someModelName, someDevice.ModelName)
assert.Equal(t, someSerialNumber, someDevice.SerialNumber)
assert.Equal(t, someFirmware, someDevice.Firmware)
assert.Equal(t, someDeviceProtocol, someDevice.DeviceProtocol)
assert.Equal(t, someDeviceType, someDevice.DeviceType)
assert.Equal(t, someCapacity, someDevice.Capacity)
})
}
48 changes: 48 additions & 0 deletions collector/pkg/detect/testdata/smartctl_info_nvme.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
{
"json_format_version": [
1,
0
],
"smartctl": {
"version": [
7,
2
],
"svn_revision": "5155",
"platform_info": "x86_64-linux-6.1.69-talos",
"build_info": "(local build)",
"argv": [
"smartctl",
"--info",
"--json",
"/dev/nvme4"
],
"exit_status": 0
},
"device": {
"name": "/dev/nvme4",
"info_name": "/dev/nvme4",
"type": "nvme",
"protocol": "NVMe"
},
"model_name": "KCD61LUL3T84",
"serial_number": "61Q0A05UT7B8",
"firmware_version": "8002",
"nvme_pci_vendor": {
"id": 7695,
"subsystem_id": 7695
},
"nvme_ieee_oui_identifier": 9233294,
"nvme_total_capacity": 3840755982336,
"nvme_unallocated_capacity": 0,
"nvme_controller_id": 1,
"nvme_version": {
"string": "1.4",
"value": 66560
},
"nvme_number_of_namespaces": 16,
"local_time": {
"time_t": 1706045146,
"asctime": "Tue Jan 23 21:25:46 2024 UTC"
}
}
Loading

0 comments on commit 44275c6

Please sign in to comment.