Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression in dashboard disk activity widget. Fixes #1978 #1979

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/rockstor/smart_manager/data_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,9 @@ def disk_stats(prev_stats):
stats_file_path = '/proc/diskstats'
cur_stats = {}
interval = 1
# TODO: Consider refactoring the following to use Disk.temp_name or
# TODO: building byid_disk_map from the same. Ideally we would have
# TODO: performance testing in place prior to this move.
# Build a list of our db's disk names, now in by-id type format.
disks = [d.name for d in Disk.objects.all()]
# /proc/diskstats has lines of the following form:
Expand Down
4 changes: 2 additions & 2 deletions src/rockstor/system/osi.py
Original file line number Diff line number Diff line change
Expand Up @@ -1620,7 +1620,7 @@ def get_dev_byid_name(device_name, remove_path=False):


def get_byid_name_map():
"""Simple wrapper around 'ls -l /dev/disk/by-id' which returns a current
"""Simple wrapper around 'ls -lr /dev/disk/by-id' which returns a current
mapping of all attached by-id device names to their sdX counterparts. When
multiple by-id names are found for the same sdX device then the longest is
preferred, or when equal in length then the first listed is used. Intended
Expand All @@ -1636,7 +1636,7 @@ def get_byid_name_map():
was encountered by run_command or no by-id type names were encountered.
"""
byid_name_map = {}
out, err, rc = run_command([LS, '-l', '/dev/disk/by-id'],
out, err, rc = run_command([LS, '-lr', '/dev/disk/by-id'],
throw=True)
if rc == 0:
for each_line in out:
Expand Down
131 changes: 130 additions & 1 deletion src/rockstor/system/tests/test_osi.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import unittest
from mock import patch

from system.osi import get_dev_byid_name, Disk, scan_disks
from system.osi import get_dev_byid_name, Disk, scan_disks, get_byid_name_map


class Pool(object):
Expand Down Expand Up @@ -1551,3 +1551,132 @@ def test_scan_disks_nvme_sys_disk(self):
'returned = ({}).\n '
'expected = ({}).'.format(returned,
expected))

def test_get_byid_name_map_prior_command_mock(self):
"""
Test get_byid_name_map() for prior mapping between canonical
and by-id device name mapping.
Note that the 'new' variant of this behaviour, tested later, only
involved the change of the mocked run_command output.
"""
# The following test data of 'ls -l' contains; sdd, sde, and sda3 with
# same length by-id mappings that are also their only mappings.
# This allows testing for returning the expected wwn-..., not the
# regression tested here of "scsi-... type name. As per
# get_dev_byid_name()'s reverse lexicographical return priority we
# would now expect the wwn-... type names if there are not others
# available and all available by-id are of the same length.
# Thanks to forum member juchong for submitting this command output.

out = [
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-HGST_HUH728080ALE600_2EKXANGP -> ../../sdb', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-VMware_Virtual_SATA_CDRW_Drive_00000000000000000001 -> ../../sr0', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68L0BN1_WD-WX11D6651995 -> ../../sdg', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68L0BN1_WD-WX31DB58YJF0 -> ../../sdh', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WX11DB4H8VJJ -> ../../sdf', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WX21DC42ELAT -> ../../sdc', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WXM1H84CXAUJ -> ../../sdi', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-35000cca252017870 -> ../../sdd', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-35000cca252017ef4 -> ../../sde', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636 -> ../../sda', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part1 -> ../../sda1', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part2 -> ../../sda2', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part3 -> ../../sda3', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca23bf728eb -> ../../sdb', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca252017870 -> ../../sdd', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca252017ef4 -> ../../sde', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee20b77be35 -> ../../sdf', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee20dd22144 -> ../../sdg', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee260e5c696 -> ../../sdi', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee26114d4cb -> ../../sdc', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee26293bbea -> ../../sdh', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636 -> ../../sda', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part1 -> ../../sda1', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part2 -> ../../sda2', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part3 -> ../../sda3', # noqa E501
]
err = ['']
rc = 0
expected = {
'sda1': 'scsi-36000c29df1181dd53db36b41b3582636-part1',
'sdf': 'ata-WDC_WD60EFRX-68MYMN1_WD-WX11DB4H8VJJ',
'sdd': 'scsi-35000cca252017870',
'sde': 'scsi-35000cca252017ef4',
'sr0': 'ata-VMware_Virtual_SATA_CDRW_Drive_00000000000000000001',
'sdg': 'ata-WDC_WD60EFRX-68L0BN1_WD-WX11D6651995',
'sda2': 'scsi-36000c29df1181dd53db36b41b3582636-part2',
'sda': 'scsi-36000c29df1181dd53db36b41b3582636',
'sdb': 'ata-HGST_HUH728080ALE600_2EKXANGP',
'sdc': 'ata-WDC_WD60EFRX-68MYMN1_WD-WX21DC42ELAT',
'sdh': 'ata-WDC_WD60EFRX-68L0BN1_WD-WX31DB58YJF0',
'sdi': 'ata-WDC_WD60EFRX-68MYMN1_WD-WXM1H84CXAUJ',
'sda3': 'scsi-36000c29df1181dd53db36b41b3582636-part3'}
self.mock_run_command.return_value = (out, err, rc)
returned = get_byid_name_map()
self.maxDiff = None
self.assertDictEqual(returned, expected)

def test_get_byid_name_map(self):
"""
Test get_byid_name_map() for expected by-id device name mapping.
"""
# The following test data of 'ls -lr' contains; sdd, sde, and sda3 with
# same length by-id mappings that are also their only mappings.
# This allows testing for returning the expected wwn-..., not the
# "scsi-... type name. As per get_dev_byid_name()'s revised reverse
# lexicographical return priority we would now expect the wwn-... type
# names if there are no others available and all available by-id are
# of the same length.
# This ls -lr output is derived from the ls -l output supplied by forum
# member juchong.

out = [
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part3 -> ../../sda3', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part2 -> ../../sda2', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636-part1 -> ../../sda1', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x6000c29df1181dd53db36b41b3582636 -> ../../sda', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee26293bbea -> ../../sdh', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee26114d4cb -> ../../sdc', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee260e5c696 -> ../../sdi', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee20dd22144 -> ../../sdg', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x50014ee20b77be35 -> ../../sdf', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca252017ef4 -> ../../sde', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca252017870 -> ../../sdd', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 wwn-0x5000cca23bf728eb -> ../../sdb', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part3 -> ../../sda3', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part2 -> ../../sda2', # noqa E501
'lrwxrwxrwx 1 root root 10 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636-part1 -> ../../sda1', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-36000c29df1181dd53db36b41b3582636 -> ../../sda', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-35000cca252017ef4 -> ../../sde', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 scsi-35000cca252017870 -> ../../sdd', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WXM1H84CXAUJ -> ../../sdi', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WX21DC42ELAT -> ../../sdc', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68MYMN1_WD-WX11DB4H8VJJ -> ../../sdf', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68L0BN1_WD-WX31DB58YJF0 -> ../../sdh', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-WDC_WD60EFRX-68L0BN1_WD-WX11D6651995 -> ../../sdg', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-VMware_Virtual_SATA_CDRW_Drive_00000000000000000001 -> ../../sr0', # noqa E501
'lrwxrwxrwx 1 root root 9 Oct 22 23:40 ata-HGST_HUH728080ALE600_2EKXANGP -> ../../sdb', # noqa E501
]
err = ['']
rc = 0
# The order of the following is unimportant as it's a dictionary but is
# preserved on the index to aid comparison with:
# test_get_byid_name_map_prior_command_mock()
expected = {
'sda1': 'wwn-0x6000c29df1181dd53db36b41b3582636-part1',
'sdf': 'ata-WDC_WD60EFRX-68MYMN1_WD-WX11DB4H8VJJ',
'sdd': 'wwn-0x5000cca252017870',
'sde': 'wwn-0x5000cca252017ef4',
'sr0': 'ata-VMware_Virtual_SATA_CDRW_Drive_00000000000000000001',
'sdg': 'ata-WDC_WD60EFRX-68L0BN1_WD-WX11D6651995',
'sda2': 'wwn-0x6000c29df1181dd53db36b41b3582636-part2',
'sda': 'wwn-0x6000c29df1181dd53db36b41b3582636',
'sdb': 'ata-HGST_HUH728080ALE600_2EKXANGP',
'sdc': 'ata-WDC_WD60EFRX-68MYMN1_WD-WX21DC42ELAT',
'sdh': 'ata-WDC_WD60EFRX-68L0BN1_WD-WX31DB58YJF0',
'sdi': 'ata-WDC_WD60EFRX-68MYMN1_WD-WXM1H84CXAUJ',
'sda3': 'wwn-0x6000c29df1181dd53db36b41b3582636-part3'}
self.mock_run_command.return_value = (out, err, rc)
returned = get_byid_name_map()
self.maxDiff = None
self.assertDictEqual(returned, expected)