Refactor VIFPortIDMixin: factor out common methods

This change is part of a series of changes which aim to refactor the
VIFPortIDMixin mixin class. The refactor aims to reduce code
duplication, and separate the neutron interactions from the more general
VIF attach/detach behaviour.

The neutron separation will be made possible by introducing an
inheritence hierarchy to the mixin class.  VIFPortIDMixin will become a
general VIF attach/detach class, with no neutron interaction.
NeutronVIFPortIDMixin will inherit from VIFPortIDMixin and override the
subclass behaviour to interact with neutron where appropriate.

Once this refactor is complete, a new fake network interface will be
added with similar VIF attach/detach behaviour to the flat and neutron
network interfaces, but without any interaction with neutron. This
interface will be useful in testing environments.

This change refactors the NeutronVIFPortIDMixin class by factoring out
some common methods into a parent class, VIFPortIDMixin. This will help
to avoid code duplication between the classes.

Change-Id: I69a104ec967572f65566b0e0d67145d135be3ac9

1 files changed, 362 insertions, 188 deletions

diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py
index fc5c44d5e..af6d2451c 100644
--- a/ironic/tests/unit/drivers/modules/network/test_common.py
+++ b/ironic/tests/unit/drivers/modules/network/test_common.py
@@ -436,13 +436,13 @@ class TestCommonFunctions(db_base.DbTestCase):
                 task, self.port)
 
 
-class TestNeutronVifPortIDMixin(db_base.DbTestCase):
+class TestVifPortIDMixin(db_base.DbTestCase):
 
     def setUp(self):
-        super(TestNeutronVifPortIDMixin, self).setUp()
+        super(TestVifPortIDMixin, self).setUp()
         self.config(enabled_drivers=['fake'])
         mgr_utils.mock_the_extension_manager()
-        self.interface = common.NeutronVIFPortIDMixin()
+        self.interface = common.VIFPortIDMixin()
         self.node = obj_utils.create_test_node(self.context,
                                                network_interface='neutron')
         self.port = obj_utils.create_test_port(
@@ -450,8 +450,121 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
             address='52:54:00:cf:2d:32',
             extra={'vif_port_id': uuidutils.generate_uuid(),
                    'client-id': 'fake1'})
