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

kola: provide internet access to qemu VM #167

Merged
merged 10 commits into from
Apr 19, 2021

Conversation

tormath1
Copy link
Contributor

@tormath1 tormath1 commented Apr 13, 2021

In this PR, we allow QEMU VMs to access Internet - it was preventing some tests to run because of this dependency.

How to use

Run kola against tests requiring Internet access, example:

$ sudo ./bin/kola run -b cl -p qemu --qemu-image ...  cl.rkt.etcd3
=== RUN   cl.rkt.etcd3
qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of qcow2)
--- PASS: cl.rkt.etcd3 (27.66s)
PASS, output in _kola_temp/qemu-2021-04-13-1711-9135

What's missing

  • update readme with more information the user does not have to perform any manual action. They are done in 3d69f85
  • add a teardown method (to remove what kola created)
  • run all tests with RequiresInternetAccess flag and remove this flag

@tormath1 tormath1 requested a review from pothos April 13, 2021 15:23
@tormath1 tormath1 self-assigned this Apr 13, 2021
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
system/ns/enter.go Outdated Show resolved Hide resolved
system/ns/enter.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
@tormath1
Copy link
Contributor Author

@pothos thanks for the review - comments have been addressed. :)

platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
@tormath1 tormath1 marked this pull request as ready for review April 15, 2021 13:09
@tormath1
Copy link
Contributor Author

@pothos thanks for reviewing again - comments have been addressed. PR is now ready to be fully reviewed :) If it looks good for you, I will rebase my fixups and reword some commits since logic has changed on the road.

platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
Copy link
Member

@pothos pothos left a comment

Choose a reason for hiding this comment

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

I think we should also remove the RequiresInternetAccess flag and PlatformsNoInternet definitions from kola/register/register.go. Or keep the RequiresInternetAccess flag untouched and just remove qemu from PlatformsNoInternet.

@tormath1
Copy link
Contributor Author

I think we should also remove the RequiresInternetAccess flag and PlatformsNoInternet definitions from kola/register/register.go. Or keep the RequiresInternetAccess flag untouched and just remove qemu from PlatformsNoInternet.

@pothos I removed RequiresInternetAccess flag in (55ea9ee and its parent commit) but not all flag since we are hitting the issue :

=== RUN   cl.locksmith.cluster
--- FAIL: cl.locksmith.cluster (0.10s)
        harness.go:523: Cluster failed starting machines: rendering Container Linux config for platform "": error: platform must be specified to use templating

which should be tracked and addressed in another issue. It's better for now to keep this definitions then.

@tormath1 tormath1 force-pushed the tormath1/mantle-qemu-tests branch from 43cc678 to 4517920 Compare April 15, 2021 15:07
@pothos
Copy link
Member

pothos commented Apr 15, 2021

The failing tests can be excluded with ExcludePlatforms: []string{"qemu"}, until then

@pothos
Copy link
Member

pothos commented Apr 15, 2021

cl.locksmith.cluster needs to be excluded

@tormath1 tormath1 force-pushed the tormath1/mantle-qemu-tests branch from 05176ce to 6564139 Compare April 16, 2021 13:51
@pothos
Copy link
Member

pothos commented Apr 16, 2021

Looks good, thanks!
The only thing I can think of is maybe checking if there is a 172.something network already present like from Docker and not pick addresses in that range (maybe a retry loop). This is not a big problem but it could happen that a container IP address clashes and then the container can't access the network while kola runs…

@tormath1
Copy link
Contributor Author

@pothos I added the retry logic: we provide the list of the host networks to the generate function then we check if the veth generated host IP is in one of this networks.

platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
platform/local/dnsmasq.go Outdated Show resolved Hide resolved
Mathieu Tortuyaux added 8 commits April 19, 2021 12:40
generation is based on a seed
this new topology allows QEMU VM to access internet. This allows
tests with RequiresInternetAccess flag to now be run.

on the implementation PoV: we basically create a virtual ethernet
pair generated from a seed (a TCP socket listener in the root namespace)
Then we configure routing and firewall to use this pair in order to
forward packet until root network namespace
this was preventing internet access from `qemu-unpriv` platform
@tormath1 tormath1 force-pushed the tormath1/mantle-qemu-tests branch from f5bcbb4 to 5bd5855 Compare April 19, 2021 10:42
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