CVM: Add boot/reboot tests #3624
Conversation
Signed-off-by: Thien Trung Vuong <[email protected]>
tpm2-tools is used to interact with a TPM device Signed-off-by: Thien Trung Vuong <[email protected]>
Signed-off-by: Thien Trung Vuong <[email protected]>
trungams
force-pushed
the
tvuong/azl-cvm-boot-test
branch
from
39b6e6c to
b900463
Compare
|
@LiliDeng @squirrelsc - This PR is ready to review. Could you help me review it when you have time? Thank you. The |
|
could you provide the full cvm mariner image name? like this format: microsoftcblmariner azure-linux-3 azure-linux-3-gen2 latest |
| # - Make sure VM boots up again | ||
| reboot_tool = node.tools[Reboot] | ||
| reboot_tool.reboot_and_check_panic(log_path) | ||
| is_ready, tcp_error_code = wait_tcp_port_ready( |
There was a problem hiding this comment.
It doesn't need to wait for ready in the test case. When you call next command, LISA will do it.
There was a problem hiding this comment.
Do you mean the wait_tcp_port_ready call can be removed? I was mostly following the pattern in smoke_test and a boot test defined in microsoft/testsuites/core/boot.py
| component is upgraded. | ||
|
|
||
| Steps: | ||
| 1. On first boot, check current PCR values for PCR4 and PCR7 |
There was a problem hiding this comment.
Add requirements on new environment, if it's needed.
There was a problem hiding this comment.
I have defined requirement for this test case below (CvmEnabled, AZURE). Does this suffice?
|
|
||
| # - PCR4 should change if any of the boot components is upgraded | ||
| # - PCR7 may change if a signing cert is changed | ||
| if boot_component_changed: |
There was a problem hiding this comment.
Is it possible to make sure all paths tested in the same test case?
There was a problem hiding this comment.
I think this logic already covers what we want to test for CVM booting, at least for Azure Linux. If you're referring to merging the 2 test cases defined in this file, I think it's possible. If that's preferred then I'll update my code
lisa/operating_system.py
Outdated
| @@ -119,6 +119,12 @@ class KernelInformation: | |||
| version_parts: List[str] | |||
|
|
|||
|
|
|||
| @dataclass | |||
| class PackageInformation: | |||
There was a problem hiding this comment.
Please modify existing get_package_information, make it compatible with any version format.
- Create a class called
LisaVersionInfo, which inherits fromVersionInfo, so all existing cases should continue to pass. - Provide the
raw_versionfield, so it can pass with non-semver. - Rewrite
parse_versionto return the new class.
There was a problem hiding this comment.
Done. Also refactored some related functions a bit. I'll need some help to validate that the change doesn't break any existing logic.
- cvm_boot: use bootctl to locate root device Signed-off-by: Thien Trung Vuong <[email protected]>
Signed-off-by: Thien Trung Vuong <[email protected]>
|
@LiliDeng The image I used to test this change is |
| security_profile_settings = cast( | ||
| SecurityProfileSettings, node.features[SecurityProfile].get_settings() | ||
| ) | ||
| if not security_profile_settings.encrypt_disk: |
There was a problem hiding this comment.
@squirrelsc @LiliDeng Ideally I want to be able to detect this setting earlier and specify it in the requirement above. I tried adding a function to lisa/features/security_profile.py as below:
DiskEncryptionEnabled = partial(
SecurityProfileSettings,
encrypt_disk=True
)and then use it similarly to how CvmEnabled is used. But it doesn't skip this test case if encrypt_disk is set to false in the runbook. Rather, it turns on that value for the running VM. That's why I had to resort to VM runtime check like this. Do you know how I can implement that kind of check to add to simple_requirement?
Signed-off-by: Thien Trung Vuong <[email protected]>
Signed-off-by: Thien Trung Vuong <[email protected]>
Signed-off-by: Thien Trung Vuong <[email protected]>
DiskWithVMGuestStateencryption setting.