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

test: add a test to check for nested virtualization #4404

Merged
merged 5 commits into from
Jan 25, 2024

Conversation

pb8o
Copy link
Contributor

@pb8o pb8o commented Jan 24, 2024

Check that nested virtualization is disabled in all our CPU templates.

Other tests already check for CPU features explicitly, but this test just checks that virtualization is not available to the guest, however the means.

Changes

[rev3]

  • Added fix: test_sec_audit. The test has failed 3 times in this PR, so I am including it even though it is unrelated.
  • Disabled some tests that don't pass. This is OK because the tests are completely disabled at the moment, so at least we enable the ones that pass. We will follow up on the 3 failing ones.

How tested

https://buildkite.com/firecracker/firecracker-pr/builds/8665#018d3caa-7488-44ee-a123-ea76ca15f318/55-278

Reason

...

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following
Developer Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • If a specific issue led to this PR, this PR closes the issue.
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • API changes follow the Runbook for Firecracker API changes.
  • User-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.
  • New TODOs link to an issue.
  • Commits meet contribution quality standards.

  • This functionality cannot be added in rust-vmm.

@pb8o pb8o added Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Awaiting author Indicates that an issue or pull request requires author action labels Jan 24, 2024
@pb8o pb8o self-assigned this Jan 24, 2024
Copy link

codecov bot commented Jan 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2455117) 81.51% compared to head (92e08e6) 81.53%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4404      +/-   ##
==========================================
+ Coverage   81.51%   81.53%   +0.02%     
==========================================
  Files         241      241              
  Lines       29332    29332              
==========================================
+ Hits        23910    23916       +6     
+ Misses       5422     5416       -6     
Flag Coverage Δ
4.14-c7g.metal 76.92% <ø> (ø)
4.14-m5d.metal 78.85% <ø> (+0.01%) ⬆️
4.14-m6a.metal 77.97% <ø> (ø)
4.14-m6g.metal 76.92% <ø> (ø)
4.14-m6i.metal 78.84% <ø> (ø)
5.10-c7g.metal 79.82% <ø> (ø)
5.10-m5d.metal 81.50% <ø> (ø)
5.10-m6a.metal 80.72% <ø> (ø)
5.10-m6g.metal 79.82% <ø> (ø)
5.10-m6i.metal 81.50% <ø> (ø)
6.1-c7g.metal 79.82% <ø> (ø)
6.1-m5d.metal 81.51% <ø> (+0.01%) ⬆️
6.1-m6a.metal 80.72% <ø> (ø)
6.1-m6g.metal 79.82% <ø> (ø)
6.1-m6i.metal 81.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pb8o pb8o force-pushed the add-nv-test branch 2 times, most recently from 4f329a0 to 482d206 Compare January 24, 2024 14:08
@pb8o pb8o added Priority: Medium Indicates than an issue or pull request should be resolved ahead of issues or pull requests labelled and removed Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` labels Jan 24, 2024
@pb8o pb8o force-pushed the add-nv-test branch 2 times, most recently from 5a95b66 to d2aa9cc Compare January 24, 2024 14:22
@pb8o pb8o added Status: Awaiting review Indicates that a pull request is ready to be reviewed and removed Status: Awaiting author Indicates that an issue or pull request requires author action labels Jan 24, 2024
roypat
roypat previously approved these changes Jan 24, 2024
@pb8o pb8o added Status: Awaiting author Indicates that an issue or pull request requires author action and removed Status: Awaiting review Indicates that a pull request is ready to be reviewed labels Jan 24, 2024
Enabling those tests makes them currently fail.

Skip for now while we investigate if they fail for valid reasons.

Signed-off-by: Pablo Barbáchano <[email protected]>
This enables the tests that are not currently on CI.

We should enable them only in Linux 6.1 since they won't work in any
other kernel.

Fixes: c358311

Signed-off-by: Pablo Barbáchano <[email protected]>
Check that nested virtualization is disabled in all our CPU templates.

Other tests already check for CPU features explicitly, but this test
just checks that virtualization is not available to the guest, however
the means.

Signed-off-by: Pablo Barbáchano <[email protected]>
Since we run them now as part of CI, there's no reason to run them
separately here.

Signed-off-by: Pablo Barbáchano <[email protected]>
Since we run this test twice in case of a PR, cargo audit locks the
database and one of the processes may print something (even with -q
specified) in stdout (‽). I believe this should be printed in stderr,
but we can workaround it here easily.

    warning: directory /usr/local/rust/advisory-db is locked
    {...}

Use `grep` to filter out the extraneous output.

Signed-off-by: Pablo Barbáchano <[email protected]>
@pb8o pb8o merged commit d761b01 into firecracker-microvm:main Jan 25, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium Indicates than an issue or pull request should be resolved ahead of issues or pull requests labelled Status: Awaiting author Indicates that an issue or pull request requires author action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants