Merge "ovn: first tear down old metadata namespaces, then deploy new" into stable/yoga

Author: Zuul

neutron/agent/ovn/metadata/agent.py

@@ -315,6 +315,12 @@ def _get_ovn_bridge(self):
                         "br-int instead.")
             return 'br-int'
 
+    def get_networks(self):
+        ports = self.sb_idl.get_ports_on_chassis(self.chassis)
+        return {(str(p.datapath.uuid),
+            ovn_utils.get_network_name_from_datapath(p.datapath))
+            for p in self._vif_ports(ports)}
+
     @_sync_lock
     def sync(self):
         """Agent sync.
@@ -323,16 +329,26 @@ def sync(self):
         chassis are serving metadata. Also, it will tear down those namespaces
         which were serving metadata but are no longer needed.
         """
-        metadata_namespaces = self.ensure_all_networks_provisioned()
+
+        # first, clean up namespaces that should no longer deploy
         system_namespaces = tuple(
             ns.decode('utf-8') if isinstance(ns, bytes) else ns
             for ns in ip_lib.list_network_namespaces())
+        nets = self.get_networks()
+        metadata_namespaces = [
+            self._get_namespace_name(net[1])
+            for net in nets
+        ]
         unused_namespaces = [ns for ns in system_namespaces if
                              ns.startswith(NS_PREFIX) and
                              ns not in metadata_namespaces]
         for ns in unused_namespaces:
             self.teardown_datapath(self._get_datapath_name(ns))
 
+        # now that all obsolete namespaces are cleaned up, deploy required
+        # networks
+        self.ensure_all_networks_provisioned(nets)
+
     @staticmethod
     def _get_veth_name(datapath):
         return ['{}{}{}'.format(n_const.TAP_DEVICE_PREFIX,
@@ -424,8 +440,6 @@ def provision_datapath(self, datapath, net_name):
         and assign the IP addresses to the interface corresponding to the
         metadata port of the network. It will also remove existing IP
         addresses that are no longer needed.
-
-        :return: The metadata namespace name of this datapath
         """
         LOG.debug("Provisioning metadata for network %s", net_name)
         port = self.sb_idl.get_metadata_port_network(datapath)
@@ -533,28 +547,13 @@ def provision_datapath(self, datapath, net_name):
             self.conf, bind_address=n_const.METADATA_V4_IP,
             network_id=net_name)
 
-        return namespace
+    def ensure_all_networks_provisioned(self, nets):
+        """Ensure that all requested datapaths are provisioned.
 
-    def ensure_all_networks_provisioned(self):
-        """Ensure that all datapaths are provisioned.
-
-        This function will make sure that all datapaths with ports bound to
-        our chassis have its namespace, VETH pair and OVS port created and
-        metadata proxy is up and running.
-
-        :return: A list with the namespaces that are currently serving
-        metadata
+        This function will make sure that requested datapaths have their
+        namespaces, VETH pair and OVS ports created and metadata proxies are up
+        and running.
         """
-        # Retrieve all VIF ports in our Chassis
-        ports = self.sb_idl.get_ports_on_chassis(self.chassis)
-        nets = {(str(p.datapath.uuid),
-            ovn_utils.get_network_name_from_datapath(p.datapath))
-            for p in self._vif_ports(ports)}
-        namespaces = []
         # Make sure that all those datapaths are serving metadata
         for datapath, net_name in nets:
-            netns = self.provision_datapath(datapath, net_name)
-            if netns:
-                namespaces.append(netns)
-
-        return namespaces
+            self.provision_datapath(datapath, net_name)

neutron/tests/unit/agent/ovn/metadata/test_agent.py

@@ -73,19 +73,29 @@ def setUp(self):
         self.agent.chassis = 'chassis'
         self.agent.ovn_bridge = 'br-int'
 
+        self.ports = []
+        for i in range(0, 3):
+            self.ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
+                external_ids={'name': 'neutron-%d' % i})))
+        self.agent.sb_idl.get_ports_on_chassis.return_value = self.ports
+
     def test_sync(self):
+
         with mock.patch.object(
                 self.agent, 'ensure_all_networks_provisioned') as enp,\
                 mock.patch.object(
                     ip_lib, 'list_network_namespaces') as lnn,\
                 mock.patch.object(
                     self.agent, 'teardown_datapath') as tdp:
-            enp.return_value = ['ovnmeta-1', 'ovnmeta-2']
             lnn.return_value = ['ovnmeta-1', 'ovnmeta-2']
 
             self.agent.sync()
 
