tests: add new spread based snapd integration test #330

mvo5 · 2023-02-28T16:07:56Z

Description

This is a first draft of adding integration tests for netplan with snapd and dbus. It uses spread and the LXD backend to run inside the github runners. A first test is designed to do basic interactions between netplan and snapd.

I put this into "draft" mode because I ran into the following issues:

following the instruction to make crashes meson inside a 22.04 container (https://paste.ubuntu.com/p/qRnJvjyddN/)
when commenting out the testing the install step fails with:

meson install -C build --destdir=/
...
Installing /home/tests/netplan.completions to //usr/share/bash-completion/completions

ERROR: Tried to install symlink to missing file //lib/netplan/../../usr/libexec/netplan/generate

I suspect (2) is a result in the most recent #323 ?

I also wonder if the build/install/build-dep step could be simplified by getting the debian packaging from somewhere but I could not figure out where it comes from (sorry for that and hints/ideas welcome).

Testing locally via spread

$ GOBIN=~/go/bin go install github.com/snapcore/spread/cmd/spread@latest
$ ~/go/bin/spread -v -debug lxd:

slyon · 2023-02-28T16:38:03Z

FYI: packaging is done in: https://git.launchpad.net/~ubuntu-core-dev/netplan/+git/ubuntu

mvo5 · 2023-02-28T17:40:48Z

tests/spread/snapd-integration/task.yaml

+    fi
+    # fake running on core (netplan only enabled on ubuntu core)
+    # FIXME: find a more offical way, this if fugly
+    cp /etc/os-release /etc/os-release.save


This needs something like canonical/snapd#12607 to really work, this hack will break at some point but we have a plan in 12607

mvo5 · 2023-02-28T20:07:13Z

src/meson.build

@@ -41,6 +41,6 @@ meson.add_install_script(meson_make_symlink,
 # It's only around for legacy reasons, see netplan/cli/utils.py: get_generator_path()
 install_symlink(
    'generate',
-    pointing_to: join_paths('..', '..') + join_paths(get_option('prefix'), libexec_netplan, 'generate'),
+    pointing_to: join_paths('..', '..', '..') + join_paths(get_option('prefix'), libexec_netplan, 'generate'),


Fwiw, this change fixed the "make install" for me (but not sure if it's correct in all cases, it seems that in 0e2a8ff the path moved from /lib/netplan/generate to /usr/libexec/netplan/generate which is one level more so one more ".." seems to be needed) [but it requires a packaging update which is why the adt test fails right now it seems]

I'm a bit worried about this change. It makes our autopkgtest GH action fail. We need to be careful with meson installation paths, as those can be different on Fedora/Debian/Ubuntu and might need to go hand-in-hand with packaging changes. Can we somehow avoid this change?

Maybe this helps: 2cded2f (i.e. rebase on top of our recent main branch). See #327 for context.

slyon · 2023-03-02T15:29:13Z

FYI: I rebased this on top of #331 to see if the spread test is now passing.

mvo5 · 2023-03-02T19:13:19Z

FYI: I rebased this on top of #331 to see if the spread test is now passing.

And it looks like it did! Nice job, thanks for fixing the issue :-D !

mvo5 · 2023-03-06T09:29:20Z

I pushed a small tweak now to properly setup/restore the state of the /etc/netplan dir (which makes the need to use multiple test bridge interfaces go away as spread will never run the same test in parallel in the same VM/container, when it runs in parallel it does so on multiple VMs/containers but not in the same).

I would love to know if you folks are interested in taking this PR (in principal, happy to tweak as needed) or if you prefer not to. If this has a chance to land I can canonical/snapd#12607 to make the setup in the snapd integration test much nicer and potentially sketch how autopktest can be integrated with spread tests.

While autopkgtest and spread are broadly doing the same our experience in snapd is that spread scales better with the (buildin) concept of suits and the more natural "prepare/execute/restore" patterns that one knows from regular unit test writing. It also scales better when many tests are used as it will use as many VMs/containers as provided (initially not a concern here though as the tests run in 2min right now ;)

slyon

Hey @mvo5 Sorry for taking too long to get through my PR review backlog..
Yes, I'm very much interested in landing this! Even as a spread test in GitHub actions, as proposed. We might want to migrate that later on into an autopkgtest, so it's also run at autopkgtest.ubuntu.com (and other instances, like DebCI).

But landing this spread test should be a good first step. It could also help to prepare infrastructure/tooling for further spread test integration, like canonical/network-manager-snap#15

The only thing I'm worried about is the src/meson.build change (see my inline comment), otherwise this LGTM!

These tests integrate into the GH workflow and run snapd and dbus based integration tests.

This is archived by moving using the "restore-each" stanza and just restoring /etc/netplan in there. This avoids the need to have two br54,br55 (or N for N tests).

slyon · 2023-03-23T14:16:18Z

Actually, I think something like this should do the trick:

meson.add_install_script(meson_make_symlink,
    join_paths(get_option('prefix'), libexec_netplan, 'generate'),
    join_paths('/', 'lib', 'netplan', 'generate'))

I rebased and force pushed to your branch. Let's see the CI results.

slyon

+1 All tests are green now.

Michael, is there anything you'd like to add/change to this (e.g. regarding canonical/snapd#12607)? Otherwise, I think we could merge this as-is and improve upon in the future.

slyon

This is providing value as-is already. So let's get it merged. We can still improve upon this once snapd landed corresponding upstream changes. Or we've prepared an autopkgtest to run those spread tests accordingly.

mvo5 force-pushed the integration-test-spread branch 2 times, most recently from d8c94e2 to ca11dfa Compare February 28, 2023 16:16

mvo5 force-pushed the integration-test-spread branch from ca11dfa to 30a5433 Compare February 28, 2023 17:31

mvo5 commented Feb 28, 2023

View reviewed changes

mvo5 force-pushed the integration-test-spread branch 5 times, most recently from ef2fe25 to b7a28ef Compare February 28, 2023 19:42

This comment was marked as resolved.

Sign in to view

mvo5 force-pushed the integration-test-spread branch from b7a28ef to 700cfcc Compare February 28, 2023 20:06

mvo5 commented Feb 28, 2023

View reviewed changes

daniloegea mentioned this pull request Mar 1, 2023

dbus: Build the copy path correctly #331

Merged

5 tasks

slyon force-pushed the integration-test-spread branch from 700cfcc to d71804c Compare March 2, 2023 15:28

mvo5 marked this pull request as ready for review March 2, 2023 19:12

mvo5 force-pushed the integration-test-spread branch 2 times, most recently from 66aabe4 to 3654e20 Compare March 6, 2023 09:21

slyon reviewed Mar 23, 2023

View reviewed changes

mvo5 and others added 3 commits March 23, 2023 15:14

tests: add new spread based snapd integration test

bb77e21

These tests integrate into the GH workflow and run snapd and dbus based integration tests.

spread.yaml: restore /etc/netplan for each test

1afd4d3

This is archived by moving using the "restore-each" stanza and just restoring /etc/netplan in there. This avoids the need to have two br54,br55 (or N for N tests).

meson: fix installation of legacy /lib/netplan/generate symlink

ae86415

slyon force-pushed the integration-test-spread branch from 3654e20 to ae86415 Compare March 23, 2023 14:15

slyon approved these changes Mar 23, 2023

View reviewed changes

slyon approved these changes Apr 3, 2023

View reviewed changes

slyon merged commit 678de93 into canonical:main Apr 3, 2023

slyon added community This PR has been proposed by somebody outside of the Netplan team and roadmap commitments. Canonical by Canonical employees outside the Netplan team labels Jul 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: add new spread based snapd integration test #330

tests: add new spread based snapd integration test #330

mvo5 commented Feb 28, 2023 •

edited

Loading

slyon commented Feb 28, 2023

mvo5 Feb 28, 2023 •

edited

Loading

This comment was marked as resolved.

mvo5 Feb 28, 2023 •

edited

Loading

slyon Mar 23, 2023

slyon commented Mar 2, 2023

mvo5 commented Mar 2, 2023

mvo5 commented Mar 6, 2023

slyon left a comment •

edited

Loading

slyon commented Mar 23, 2023

slyon left a comment

slyon left a comment

tests: add new spread based snapd integration test #330

tests: add new spread based snapd integration test #330

Conversation

mvo5 commented Feb 28, 2023 • edited Loading

Description

Testing locally via spread

slyon commented Feb 28, 2023

mvo5 Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

mvo5 Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

slyon Mar 23, 2023

Choose a reason for hiding this comment

slyon commented Mar 2, 2023

mvo5 commented Mar 2, 2023

mvo5 commented Mar 6, 2023

slyon left a comment • edited Loading

Choose a reason for hiding this comment

slyon commented Mar 23, 2023

slyon left a comment

Choose a reason for hiding this comment

slyon left a comment

Choose a reason for hiding this comment

mvo5 commented Feb 28, 2023 •

edited

Loading

mvo5 Feb 28, 2023 •

edited

Loading

mvo5 Feb 28, 2023 •

edited

Loading

slyon left a comment •

edited

Loading