-        self.neutron_port = {'id': '132f871f-eaec-4fed-9475-0d54465e0f00',
-                             'mac_address': '52:54:00:cf:2d:32'}
+
+    def test__save_vif_to_port_like_obj_port(self):
+        self.port.extra = {}
+        self.port.save()
+        vif_id = "fake_vif_id"
+        self.interface._save_vif_to_port_like_obj(self.port, vif_id)
+        self.port.refresh()
+        self.assertIn(common.TENANT_VIF_KEY, self.port.internal_info)
+        self.assertEqual(vif_id,
+                         self.port.internal_info[common.TENANT_VIF_KEY])
+        self.assertEqual({}, self.port.extra)
+
+    def test__save_vif_to_port_like_obj_portgroup(self):
+        vif_id = "fake_vif_id"
+        pg = obj_utils.create_test_portgroup(
+            self.context, node_id=self.node.id)
+        obj_utils.create_test_port(
+            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
+            portgroup_id=pg.id, uuid=uuidutils.generate_uuid()
+        )
+        self.interface._save_vif_to_port_like_obj(pg, vif_id)
+        pg.refresh()
+        self.assertIn(common.TENANT_VIF_KEY, pg.internal_info)
+        self.assertEqual(vif_id,
+                         pg.internal_info[common.TENANT_VIF_KEY])
+        self.assertEqual({}, pg.extra)
+
+    def test__clear_vif_from_port_like_obj_in_extra_port(self):
+        self.interface._clear_vif_from_port_like_obj(self.port)
+        self.port.refresh()
+        self.assertNotIn('vif_port_id', self.port.extra)
+        self.assertNotIn(common.TENANT_VIF_KEY, self.port.internal_info)
+
+    def test__clear_vif_from_port_like_obj_in_internal_info_port(self):
+        self.port.internal_info = {
+            common.TENANT_VIF_KEY: self.port.extra['vif_port_id']}
+        self.port.extra = {}
+        self.port.save()
+
+        self.interface._clear_vif_from_port_like_obj(self.port)
+        self.port.refresh()
+        self.assertNotIn('vif_port_id', self.port.extra)
+        self.assertNotIn(common.TENANT_VIF_KEY, self.port.internal_info)
+
+    def test__clear_vif_from_port_like_obj_in_extra_portgroup(self):
+        vif_id = uuidutils.generate_uuid()
+        pg = obj_utils.create_test_portgroup(
+            self.context, node_id=self.node.id,
+            extra={'vif_port_id': vif_id})
+        obj_utils.create_test_port(
+            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
+            portgroup_id=pg.id, uuid=uuidutils.generate_uuid()
+        )
+        self.interface._clear_vif_from_port_like_obj(pg)
+        pg.refresh()
+        self.assertNotIn('vif_port_id', pg.extra)
+        self.assertNotIn(common.TENANT_VIF_KEY, pg.internal_info)
+
+    def test__clear_vif_from_port_like_obj_in_internal_info_portgroup(self):
+        vif_id = uuidutils.generate_uuid()
+        pg = obj_utils.create_test_portgroup(
+            self.context, node_id=self.node.id,
+            internal_info={common.TENANT_VIF_KEY: vif_id})
+        obj_utils.create_test_port(
+            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
+            portgroup_id=pg.id, uuid=uuidutils.generate_uuid()
+        )
+        self.interface._clear_vif_from_port_like_obj(pg)
+        pg.refresh()
+        self.assertNotIn('vif_port_id', pg.extra)
+        self.assertNotIn(common.TENANT_VIF_KEY, pg.internal_info)
+
+    def test__get_port_like_obj_by_vif_id_in_extra(self):
+        vif_id = self.port.extra["vif_port_id"]
+        with task_manager.acquire(self.context, self.node.id) as task:
+            result = self.interface._get_port_like_obj_by_vif_id(task, vif_id)
+        self.assertEqual(self.port.id, result.id)
+
+    def test__get_port_like_obj_by_vif_id_in_internal_info(self):
+        vif_id = self.port.extra["vif_port_id"]
+        self.port.internal_info = {common.TENANT_VIF_KEY: vif_id}
+        self.port.extra = {}
+        self.port.save()
+        with task_manager.acquire(self.context, self.node.id) as task:
+            result = self.interface._get_port_like_obj_by_vif_id(task, vif_id)
+        self.assertEqual(self.port.id, result.id)
+
+    def test__get_port_like_obj_by_vif_id_not_attached(self):
+        vif_id = self.port.extra["vif_port_id"]
+        self.port.extra = {}
+        self.port.save()
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.assertRaisesRegex(exception.VifNotAttached,
+                                   "it is not attached to it.",
+                                   self.interface._get_port_like_obj_by_vif_id,
+                                   task, vif_id)
+
+    def test__get_vif_id_by_port_like_obj_in_extra(self):
+        vif_id = self.port.extra["vif_port_id"]
+        result = self.interface._get_vif_id_by_port_like_obj(self.port)
+        self.assertEqual(vif_id, result)
+
+    def test__get_vif_id_by_port_like_obj_in_internal_info(self):
+        vif_id = self.port.extra["vif_port_id"]
+        self.port.internal_info = {common.TENANT_VIF_KEY: vif_id}
+        self.port.extra = {}
+        self.port.save()
+        result = self.interface._get_vif_id_by_port_like_obj(self.port)
+        self.assertEqual(vif_id, result)
+
+    def test__get_vif_id_by_port_like_obj_not_attached(self):
+        self.port.extra = {}
+        self.port.save()
+        result = self.interface._get_vif_id_by_port_like_obj(self.port)
+        self.assertIsNone(result)
 
     def test_vif_list_extra(self):
         vif_id = uuidutils.generate_uuid()
