Update vif_attach from NeutronVIFPortIDMixin

This patch updates vif_attach from NeutronVIFPortIDMixin to rely
on presence of Neutron. Drop incorrect logic that skips mac_address
update when show_port() to neutron failed as NeutronVIFPortIDMixin
is used only in flat and neutron network interfaces which require
neutron presence.
Update api tests to use real neutron port.

Change-Id: Iffaf3569aa296b4466729cf7f62c92995b6b3723

ironic/drivers/modules/network/common.py

@@ -559,32 +559,15 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin):
 
         # Address is optional for portgroups
         if port_like_obj.address:
-            # Check if the requested vif_id is a neutron port. If it is
-            # then attempt to update the port's MAC address.
             try:
-                client.show_port(vif_id)
-            except neutron_exceptions.NeutronClientException:
-                # TODO(mgoddard): Remove this except clause and handle errors
-                # properly. We can do this once a strategy has been determined
-                # for handling the tempest VIF tests in an environment that
-                # may not support neutron.
-                # NOTE(sambetts): If a client error occurs this is because
-                # either neutron doesn't exist because we're running in
-                # standalone environment or we can't find a matching neutron
-                # port which means a user might be requesting a non-neutron
-                # port. So skip trying to update the neutron port MAC address
-                # in these cases.
-                pass
-            else:
-                try:
-                    neutron.update_port_address(vif_id, port_like_obj.address)
-                except exception.FailedToUpdateMacOnPort:
-                    raise exception.NetworkError(_(
-                        "Unable to attach VIF %(vif)s because Ironic can not "
-                        "update Neutron port %(port)s MAC address to match "
-                        "physical MAC address %(mac)s") % {
-                            'vif': vif_id, 'port': vif_id,
-                            'mac': port_like_obj.address})
+                neutron.update_port_address(vif_id, port_like_obj.address)
+            except exception.FailedToUpdateMacOnPort:
+                raise exception.NetworkError(_(
+                    "Unable to attach VIF %(vif)s because Ironic can not "
+                    "update Neutron port %(port)s MAC address to match "
+                    "physical MAC address %(mac)s") % {
+                        'vif': vif_id, 'port': vif_id,
+                        'mac': port_like_obj.address})
 
         self._save_vif_to_port_like_obj(port_like_obj, vif_id)
 

ironic/tests/unit/drivers/modules/network/test_common.py

@@ -11,7 +11,6 @@
 #    under the License.
 
 import mock
-from neutronclient.common import exceptions as neutron_exceptions
 from oslo_config import cfg
 from oslo_utils import uuidutils
 
@@ -684,8 +683,6 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
         mock_client.assert_called_once_with()
         self.assertFalse(mock_gpbpi.called)
         mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
-        mock_client.return_value.show_port.assert_called_once_with(
-            'fake_vif_id')
         mock_upa.assert_called_once_with("fake_vif_id", self.port.address)
         mock_save.assert_called_once_with(self.port, "fake_vif_id")
 
@@ -724,8 +721,6 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
         mock_gpbpi.assert_called_once_with(mock_client.return_value,
                                            'fake_vif_id')
         mock_gfp.assert_called_once_with(task, 'fake_vif_id', {'physnet1'})
-        mock_client.return_value.show_port.assert_called_once_with(
-            'fake_vif_id')
         mock_upa.assert_called_once_with("fake_vif_id", self.port.address)
         mock_save.assert_called_once_with(self.port, "fake_vif_id")
 
@@ -747,10 +742,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
         mock_client.assert_called_once_with()
         self.assertFalse(mock_gpbpi.called)
         mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
-        mock_client.return_value.show_port.assert_called_once_with(
-            'fake_vif_id')
         mock_upa.assert_called_once_with("fake_vif_id", self.port.address)
-        mock_save.assert_called_once_with(self.port, "fake_vif_id")
         mock_plug.assert_called_once_with(task, self.port, mock.ANY)
 
     @mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj')
@@ -774,8 +766,6 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
         mock_client.assert_called_once_with()
         self.assertFalse(mock_gpbpi.called)
         mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
-        mock_client.return_value.show_port.assert_called_once_with(
-            'fake_vif_id')
         mock_upa.assert_called_once_with("fake_vif_id", self.port.address)
         mock_save.assert_called_once_with(self.port, "fake_vif_id")
         mock_plug.assert_called_once_with(task, self.port, mock.ANY)
@@ -822,36 +812,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
         mock_client.assert_called_once_with()
         mock_gpbpi.assert_called_once_with(mock_client.return_value,
                                            'fake_vif_id')
-        mock_client.return_value.show_port.assert_called_once_with(
-            'fake_vif_id')
         self.assertFalse(mock_save.called)
 
