From a951e452e14b37d7ea25a90e5ec2bfe6bd64b105 Mon Sep 17 00:00:00 2001
From: Vasyl Saienko <vsaienko@mirantis.com>
Date: Mon, 13 Feb 2017 13:11:27 +0200
Subject: [PATCH] Walk over all objects when doing VIF detach

We apply specific rules when doing VIF attach, do not duplicate them
in vif_detach just walk over all port and portgroup objects.
This also fixes an issue when VIF was attached to port manually
(set vif_port_id key in port.extra) and it is impossible to detach
such VIF via v1/nodes/<node_ident>/vifs API.

Closes-Bug: #1663660

Change-Id: I70fe87bb4ae15a506fec705d10beaed572c0b45e
---
 ironic/drivers/modules/network/common.py         |  7 ++++---
 .../unit/drivers/modules/network/test_common.py  | 16 ++++++++++++++++
 .../notes/fix-vif-detach-fca221f1a1c0e9fa.yaml   |  5 +++++
 3 files changed, 25 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/fix-vif-detach-fca221f1a1c0e9fa.yaml

diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py
index 3cf32ba37c..57cb687e7d 100644
--- a/ironic/drivers/modules/network/common.py
+++ b/ironic/drivers/modules/network/common.py
@@ -327,9 +327,10 @@ class VIFPortIDMixin(object):
         :raises: VifNotAttached
         """
 
-        ports = [p for p in task.ports if p.portgroup_id is None]
-        portgroups = task.portgroups
-        for port_like_obj in portgroups + ports:
+        # NOTE(vsaienko) We picking object to attach on vif-attach side.
+        # Here we should only detach VIF and shouldn't duplicate/follow
+        # attach rules, just walk over all objects and detach VIF.
+        for port_like_obj in task.portgroups + task.ports:
             # FIXME(sambetts) Remove this when we no longer support a nova
             # driver that uses port.extra
             if (port_like_obj.extra.get("vif_port_id") == vif_id or
diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py
index 67d17f528f..f826d0bb87 100644
--- a/ironic/tests/unit/drivers/modules/network/test_common.py
+++ b/ironic/tests/unit/drivers/modules/network/test_common.py
@@ -381,6 +381,22 @@ class TestVifPortIDMixin(db_base.DbTestCase):
             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()
+        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(
diff --git a/releasenotes/notes/fix-vif-detach-fca221f1a1c0e9fa.yaml b/releasenotes/notes/fix-vif-detach-fca221f1a1c0e9fa.yaml
new file mode 100644
index 0000000000..dd493e21bc
--- /dev/null
+++ b/releasenotes/notes/fix-vif-detach-fca221f1a1c0e9fa.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - An issue when it was impossible to detach manually attached VIF
+    to port (port.extra) when port is in portgroup by using DELETE
+    ``v1/nodes/<node_ident>/vifs`` API.