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

Add nested virtualization tests #55

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

Conversation

k-304
Copy link
Contributor

@k-304 k-304 commented Jan 14, 2025

Test if KVM virtualization works with QEMU.

@k-304 k-304 force-pushed the add-nested-virtualization-support branch 4 times, most recently from dd8231c to 6373028 Compare January 15, 2025 07:33
@k-304 k-304 force-pushed the add-nested-virtualization-support branch from 6373028 to 3217559 Compare January 15, 2025 07:48
@k-304 k-304 requested a review from href January 15, 2025 08:01
@@ -222,6 +224,12 @@ def test_common_images_have_a_hosts_file(create_server, image):
server = create_server(image=image)
```

### Update README tests table
If you add new tests, update the tests table:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The rest of the README uses spaces after headings, and before code-blocks.

resources.py Outdated
@@ -910,6 +910,53 @@ def enable_dhcp_in_networkd(self, interface):
# Wait a few seconds for the DHCP to be applied
time.sleep(5)

def prepare_nested_virtualization(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Functions should only be part of resources.py, when it would make the tests too complicated if they were included in the test functions directly, distracting from what we actually want to test.

The idea with the test_ files is: A customer can read them and understand what we are doing, maybe without having to understand complicated details. A bit of an exception are LBaaS tests, as they would get very long and hard to understand, if we included the whole LBaaS setup with each test.

I don't think your functions here qualify: You should inline your code in the test functions, to demonstrate what we are checking / what the platform is meant to be capable of.

""" Nested virtualization is supported. """

vm_os = 'alpinelinux3.17' # Needs to match one virt os-variant
vm_iso_url = (
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not use externally hosted ISOs for test. They cost bandwidth and causes our tests to fail if there's an issue with someone else's CDN.

We already store custom alpine images here: https://at-images.objects.lpg.cloudscale.ch/ (search for it in the code), I think we can add an ISO there, making the URL much shorter in the process.

Even easier might be to just download our custom alpine image, and run that. If I'm not missing something you are only downloading the ISO anyway, not actually mounting it or anything.

)

# Start a server with the flex-4-1 flavor
server = create_server(flavor='flex-4-1')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why create a server by hand, instead of just using server as fixture?

resources.py Outdated
self.run('sudo virsh net-start default')
self.run('sudo virsh net-autostart default')
self.run(f'qemu-img create -f qcow2 {vm_os}.qcow2 8G')
self.run(oneliner(f"""
Copy link
Contributor

Choose a reason for hiding this comment

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

If we use our own custom alpine image, we could start that, then get the console output and check that it boots.

@k-304 k-304 force-pushed the add-nested-virtualization-support branch 2 times, most recently from 5891186 to d1d3b9c Compare January 24, 2025 07:41
@k-304 k-304 force-pushed the add-nested-virtualization-support branch from d1d3b9c to c789be0 Compare January 24, 2025 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants