summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Alvarez <dalvarez@redhat.com>2018-10-08 02:19:29 +0000
committerDaniel Alvarez <dalvarez@redhat.com>2018-12-11 17:24:25 +0000
commit3e5d9213aa9d59029107ba51d6c0876d6d7d8b2a (patch)
treeb13c0d2584f3a1df7f3db46e19bdbdc915f135c3
parentcfdd81c6375f445116f56e656a042916db513df9 (diff)
Metadata agent: fetch ovn-bridge from OVSDB and not from config
Prior to this patch, metadata agent was fetching the OVN bridge from the settings but this could lead to inconsistencies if the config is different from the actual OVSDB configuration as ovn-controller would be using one bridge and Metadata agent another one. In order to eliminate these inconsistencies, this patch is changing the behavior of the agent so that it reads the OVN bridge from OVSDB as ovn-controller does. Also, when the agent is restarted, if the bridge has changed, it'll plug all the VETH pairs to the right bridge and unplug it from the older one (this is handy for the migration from ML2/OVS where an intermediate OVS bridge is used temporarily). The config option is not removed by this patch but ignored to avoid issues when it doesn't match OVSDB configuration. Change-Id: I33f3509b8e5fe0398a27d35923e46f0409a1935d Closes-Bug: #1799216 Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
Notes
Notes (review): Code-Review+1: Terry Wilson <twilson@redhat.com> Code-Review+2: Miguel Angel Ajo <mangelajo@redhat.com> Code-Review+2: Lucas Alvares Gomes <lucasagomes@gmail.com> Workflow+1: Lucas Alvares Gomes <lucasagomes@gmail.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Mon, 24 Dec 2018 14:29:08 +0000 Reviewed-on: https://review.openstack.org/612406 Project: openstack/networking-ovn Branch: refs/heads/master
-rw-r--r--networking_ovn/agent/metadata/agent.py40
-rw-r--r--networking_ovn/releasenotes/notes/ignore-ovs-integration-bridge-from-metadata-agent-2752193adbbdeec9.yaml12
-rw-r--r--networking_ovn/tests/functional/test_metadata_agent.py5
-rw-r--r--networking_ovn/tests/unit/agent/metadata/test_agent.py12
4 files changed, 63 insertions, 6 deletions
diff --git a/networking_ovn/agent/metadata/agent.py b/networking_ovn/agent/metadata/agent.py
index 7cdecb9..a227820 100644
--- a/networking_ovn/agent/metadata/agent.py
+++ b/networking_ovn/agent/metadata/agent.py
@@ -21,6 +21,7 @@ from neutron.common import utils
21from neutron_lib import constants as n_const 21from neutron_lib import constants as n_const
22from oslo_concurrency import lockutils 22from oslo_concurrency import lockutils
23from oslo_log import log 23from oslo_log import log
24from oslo_utils import excutils
24from oslo_utils import uuidutils 25from oslo_utils import uuidutils
25from ovsdbapp.backend.ovs_idl import event as row_event 26from ovsdbapp.backend.ovs_idl import event as row_event
26from ovsdbapp.backend.ovs_idl import vlog 27from ovsdbapp.backend.ovs_idl import vlog
@@ -153,6 +154,7 @@ class MetadataAgent(object):
153 # Open the connection to OVS database 154 # Open the connection to OVS database
154 self.ovs_idl = ovsdb.MetadataAgentOvsIdl().start() 155 self.ovs_idl = ovsdb.MetadataAgentOvsIdl().start()
155 self.chassis = self._get_own_chassis_name() 156 self.chassis = self._get_own_chassis_name()
157 self.ovn_bridge = self._get_ovn_bridge()
156 158
157 # Open the connection to OVN SB database. 159 # Open the connection to OVN SB database.
158 self.sb_idl = ovsdb.MetadataAgentOvnSbIdl( 160 self.sb_idl = ovsdb.MetadataAgentOvnSbIdl(
@@ -185,6 +187,22 @@ class MetadataAgent(object):
185 'Open_vSwitch', '.', 'external_ids').execute() 187 'Open_vSwitch', '.', 'external_ids').execute()
186 return ext_ids['system-id'] 188 return ext_ids['system-id']
187 189
190 def _get_ovn_bridge(self):
191 """Return the external_ids:ovn-bridge value of the Open_vSwitch table.
192
193 This is the OVS bridge used to plug the metadata ports to.
194 If the key doesn't exist, this method will return 'br-int' as default.
195 """
196 ext_ids = self.ovs_idl.db_get(
197 'Open_vSwitch', '.', 'external_ids').execute()
198 try:
199 ovn_bridge = ext_ids['ovn-bridge']
200 except KeyError:
201 LOG.warning("Can't read ovn-bridge external-id from OVSDB. Using "
202 "br-int instead.")
203 return 'br-int'
204 return ovn_bridge
205
188 @_sync_lock 206 @_sync_lock
189 def sync(self): 207 def sync(self):
190 """Agent sync. 208 """Agent sync.
@@ -234,8 +252,7 @@ class MetadataAgent(object):
234 self._process_monitor, datapath, self.conf, namespace) 252 self._process_monitor, datapath, self.conf, namespace)
235 253
236 veth_name = self._get_veth_name(datapath) 254 veth_name = self._get_veth_name(datapath)
237 self.ovs_idl.del_port( 255 self.ovs_idl.del_port(veth_name[0]).execute()
238 veth_name[0], bridge=self.conf.ovs_integration_bridge).execute()
239 if ip_lib.device_exists(veth_name[0]): 256 if ip_lib.device_exists(veth_name[0]):
240 ip_lib.IPWrapper().del_veth(veth_name[0]) 257 ip_lib.IPWrapper().del_veth(veth_name[0])
241 258
@@ -342,9 +359,26 @@ class MetadataAgent(object):
342 if utils.get_ip_version(ipaddr) == 4: 359 if utils.get_ip_version(ipaddr) == 4:
343 ip2.addr.add(ipaddr) 360 ip2.addr.add(ipaddr)
344 361
362 # Check that this port is not attached to any other OVS bridge. This
363 # can happen when the OVN bridge changes (for example, during a
364 # migration from ML2/OVS).
365 ovs_bridges = set(self.ovs_idl.list_br().execute())
366 try:
367 ovs_bridges.remove(self.ovn_bridge)
368 except KeyError:
369 with excutils.save_and_reraise_exception():
370 LOG.exception("Configured OVN bridge %s cannot be found in "
371 "the system.", self.ovn_bridge)
372
373 if ovs_bridges:
374 with self.ovs_idl.transaction() as txn:
375 for br in ovs_bridges:
376 txn.add(self.ovs_idl.del_port(veth_name[0], bridge=br,
377 if_exists=True))
378
345 # Configure the OVS port and add external_ids:iface-id so that it 379 # Configure the OVS port and add external_ids:iface-id so that it
346 # can be tracked by OVN. 380 # can be tracked by OVN.
347 self.ovs_idl.add_port(self.conf.ovs_integration_bridge, 381 self.ovs_idl.add_port(self.ovn_bridge,
348 veth_name[0]).execute() 382 veth_name[0]).execute()
349 self.ovs_idl.db_set( 383 self.ovs_idl.db_set(
350 'Interface', veth_name[0], 384 'Interface', veth_name[0],
diff --git a/networking_ovn/releasenotes/notes/ignore-ovs-integration-bridge-from-metadata-agent-2752193adbbdeec9.yaml b/networking_ovn/releasenotes/notes/ignore-ovs-integration-bridge-from-metadata-agent-2752193adbbdeec9.yaml
new file mode 100644
index 0000000..41ce53d
--- /dev/null
+++ b/networking_ovn/releasenotes/notes/ignore-ovs-integration-bridge-from-metadata-agent-2752193adbbdeec9.yaml
@@ -0,0 +1,12 @@
1---
2fixes:
3 - |
4 The configuration option ``ovs_integration_bridge`` used by networking-ovn
5 metadata agent can only lead to problems as the bridge used by
6 ``ovn-controller`` to install the flows is stored in OVSDB.
7 The metadata agent will now use OVSDB instead of the configuration option
8 to plug its ports, as a mismatch between both will break metadata.
9 There is no real use case for this option to exist and systems currently
10 using it will *not* be impacted by this change.
11 For more information see bug `1799216
12 <https://bugs.launchpad.net/networking-ovn/+bug/1799216>`_.
diff --git a/networking_ovn/tests/functional/test_metadata_agent.py b/networking_ovn/tests/functional/test_metadata_agent.py
index 4845fcd..6cf31ee 100644
--- a/networking_ovn/tests/functional/test_metadata_agent.py
+++ b/networking_ovn/tests/functional/test_metadata_agent.py
@@ -84,8 +84,11 @@ class TestMetadataAgent(base.TestOVNFunctionalBase):
84 84
85 self.chassis_name = self.add_fake_chassis('ovs-host-fake') 85 self.chassis_name = self.add_fake_chassis('ovs-host-fake')
86 with mock.patch.object(agent.MetadataAgent, 86 with mock.patch.object(agent.MetadataAgent,
87 '_get_own_chassis_name') as mock_get_ch_name: 87 '_get_own_chassis_name') as mock_get_ch_name,\
88 mock.patch.object(agent.MetadataAgent,
89 '_get_ovn_bridge') as mock_get_ovn_br:
88 mock_get_ch_name.return_value = self.chassis_name 90 mock_get_ch_name.return_value = self.chassis_name
91 mock_get_ovn_br.return_value = 'br-int'
89 agt = agent.MetadataAgent(conf) 92 agt = agent.MetadataAgent(conf)
90 agt.start() 93 agt.start()
91 # Metadata agent will open connections to OVS and SB databases. 94 # Metadata agent will open connections to OVS and SB databases.
diff --git a/networking_ovn/tests/unit/agent/metadata/test_agent.py b/networking_ovn/tests/unit/agent/metadata/test_agent.py
index f52c872..74670e2 100644
--- a/networking_ovn/tests/unit/agent/metadata/test_agent.py
+++ b/networking_ovn/tests/unit/agent/metadata/test_agent.py
@@ -63,7 +63,9 @@ class TestMetadataAgent(base.BaseTestCase):
63 self.agent = agent.MetadataAgent(self.fake_conf) 63 self.agent = agent.MetadataAgent(self.fake_conf)
64 self.agent.sb_idl = mock.Mock() 64 self.agent.sb_idl = mock.Mock()
65 self.agent.ovs_idl = mock.Mock() 65 self.agent.ovs_idl = mock.Mock()
66 self.agent.ovs_idl.transaction = mock.MagicMock()
66 self.agent.chassis = 'chassis' 67 self.agent.chassis = 'chassis'
68 self.agent.ovn_bridge = 'br-int'
67 69
68 def test_sync(self): 70 def test_sync(self):
69 with mock.patch.object( 71 with mock.patch.object(
@@ -181,8 +183,7 @@ class TestMetadataAgent(base.BaseTestCase):
181 self.agent.teardown_datapath('1') 183 self.agent.teardown_datapath('1')
182 184
183 destroy_mdp.assert_called_once() 185 destroy_mdp.assert_called_once()
184 self.agent.ovs_idl.del_port.assert_called_once_with( 186 self.agent.ovs_idl.del_port.assert_called_once_with('veth_0')
185 'veth_0', bridge='br-int')
186 del_veth.assert_called_once_with('veth_0') 187 del_veth.assert_called_once_with('veth_0')
187 garbage_collect.assert_called_once() 188 garbage_collect.assert_called_once()
188 189
@@ -226,8 +227,15 @@ class TestMetadataAgent(base.BaseTestCase):
226 driver.MetadataDriver, 227 driver.MetadataDriver,
227 'spawn_monitored_metadata_proxy') as spawn_mdp: 228 'spawn_monitored_metadata_proxy') as spawn_mdp:
228 229
230 # Simulate that the VETH pair was already present in 'br-fake'.
231 # We need to assert that it was deleted first.
232 self.agent.ovs_idl.list_br.return_value.execute.return_value = (
233 ['br-int', 'br-fake'])
229 self.agent.provision_datapath('1') 234 self.agent.provision_datapath('1')
230 235
236 # Check that the port was deleted from br-fake
237 self.agent.ovs_idl.del_port.assert_called_once_with(
238 'veth_0', bridge='br-fake', if_exists=True)
231 # Check that the VETH pair is created 239 # Check that the VETH pair is created
232 add_veth.assert_called_once_with('veth_0', 'veth_1', 'namespace') 240 add_veth.assert_called_once_with('veth_0', 'veth_1', 'namespace')
233 # Make sure that the two ends of the VETH pair have been set as up. 241 # Make sure that the two ends of the VETH pair have been set as up.