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

Skip storage testcases if nvme cmd doesn't have 'NameSpace' in output. #3685

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Skip storage testcases if nvme cmd doesn't have 'NameSpace' in output. #3685

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8a5ec48
Skip storage testcases if nvme cmd doesn't have 'NameSpace' in output.
SRIKKANTH Feb 25, 2025
4a1aa66
Update nvmecli.py
SRIKKANTH Feb 27, 2025
33dc322
Mariner 2 issue fix
SRIKKANTH Feb 28, 2025
846c6d5
Update nvmecli.py
SRIKKANTH Feb 28, 2025
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
Next Next commit
Skip storage testcases if nvme cmd doesn't have 'NameSpace' in output. 
Older versions of nvme-cli do not have the NameSpace key in the output skip the test if NameSpace key is not available instead of failing.
  • Loading branch information
@SRIKKANTH
SRIKKANTH committed Feb 27, 2025
commit 8a5ec480b4aca04b11942c610183a555eba75229
9 changes: 8 additions & 1 deletion lisa/tools/nvmecli.py
View file
Original file line number Diff line number Diff line change
@@ -7,8 +7,8 @@
from lisa.executable import Tool
from lisa.operating_system import Posix
from lisa.tools import Git, Make
from lisa.util import find_patterns_in_lines
from lisa.util.process import ExecutableResult
from lisa.util import find_patterns_in_lines, SkippedException


class Nvmecli(Tool):
@@ -161,6 +161,13 @@ def get_disks(self, force_run: bool = False) -> List[str]:

def get_namespace_ids(self, force_run: bool = False) -> List[Dict[str, int]]:
nvme_devices = self.get_devices(force_run=force_run)
# Older versions of nvme-cli do not have the NameSpace key in the output
# skip the test if NameSpace key is not available
if not nvme_devices or "NameSpace" not in nvme_devices[0]:
raise SkippedException(
"'NameSpace' key is not available in 'nvme -list -o json' output"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message is not clear enough. It says what's verified, but not explain why the test case is skipped from the test purpose. Why is the test skipped, when the NameSpace is not available? Even the Namespace doesn't exist, it's still a nvme device.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Older images like CentOS don't have latest nvme cli which is missing support for "Namespace" field in the output.

This is needed only for serial hot add storage testcases (with disk controller == NVMe) but not for actual NVMe testcases.
Parallel storage testcases and other tests are not impacted by this.

The "Namespace" field is used to get the LUN numbers of the remote disks which is used by all serial hot add disk tests. Without this the tests will not work on VMs with NVMe disk controller.

Modified the exception message saying the reason and added a debug message

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing, it's better, and can be improved too.

  1. It needs to raise skipped exceptions in tool level, because it doesn't know how the tools will be used. Some test cases maybe skipped unexpected. Please raise general exception here, and catch/rethrow it to skipped in test cases.
  2. The message should tell the details like in the review comments. Some information is important to help understanding why it's skipped like: The "Namespace" field is used to get the LUN numbers of the remote disks which is used by all serial hot add disk tests. Without this the tests will not work on VMs with NVMe disk controller.

)

return [
{device["DevicePath"]: int(device["NameSpace"])} for device in nvme_devices
]
Loading
Oops, something went wrong.