when deviceType not specified in collector config, scrutiny will igno…

…re the device. We need to make sure we correctly override the device. fixes #255
AnalogJ · Jun 25, 2022 · 3d9001a · 3d9001a
1 parent fbe7d63
commit 3d9001a
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 2 deletions.
diff --git a/collector/pkg/detect/detect.go b/collector/pkg/detect/detect.go
@@ -149,10 +149,35 @@ func (d *Detect) TransformDetectedDevices(detectedDeviceConns models.Scan) []mod
 			//create a new device group, and replace the one generated by smartctl --scan
 			overrideDeviceGroup := []models.Device{}
 
-			for _, overrideDeviceType := range overrideDevice.DeviceType {
+			if overrideDevice.DeviceType != nil {
+				for _, overrideDeviceType := range overrideDevice.DeviceType {
+					overrideDeviceGroup = append(overrideDeviceGroup, models.Device{
+						HostId:     d.Config.GetString("host.id"),
+						DeviceType: overrideDeviceType,
+						DeviceName: strings.TrimPrefix(overrideDeviceFile, DevicePrefix()),
+					})
+				}
+			} else {
+				//user may have specified device in config file without device type (default to scanned device type)
+
+				//check if the device file was detected by the scanner
+				var deviceType string
+				if scannedDevice, foundScannedDevice := groupedDevices[overrideDeviceFile]; foundScannedDevice {
+					if len(scannedDevice) > 0 {
+						//take the device type from the first grouped device
+						deviceType = scannedDevice[0].DeviceType
+					} else {
+						deviceType = "ata"
+					}
+
+				} else {
+					//fallback to ata if no scanned device detected
+					deviceType = "ata"
+				}
+
 				overrideDeviceGroup = append(overrideDeviceGroup, models.Device{
 					HostId:     d.Config.GetString("host.id"),
-					DeviceType: overrideDeviceType,
+					DeviceType: deviceType,
 					DeviceName: strings.TrimPrefix(overrideDeviceFile, DevicePrefix()),
 				})
 			}

diff --git a/collector/pkg/detect/detect_test.go b/collector/pkg/detect/detect_test.go
@@ -241,3 +241,59 @@ func TestDetect_TransformDetectedDevices_Simple(t *testing.T) {
 	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
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	fakeConfig := mock_config.NewMockInterface(mockCtrl)
+	fakeConfig.EXPECT().GetString("host.id").AnyTimes().Return("")
+	fakeConfig.EXPECT().GetString("commands.metrics_smartctl_bin").AnyTimes().Return("smartctl")
+	fakeConfig.EXPECT().GetString("commands.metrics_scan_args").AnyTimes().Return("--scan --json")
+	fakeConfig.EXPECT().GetDeviceOverrides().AnyTimes().Return([]models.ScanOverride{{Device: "/dev/sda"}})
+	detectedDevices := models.Scan{
+		Devices: []models.ScanDevice{
+			{
+				Name:     "/dev/sda",
+				InfoName: "/dev/sda",
+				Protocol: "ata",
+				Type:     "scsi",
+			},
+		},
+	}
+
+	d := detect.Detect{
+		Config: fakeConfig,
+	}
+
+	//test
+	transformedDevices := d.TransformDetectedDevices(detectedDevices)
+
+	//assert
+	require.Equal(t, 1, len(transformedDevices))
+	require.Equal(t, "scsi", transformedDevices[0].DeviceType)
+}
+
+func TestDetect_TransformDetectedDevices_WhenDeviceNotDetected(t *testing.T) {
+	//setup
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	fakeConfig := mock_config.NewMockInterface(mockCtrl)
+	fakeConfig.EXPECT().GetString("host.id").AnyTimes().Return("")
+	fakeConfig.EXPECT().GetString("commands.metrics_smartctl_bin").AnyTimes().Return("smartctl")
+	fakeConfig.EXPECT().GetString("commands.metrics_scan_args").AnyTimes().Return("--scan --json")
+	fakeConfig.EXPECT().GetDeviceOverrides().AnyTimes().Return([]models.ScanOverride{{Device: "/dev/sda"}})
+	detectedDevices := models.Scan{}
+
+	d := detect.Detect{
+		Config: fakeConfig,
+	}
+
+	//test
+	transformedDevices := d.TransformDetectedDevices(detectedDevices)
+
+	//assert
+	require.Equal(t, 1, len(transformedDevices))
+	require.Equal(t, "ata", transformedDevices[0].DeviceType)
+}