-            enp.assert_called_once_with()
+            enp.assert_called_once_with({
+                (p.datapath.uuid, p.datapath.uuid)
+                for p in self.ports
+            })
+
             lnn.assert_called_once_with()
             tdp.assert_not_called()
 
@@ -97,18 +107,20 @@ def test_sync_teardown_namespace(self):
                     ip_lib, 'list_network_namespaces') as lnn,\
                 mock.patch.object(
                     self.agent, 'teardown_datapath') as tdp:
-            enp.return_value = ['ovnmeta-1', 'ovnmeta-2']
             lnn.return_value = ['ovnmeta-1', 'ovnmeta-2', 'ovnmeta-3',
                                 'ns1', 'ns2']
 
             self.agent.sync()
 
-            enp.assert_called_once_with()
+            enp.assert_called_once_with({
+                (p.datapath.uuid, p.datapath.uuid)
+                for p in self.ports
+            })
             lnn.assert_called_once_with()
             tdp.assert_called_once_with('3')
 
-    def test_ensure_all_networks_provisioned(self):
-        """Test networks are provisioned.
+    def test_get_networks(self):
+        """Test which networks are provisioned.
 
         This test simulates that this chassis has the following ports:
             * datapath '0': 1 port
@@ -117,61 +129,37 @@ def test_ensure_all_networks_provisioned(self):
             * datapath '3': 1 port with type 'external'
             * datapath '5': 1 port with type 'unknown'
 
-        It is expected that only datapaths '0', '1' and '2' are provisioned
-        once.
+        It is expected that only datapaths '0', '1' and '2' are scheduled for
+        provisioning.
         """
 
-        ports = []
-        for i in range(0, 3):
-            ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
-                external_ids={'name': 'neutron-%d' % i})))
-        ports.append(makePort(datapath=DatapathInfo(uuid='1',
+        self.ports.append(makePort(datapath=DatapathInfo(uuid='1',
             external_ids={'name': 'neutron-1'})))
-        ports.append(makePort(datapath=DatapathInfo(uuid='3',
+        self.ports.append(makePort(datapath=DatapathInfo(uuid='3',
             external_ids={'name': 'neutron-3'}), type='external'))
-        ports.append(makePort(datapath=DatapathInfo(uuid='5',
+        self.ports.append(makePort(datapath=DatapathInfo(uuid='5',
             external_ids={'name': 'neutron-5'}), type='unknown'))
 
-        with mock.patch.object(self.agent, 'provision_datapath',
-                               return_value=None) as pdp,\
-                mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis',
-                                  return_value=ports):
-            self.agent.ensure_all_networks_provisioned()
-
-            expected_calls = [mock.call(str(i), str(i)) for i in range(0, 4)]
-            self.assertEqual(sorted(expected_calls),
-                             sorted(pdp.call_args_list))
+        expected_networks = {(str(i), str(i)) for i in range(0, 4)}
+        self.assertEqual(expected_networks, self.agent.get_networks())
 
     def test_update_datapath_provision(self):
-        ports = []
-        for i in range(0, 3):
-            ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
-                external_ids={'name': 'neutron-%d' % i})))
-        ports.append(makePort(datapath=DatapathInfo(uuid='3',
+        self.ports.append(makePort(datapath=DatapathInfo(uuid='3',
             external_ids={'name': 'neutron-3'}), type='external'))
 
         with mock.patch.object(self.agent, 'provision_datapath',
                                return_value=None) as pdp,\
-                mock.patch.object(self.agent, 'teardown_datapath') as tdp,\
-                mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis',
-                                  return_value=ports):
+                mock.patch.object(self.agent, 'teardown_datapath') as tdp:
             self.agent.update_datapath('1', 'a')
             self.agent.update_datapath('3', 'b')
             expected_calls = [mock.call('1', 'a'), mock.call('3', 'b')]
             pdp.assert_has_calls(expected_calls)
             tdp.assert_not_called()
 
     def test_update_datapath_teardown(self):
-        ports = []
-        for i in range(0, 3):
-            ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
-                external_ids={'name': 'neutron-%d' % i})))
-
         with mock.patch.object(self.agent, 'provision_datapath',
                                return_value=None) as pdp,\
-                mock.patch.object(self.agent, 'teardown_datapath') as tdp,\
-                mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis',
-                                  return_value=ports):
+                mock.patch.object(self.agent, 'teardown_datapath') as tdp:
             self.agent.update_datapath('5', 'a')
             tdp.assert_called_once_with('5', 'a')
             pdp.assert_not_called()