@@ -495,36 +608,111 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
             vifs = self.interface.vif_list(task)
             self.assertEqual([{'id': vif_id}], vifs)
 
+    def test_get_current_vif_extra_vif_port_id(self):
+        extra = {'vif_port_id': 'foo'}
+        self.port.extra = extra
+        self.port.save()
+        with task_manager.acquire(self.context, self.node.id) as task:
+            vif = self.interface.get_current_vif(task, self.port)
+            self.assertEqual('foo', vif)
+
+    def test_get_current_vif_internal_info_cleaning(self):
+        internal_info = {'cleaning_vif_port_id': 'foo',
+                         'tenant_vif_port_id': 'bar'}
+        self.port.internal_info = internal_info
+        self.port.save()
+        with task_manager.acquire(self.context, self.node.id) as task:
+            vif = self.interface.get_current_vif(task, self.port)
+            self.assertEqual('foo', vif)
+
+    def test_get_current_vif_internal_info_provisioning(self):
+        internal_info = {'provisioning_vif_port_id': 'foo',
+                         'vif_port_id': 'bar'}
+        self.port.internal_info = internal_info
+        self.port.save()
+        with task_manager.acquire(self.context, self.node.id) as task:
+            vif = self.interface.get_current_vif(task, self.port)
+            self.assertEqual('foo', vif)
+
+    def test_get_current_vif_internal_info_tenant_vif(self):
+        internal_info = {'tenant_vif_port_id': 'bar'}
+        self.port.internal_info = internal_info
+        self.port.save()
+        with task_manager.acquire(self.context, self.node.id) as task:
+            vif = self.interface.get_current_vif(task, self.port)
+            self.assertEqual('bar', vif)
+
+    def test_get_current_vif_none(self):
+        internal_info = extra = {}
+        self.port.internal_info = internal_info
+        self.port.extra = extra
+        self.port.save()
+        with task_manager.acquire(self.context, self.node.id) as task:
+            vif = self.interface.get_current_vif(task, self.port)
+            self.assertIsNone(vif)
+
+
+class TestNeutronVifPortIDMixin(db_base.DbTestCase):
+
+    def setUp(self):
+        super(TestNeutronVifPortIDMixin, self).setUp()
+        self.config(enabled_drivers=['fake'])
+        mgr_utils.mock_the_extension_manager()
+        self.interface = common.NeutronVIFPortIDMixin()
+        self.node = obj_utils.create_test_node(self.context,
+                                               network_interface='neutron')
+        self.port = obj_utils.create_test_port(
+            self.context, node_id=self.node.id,
+            address='52:54:00:cf:2d:32',
+            extra={'vif_port_id': uuidutils.generate_uuid(),
+                   'client-id': 'fake1'})
+        self.neutron_port = {'id': '132f871f-eaec-4fed-9475-0d54465e0f00',
+                             'mac_address': '52:54:00:cf:2d:32'}
+
+    @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', autospec=True)
     @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
                        autospec=True)
-    def test_vif_attach(self, mock_gpbpi, mock_upa, mock_client, mock_gfp):
-        self.port.extra = {}
-        self.port.save()
+    def test_vif_attach(self, mock_gpbpi, mock_upa, mock_client, mock_gfp,
+                        mock_save):
         vif = {'id': "fake_vif_id"}
         mock_gfp.return_value = self.port
         with task_manager.acquire(self.context, self.node.id) as task:
             self.interface.vif_attach(task, vif)
-            self.port.refresh()
-            self.assertEqual("fake_vif_id", self.port.internal_info.get(
-                common.TENANT_VIF_KEY))
-            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_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.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', autospec=True)
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach_failure(self, mock_gpbpi, mock_upa, mock_client,
+                                mock_gfp, mock_save):
+        vif = {'id': "fake_vif_id"}
+        mock_gfp.side_effect = exception.NoFreePhysicalPorts(vif='fake-vif')
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.assertRaises(exception.NoFreePhysicalPorts,
+                              self.interface.vif_attach, task, vif)
+        mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
+        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', autospec=True)
     @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
                        autospec=True)
     def test_vif_attach_with_physnet(self, mock_gpbpi, mock_upa, mock_client,
-                                     mock_gfp):
-        self.port.extra = {}
+                                     mock_gfp, mock_save):
         self.port.physical_network = 'physnet1'
         self.port.save()
         vif = {'id': "fake_vif_id"}
@@ -532,48 +720,94 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
         mock_gfp.return_value = self.port
         with task_manager.acquire(self.context, self.node.id) as task:
             self.interface.vif_attach(task, vif)
-            self.port.refresh()
-            self.assertEqual("fake_vif_id", self.port.internal_info.get(
-                common.TENANT_VIF_KEY))
-            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', {'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_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', {'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")
+
+    @mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj')
+    @mock.patch.object(common, 'plug_port_to_tenant_network', autospec=True)
+    @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', autospec=True)
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach_active_node(self, mock_gpbpi, mock_upa, mock_client,
+                                    mock_gfp, mock_plug, mock_save):
+        self.node.provision_state = states.ACTIVE
+        self.node.save()
+        vif = {'id': "fake_vif_id"}
+        mock_gfp.return_value = self.port
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.interface.vif_attach(task, vif)
+        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')
+    @mock.patch.object(common, 'plug_port_to_tenant_network', autospec=True)
+    @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', autospec=True)
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach_active_node_failure(self, mock_gpbpi, mock_upa,
+                                            mock_client, mock_gfp, mock_plug,
+                                            mock_save):
+        self.node.provision_state = states.ACTIVE
+        self.node.save()
+        vif = {'id': "fake_vif_id"}
+        mock_gfp.return_value = self.port
+        mock_plug.side_effect = exception.NetworkError
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.assertRaises(exception.NetworkError,
+                              self.interface.vif_attach, task, vif)
+        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')
     @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_portgroup_no_address(self, mock_gpbpi, mock_upa,
-                                             mock_client, mock_gfp):
+                                             mock_client, mock_gfp, mock_save):
         pg = obj_utils.create_test_portgroup(
             self.context, node_id=self.node.id, address=None)
         mock_gfp.return_value = pg
-        obj_utils.create_test_port(
-            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
-            uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
         vif = {'id': "fake_vif_id"}
         with task_manager.acquire(self.context, self.node.id) as task:
             self.interface.vif_attach(task, vif)
-            pg.refresh()
-            self.assertEqual(vif['id'],
-                             pg.internal_info[common.TENANT_VIF_KEY])
-            mock_client.assert_called_once_with()
-            self.assertFalse(mock_gpbpi.called)
-            mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
-            self.assertFalse(mock_client.return_value.show_port.called)
-            self.assertFalse(mock_upa.called)
-
+        mock_client.assert_called_once_with()
+        self.assertFalse(mock_gpbpi.called)
+        mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
+        self.assertFalse(mock_client.return_value.show_port.called)
+        self.assertFalse(mock_upa.called)
+        mock_save.assert_called_once_with(pg, "fake_vif_id")
+
+    @mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj')
     @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_update_port_exception(self, mock_gpbpi, mock_upa,
-                                              mock_client):
+                                              mock_client, mock_save):
         self.port.extra = {}
         self.port.physical_network = 'physnet1'
         self.port.save()
@@ -585,20 +819,21 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
             self.assertRaisesRegex(
                 exception.NetworkError, "can not update Neutron port",
                 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_client.return_value.show_port.assert_called_once_with(
-                'fake_vif_id')
-
+        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):
-        self.port.extra = {}
+                                       mock_client, mock_gfp, mock_save):
         self.port.physical_network = 'physnet1'
         self.port.save()
         vif = {'id': "fake_vif_id"}
