Add extra check to avoid double delete of instances

JIRA: FUNCTEST-748

Sometimes Openstack doesn't handle deletion in a timely manner which can lead
to openstack_clean to think there are leftovers from a previous test when in
fact the instances are pending deletion. This patch tries to better handle
this kind of situation that would otherwise result in a double free and
eventually <class 'nova.exception.InstanceInvalidState'> (HTTP 500)

Change-Id: Id0d9b61d8380e9b12fc7acd46cd84260714f4baf
Signed-off-by: Ciprian Barbu <ciprian.barbu@enea.com>

2 files changed, 62 insertions, 2 deletions

diff --git a/functest/tests/unit/utils/test_openstack_clean.py b/functest/tests/unit/utils/test_openstack_clean.py
index 28eab4f8a..156695380 100644
--- a/functest/tests/unit/utils/test_openstack_clean.py
+++ b/functest/tests/unit/utils/test_openstack_clean.py
@@ -20,13 +20,41 @@ class OSCleanTesting(unittest.TestCase):
     def _get_instance(self, key):
         mock_obj = mock.Mock()
         attrs = {'id': 'id' + str(key), 'name': 'name' + str(key),
-                 'ip': 'ip' + str(key)}
+                 'ip': 'ip' + str(key), 'status': 'ACTIVE',
+                 'OS-EXT-STS:task_state': '-'}
+        mock_obj.configure_mock(**attrs)
+        return mock_obj
+
+    def _get_instance_deleted(self, key):
+        mock_obj = mock.Mock()
+        attrs = {'id': 'id' + str(key), 'name': 'name' + str(key),
+                 'ip': 'ip' + str(key), 'status': 'DELETED',
+                 'OS-EXT-STS:task_state': '-'}
+        mock_obj.configure_mock(**attrs)
+        return mock_obj
+
+    def _get_instance_deleting(self, key):
+        mock_obj = mock.Mock()
+        attrs = {'id': 'id' + str(key), 'name': 'name' + str(key),
+                 'ip': 'ip' + str(key), 'status': 'BUILD',
+                 'OS-EXT-STS:task_state': 'deleting'}
+        mock_obj.configure_mock(**attrs)
+        return mock_obj
+
+    def _get_instance_other(self, key):
+        mock_obj = mock.Mock()
+        attrs = {'id': 'id' + str(key), 'name': 'name' + str(key),
+                 'ip': 'ip' + str(key), 'status': 'BUILD',
+                 'OS-EXT-STS:task_state': 'networking'}
         mock_obj.configure_mock(**attrs)
         return mock_obj
 
     def setUp(self):
         self.client = mock.Mock()
         self.test_list = [self._get_instance(1), self._get_instance(2)]
+        self.deleted_list = [self._get_instance_deleted(5),
+                             self._get_instance_deleting(6)]
+        self.other_list = [self._get_instance_other(7)]
         self.update_list = {'id1': 'name1', 'id2': 'name2'}
         self.remove_list = {'id3': 'name3', 'id4': 'name4'}
         self.test_dict_list = [{'id': 'id1', 'name': 'name1', 'ip': 'ip1',
@@ -77,6 +105,33 @@ class OSCleanTesting(unittest.TestCase):
                                                                     " '\s*\S+'"
                                                                     " ..."))
 
+    @mock.patch('functest.utils.openstack_clean.logger.debug')
+    def test_remove_instances_pending_delete_success(self, mock_logger_debug):
+        with mock.patch('functest.utils.openstack_clean.os_utils'
+                        '.get_instances', return_value=self.deleted_list), \
+                mock.patch('functest.utils.openstack_clean.os_utils'
+                           '.delete_instance', return_value=True):
+            openstack_clean.remove_instances(self.client, self.remove_list)
+            mock_logger_debug.assert_any_call("Removing Nova instances...")
+            mock_logger_debug.test_utils.RegexMatch("Removing"
+                                                    " instance"
+                                                    " '\s*\S+'"
+                                                    " ...").assert_not_called()
+
+    @mock.patch('functest.utils.openstack_clean.logger.debug')
+    def test_remove_instances_other_delete_success(self, mock_logger_debug):
+        with mock.patch('functest.utils.openstack_clean.os_utils'
+                        '.get_instances', return_value=self.other_list), \
+                mock.patch('functest.utils.openstack_clean.os_utils'
+                           '.delete_instance', return_value=True):
+            openstack_clean.remove_instances(self.client, self.remove_list)
+            mock_logger_debug.assert_any_call("Removing Nova instances...")
+            mock_logger_debug.assert_any_call("  > Request sent.")
+            mock_logger_debug.assert_any_call(test_utils.RegexMatch("Removing"
+                                                                    " instance"
+                                                                    " '\s*\S+'"
+                                                                    " ..."))
+
     @mock.patch('functest.utils.openstack_clean.logger.error')
     @mock.patch('functest.utils.openstack_clean.logger.debug')
     def test_remove_instances_delete_failed(self, mock_logger_debug,
diff --git a/functest/utils/openstack_clean.py b/functest/utils/openstack_clean.py
index 15a8f33d5..ce61fcacf 100755
--- a/functest/utils/openstack_clean.py
+++ b/functest/utils/openstack_clean.py
@@ -49,9 +49,14 @@ def remove_instances(nova_client, default_instances):
     for instance in instances:
         instance_name = getattr(instance, 'name')
         instance_id = getattr(instance, 'id')
+        instance_status = getattr(instance, 'status')
+        instance_state = getattr(instance, 'OS-EXT-STS:task_state')
+
         logger.debug("'%s', ID=%s " % (instance_name, instance_id))
         if (instance_id not in default_instances and
-                instance_name not in default_instances.values()):
+                instance_name not in default_instances.values() and
+                instance_status != 'DELETED' and
+                (instance_status != 'BUILD' or instance_state != 'deleting')):
             logger.debug("Removing instance '%s' ..." % instance_id)
             if os_utils.delete_instance(nova_client, instance_id):
                 logger.debug("  > Request sent.")