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.

Sign up for GitHub

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

Refactor lsblk, resolves issue re leading space in values #2907 #2909

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Refactor lsblk, resolves issue re leading space in values #2907 #2909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,6 @@ rockstor-tasks-huey.db*
# Poetry error logs
poetry-installer-error*
/build-output.txt

# Pyenv version
.python-version
27 changes: 14 additions & 13 deletions src/rockstor/system/osi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,19 +326,20 @@ def scan_disks(min_size: int, test_mode: bool = False) -> list[Disk]:
if line == "" or re.match("NAME", line) is None:
continue
# lsblk example line (incomplete): 'NAME="/dev/sdb" MODEL="TOSHIBA MK1652GS" VENDOR="ATA " LABEL="" UUID=""'
# line.strip().split('" ') = ['NAME="/dev/sdb', 'MODEL="TOSHIBA MK1652GS', 'VENDOR="ATA ', 'LABEL="', 'UUID=""'] # noqa E501
# Device information built from each lsblk line in turn.
clean_line = re.sub('"\s+"', '""', line).strip()
# logger.debug(f"Scan_disks() using lsblk clean_line={clean_line}")
# Device information built from each lsblk line in turn
# 'KEY_1="VALUE_1" KEY_2="VALUE_2" ... KEY_N="VALUE_N"' will become
# ['KEY_1', '', 'VALUE_1', 'KEY_2', '', 'VALUE_2', ... 'KEY_N', '', 'VALUE_N', '']
line = [item.strip() for item in re.split(r'["=]', line)]
# Remove every third value starting from index 2 to turn the list into
# ['KEY_1', 'VALUE_1', 'KEY_2', 'VALUE_2', 'KEY_N', 'VALUE_N', '']
# Trailing item is ok due to how zip is done
line = [item for i, item in enumerate(line, 2) if i % 3 != 0]
# logger.debug(f"Scan_disks() using lsblk line={line}")
blk_dev_properties: dict = {
key.lower()
if key != "TRAN"
else "transport": value.replace('"', "").strip()
if value.replace('"', "").strip() != ""
else None
for key, value in (
key_value.split("=") for key_value in clean_line.split('" ')
key.lower() if key != "TRAN" else "transport": (
value if value != "" else None
)
for key, value in zip(line[::2], line[1::2])
}
logger.debug(f"Scan_disks() using: blk_dev_properties={blk_dev_properties}")
# Disk namedtuple from lsblk line dictionary.
Expand Down Expand Up @@ -1995,8 +1996,8 @@ def get_devname(device_name, addPath=False):

def get_libs(program_path: str) -> list[str]:
"""
Wrapper around `ldd program_path`
@param program_path: Binary to query ldd about
Wrapper around `ldd program_path`
@param program_path: Binary to query ldd about
@return: list of OS paths or empty list if error or non found.
"""
libs: list[str] = []
Expand Down
22 changes: 17 additions & 5 deletions src/rockstor/system/tests/test_osi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1650,11 +1650,6 @@ def test_scan_disks_lsblk_parse_fail(self):
cd /opt/rockstor/src/rockstor/system/tests
/opt/rockstor/.venv/bin/python -m unittest test_osi.OSITests.test_scan_disks_lsblk_parse_fail
"""
# E.g 'VENDOR=" "' failed with: "ValueError: not enough values to unpack (expected 2, got 1)"
# We use dictionary comprehension to parse lsblk output and rely on: 'line.strip().split('" ')'
# this breaks when a disk property begins with a space.
# See: https://github.com/rockstor/rockstor-core/issues/2853
# Reported/encountered/addressed failure concerns values consisting solely of whitespace.
out = [
[
'NAME="/dev/fd0" MODEL="" SERIAL="" SIZE="4K" TRAN="" VENDOR="" HCTL="" TYPE="disk" FSTYPE="" LABEL="" UUID=""', # noqa E501
Expand All @@ -1664,6 +1659,7 @@ def test_scan_disks_lsblk_parse_fail(self):
'NAME="/dev/sdb2" MODEL="" SERIAL="" SIZE="64M" TRAN="" VENDOR="" HCTL="" TYPE="part" FSTYPE="vfat" LABEL="EFI" UUID="1381-2428"', # noqa E501
'NAME="/dev/sdb3" MODEL="" SERIAL="" SIZE="2G" TRAN="" VENDOR="" HCTL="" TYPE="part" FSTYPE="swap" LABEL="SWAP" UUID="ee6de8d5-eb1c-4d19-943c-0d24ecee92a7"', # noqa E501
'NAME="/dev/sdb4" MODEL="" SERIAL="" SIZE="29.9G" TRAN="" VENDOR="" HCTL="" TYPE="part" FSTYPE="btrfs" LABEL="ROOT" UUID="46befa9e-b399-480d-b10d-338a4c875615"', # noqa E501
'NAME="/dev/sdd" MODEL="SanDisk 3.2Gen1" SERIAL="01016c73a2fabf22051f78f30546627c9fb5e8083fbc14d477f85d6a269220dd15710000000000000000000060a25f5700804f0091558107a3ab6843" SIZE="57.3G" TRAN="usb" VENDOR=" USB " HCTL="6:0:0:0" TYPE="disk" FSTYPE="" LABEL="" UUID=""', # noqa E501
cellisten marked this conversation as resolved.
Show resolved Hide resolved
"",
]
]
Expand Down Expand Up @@ -1703,6 +1699,22 @@ def test_scan_disks_lsblk_parse_fail(self):
root=False,
partitions={"/dev/sdb4": "btrfs"},
),
Disk(
name="/dev/sdd",
model="SanDisk 3.2Gen1",
serial="01016c73a2fabf22051f78f30546627c9fb5e8083fbc14d477f85d6a269220dd15710000000000000000000060a25f5700804f0091558107a3ab6843",
size=60083404,
transport="usb",
vendor="USB",
hctl="6:0:0:0",
type="disk",
fstype=None,
label=None,
uuid=None,
parted=False,
root=False,
partitions={}
)
]
]
# As all serials are available via the lsblk we can avoid mocking
Expand Down