@@ -608,14 +843,16 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
         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)
-
+        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')
     @mock.patch.object(neutron_common, 'update_port_address')
@@ -623,8 +860,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
                        autospec=True)
     def test_vif_attach_portgroup_physnet_inconsistent(self, mock_gpbpi,
                                                        mock_upa, mock_client,
-                                                       mock_gfp):
-        self.port.extra = {}
+                                                       mock_gfp, mock_save):
         self.port.physical_network = 'physnet1'
         self.port.save()
         vif = {'id': "fake_vif_id"}
@@ -635,19 +871,21 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
             self.assertRaises(
                 exception.PortgroupPhysnetInconsistent,
                 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')
-            self.assertFalse(mock_upa.called)
+        mock_client.assert_called_once_with()
+        mock_gpbpi.assert_called_once_with(mock_client.return_value,
+                                           'fake_vif_id')
+        self.assertFalse(mock_upa.called)
+        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')
     @mock.patch.object(neutron_common, 'update_port_address')
     @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
                        autospec=True)
     def test_vif_attach_multiple_segment_mappings(self, mock_gpbpi, mock_upa,
-                                                  mock_client, mock_gfp):
-        self.port.extra = {}
+                                                  mock_client, mock_gfp,
+                                                  mock_save):
         self.port.physical_network = 'physnet1'
         self.port.save()
         obj_utils.create_test_port(
@@ -659,143 +897,79 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
             self.assertRaises(
                 exception.VifInvalidForAttach,
                 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')
-            self.assertFalse(mock_gfp.called)
-            self.assertFalse(mock_upa.called)
-
-    def test_vif_detach_in_extra(self):
-        with task_manager.acquire(self.context, self.node.id) as task:
-            self.interface.vif_detach(
-                task, self.port.extra['vif_port_id'])
-            self.port.refresh()
-            self.assertFalse('vif_port_id' in self.port.extra)
-            self.assertFalse(common.TENANT_VIF_KEY in self.port.internal_info)
-
+        mock_client.assert_called_once_with()
+        mock_gpbpi.assert_called_once_with(mock_client.return_value,
+                                           'fake_vif_id')
+        self.assertFalse(mock_gfp.called)
+        self.assertFalse(mock_upa.called)
+        self.assertFalse(mock_save.called)
+
+    @mock.patch.object(common.VIFPortIDMixin, '_clear_vif_from_port_like_obj')
     @mock.patch.object(neutron_common, 'unbind_neutron_port', autospec=True)
-    def test_vif_detach_in_internal_info(self, mock_unp):
-        self.port.internal_info = {
-            common.TENANT_VIF_KEY: self.port.extra['vif_port_id']}
-        self.port.extra = {}
-        self.port.save()
+    @mock.patch.object(common.VIFPortIDMixin, '_get_port_like_obj_by_vif_id')
+    def test_vif_detach(self, mock_get, mock_unp, mock_clear):
+        mock_get.return_value = self.port
         with task_manager.acquire(self.context, self.node.id) as task:
-            self.interface.vif_detach(
-                task, self.port.internal_info[common.TENANT_VIF_KEY])
-            self.port.refresh()
-            self.assertFalse('vif_port_id' in self.port.extra)
-            self.assertFalse(common.TENANT_VIF_KEY in self.port.internal_info)
+            self.interface.vif_detach(task, 'fake_vif_id')
+        mock_get.assert_called_once_with(task, 'fake_vif_id')
         self.assertFalse(mock_unp.called)
+        mock_clear.assert_called_once_with(self.port)
 
-    def test_vif_detach_in_extra_portgroup(self):
-        vif_id = uuidutils.generate_uuid()
-        pg = obj_utils.create_test_portgroup(
-            self.context, node_id=self.node.id,
-            extra={'vif_port_id': vif_id})
-        obj_utils.create_test_port(
-            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
-            portgroup_id=pg.id, uuid=uuidutils.generate_uuid()
-        )
-        with task_manager.acquire(self.context, self.node.id) as task:
-            self.interface.vif_detach(task, vif_id)
-            pg.refresh()
-            self.assertFalse('vif_port_id' in pg.extra)
-            self.assertFalse(common.TENANT_VIF_KEY in pg.internal_info)
-
-    def test_vif_detach_in_extra_portgroup_manually_attached_to_port(self):
-        vif_id = uuidutils.generate_uuid()
+    @mock.patch.object(common.VIFPortIDMixin, '_clear_vif_from_port_like_obj')
+    @mock.patch.object(neutron_common, 'unbind_neutron_port', autospec=True)
+    @mock.patch.object(common.VIFPortIDMixin, '_get_port_like_obj_by_vif_id')
+    def test_vif_detach_portgroup(self, mock_get, mock_unp, mock_clear):
         pg = obj_utils.create_test_portgroup(
             self.context, node_id=self.node.id)
-        port = obj_utils.create_test_port(
-            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
-            portgroup_id=pg.id, uuid=uuidutils.generate_uuid(),
-            extra={'vif_port_id': vif_id}
-        )
-        with task_manager.acquire(self.context, self.node.id) as task:
-            self.interface.vif_detach(task, vif_id)
-            port.refresh()
-            pg.refresh()
-            self.assertFalse('vif_port_id' in port.extra)
-            self.assertFalse(common.TENANT_VIF_KEY in pg.internal_info)
-
-    def test_vif_detach_in_internal_info_portgroup(self):
-        vif_id = uuidutils.generate_uuid()
-        pg = obj_utils.create_test_portgroup(
-            self.context, node_id=self.node.id,
-            internal_info={common.TENANT_VIF_KEY: vif_id})
-        obj_utils.create_test_port(
-            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
-            portgroup_id=pg.id, uuid=uuidutils.generate_uuid()
-        )
+        mock_get.return_value = pg
         with task_manager.acquire(self.context, self.node.id) as task:
-            self.interface.vif_detach(task, vif_id)
-            pg.refresh()
-            self.assertFalse('vif_port_id' in pg.extra)
-            self.assertFalse(common.TENANT_VIF_KEY in pg.internal_info)
+            self.interface.vif_detach(task, 'fake_vif_id')
+        mock_get.assert_called_once_with(task, 'fake_vif_id')
+        self.assertFalse(mock_unp.called)
+        mock_clear.assert_called_once_with(pg)
 
-    def test_vif_detach_not_attached(self):
+    @mock.patch.object(common.VIFPortIDMixin, '_clear_vif_from_port_like_obj')
+    @mock.patch.object(neutron_common, 'unbind_neutron_port', autospec=True)
+    @mock.patch.object(common.VIFPortIDMixin, '_get_port_like_obj_by_vif_id')
+    def test_vif_detach_not_attached(self, mock_get, mock_unp, mock_clear):
+        mock_get.side_effect = exception.VifNotAttached(vif='fake-vif',
+                                                        node='fake-node')
         with task_manager.acquire(self.context, self.node.id) as task:
             self.assertRaisesRegex(
                 exception.VifNotAttached, "it is not attached to it.",
-                self.interface.vif_detach, task, 'aaa')
+                self.interface.vif_detach, task, 'fake_vif_id')
+        mock_get.assert_called_once_with(task, 'fake_vif_id')
+        self.assertFalse(mock_unp.called)
+        self.assertFalse(mock_clear.called)
 
+    @mock.patch.object(common.VIFPortIDMixin, '_clear_vif_from_port_like_obj')
     @mock.patch.object(neutron_common, 'unbind_neutron_port', autospec=True)
-    def test_vif_detach_active_node(self, mock_unp):
-        vif_id = self.port.extra['vif_port_id']
-        self.port.internal_info = {
-            common.TENANT_VIF_KEY: vif_id}
-        self.port.extra = {}
-        self.port.save()
+    @mock.patch.object(common.VIFPortIDMixin, '_get_port_like_obj_by_vif_id')
+    def test_vif_detach_active_node(self, mock_get, mock_unp, mock_clear):
         self.node.provision_state = states.ACTIVE
         self.node.save()
+        mock_get.return_value = self.port
         with task_manager.acquire(self.context, self.node.id) as task:
-            self.interface.vif_detach(
-                task, self.port.internal_info[common.TENANT_VIF_KEY])
-            self.port.refresh()
-            mock_unp.assert_called_once_with(vif_id)
-
-    def test_get_current_vif_extra_vif_port_id(self):
-        extra = {'vif_port_id': 'foo'}
-        self.port.extra = extra
-        self.port.save()
-        with task_manager.acquire(self.context, self.node.id) as task:
-            vif = self.interface.get_current_vif(task, self.port)
-            self.assertEqual('foo', vif)
-
-    def test_get_current_vif_internal_info_cleaning(self):
-        internal_info = {'cleaning_vif_port_id': 'foo',
-                         'tenant_vif_port_id': 'bar'}
-        self.port.internal_info = internal_info
-        self.port.save()
-        with task_manager.acquire(self.context, self.node.id) as task:
-            vif = self.interface.get_current_vif(task, self.port)
-            self.assertEqual('foo', vif)
-
-    def test_get_current_vif_internal_info_provisioning(self):
-        internal_info = {'provisioning_vif_port_id': 'foo',
-                         'vif_port_id': 'bar'}
-        self.port.internal_info = internal_info
-        self.port.save()
-        with task_manager.acquire(self.context, self.node.id) as task:
-            vif = self.interface.get_current_vif(task, self.port)
-            self.assertEqual('foo', vif)
-
-    def test_get_current_vif_internal_info_tenant_vif(self):
-        internal_info = {'tenant_vif_port_id': 'bar'}
-        self.port.internal_info = internal_info
-        self.port.save()
-        with task_manager.acquire(self.context, self.node.id) as task:
-            vif = self.interface.get_current_vif(task, self.port)
-            self.assertEqual('bar', vif)
+            self.interface.vif_detach(task, 'fake_vif_id')
+        mock_get.assert_called_once_with(task, 'fake_vif_id')
+        mock_clear.assert_called_once_with(self.port)
+        mock_unp.assert_called_once_with('fake_vif_id')
 
-    def test_get_current_vif_none(self):
-        internal_info = extra = {}
-        self.port.internal_info = internal_info
-        self.port.extra = extra
-        self.port.save()
-        with task_manager.acquire(self.context, self.node.id) as task:
-            vif = self.interface.get_current_vif(task, self.port)
-            self.assertIsNone(vif)
+    @mock.patch.object(common.VIFPortIDMixin, '_clear_vif_from_port_like_obj')
+    @mock.patch.object(neutron_common, 'unbind_neutron_port', autospec=True)
+    @mock.patch.object(common.VIFPortIDMixin, '_get_port_like_obj_by_vif_id')
+    def test_vif_detach_active_node_failure(self, mock_get, mock_unp,
+                                            mock_clear):
+        self.node.provision_state = states.ACTIVE
+        self.node.save()
+        mock_get.return_value = self.port
+        mock_unp.side_effect = exception.NetworkError
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.assertRaises(exception.NetworkError,
+                              self.interface.vif_detach, task, 'fake_vif_id')
+        mock_get.assert_called_once_with(task, 'fake_vif_id')
+        mock_clear.assert_called_once_with(self.port)
+        mock_unp.assert_called_once_with('fake_vif_id')
 
     @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
                        autospec=True)