Openvswitch support (LP: #1728134) #154
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Initial first-try framework for OpenVSwitch schema This basically adds support for generating systemd unit files that would then later configure the OVS bits for devices that require it. The only schema piece supported right now is 'other-config' and 'external-ids', which were quite easy to do as they directly pass-through to OVS. This adds support for both the per-interface and global config. Added some basic unit testing for 100% coverage. Co-authored-by: Łukasz 'sil2100' Zemczak <lukasz.zemczak@canonical.com>
This adds support for OpenVSwitch bonds, with the additional parameters->mode (balance-tcp & balance-slb) and openvswitch->lacp fields. It implements netplan apply for OVS devices, defines systemd unit-file dependecies for OVS devices and adds a new, basic integration test for OVS. Commits: * ovs: initial bond setup * tests: cleanup * ovs: delete default bond iface * Set OpenVSwitch renderer implicitly * Handle all bond setup from within the bond netdef/unit itself * Allow to define a dependency on a systemd netplan-ovs-* unit * Bond setup: Add tests for error cases * Refactor bond interface setup into its own function * OVS: Add 'lacp' key for bonds * OVS: Add additional bond modes * OVS supports only 3 bond modes * Mark OVS bonds as external-ids:netplan=true * OVS: make use of id_escaped for bonds * OVS: 'netplan apply' for bonds + basic integration test * OVS: keep using networkd backend, if only external-ids or other-config are given in 'openvswitch:' * OVS: refactor bond params in separate functions * OVS: handle all bonds with 'openvswitch:' key via OVS backend * OVS: free GList after use * OVS: reduce boilerplate
…tings (#144) This adds support for OpenVSwitch bridges as per the OVS spec. This in theory should be feature complete minus the 'controllers' section which I did not manage to add yet due to time constraints. Another important-ish part of this branch is adding support for writing the additional .network file configuration for OpenVSwitch bridges and bonds to configure non-layer-2 settings like IP addresses etc. Not sure if this is enough, but for sure that's a start. With this, after our quick chat yesterday, I also streamlined the use of id_escaped, using it only in places where it's actually necessary. Commits: * ovs: initial bond setup * tests: cleanup * ovs: delete default bond iface * Set OpenVSwitch renderer implicitly * Handle all bond setup from within the bond netdef/unit itself * Allow to define a dependency on a systemd netplan-ovs-* unit * Bond setup: Add tests for error cases * Refactor bond interface setup into its own function * OVS: Add 'lacp' key for bonds * OVS: Add additional bond modes * OVS supports only 3 bond modes * Mark OVS bonds as external-ids:netplan=true * OVS: make use of id_escaped for bonds * OVS: 'netplan apply' for bonds + basic integration test * OVS: keep using networkd backend, if only external-ids or other-config are given in 'openvswitch:' * OVS: refactor bond params in separate functions * OVS: handle all bonds with 'openvswitch:' key via OVS backend * OVS: free GList after use * Initial WIP bridges support. * Add more missing bridge fields, add tests, add support for generating a base networkd config for all the devices. * Try to streamline the use of id_escaped now that we know how it's needed by systemd * Changes per Lukas's review * Add tests for coverage, update docs and add OpenFlow16 support for the future. Co-authored-by: Lukas Märdian <lukas.maerdian@canonical.com> Co-authored-by: Łukasz 'sil2100' Zemczak <lukasz.zemczak@canonical.com>
Adds support for the OpenVSwitch Bridge controller.addresses config key to specify OpenFlow controller targets and controller.connection-mode config key, to define if the controller shall be used in a in-band or out-of-band configuration. Additionally, it adds support for the following global openvswitch.ssl configs, which need to be used if a ssl: or pssl: OpenFlow controller target is to be used. ca-cert certificate private-key It adds a new integration test in tests/integration/ovs.py, which we need to make sure to integrate into the debian package (via debian/tests/control), to have it run during autopkgtest. Commits: * OVS: bridge controller addresses/targets parsing * OVS: bridge controller cleanup, documentation & tests * OVS: add basic bridge integration test * OVS: update bridge controller wording, as defined per spec * OVS: implement global ssl settings, used by controller.addresses * OVS: adopt integration tests for SSL * OVS: check_ssl improvements from review
This adds support for OVS patch ports, implements the openvswitch.ssl setting in a global way (instead of per port), constructs OVS interfaces/ports only if they are not there yet, using --may-exist, as suggested by James/OpenStack and improves the integration tests for OpenVSwitch. Additionally, it fixes a bug in IP4/IP6 parsing, where the same address might have been added multiple times, if the config needed to be parsed in multiple runs. Commits: * OVS: initial patch port implementation * OVS: automatically choose OVS backend for bridge/bond, if it contains an OVS interface * OVS: configure SSL globally * OVS: make patch-ports work in 2nd and 1st pass * parse: fix duplicate IP4/IP6 after multiple passes * doc: improve OpenVSwitch section * OVS: improve patch-port tests * OVS:Bridge: do not add-port for OVS bonds, they'll to it in their own systemd unit * OVS: Adopt integration tests * OVS: integrate some feedback from James * doc: improve wording * cleanup * parse: fix stylistic issues
If the target is "ptcp:1337", it will check_ovs_target() on "1337".
g_strsplit() will result in vec = { "1337", NULL } – so vec[2] is
undefined!
… OVS interfaces (#149) Throw error, if OVS backend is applied to an unsupported interface type. Check for existence of all OVS systemd unit files in unit-tests Add (preliminary) VLAN-Bridge integration test for OVS Commits: * OVS: throw error on unsupported device/interface type * networkd: do not require networkd Bridge= if the bridge is managed by OVS * OVS: improve tests * Revert "networkd: do not require networkd Bridge= if the bridge is managed by OVS" * cleanup * OVS: add bridge_vlan integration test
The spec defined fake VLAN bridge structures, but wasn't very clear about it being in scope. Let's add those now. Co-authored-by: Łukasz 'sil2100' Zemczak <lukasz.zemczak@canonical.com> Co-authored-by: Lukas Märdian <lukas.maerdian@canonical.com>
To verify normal bonds are not ignored if they are set up as OVS bridge interfaces.
Co-authored-by: Łukasz 'sil2100' Zemczak <lukasz.zemczak@canonical.com>
This provides general cleanup of the OVS codebase, such as test and comment improvements. It also makes sure that left-over OVS interfaces (tagged netplan=true) are cleaned up during netplan apply, by executing del-br/del-port on those interfaces. All ExecStop= commands and RemainAfterExit=true have been removed and all cleanup is now done via the netplan-ovs-cleanup.service unit, which runs prior to the other netplan-ovs-*.service units. I'm adding a new parameter netplan apply --ovs-only (called via netplan-ovs-cleanup.service as well), which will just clean up all netplan=true taged OVS interfaces (Bridges/Ports/Bonds), if they are not defined in the current YAML config. I reduced the usage of systemd_escape() even more, to have it only escape interface names, if they are part of an escaped systemd "path" where they actually need to be escaped (such as sys-subsystem-net-devices-<ESCAPED_ID>.device). This avoids having normal interface names, such as br-data escaped, which leads to a partly escaped systemd service unit name (e.g. netplan-ovs-br\x2ddata.service) and unescaping problems when passing those through subprocess calls.
…ight be different per test environment.
…them via Interface instead of Bridge.
Codecov Report
@@ Coverage Diff @@
## master #154 +/- ##
===========================================
- Coverage 100.00% 93.12% -6.88%
===========================================
Files 41 9 -32
Lines 5166 2618 -2548
===========================================
- Hits 5166 2438 -2728
- Misses 0 180 +180
Continue to review full report at Codecov.
|
* Fix only_ovs_cleanup argument, to make it execute the correct code. * Execute the oneshot OVS systemd units synchronously, to avoid race conditions during systemctl start .... * Set OVS patch ports atomically, to avoid logging errors. * Clean up OVS fake VLAN bridges from the Interface table via del-br
|
I started to prepare packaging changes here: https://git.launchpad.net/~slyon/netplan/+git/ubuntu?h=slyon/ovs-wireguard-100 |
This PR implements the "Common tags for all device types" section of the spec. Whenever any OVS setting is applied inside the ovsdb, this code adds another setting for the corresponding interface inside the external-ids table, e.g.: ovs-vsctl set-ssl client-key client-cert ca-cert ovs-vsctl set open_vswitch . external-ids:netplan/global/set-ssl=client-key,client-cert,ca-cert or: ovs-vsctl set Bridge ovs0 other-config:some-key=42 ovs-vsctl set Bridge ovs0 external-ids:netplan/other-config/some-key=42 or: ovs-vsctl set Port ovs1 lacp=off ovs-vsctl set Port ovs1 external-ids:netplan/lacp=off The netplan settings tag (netplan/<column>[/<key>]=<value>) marks the settings (column[:key]=value) inside ovsdb state, so that it can be cleaned up via netplan apply --only-ovs-cleanup. The cleanup will only happen if the setting and value are exactly the same as set/tagged by netplan. If the setting was touched manually (or by another process) netplan cleanup will not touch it. This way we can make sure to always start with a clean state before executing the OVS setup commands and submitting the new config/settings to ovsdb. Additionally, this PR fixes the creation and tagging of OVS patch_ports and the setup of global OpenFlow protocols. Commits: * OVS: tag all global & interface settings in external-ids * OVS: additional data can only be set if a row for a non-ovs interface has been created in ovsdb * OVS:cli: fix cleanup of implicit OVS interfaces * parse: fix compiler warning * OVS: first draft for cleanup of arbitrary interface & global settings * OVS: fix potential patch port setup race & port tagging issue * OVS: cleanup patch ports if they are detected to be a bond interface * OVS: fix cleanup of patch ports/interfaces * OVS: change handling of global protocols key * OVS: cleanup controller settings + test * OVS: add configmanager tests * OVS: improve SSL tagging * Add unit-tests for OVS cleanup * OVS: avoid spaces in settings tag values, fix global iface argument * OVS: adopt test_bridge_base to settings tagging * OVS: comments and cleanup * OVS: cleanup & reducing some duplication * OVS: improve systemd dependency, to avoid failure if OVS is not installed
slyon
approved these changes
Aug 28, 2020
There was a problem hiding this comment.
I had to add a tiny fix to make all integration tests work (if run in sequence), they have been only working if run in isolation. Now everything is looking good. +1 to merge this from my side!
Test results:
https://git.launchpad.net/~slyon/+git/files/tree/netplan-PR154/openvswitch-support-test-results.txt
|
Ok, did a sanity review of the changes and didn't see anything wrong. Let's get this merged! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Finally merge the work-in-progress OpenVSwitch support branch to master. Currently the branch should be feature-complete as per the initial specification, meaning we support:
This version also attempts to do smart cleanup of the configuration, removing no-longer relevant configuration on every
netplan applyor system reboot.Checklist
make checksuccessfully.make check-coverage).