Fix removal of defunct vlan interfaces (LP #1959147) #256
Conversation
|
Hi! Thanks for the patch, it looks good for me. Don't worry about the autopkgtest suite failing, I'll run them manually. However, could you perhaps add a new integration test running the scenario outlined in the LP issue? |
|
I'll see if I can come up with a decent test tomorrow. The OVS tests are rather sparse so I'll have to figure out how to mock a network config change. |
|
You should be able to find examples of OVS integration tests in
tests/integration/ovs.py. Since those are run in a VM or container,
there's no need for mocking, you can set up the system using the actual
commands.
|
Codecov Report
@@ Coverage Diff @@
## main #256 +/- ##
=======================================
Coverage 99.14% 99.14%
=======================================
Files 60 60
Lines 10511 10511
=======================================
Hits 10421 10421
Misses 90 90
Continue to review full report at Codecov.
|
|
You wrote integration test but I saw unit test :) I added a test that I have moderate confidence should work. I don't have your integration test setup so I can't make sure it's working as I intended. On a side note: I think the removal of VLAN interfaces doesn't work for non-OVS interfaces as well (this is with including the |
|
Thank you for adding the tests :)
You can relatively easily run the tests by looking at the GH action
entry, if you wish. I'll run them on my end, and fix them up if
necessary.
Don't hesitate to file a new bug for the non-ovs vlan bug if it actually
is one!
|
|
Anything else I can do to help this along? The potential problem I referenced earlier is a directory structure "bug" (https://bugs.launchpad.net/netplan/+bug/1959706) and completely distinct from this patch. |
|
Sorry for the delay. I've finally managed to take the time to run the new test, and it fails early, as it seems the VLAN interface is never created: I couldn't really look further into it. |
|
I blindly copied and pasted the boilerplate test setup without really understanding what it all did; I inadvertently had it waiting for the just deleted network interface to settle which, as it no longer exists, failed. If you don't mind running the tests one more time, I believe it should work this time. Sorry about that! |
|
I ran the tests anew, they work now :). I also checked that they do fail on As soon as the CI passes against the rebased branch I'll merge. Thanks for your contribution :) |
Description
From my Launchpad bug report:
netplan fails to delete OVS vlan interfaces that were previously defined but were then removed from the configuration. The underlying problem appears to be that the
ovs-vsctl iface-to-brcommand used to test whether an interface is bonded or a vlan fails on "fake bridges" (this is how OVS defines vlans) which then causes it be treated incorrectly as a bonded interface. My testing seems to indicate theovs-vsctl br-existscommand may be a better choice for this test.Checklist
make checksuccessfully.make check-coverage). - Looks like there is a pre-existing lack of coverage on netplan/cli/commands/generate.py