Browse Source

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>
tags/6.0.0.0b1
Daniel Alvarez 10 months ago
parent
commit
3e5d9213aa

+ 37
- 3
networking_ovn/agent/metadata/agent.py View File

@@ -21,6 +21,7 @@ from neutron.common import utils
21 21
 from neutron_lib import constants as n_const
22 22
 from oslo_concurrency import lockutils
23 23
 from oslo_log import log
24
+from oslo_utils import excutils
24 25
 from oslo_utils import uuidutils
25 26
 from ovsdbapp.backend.ovs_idl import event as row_event
26 27
 from ovsdbapp.backend.ovs_idl import vlog
@@ -153,6 +154,7 @@ class MetadataAgent(object):
153 154
         # Open the connection to OVS database
154 155
         self.ovs_idl = ovsdb.MetadataAgentOvsIdl().start()
155 156
         self.chassis = self._get_own_chassis_name()
157
+        self.ovn_bridge = self._get_ovn_bridge()
156 158
 
157 159
         # Open the connection to OVN SB database.
158 160
         self.sb_idl = ovsdb.MetadataAgentOvnSbIdl(
@@ -185,6 +187,22 @@ class MetadataAgent(object):
185 187
             'Open_vSwitch', '.', 'external_ids').execute()
186 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 206
     @_sync_lock
189 207
     def sync(self):
190 208
         """Agent sync.
@@ -234,8 +252,7 @@ class MetadataAgent(object):
234 252
             self._process_monitor, datapath, self.conf, namespace)
235 253
 
236 254
         veth_name = self._get_veth_name(datapath)
237
-        self.ovs_idl.del_port(
238
-            veth_name[0], bridge=self.conf.ovs_integration_bridge).execute()
255
+        self.ovs_idl.del_port(veth_name[0]).execute()
239 256
         if ip_lib.device_exists(veth_name[0]):
240 257
             ip_lib.IPWrapper().del_veth(veth_name[0])
241 258
 
@@ -342,9 +359,26 @@ class MetadataAgent(object):
342 359
             if utils.get_ip_version(ipaddr) == 4:
343 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 379
         # Configure the OVS port and add external_ids:iface-id so that it
346 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 382
                               veth_name[0]).execute()
349 383
         self.ovs_idl.db_set(
350 384
             'Interface', veth_name[0],

+ 12
- 0
networking_ovn/releasenotes/notes/ignore-ovs-integration-bridge-from-metadata-agent-2752193adbbdeec9.yaml View File

@@ -0,0 +1,12 @@
1
+---
2
+fixes:
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>`_.

+ 4
- 1
networking_ovn/tests/functional/test_metadata_agent.py View File

@@ -84,8 +84,11 @@ class TestMetadataAgent(base.TestOVNFunctionalBase):
84 84
 
85 85
         self.chassis_name = self.add_fake_chassis('ovs-host-fake')
86 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 90
             mock_get_ch_name.return_value = self.chassis_name
91
+            mock_get_ovn_br.return_value = 'br-int'
89 92
             agt = agent.MetadataAgent(conf)
90 93
             agt.start()
91 94
             # Metadata agent will open connections to OVS and SB databases.

+ 10
- 2
networking_ovn/tests/unit/agent/metadata/test_agent.py View File

@@ -63,7 +63,9 @@ class TestMetadataAgent(base.BaseTestCase):
63 63
         self.agent = agent.MetadataAgent(self.fake_conf)
64 64
         self.agent.sb_idl = mock.Mock()
65 65
         self.agent.ovs_idl = mock.Mock()
66
+        self.agent.ovs_idl.transaction = mock.MagicMock()
66 67
         self.agent.chassis = 'chassis'
68
+        self.agent.ovn_bridge = 'br-int'
67 69
 
68 70
     def test_sync(self):
69 71
         with mock.patch.object(
@@ -181,8 +183,7 @@ class TestMetadataAgent(base.BaseTestCase):
181 183
             self.agent.teardown_datapath('1')
182 184
 
183 185
             destroy_mdp.assert_called_once()
184
-            self.agent.ovs_idl.del_port.assert_called_once_with(
185
-                'veth_0', bridge='br-int')
186
+            self.agent.ovs_idl.del_port.assert_called_once_with('veth_0')
186 187
             del_veth.assert_called_once_with('veth_0')
187 188
             garbage_collect.assert_called_once()
188 189
 
@@ -226,8 +227,15 @@ class TestMetadataAgent(base.BaseTestCase):
226 227
                     driver.MetadataDriver,
227 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 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 239
             # Check that the VETH pair is created
232 240
             add_veth.assert_called_once_with('veth_0', 'veth_1', 'namespace')
233 241
             # Make sure that the two ends of the VETH pair have been set as up.

Loading…
Cancel
Save