Add verify_supports_nvme #3122
Add verify_supports_nvme #3122
Conversation
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
from
529ffbb
to
3ec49f0
Compare
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
2 times, most recently
from
162bd02
to
f7fed9b
Compare
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
from
f7fed9b
to
d9c9396
Compare
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
from
d9c9396
to
e7b85e7
Compare
|
Rebased on PR #3255 |
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
3 times, most recently
from
80a0bca
to
abbecf8
Compare
| ) | ||
| image = node_runbook.image | ||
| # Failure conditions | ||
| if image and not isinstance(image, VhdSchema): |
There was a problem hiding this comment.
VHD should be able to set in the runbook to specify it supports nvme.
There was a problem hiding this comment.
We can't rely on people manually specifying things in the runbook because it causes confusion. This means we need a good default value. If we don't include NVMe in VHD default, people will be confused why they can't deploy VHD with NVMe disk controller type. On the other hand, if default value has NVMe (as it currently does) then there will be confusing failures on this test case. The test case will assume VHD supports NVMe by default when this isn't necessarily true.
If someone wants to check if their VHD supports VHD all they have to do is confirm that this test case is pass and not skip. VHDs aren't tagged with any info so there shouldn't be an expectation that LISA will check if they are correctly tagged.
There was a problem hiding this comment.
Can you list the situations in a table? With current logic, the test case is always disabled for vhd.
There was a problem hiding this comment.
| Marketplace | SIG / CGI | VHD | |
|---|---|---|---|
| Has drivers | PASS | PASS | PASS |
| Missing drivers, not tagged | SKIP | SKIP | SKIP |
| Missing drivers, tagged capable | FAIL | FAIL | VHD is never tagged |
There was a problem hiding this comment.
The main purpose of this test case is "Tag the image if and only if the test case passes" to be used before the image is published publicly to the marketplace. We don't really care about the skip vs. failure condition unless an image published in the marketplace is tagged but doesn't have drivers. This test case does not really care about VHDs because they aren't tagged.
There was a problem hiding this comment.
It maybe a case, that we need to verify a VHD is good with the tag before it's published. If the runbook is not modified, the vhd can be skipped to verify. It needs to be capable to verify vhd support that or not by adding tag in runbook. How about create unsupported NVMe capable for vhd by default?
There was a problem hiding this comment.
Here is how I see the options
| Deploying VHD on NVMe | Running verify_support_nvme | |
|---|---|---|
| Default support NVMe | Works! | Why is test case failing? :( |
| Default don't support NVMe | Doesn't deploy :( | Catches Failures |
I could add a tag which it separate from disk_controller_type for the image, but it would be quite niche use case. It would be confusing and I don't think it would really be used.
There was a problem hiding this comment.
If we add a test variable for the test case, can it solve the problem?
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
from
abbecf8
to
5337e54
Compare
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
from
5337e54
to
c4c6608
Compare
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
from
c4c6608
to
c97b143
Compare
This test case checks whether the NVMe drivers are present such that the image can boot with DiskControllerType NVMe.
kamalca
force-pushed
the
kameroncarr/verify-nvme-boot
branch
from
187c3f3
to
02dab93
Compare
This test case checks whether the NVMe drivers are present such that the image can boot with DiskControllerType NVMe.
We needed a test that will do the same thing as nvme_enabled in the info section. This will enable AzQualify to check images before tagging them.
We cannot simply do verify_deployment_provision with disk_controller_type set to NVMe or use verify_nvme_disk_controller_type because deployment will fail if the image is not yet tagged as NVMe.