-    @mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj')
-    @mock.patch.object(common, 'get_free_port_like_object', autospec=True)
-    @mock.patch.object(neutron_common, 'get_client', autospec=True)
-    @mock.patch.object(neutron_common, 'update_port_address')
-    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
-                       autospec=True)
-    def test_vif_attach_neutron_absent(self, mock_gpbpi, mock_upa,
-                                       mock_client, mock_gfp, mock_save):
-        self.port.physical_network = 'physnet1'
-        self.port.save()
-        vif = {'id': "fake_vif_id"}
-        mock_gfp.return_value = self.port
-        mock_client.return_value.show_port.side_effect = (
-            neutron_exceptions.NeutronClientException)
-        mock_gpbpi.side_effect = exception.NetworkError
-        with task_manager.acquire(self.context, self.node.id) as task:
-            self.interface.vif_attach(task, vif)
-        mock_client.assert_called_once_with()
-        mock_gpbpi.assert_called_once_with(mock_client.return_value,
-                                           'fake_vif_id')
-        mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
-        mock_client.return_value.show_port.assert_called_once_with(
-            'fake_vif_id')
-        self.assertFalse(mock_upa.called)
-        self.assertTrue(mock_save.called)
-
     @mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj')
     @mock.patch.object(common, 'get_free_port_like_object', autospec=True)
     @mock.patch.object(neutron_common, 'get_client')

ironic_tempest_plugin/tests/api/admin/test_nodes.py

@@ -299,6 +299,16 @@ class TestNodesVif(base.BaseBaremetalTest):
 
         _, self.chassis = self.create_chassis()
         _, self.node = self.create_node(self.chassis['uuid'])
+        self.net = self.admin_manager.networks_client.create_network()
+
+        self.nport_id = self.admin_manager.ports_client.create_port(
+            network_id=self.net['network']['id'])['port']['id']
+
+    def tearDown(self):
+        super(TestNodesVif, self).tearDown()
+        self.admin_manager.ports_client.delete_port(self.nport_id)
+        self.admin_manager.networks_client.delete_network(
+            self.net['network']['id'])
 
     @decorators.idempotent_id('a3d319d0-cacb-4e55-a3dc-3fa8b74880f1')
     def test_vif_on_port(self):
@@ -316,13 +326,13 @@ class TestNodesVif(base.BaseBaremetalTest):
             api_microversion_fixture.APIMicroversionFixture('1.28'))
         _, self.port = self.create_port(self.node['uuid'],
                                         data_utils.rand_mac_address())
-        self.client.vif_attach(self.node['uuid'], 'test-vif')
+        self.client.vif_attach(self.node['uuid'], self.nport_id)
         _, body = self.client.vif_list(self.node['uuid'])
-        self.assertEqual({'vifs': [{'id': 'test-vif'}]}, body)
+        self.assertEqual({'vifs': [{'id': self.nport_id}]}, body)
         _, port = self.client.show_port(self.port['uuid'])
-        self.assertEqual('test-vif',
+        self.assertEqual(self.nport_id,
                          port['internal_info']['tenant_vif_port_id'])
-        self.client.vif_detach(self.node['uuid'], 'test-vif')
+        self.client.vif_detach(self.node['uuid'], self.nport_id)
         _, body = self.client.vif_list(self.node['uuid'])
         self.assertEqual({'vifs': []}, body)
         _, port = self.client.show_port(self.port['uuid'])
@@ -355,17 +365,17 @@ class TestNodesVif(base.BaseBaremetalTest):
                   'value': self.portgroup['uuid']}]
         self.client.update_port(self.port['uuid'], patch)
 
-        self.client.vif_attach(self.node['uuid'], 'test-vif')
+        self.client.vif_attach(self.node['uuid'], self.nport_id)
         _, body = self.client.vif_list(self.node['uuid'])
-        self.assertEqual({'vifs': [{'id': 'test-vif'}]}, body)
+        self.assertEqual({'vifs': [{'id': self.nport_id}]}, body)
 
         _, port = self.client.show_port(self.port['uuid'])
         self.assertNotIn('tenant_vif_port_id', port['internal_info'])
         _, portgroup = self.client.show_portgroup(self.portgroup['uuid'])
-        self.assertEqual('test-vif',
+        self.assertEqual(self.nport_id,
                          portgroup['internal_info']['tenant_vif_port_id'])
 
-        self.client.vif_detach(self.node['uuid'], 'test-vif')
+        self.client.vif_detach(self.node['uuid'], self.nport_id)
         _, body = self.client.vif_list(self.node['uuid'])
         self.assertEqual({'vifs': []}, body)
         _, portgroup = self.client.show_portgroup(self.portgroup['uuid'])
@@ -379,13 +389,13 @@ class TestNodesVif(base.BaseBaremetalTest):
                                         data_utils.rand_mac_address())
         patch = [{'path': '/extra/vif_port_id',
                   'op': 'add',
-                  'value': 'test-vif'}]
+                  'value': self.nport_id}]
         self.client.update_port(self.port['uuid'], patch)
 
         _, body = self.client.vif_list(self.node['uuid'])
-        self.assertEqual({'vifs': [{'id': 'test-vif'}]}, body)
+        self.assertEqual({'vifs': [{'id': self.nport_id}]}, body)
 
         self.assertRaises(lib_exc.Conflict, self.client.vif_attach,
-                          self.node['uuid'], 'test-vif')
+                          self.node['uuid'], self.nport_id)
 
-        self.client.vif_detach(self.node['uuid'], 'test-vif')
+        self.client.vif_detach(self.node['uuid'], self.nport_id)