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

Implement YAML state tracking and use it in the DBus API and netplan-try (LP: #1943120) (FR-1745) #231

Merged
merged 7 commits into from
Sep 27, 2021

Conversation

slyon
Copy link
Collaborator

@slyon slyon commented Sep 24, 2021

Description

Allow to pass an optional --state parameter to netplan try/apply describing a directory that contains a netplan configuration tree/state (i.e. /{etc,run,lib}/netplan/*.yaml).
Netplan will make use of this "old state" to calculate the delta of dropped interface definitions, like bridges/bonds/vlans/tunnels that have been configured before but are not part of the current YAML configuration anymore. It will then try to delete those virtual interfaces (via ip link del dev IFACE) if they still exist.

The same functionality is used to roll back a netplan try command that failed or was rejected.

Generally, the state needs to be provided manually. The DBus API (using io.netplan.Netplan.Config.Try/Apply) is an exception, as the previous state can be backed up automatically in this case.

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: LP#1943120

@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2021

Codecov Report

Merging #231 (3f859f0) into main (17d3848) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #231   +/-   ##
=======================================
  Coverage   99.04%   99.05%           
=======================================
  Files          57       57           
  Lines        9524     9592   +68     
=======================================
+ Hits         9433     9501   +68     
  Misses         91       91           
Impacted Files Coverage Δ
netplan/cli/commands/apply.py 100.00% <100.00%> (ø)
netplan/cli/commands/try_command.py 100.00% <100.00%> (ø)
netplan/configmanager.py 100.00% <100.00%> (ø)
src/dbus.c 100.00% <100.00%> (ø)
tests/dbus/test_dbus.py 100.00% <100.00%> (ø)
tests/test_cli_units.py 100.00% <100.00%> (ø)
tests/test_configmanager.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17d3848...3f859f0. Read the comment docs.

@slyon slyon force-pushed the slyon/clear-virtual-interfaces-lp1943120 branch from 300ff5c to 3f859f0 Compare September 24, 2021 15:40
@slyon slyon marked this pull request as ready for review September 24, 2021 15:43
Copy link
Contributor

@schopin-pro schopin-pro left a comment

Choose a reason for hiding this comment

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

I have a few non-blocking nitpicks but all in all LGTM.

Have you run the integration tests?

netplan/cli/commands/apply.py Outdated Show resolved Hide resolved
tests/test_cli_units.py Outdated Show resolved Hide resolved
src/dbus.c Show resolved Hide resolved
@slyon
Copy link
Collaborator Author

slyon commented Sep 27, 2021

Thanks for your feedback. I've made the devices parameter a named argument and added the extra code path and relevant testing around it.

As this have only been nitpicks and the integration tests are passing (incl. the new one that explicitly checks for this case), too, let's get this merged.

$ time autopkgtest ../netplan.io_0.103-0ubuntu6_amd64.changes -U -- qemu /data/autopkgtest-impish-amd64.img
autopkgtest [15:39:14]: starting date: 2021-09-27
autopkgtest [15:39:14]: version 5.16ubuntu1
autopkgtest [15:39:14]: host abaconcy; command line: /usr/bin/autopkgtest ../netplan.io_0.103-0ubuntu6_amd64.changes -U -- qemu /data/autopkgtest-impish-amd64.img
[...]
autopkgtest [16:12:15]: @@@@@@@@@@@@@@@@@@@@ summary
ovs                  PASS
ethernets            PASS
bridges              PASS
bonds                PASS
routing              PASS
vlans                PASS
wifi                 FLAKY non-zero exit status 1
tunnels              PASS
scenarios            PASS
regressions          PASS
autostart            PASS
cloud-init           PASS
qemu-system-x86_64: terminating on signal 15 from pid 375488 (/usr/bin/python3)

real	33m1,326s
user	9m28,119s
sys	1m2,647s

@slyon slyon merged commit 730fbbd into main Sep 27, 2021
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