Make sure datapath_type is updated on bridges changed
When changing datapath_type in the config, physical and tunnel bridges do not have their datapath_type updated. Calling create() on already created bridges should be safe as it passes '--may-exist' when adding the bridge, which will do nothing if the bridge already exists, but the second part of the transaction will still update things like datapath_type. It should be noted that ancillary bridges (like br-ex) are not modified by this patch as datapath_type was never applied to them to begin with. Incidentally, the native and vsctl versions behaved slightly differently when handling datapath_type: vsctl builds the multi-cmd transaction with add-br ... -- set ..., so that the second cmd would actually complete. The native just bailed if may_exist and the bridge existed. This is fixed as part of this patch. Change-Id: Ib8bc817c7bc724d80193d0ca7af480a7ea103f77 Closes-Bug: 1532273
This commit is contained in:
parent
8e54248653
commit
71fa182000
|
@ -61,6 +61,8 @@ class AddBridgeCommand(BaseCommand):
|
||||||
br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name',
|
br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name',
|
||||||
self.name, None)
|
self.name, None)
|
||||||
if br:
|
if br:
|
||||||
|
if self.datapath_type:
|
||||||
|
br.datapath_type = self.datapath_type
|
||||||
return
|
return
|
||||||
row = txn.insert(self.api._tables['Bridge'])
|
row = txn.insert(self.api._tables['Bridge'])
|
||||||
row.name = self.name
|
row.name = self.name
|
||||||
|
|
|
@ -998,8 +998,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
|
||||||
self.tun_br = self.br_tun_cls(tun_br_name)
|
self.tun_br = self.br_tun_cls(tun_br_name)
|
||||||
self.tun_br.set_agent_uuid_stamp(self.agent_uuid_stamp)
|
self.tun_br.set_agent_uuid_stamp(self.agent_uuid_stamp)
|
||||||
|
|
||||||
if not self.tun_br.bridge_exists(self.tun_br.br_name):
|
# tun_br.create() won't recreate bridge if it exists, but will handle
|
||||||
self.tun_br.create(secure_mode=True)
|
# cases where something like datapath_type has changed
|
||||||
|
self.tun_br.create(secure_mode=True)
|
||||||
self.tun_br.setup_controllers(self.conf)
|
self.tun_br.setup_controllers(self.conf)
|
||||||
if (not self.int_br.port_exists(self.conf.OVS.int_peer_patch_port) or
|
if (not self.int_br.port_exists(self.conf.OVS.int_peer_patch_port) or
|
||||||
self.patch_tun_ofport == ovs_lib.INVALID_OFPORT):
|
self.patch_tun_ofport == ovs_lib.INVALID_OFPORT):
|
||||||
|
@ -1057,6 +1058,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
|
||||||
'bridge': bridge})
|
'bridge': bridge})
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
br = self.br_phys_cls(bridge)
|
br = self.br_phys_cls(bridge)
|
||||||
|
# The bridge already exists, so create won't recreate it, but will
|
||||||
|
# handle things like changing the datapath_type
|
||||||
|
br.create()
|
||||||
br.setup_controllers(self.conf)
|
br.setup_controllers(self.conf)
|
||||||
br.setup_default_table()
|
br.setup_default_table()
|
||||||
self.phys_brs[physical_network] = br
|
self.phys_brs[physical_network] = br
|
||||||
|
|
|
@ -58,6 +58,8 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
|
||||||
prefix='br-int')
|
prefix='br-int')
|
||||||
self.br_tun = base.get_rand_name(n_const.DEVICE_NAME_MAX_LEN,
|
self.br_tun = base.get_rand_name(n_const.DEVICE_NAME_MAX_LEN,
|
||||||
prefix='br-tun')
|
prefix='br-tun')
|
||||||
|
self.br_phys = base.get_rand_name(n_const.DEVICE_NAME_MAX_LEN,
|
||||||
|
prefix='br-phys')
|
||||||
patch_name_len = n_const.DEVICE_NAME_MAX_LEN - len("-patch-tun")
|
patch_name_len = n_const.DEVICE_NAME_MAX_LEN - len("-patch-tun")
|
||||||
self.patch_tun = "%s-patch-tun" % self.br_int[patch_name_len:]
|
self.patch_tun = "%s-patch-tun" % self.br_int[patch_name_len:]
|
||||||
self.patch_int = "%s-patch-int" % self.br_tun[patch_name_len:]
|
self.patch_int = "%s-patch-int" % self.br_tun[patch_name_len:]
|
||||||
|
@ -102,17 +104,20 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
|
||||||
tunnel_types = [p_const.TYPE_VXLAN]
|
tunnel_types = [p_const.TYPE_VXLAN]
|
||||||
else:
|
else:
|
||||||
tunnel_types = None
|
tunnel_types = None
|
||||||
bridge_mappings = ['physnet:%s' % self.br_int]
|
bridge_mappings = ['physnet:%s' % self.br_phys]
|
||||||
self.config.set_override('tunnel_types', tunnel_types, "AGENT")
|
self.config.set_override('tunnel_types', tunnel_types, "AGENT")
|
||||||
self.config.set_override('polling_interval', 1, "AGENT")
|
self.config.set_override('polling_interval', 1, "AGENT")
|
||||||
self.config.set_override('prevent_arp_spoofing', False, "AGENT")
|
self.config.set_override('prevent_arp_spoofing', False, "AGENT")
|
||||||
self.config.set_override('local_ip', '192.168.10.1', "OVS")
|
self.config.set_override('local_ip', '192.168.10.1', "OVS")
|
||||||
self.config.set_override('bridge_mappings', bridge_mappings, "OVS")
|
self.config.set_override('bridge_mappings', bridge_mappings, "OVS")
|
||||||
|
# Physical bridges should be created prior to running
|
||||||
|
self._bridge_classes()['br_phys'](self.br_phys).create()
|
||||||
agent = ovs_agent.OVSNeutronAgent(self._bridge_classes(),
|
agent = ovs_agent.OVSNeutronAgent(self._bridge_classes(),
|
||||||
self.config)
|
self.config)
|
||||||
self.addCleanup(self.ovs.delete_bridge, self.br_int)
|
self.addCleanup(self.ovs.delete_bridge, self.br_int)
|
||||||
if tunnel_types:
|
if tunnel_types:
|
||||||
self.addCleanup(self.ovs.delete_bridge, self.br_tun)
|
self.addCleanup(self.ovs.delete_bridge, self.br_tun)
|
||||||
|
self.addCleanup(self.ovs.delete_bridge, self.br_phys)
|
||||||
agent.sg_agent = mock.Mock()
|
agent.sg_agent = mock.Mock()
|
||||||
agent.ancillary_brs = []
|
agent.ancillary_brs = []
|
||||||
return agent
|
return agent
|
||||||
|
|
|
@ -41,14 +41,14 @@ class TestOVSAgent(base.OVSAgentTestFramework):
|
||||||
"OVS")
|
"OVS")
|
||||||
agent = self.create_agent()
|
agent = self.create_agent()
|
||||||
self.start_agent(agent)
|
self.start_agent(agent)
|
||||||
actual = self.ovs.db_get_val('Bridge',
|
for br_name in (getattr(self, br) for br in
|
||||||
agent.int_br.br_name,
|
('br_int', 'br_tun', 'br_phys')):
|
||||||
'datapath_type')
|
actual = self.ovs.db_get_val('Bridge', br_name, 'datapath_type')
|
||||||
self.assertEqual(expected, actual)
|
self.assertEqual(expected, actual)
|
||||||
actual = self.ovs.db_get_val('Bridge',
|
|
||||||
agent.tun_br.br_name,
|
def test_datapath_type_change(self):
|
||||||
'datapath_type')
|
self._check_datapath_type_netdev('system')
|
||||||
self.assertEqual(expected, actual)
|
self._check_datapath_type_netdev('netdev')
|
||||||
|
|
||||||
def test_datapath_type_netdev(self):
|
def test_datapath_type_netdev(self):
|
||||||
self._check_datapath_type_netdev(
|
self._check_datapath_type_netdev(
|
||||||
|
|
|
@ -980,6 +980,7 @@ class TestOvsNeutronAgent(object):
|
||||||
self.agent.setup_physical_bridges({"physnet1": "br-eth"})
|
self.agent.setup_physical_bridges({"physnet1": "br-eth"})
|
||||||
expected_calls = [
|
expected_calls = [
|
||||||
mock.call.phys_br_cls('br-eth'),
|
mock.call.phys_br_cls('br-eth'),
|
||||||
|
mock.call.phys_br.create(),
|
||||||
mock.call.phys_br.setup_controllers(mock.ANY),
|
mock.call.phys_br.setup_controllers(mock.ANY),
|
||||||
mock.call.phys_br.setup_default_table(),
|
mock.call.phys_br.setup_default_table(),
|
||||||
mock.call.int_br.db_get_val('Interface', 'int-br-eth',
|
mock.call.int_br.db_get_val('Interface', 'int-br-eth',
|
||||||
|
@ -1060,6 +1061,7 @@ class TestOvsNeutronAgent(object):
|
||||||
self.agent.setup_physical_bridges({"physnet1": "br-eth"})
|
self.agent.setup_physical_bridges({"physnet1": "br-eth"})
|
||||||
expected_calls = [
|
expected_calls = [
|
||||||
mock.call.phys_br_cls('br-eth'),
|
mock.call.phys_br_cls('br-eth'),
|
||||||
|
mock.call.phys_br.create(),
|
||||||
mock.call.phys_br.setup_controllers(mock.ANY),
|
mock.call.phys_br.setup_controllers(mock.ANY),
|
||||||
mock.call.phys_br.setup_default_table(),
|
mock.call.phys_br.setup_default_table(),
|
||||||
mock.call.int_br.delete_port('int-br-eth'),
|
mock.call.int_br.delete_port('int-br-eth'),
|
||||||
|
|
|
@ -188,6 +188,7 @@ class TunnelTest(object):
|
||||||
]
|
]
|
||||||
|
|
||||||
self.mock_map_tun_bridge_expected = [
|
self.mock_map_tun_bridge_expected = [
|
||||||
|
mock.call.create(),
|
||||||
mock.call.setup_controllers(mock.ANY),
|
mock.call.setup_controllers(mock.ANY),
|
||||||
mock.call.setup_default_table(),
|
mock.call.setup_default_table(),
|
||||||
mock.call.add_patch_port('phy-%s' % self.MAP_TUN_BRIDGE,
|
mock.call.add_patch_port('phy-%s' % self.MAP_TUN_BRIDGE,
|
||||||
|
@ -214,8 +215,7 @@ class TunnelTest(object):
|
||||||
|
|
||||||
self.mock_tun_bridge_expected = [
|
self.mock_tun_bridge_expected = [
|
||||||
mock.call.set_agent_uuid_stamp(mock.ANY),
|
mock.call.set_agent_uuid_stamp(mock.ANY),
|
||||||
mock.call.bridge_exists(mock.ANY),
|
mock.call.create(secure_mode=True),
|
||||||
nonzero(mock.call.bridge_exists()),
|
|
||||||
mock.call.setup_controllers(mock.ANY),
|
mock.call.setup_controllers(mock.ANY),
|
||||||
mock.call.port_exists('patch-int'),
|
mock.call.port_exists('patch-int'),
|
||||||
nonzero(mock.call.port_exists()),
|
nonzero(mock.call.port_exists()),
|
||||||
|
@ -627,6 +627,7 @@ class TunnelTestUseVethInterco(TunnelTest):
|
||||||
]
|
]
|
||||||
|
|
||||||
self.mock_map_tun_bridge_expected = [
|
self.mock_map_tun_bridge_expected = [
|
||||||
|
mock.call.create(),
|
||||||
mock.call.setup_controllers(mock.ANY),
|
mock.call.setup_controllers(mock.ANY),
|
||||||
mock.call.setup_default_table(),
|
mock.call.setup_default_table(),
|
||||||
mock.call.add_port(self.intb),
|
mock.call.add_port(self.intb),
|
||||||
|
@ -646,8 +647,7 @@ class TunnelTestUseVethInterco(TunnelTest):
|
||||||
|
|
||||||
self.mock_tun_bridge_expected = [
|
self.mock_tun_bridge_expected = [
|
||||||
mock.call.set_agent_uuid_stamp(mock.ANY),
|
mock.call.set_agent_uuid_stamp(mock.ANY),
|
||||||
mock.call.bridge_exists(mock.ANY),
|
mock.call.create(secure_mode=True),
|
||||||
nonzero(mock.call.bridge_exists()),
|
|
||||||
mock.call.setup_controllers(mock.ANY),
|
mock.call.setup_controllers(mock.ANY),
|
||||||
mock.call.port_exists('patch-int'),
|
mock.call.port_exists('patch-int'),
|
||||||
nonzero(mock.call.port_exists()),
|
nonzero(mock.call.port_exists()),
|
||||||
|
|
Loading…
Reference in New Issue