Skip to content

tests:base:ethernets: Improve stability of autopkgtests #223

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

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

slyon
Copy link
Collaborator

@slyon slyon commented Aug 20, 2021

Make sure NetworkManager's unmanaged-devices are kept in check so that NM does not take control over some network interfaces that are controlled by the test system, such as the routers veth42, veth42, fake wifi AP, ...

Replaces the "autopkgtest-fixes.patch" Distro patch.

Changing the location of the denylist from /run to /etc because the changes in /run would be overwritten by our test system's /etc/NetworkManager/conf.d/90-test-ignore.conf, because /etc/ drop-in configs are read after /run/ drop-ins and the /etc/ drop-in uses a hard overwrite of unmanaged-devices=... in order to reset the system's default in /usr/lib/NetworkManager/conf.d/10-globally-managed-devices.conf (guess we could also downgrade the test systems static /etc/ drop-in to /run/ which should have the same effect).

So with this change we would have the following order:

  1. base system: (system config, to be overwritten)
    /usr/lib/NetworkManager/conf.d/10-globally-managed-devices.conf
  2. setUpClass: (static test system config, i.e. management interface eth0, en*)
    /etc/NetworkManager/conf.d/90-test-ignore.conf
  3. create_devices: (dynamic test interfaces, like eth42, eth43, wlan1, ...)
    /etc/NetworkManager/conf.d/99-test-denylist.conf

[...] you can add additional .conf files to the /etc/NetworkManager/conf.d directory. These will be read in order, with later files overriding earlier ones. Packages might install further configuration snippets to /usr/lib/NetworkManager/conf.d. This directory is parsed first, even before NetworkManager.conf. Scripts can also put per-boot configuration into /run/NetworkManager/conf.d. This directory is parsed second, also before NetworkManager.conf. The loading of a file /run/NetworkManager/conf.d/name.conf can be prevented by adding a file /etc/NetworkManager/conf.d/name.conf. Likewise, a file /usr/lib/NetworkManager/conf.d/name.conf can be shadowed by putting a file of the same name to either /etc/NetworkManager/conf.d or /run/NetworkManager/conf.d.

https://developer.gnome.org/NetworkManager/stable/NetworkManager.conf.html

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@slyon slyon force-pushed the slyon/autopkgtest-stability branch from 59ce930 to 60c072b Compare August 20, 2021 14:03
@codecov-commenter
Copy link

codecov-commenter commented Aug 20, 2021

Codecov Report

Merging #223 (3ed067e) into main (6a47c70) will decrease coverage by 0.18%.
The diff coverage is n/a.

❗ Current head 3ed067e differs from pull request most recent head e7c4638. Consider uploading reports for the commit e7c4638 to get more accurate results

@@            Coverage Diff             @@
##             main     #223      +/-   ##
==========================================
- Coverage   99.19%   99.02%   -0.18%     
==========================================
  Files          61       56       -5     
  Lines       11206     9343    -1863     
==========================================
- Hits        11116     9252    -1864     
- Misses         90       91       +1     
Impacted Files Coverage Δ
src/nm.c 99.46% <0.00%> (-0.54%) ⬇️
src/validation.c 99.57% <0.00%> (-0.04%) ⬇️
src/dbus.c 100.00% <0.00%> (ø)
src/util.c 100.00% <0.00%> (ø)
src/error.c 100.00% <0.00%> (ø)
src/sriov.c 100.00% <0.00%> (ø)
src/netplan.c 100.00% <0.00%> (ø)
src/generate.c 100.00% <0.00%> (ø)
src/networkd.c 100.00% <0.00%> (ø)
src/parse-nm.c 100.00% <0.00%> (ø)
... and 40 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@slyon slyon marked this pull request as draft August 20, 2021 14:19
@slyon slyon force-pushed the slyon/autopkgtest-stability branch from 1906350 to d4966ba Compare August 20, 2021 15:30
@@ -444,8 +445,8 @@ def create_devices(klass):
klass.dev_w_client = devs[1]

# don't let NM trample over our fake AP
with open('/run/NetworkManager/conf.d/test-blacklist.conf', 'w') as f:
f.write('[main]\nplugins=keyfile\n[keyfile]\nunmanaged-devices+=nptestsrv,%s\n' % klass.dev_w_ap)
with open('/etc/NetworkManager/conf.d/99-test-denylist.conf', 'w') as f:
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why the change from /run to /etc ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because the changes in /run would be overwritten by our test system's /etc/NetworkManager/conf.d/90-test-ignore.conf, because /etc/ drop-in configs are read after /run/ drop-ins and the /etc/ drop-in uses a hard overwrite of unmanaged-devices=... in order to reset the system's default in /usr/lib/NetworkManager/conf.d/10-globally-managed-devices.conf (guess we could also downgrade the test systems static /etc/ drop-in to /run/ which should have the same effect).

So with this change we would have the following order:

  1. base system: (system config, to be overwritten)
    /usr/lib/NetworkManager/conf.d/10-globally-managed-devices.conf
  2. setUpClass: (static test system config, i.e. management interface eth0, en*)
    /etc/NetworkManager/conf.d/90-test-ignore.conf
  3. create_devices: (dynamic test interfaces, like eth42, eth43, wlan1, ...)
    /etc/NetworkManager/conf.d/99-test-denylist.conf

[...] you can add additional .conf files to the /etc/NetworkManager/conf.d directory. These will be read in order, with later files overriding earlier ones. Packages might install further configuration snippets to /usr/lib/NetworkManager/conf.d. This directory is parsed first, even before NetworkManager.conf. Scripts can also put per-boot configuration into /run/NetworkManager/conf.d. This directory is parsed second, also before NetworkManager.conf. The loading of a file /run/NetworkManager/conf.d/name.conf can be prevented by adding a file /etc/NetworkManager/conf.d/name.conf. Likewise, a file /usr/lib/NetworkManager/conf.d/name.conf can be shadowed by putting a file of the same name to either /etc/NetworkManager/conf.d or /run/NetworkManager/conf.d.

https://developer.gnome.org/NetworkManager/stable/NetworkManager.conf.html

@slyon
Copy link
Collaborator Author

slyon commented Sep 6, 2021

@slyon slyon force-pushed the slyon/autopkgtest-stability branch 2 times, most recently from 9fe1820 to 9ba6a33 Compare February 1, 2023 16:14
@slyon slyon marked this pull request as ready for review February 1, 2023 16:14
@slyon slyon requested a review from daniloegea February 1, 2023 16:27
Make sure the "unmanaged-devices" are kept in check so that
NetworkManager does not take control over some network interfaces that
are controlled by the test system, such as the routers veth42, veth42,
fake wifi AP, ...
@slyon slyon force-pushed the slyon/autopkgtest-stability branch from 9ba6a33 to e7c4638 Compare February 8, 2023 09:03
@slyon slyon merged commit d265555 into main Feb 8, 2023
@slyon slyon deleted the slyon/autopkgtest-stability branch February 8, 2023 16:39
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.

3 participants