From 824380efdd95311bbab40a25b4e3284c809dab11 Mon Sep 17 00:00:00 2001 From: Danilo Egea Gondolfo Date: Fri, 30 Aug 2024 17:49:25 +0100 Subject: [PATCH] ovs: quote external-ids and other-config values For complex values, ovs-vsctl requires that they are quoted or it will error out. LP: #2070318 While here, add some debugging information so we can see the ovs-vsctl command executed by "netplan apply" with --debug. --- netplan_cli/cli/ovs.py | 18 +++--- src/openvswitch.c | 6 +- tests/generator/base.py | 8 +-- tests/generator/test_ovs.py | 112 ++++++++++++++++++------------------ tests/integration/ovs.py | 3 + tests/test_ovs.py | 4 +- 6 files changed, 77 insertions(+), 74 deletions(-) diff --git a/netplan_cli/cli/ovs.py b/netplan_cli/cli/ovs.py index 0d559abf1..a3ada29e0 100644 --- a/netplan_cli/cli/ovs.py +++ b/netplan_cli/cli/ovs.py @@ -18,7 +18,6 @@ import logging import os import subprocess -import re from .utils import systemctl_is_active, systemctl_is_installed @@ -52,21 +51,22 @@ def _del_col(type, iface, column, value): default = DEFAULTS.get(column) if default is None: # removes the exact value only if it was set by netplan - subprocess.check_call([OPENVSWITCH_OVS_VSCTL, 'remove', type, iface, column, value]) + cmd = [OPENVSWITCH_OVS_VSCTL, 'remove', type, iface, column, value] + logging.debug('Running: %s' % ' '.join(cmd)) + subprocess.check_call(cmd) elif default and default != value: # reset to default, if its not the default already - subprocess.check_call([OPENVSWITCH_OVS_VSCTL, 'set', type, iface, '%s=%s' % (column, default)]) + cmd = [OPENVSWITCH_OVS_VSCTL, 'set', type, iface, '%s=%s' % (column, default)] + logging.debug('Running: %s' % ' '.join(cmd)) + subprocess.check_call(cmd) def _del_dict(type, iface, column, key, value): """Cleanup values from a dictionary (i.e. "column:key=value")""" # removes the exact value only if it was set by netplan - subprocess.check_call([OPENVSWITCH_OVS_VSCTL, 'remove', type, iface, column, key, _escape_colon(value)]) - - -# for ovsdb remove: column key's value can not contain bare ':', need to escape with '\' -def _escape_colon(literal): - return re.sub(r'([^\\]):', r'\g<1>\:', literal) + cmd = [OPENVSWITCH_OVS_VSCTL, 'remove', type, iface, column, '%s=\"%s\"' % (key, value)] + logging.debug('Running: %s' % ' '.join(cmd)) + subprocess.check_call(cmd) def _del_global(type, iface, key, value): diff --git a/src/openvswitch.c b/src/openvswitch.c index 09864b560..dd9ee7636 100644 --- a/src/openvswitch.c +++ b/src/openvswitch.c @@ -134,7 +134,7 @@ write_ovs_tag_setting(const gchar* id, const char* type, const char* col, const g_string_append_printf(s, "%s", col); if (key) g_string_append_printf(s, "/%s", key); - g_string_append_printf(s, "=%s", clean_value); + g_string_append_printf(s, "=\"%s\"", clean_value); append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s", type, id, s->str); g_string_free(s, TRUE); } @@ -150,7 +150,7 @@ write_ovs_additional_data(GHashTable *data, const char* type, const gchar* id, G while (g_hash_table_iter_next(&iter, (gpointer) &key, (gpointer) &value)) { /* XXX: we need to check what happens when an invalid key=value pair gets supplied here. We might want to handle this somehow. */ - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s:%s=%s", + append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s:%s=\"%s\"", type, id, setting, key, value); write_ovs_tag_setting(id, type, setting, key, value, cmds); } @@ -210,7 +210,7 @@ STATIC void write_ovs_tag_netplan(const gchar* id, const char* type, GString* cmds) { /* Mark this bridge/port/interface as created by netplan */ - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s external-ids:netplan=true", + append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s external-ids:netplan=\"true\"", type, id); } diff --git a/tests/generator/base.py b/tests/generator/base.py index 30d390a70..b32b04234 100644 --- a/tests/generator/base.py +++ b/tests/generator/base.py @@ -65,11 +65,11 @@ OVS_PHYSICAL = _OVS_BASE + 'Requires=sys-subsystem-net-devices-%(iface)s.device\nAfter=sys-subsystem-net-devices-%(iface)s\ .device\nAfter=netplan-ovs-cleanup.service\nBefore=network.target\nWants=network.target\n%(extra)s' OVS_VIRTUAL = _OVS_BASE + 'After=netplan-ovs-cleanup.service\nBefore=network.target\nWants=network.target\n%(extra)s' -OVS_BR_DEFAULT = 'ExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan=true\nExecStart=/usr/bin/ovs-vsctl \ +OVS_BR_DEFAULT = 'ExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan=\"true\"\nExecStart=/usr/bin/ovs-vsctl \ set-fail-mode %(iface)s standalone\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan/global/set-fail-mode=\ -standalone\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s mcast_snooping_enable=false\nExecStart=/usr/bin/ovs-vsctl set \ -Bridge %(iface)s external-ids:netplan/mcast_snooping_enable=false\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s \ -rstp_enable=false\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan/rstp_enable=false\n' +\"standalone\"\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s mcast_snooping_enable=false\nExecStart=/usr/bin/ovs-vsctl set \ +Bridge %(iface)s external-ids:netplan/mcast_snooping_enable="false"\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s \ +rstp_enable=false\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan/rstp_enable=\"false\"\n' OVS_BR_EMPTY = _OVS_BASE + 'After=netplan-ovs-cleanup.service\nBefore=network.target\nWants=network.target\n\n[Service]\n\ Type=oneshot\nTimeoutStartSec=10s\nExecStart=/usr/bin/ovs-vsctl --may-exist add-br %(iface)s\n' + OVS_BR_DEFAULT OVS_CLEANUP = _OVS_BASE + 'ConditionFileIsExecutable=/usr/bin/ovs-vsctl\nBefore=network.target\nWants=network.target\n\n\ diff --git a/tests/generator/test_ovs.py b/tests/generator/test_ovs.py index 5a9d69483..a3a422ef9 100644 --- a/tests/generator/test_ovs.py +++ b/tests/generator/test_ovs.py @@ -67,10 +67,10 @@ def test_interface_external_ids_other_config(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:iface-id=myhostname -ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:netplan/external-ids/iface-id=myhostname -ExecStart=/usr/bin/ovs-vsctl set Interface eth0 other-config:disable-in-band=true -ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:netplan/other-config/disable-in-band=true +ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:iface-id="myhostname" +ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:netplan/external-ids/iface-id="myhostname" +ExecStart=/usr/bin/ovs-vsctl set Interface eth0 other-config:disable-in-band="true" +ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:netplan/other-config/disable-in-band="true" '''}, 'eth1.service': OVS_PHYSICAL % {'iface': 'eth1', 'extra': '''\ Requires=netplan-ovs-ovs0.service @@ -79,8 +79,8 @@ def test_interface_external_ids_other_config(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set Interface eth1 other-config:disable-in-band=false -ExecStart=/usr/bin/ovs-vsctl set Interface eth1 external-ids:netplan/other-config/disable-in-band=false +ExecStart=/usr/bin/ovs-vsctl set Interface eth1 other-config:disable-in-band="false" +ExecStart=/usr/bin/ovs-vsctl set Interface eth1 external-ids:netplan/other-config/disable-in-band="false" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -118,10 +118,10 @@ def test_global_external_ids_other_config(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:iface-id=myhostname -ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:netplan/external-ids/iface-id=myhostname -ExecStart=/usr/bin/ovs-vsctl set open_vswitch . other-config:disable-in-band=true -ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:netplan/other-config/disable-in-band=true +ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:iface-id="myhostname" +ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:netplan/external-ids/iface-id="myhostname" +ExecStart=/usr/bin/ovs-vsctl set open_vswitch . other-config:disable-in-band="true" +ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:netplan/other-config/disable-in-band="true" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -143,7 +143,7 @@ def test_global_set_protocols(self): ExecStart=/usr/bin/ovs-vsctl --may-exist add-br ovs0 ''' + OVS_BR_DEFAULT % {'iface': 'ovs0'} + '''\ ExecStart=/usr/bin/ovs-vsctl set Bridge ovs0 protocols=OpenFlow10,OpenFlow11,OpenFlow12 -ExecStart=/usr/bin/ovs-vsctl set Bridge ovs0 external-ids:netplan/protocols=OpenFlow10,OpenFlow11,OpenFlow12 +ExecStart=/usr/bin/ovs-vsctl set Bridge ovs0 external-ids:netplan/protocols="OpenFlow10,OpenFlow11,OpenFlow12" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -198,11 +198,11 @@ def test_bond_setup(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-bond br0 bond0 eth1 eth2 -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan="true" ExecStart=/usr/bin/ovs-vsctl set Port bond0 lacp=off -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp=off -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:iface-id=myhostname -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/external-ids/iface-id=myhostname +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp="off" +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:iface-id="myhostname" +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/external-ids/iface-id="myhostname" '''}, 'br0.service': OVS_BR_EMPTY % {'iface': 'br0'}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) @@ -267,9 +267,9 @@ def test_bond_lacp(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-bond br0 bond0 eth1 eth2 -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan="true" ExecStart=/usr/bin/ovs-vsctl set Port bond0 lacp=active -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp=active +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp="active" '''}, 'br0.service': OVS_BR_EMPTY % {'iface': 'br0'}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) @@ -333,11 +333,11 @@ def test_bond_mode_implicit_params(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-bond br0 bond0 eth1 eth2 -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan="true" ExecStart=/usr/bin/ovs-vsctl set Port bond0 lacp=off -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp=off +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp="off" ExecStart=/usr/bin/ovs-vsctl set Port bond0 bond_mode=balance-tcp -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/bond_mode=balance-tcp +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/bond_mode="balance-tcp" '''}, 'br0.service': OVS_BR_EMPTY % {'iface': 'br0'}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) @@ -373,11 +373,11 @@ def test_bond_mode_explicit_params(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-bond br0 bond0 eth1 eth2 -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan="true" ExecStart=/usr/bin/ovs-vsctl set Port bond0 lacp=off -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp=off +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp="off" ExecStart=/usr/bin/ovs-vsctl set Port bond0 bond_mode=active-backup -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/bond_mode=active-backup +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/bond_mode="active-backup" '''}, 'br0.service': OVS_BR_EMPTY % {'iface': 'br0'}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) @@ -451,10 +451,10 @@ def test_bridge_external_ids_other_config(self): TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-br br0 ''' + OVS_BR_DEFAULT % {'iface': 'br0'} + '''\ -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:iface-id=myhostname -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/external-ids/iface-id=myhostname -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 other-config:disable-in-band=true -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/other-config/disable-in-band=true +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:iface-id="myhostname" +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/external-ids/iface-id="myhostname" +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 other-config:disable-in-band="true" +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/other-config/disable-in-band="true" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the bridge has been only configured for OVS @@ -483,13 +483,13 @@ def test_bridge_non_default_parameters(self): ExecStart=/usr/bin/ovs-vsctl --may-exist add-br br0 ExecStart=/usr/bin/ovs-vsctl --may-exist add-port br0 eth1 ExecStart=/usr/bin/ovs-vsctl --may-exist add-port br0 eth2 -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan="true" ExecStart=/usr/bin/ovs-vsctl set-fail-mode br0 secure -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/global/set-fail-mode=secure +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/global/set-fail-mode="secure" ExecStart=/usr/bin/ovs-vsctl set Bridge br0 mcast_snooping_enable=true -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/mcast_snooping_enable=true +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/mcast_snooping_enable="true" ExecStart=/usr/bin/ovs-vsctl set Bridge br0 rstp_enable=true -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/rstp_enable=true +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/rstp_enable="true" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -543,7 +543,7 @@ def test_bridge_set_protocols(self): ExecStart=/usr/bin/ovs-vsctl --may-exist add-br br0 ''' + OVS_BR_DEFAULT % {'iface': 'br0'} + '''\ ExecStart=/usr/bin/ovs-vsctl set Bridge br0 protocols=OpenFlow10,OpenFlow11,OpenFlow15 -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/protocols=OpenFlow10,OpenFlow11,OpenFlow15 +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/protocols="OpenFlow10,OpenFlow11,OpenFlow15" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -594,18 +594,18 @@ def test_bridge_controller(self): ''' + OVS_BR_DEFAULT % {'iface': 'br0'} + '''\ ExecStart=/usr/bin/ovs-vsctl set-controller br0 ptcp: ptcp:1337 ptcp:1337:[fe80::1234%eth0] pssl:1337:[fe80::1] ssl:10.10.10.1 \ tcp:127.0.0.1:1337 tcp:[fe80::1234%eth0] tcp:[fe80::1]:1337 unix:/some/path punix:other/path -ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/global/set-controller=ptcp:,ptcp:1337,\ +ExecStart=/usr/bin/ovs-vsctl set Bridge br0 external-ids:netplan/global/set-controller="ptcp:,ptcp:1337,\ ptcp:1337:[fe80::1234%eth0],pssl:1337:[fe80::1],ssl:10.10.10.1,tcp:127.0.0.1:1337,tcp:[fe80::1234%eth0],tcp:[fe80::1]:1337,\ -unix:/some/path,punix:other/path +unix:/some/path,punix:other/path" ExecStart=/usr/bin/ovs-vsctl set Controller br0 connection-mode=out-of-band -ExecStart=/usr/bin/ovs-vsctl set Controller br0 external-ids:netplan/connection-mode=out-of-band +ExecStart=/usr/bin/ovs-vsctl set Controller br0 external-ids:netplan/connection-mode="out-of-band" '''}, 'global.service': OVS_VIRTUAL % {'iface': 'global', 'extra': ''' [Service] Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl set-ssl /key/path /some/path /another/path -ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:netplan/global/set-ssl=/key/path,/some/path,/another/path +ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:netplan/global/set-ssl="/key/path,/some/path,/another/path" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -703,7 +703,7 @@ def test_global_ssl(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl set-ssl /key/path /some/path /another/path -ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:netplan/global/set-ssl=/key/path,/some/path,/another/path +ExecStart=/usr/bin/ovs-vsctl set open_vswitch . external-ids:netplan/global/set-ssl="/key/path,/some/path,/another/path" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -808,9 +808,9 @@ def test_bridge_auto_ovs_backend(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-bond br0 bond0 eth1 eth2 -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan="true" ExecStart=/usr/bin/ovs-vsctl set Port bond0 lacp=off -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp=off +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp="off" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) self.assert_networkd({'br0.network': ND_WITHIP % ('br0', '192.170.1.1/24'), @@ -867,9 +867,9 @@ def test_bond_auto_ovs_backend(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-bond br0 bond0 patchy eth0 -- set Interface patchy type=patch options:peer=patchx -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan="true" ExecStart=/usr/bin/ovs-vsctl set Port bond0 lacp=off -ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp=off +ExecStart=/usr/bin/ovs-vsctl set Port bond0 external-ids:netplan/lacp="off" '''}, 'patchx.service': OVS_VIRTUAL % {'iface': 'patchx', 'extra': '''Requires=netplan-ovs-br1.service @@ -878,7 +878,7 @@ def test_bond_auto_ovs_backend(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set Port patchx external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port patchx external-ids:netplan="true" '''}, 'patchy.service': OVS_VIRTUAL % {'iface': 'patchy', 'extra': '''Requires=netplan-ovs-bond0.service @@ -887,7 +887,7 @@ def test_bond_auto_ovs_backend(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set Interface patchy external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Interface patchy external-ids:netplan="true" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) self.assert_networkd({'br0.network': ND_WITHIP % ('br0', '192.170.1.1/24'), @@ -932,7 +932,7 @@ def test_patch_ports(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set Port patch0-1 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port patch0-1 external-ids:netplan="true" '''}, 'patch1-0.service': OVS_VIRTUAL % {'iface': 'patch1-0', 'extra': '''Requires=netplan-ovs-br1.service @@ -941,7 +941,7 @@ def test_patch_ports(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set Port patch1-0 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Port patch1-0 external-ids:netplan="true" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) self.assert_networkd({'br0.network': ND_WITHIP % ('br0', '192.168.1.1/24'), @@ -976,7 +976,7 @@ def test_fake_vlan_bridge_setup(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-br br0.100 br0 100 -ExecStart=/usr/bin/ovs-vsctl set Interface br0.100 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Interface br0.100 external-ids:netplan="true" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -1006,7 +1006,7 @@ def test_implicit_fake_vlan_bridge_setup(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-br br0.100 br0 100 -ExecStart=/usr/bin/ovs-vsctl set Interface br0.100 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Interface br0.100 external-ids:netplan="true" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane @@ -1101,9 +1101,9 @@ def test_ovs_duplicates_when_parser_needs_second_pass(self): ExecStart=/usr/bin/ovs-vsctl --may-exist add-port br123 nic1 ''' + OVS_BR_DEFAULT % {'iface': 'br123'} + ('\ ExecStart=/usr/bin/ovs-vsctl set Bridge br123 protocols=OpenFlow10,OpenFlow11,OpenFlow12\n\ -ExecStart=/usr/bin/ovs-vsctl set Bridge br123 external-ids:netplan/protocols=OpenFlow10,OpenFlow11,OpenFlow12\n\ +ExecStart=/usr/bin/ovs-vsctl set Bridge br123 external-ids:netplan/protocols="OpenFlow10,OpenFlow11,OpenFlow12"\n\ ExecStart=/usr/bin/ovs-vsctl set-controller br123 tcp:127.0.0.1:6653\n\ -ExecStart=/usr/bin/ovs-vsctl set Bridge br123 external-ids:netplan/global/set-controller=tcp:127.0.0.1:6653\n\ +ExecStart=/usr/bin/ovs-vsctl set Bridge br123 external-ids:netplan/global/set-controller="tcp:127.0.0.1:6653"\n\ ')}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) @@ -1136,7 +1136,7 @@ def test_file_paths_escaped(self): Type=oneshot TimeoutStartSec=10s ExecStart=/usr/bin/ovs-vsctl --may-exist add-br abc/../../123.100 abc/../../123 100 -ExecStart=/usr/bin/ovs-vsctl set Interface abc/../../123.100 external-ids:netplan=true +ExecStart=/usr/bin/ovs-vsctl set Interface abc/../../123.100 external-ids:netplan="true" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) @@ -1175,10 +1175,10 @@ def test_control_characters_and_semicolons_escaping(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:a\\n1\\ra= \\; a \\; 1 ;a; ;b\\t;\\t3 ;\\ta\\t; 1 -ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:netplan/external-ids/a\\n1\\ra=,;,a,;,1,;a;,;b\\t;\\t3,;\\ta\\t;,1 -ExecStart=/usr/bin/ovs-vsctl set Interface eth0 other-config:a\\n1\\ra= \\; a \\; 1 ;a; ;b\\t;\\t3 ;\\ta\\t; 1 -ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:netplan/other-config/a\\n1\\ra=,;,a,;,1,;a;,;b\\t;\\t3,;\\ta\\t;,1 +ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:a\\n1\\ra=" \\; a \\; 1 ;a; ;b\\t;\\t3 ;\\ta\\t; 1" +ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:netplan/external-ids/a\\n1\\ra=",;,a,;,1,;a;,;b\\t;\\t3,;\\ta\\t;,1" +ExecStart=/usr/bin/ovs-vsctl set Interface eth0 other-config:a\\n1\\ra=" \\; a \\; 1 ;a; ;b\\t;\\t3 ;\\ta\\t; 1" +ExecStart=/usr/bin/ovs-vsctl set Interface eth0 external-ids:netplan/other-config/a\\n1\\ra=",;,a,;,1,;a;,;b\\t;\\t3,;\\ta\\t;,1" '''}, 'eth1.service': OVS_PHYSICAL % {'iface': 'eth1', 'extra': '''\ Requires=netplan-ovs-ovs0.service @@ -1187,8 +1187,8 @@ def test_control_characters_and_semicolons_escaping(self): [Service] Type=oneshot TimeoutStartSec=10s -ExecStart=/usr/bin/ovs-vsctl set Interface eth1 other-config:disable-in-band=false -ExecStart=/usr/bin/ovs-vsctl set Interface eth1 external-ids:netplan/other-config/disable-in-band=false +ExecStart=/usr/bin/ovs-vsctl set Interface eth1 other-config:disable-in-band="false" +ExecStart=/usr/bin/ovs-vsctl set Interface eth1 external-ids:netplan/other-config/disable-in-band="false" '''}, 'cleanup.service': OVS_CLEANUP % {'iface': 'cleanup'}}) # Confirm that the networkd config is still sane diff --git a/tests/integration/ovs.py b/tests/integration/ovs.py index 834a7bd45..bfcfc64fc 100644 --- a/tests/integration/ovs.py +++ b/tests/integration/ovs.py @@ -526,6 +526,7 @@ def test_settings_tag_cleanup(self): mcast-snooping: true external-ids: iface-id: myhostname + ovn-cms-options: "card-serial-number=MT42424242N8,enable-chassis-as-gw" other-config: disable-in-band: true hwaddr: aa:bb:cc:dd:ee:ff @@ -600,6 +601,8 @@ def test_settings_tag_cleanup(self): self.assertNotIn(b'somekey=55:44:33:22:11:00', after['external-ids-Open_vSwitch']) self.assertIn(b'iface-id=myhostname', before['external-ids-Bridge']) self.assertNotIn(b'iface-id=myhostname', after['external-ids-Bridge']) + self.assertIn(b'ovn-cms-options=card-serial-number=MT42424242N8,enable-chassis-as-gw', before['external-ids-Bridge']) + self.assertNotIn(b'ovn-cms-options=card-serial-number=MT42424242N8,enable-chassis-as-gw', after['external-ids-Bridge']) self.assertIn(b'iface-id=mylocaliface', before['external-ids-Interface']) self.assertNotIn(b'iface-id=mylocaliface', after['external-ids-Interface']) for tbl in ('Bridge', 'Port'): diff --git a/tests/test_ovs.py b/tests/test_ovs.py index c2865e622..e238c67f6 100644 --- a/tests/test_ovs.py +++ b/tests/test_ovs.py @@ -94,7 +94,7 @@ def test_no_clear_global_different(self, mock, mock_out): def test_clear_dict(self, mock): ovs.clear_setting('Bridge', 'ovs0', 'netplan/other-config/key', 'value') mock.assert_has_calls([ - call([OVS, 'remove', 'Bridge', 'ovs0', 'other-config', 'key', 'value']), + call([OVS, 'remove', 'Bridge', 'ovs0', 'other-config', 'key=\"value\"']), call([OVS, 'remove', 'Bridge', 'ovs0', 'external-ids', 'netplan/other-config/key']) ]) @@ -118,7 +118,7 @@ def test_clear_col_default(self, mock): def test_clear_dict_colon(self, mock): ovs.clear_setting('Bridge', 'ovs0', 'netplan/other-config/key', 'fa:16:3e:4b:19:3a') mock.assert_has_calls([ - call([OVS, 'remove', 'Bridge', 'ovs0', 'other-config', 'key', r'fa\:16\:3e\:4b\:19\:3a']), + call([OVS, 'remove', 'Bridge', 'ovs0', 'other-config', 'key=\"fa:16:3e:4b:19:3a\"']), call([OVS, 'remove', 'Bridge', 'ovs0', 'external-ids', 'netplan/other-config/key']) ]) mock.mock_calls