From 9bff6e98c84344bf82d84ebf9f9db0c5def4a685 Mon Sep 17 00:00:00 2001 From: spisarski Date: Mon, 5 Feb 2018 11:52:55 -0700 Subject: Fixed timeout logic when attaching/detaching volumes. The timeout logic in nova_utils.attach_volume() and detach_volume() was not correct which may have been the root cause behind the issue FUNCTEST-927. Timeout in both attach and detach is no longer optional. Also added a test to attach and detach without timeout as that path was never tested. Updated associated test documentation as well. JIRA: SNAPS-263 JIRA: FUNCTEST-927 Change-Id: Iea3aeab59c378917fbd175d673113e8d30e2e4b9 Signed-off-by: spisarski --- docs/how-to-use/APITests.rst | 11 ++- snaps/openstack/create_instance.py | 10 ++- snaps/openstack/utils/nova_utils.py | 67 ++++++++------- snaps/openstack/utils/tests/nova_utils_tests.py | 110 +++++++++++++++++++++--- 4 files changed, 149 insertions(+), 49 deletions(-) diff --git a/docs/how-to-use/APITests.rst b/docs/how-to-use/APITests.rst index 50cd437..6239a08 100644 --- a/docs/how-to-use/APITests.rst +++ b/docs/how-to-use/APITests.rst @@ -435,7 +435,16 @@ nova_utils_tests.py - NovaUtilsInstanceVolumeTests | Test Name | Nova API | Description | +=======================================+===============+===========================================================+ | test_add_remove_volume | 2 | Ensures that a VM instance can properly attach and detach | -| | | a volume using the nova interface | +| | | a volume using the nova interface while waiting for | +| | | the update to fully occur | ++---------------------------------------+---------------+-----------------------------------------------------------+ +| test_attach_volume_nowait | 2 | Ensures that the call to nova_utils.attach_volume raises | +| | | an exception when the timeout is too short to return an | +| | | properly updated VmInst object | ++---------------------------------------+---------------+-----------------------------------------------------------+ +| test_detach_volume_nowait | 2 | Ensures that the call to nova_utils.detach_volume raises | +| | | an exception when the timeout is too short to return an | +| | | properly updated VmInst object | +---------------------------------------+---------------+-----------------------------------------------------------+ create_flavor_tests.py - CreateFlavorTests diff --git a/snaps/openstack/create_instance.py b/snaps/openstack/create_instance.py index cdc778f..73cdf8a 100644 --- a/snaps/openstack/create_instance.py +++ b/snaps/openstack/create_instance.py @@ -30,6 +30,7 @@ __author__ = 'spisarski' logger = logging.getLogger('create_instance') POLL_INTERVAL = 3 +VOL_DETACH_TIMEOUT = 120 STATUS_ACTIVE = 'ACTIVE' STATUS_DELETED = 'DELETED' @@ -164,15 +165,15 @@ class OpenStackVmInstance(OpenStackComputeObject): cinder, volume_name=volume_name) if volume and self.vm_active(block=True): - timeout = 30 vm = nova_utils.attach_volume( - self._nova, self.__neutron, self.__vm, volume, timeout) + self._nova, self.__neutron, self.__vm, volume, + VOL_DETACH_TIMEOUT) if vm: self.__vm = vm else: logger.warn('Volume [%s] not attached within timeout ' - 'of [%s]', volume.name, timeout) + 'of [%s]', volume.name, VOL_DETACH_TIMEOUT) else: logger.warn('Unable to attach volume named [%s]', volume_name) @@ -271,7 +272,8 @@ class OpenStackVmInstance(OpenStackComputeObject): cinder, volume_rec['id']) if volume: vm = nova_utils.detach_volume( - self._nova, self.__neutron, self.__vm, volume, 30) + self._nova, self.__neutron, self.__vm, volume, + VOL_DETACH_TIMEOUT) if vm: self.__vm = vm else: diff --git a/snaps/openstack/utils/nova_utils.py b/snaps/openstack/utils/nova_utils.py index a8e051e..ba902f9 100644 --- a/snaps/openstack/utils/nova_utils.py +++ b/snaps/openstack/utils/nova_utils.py @@ -35,6 +35,8 @@ __author__ = 'spisarski' logger = logging.getLogger('nova_utils') +POLL_INTERVAL = 3 + """ Utilities for basic OpenStack Nova API calls """ @@ -711,60 +713,65 @@ def update_quotas(nova, project_id, compute_quotas): return nova.quotas.update(project_id, **update_values) -def attach_volume(nova, neutron, server, volume, timeout=None): +def attach_volume(nova, neutron, server, volume, timeout=120): """ - Attaches a volume to a server + Attaches a volume to a server. When the timeout parameter is used, a VmInst + object with the proper volume updates is returned unless it has not been + updated in the allotted amount of time then an Exception will be raised. :param nova: the nova client :param neutron: the neutron client :param server: the VMInst domain object :param volume: the Volume domain object :param timeout: denotes the amount of time to block to determine if the - has been properly attached. When None, do not wait. - :return: the value from the nova call + has been properly attached. + :return: updated VmInst object """ nova.volumes.create_server_volume(server.id, volume.id) - if timeout: - start_time = time.time() - while time.time() < start_time + timeout: - vm = get_server_object_by_id(nova, neutron, server.id) - for vol_dict in vm.volume_ids: - if volume.id == vol_dict['id']: - return vm + start_time = time.time() + while time.time() < start_time + timeout: + vm = get_server_object_by_id(nova, neutron, server.id) + for vol_dict in vm.volume_ids: + if volume.id == vol_dict['id']: + return vm + time.sleep(POLL_INTERVAL) - return None - else: - return get_server_object_by_id(nova, neutron, server.id) + raise NovaException( + 'Attach failed on volume - {} and server - {}'.format( + volume.id, server.id)) -def detach_volume(nova, neutron, server, volume, timeout=None): +def detach_volume(nova, neutron, server, volume, timeout=120): """ - Attaches a volume to a server + Detaches a volume to a server. When the timeout parameter is used, a VmInst + object with the proper volume updates is returned unless it has not been + updated in the allotted amount of time then an Exception will be raised. :param nova: the nova client :param neutron: the neutron client :param server: the VMInst domain object :param volume: the Volume domain object :param timeout: denotes the amount of time to block to determine if the - has been properly detached. When None, do not wait. - :return: the value from the nova call + has been properly detached. + :return: updated VmInst object """ nova.volumes.delete_server_volume(server.id, volume.id) - if timeout: - start_time = time.time() - while time.time() < start_time + timeout: - vm = get_server_object_by_id(nova, neutron, server.id) - found = False + start_time = time.time() + while time.time() < start_time + timeout: + vm = get_server_object_by_id(nova, neutron, server.id) + if len(vm.volume_ids) == 0: + return vm + else: + ids = list() for vol_dict in vm.volume_ids: - if volume.id == vol_dict['id']: - found = True - - if not found: + ids.append(vol_dict['id']) + if volume.id not in ids: return vm + time.sleep(POLL_INTERVAL) - return None - else: - return get_server_object_by_id(nova, neutron, server.id) + raise NovaException( + 'Detach failed on volume - {} server - {}'.format( + volume.id, server.id)) class RebootType(enum.Enum): diff --git a/snaps/openstack/utils/tests/nova_utils_tests.py b/snaps/openstack/utils/tests/nova_utils_tests.py index 77dc5dd..9383088 100644 --- a/snaps/openstack/utils/tests/nova_utils_tests.py +++ b/snaps/openstack/utils/tests/nova_utils_tests.py @@ -33,6 +33,7 @@ from snaps.openstack.tests import openstack_tests from snaps.openstack.tests.os_source_file_test import OSComponentTestCase from snaps.openstack.utils import ( nova_utils, neutron_utils, glance_utils, cinder_utils) +from snaps.openstack.utils.nova_utils import NovaException __author__ = 'spisarski' @@ -440,7 +441,8 @@ class NovaUtilsInstanceVolumeTests(OSComponentTestCase): def test_add_remove_volume(self): """ - Tests the nova_utils.create_server() method + Tests the nova_utils.attach_volume() and detach_volume functions with + a timeout value :return: """ @@ -449,23 +451,27 @@ class NovaUtilsInstanceVolumeTests(OSComponentTestCase): # Attach volume to VM neutron = neutron_utils.neutron_client(self.os_creds) - nova_utils.attach_volume( + self.assertIsNotNone(nova_utils.attach_volume( self.nova, neutron, self.instance_creator.get_vm_inst(), - self.volume_creator.get_volume(), 120) + self.volume_creator.get_volume())) - vol_attach = cinder_utils.get_volume_by_id( - self.cinder, self.volume_creator.get_volume().id) - vm_attach = nova_utils.get_server_object_by_id( - self.nova, neutron, self.instance_creator.get_vm_inst().id) + vol_attach = None + attached = False + start_time = time.time() + while time.time() < start_time + 30: + vol_attach = cinder_utils.get_volume_by_id( + self.cinder, self.volume_creator.get_volume().id) - # Detach volume to VM - nova_utils.detach_volume( - self.nova, neutron, self.instance_creator.get_vm_inst(), - self.volume_creator.get_volume(), 120) + if len(vol_attach.attachments) > 0: + attached = True + break - vol_detach = cinder_utils.get_volume_by_id( - self.cinder, self.volume_creator.get_volume().id) - vm_detach = nova_utils.get_server_object_by_id( + time.sleep(3) + + self.assertTrue(attached) + self.assertIsNotNone(vol_attach) + + vm_attach = nova_utils.get_server_object_by_id( self.nova, neutron, self.instance_creator.get_vm_inst().id) # Validate Attachment @@ -475,8 +481,84 @@ class NovaUtilsInstanceVolumeTests(OSComponentTestCase): self.assertEqual(vm_attach.volume_ids[0]['id'], vol_attach.attachments[0]['volume_id']) + # Detach volume to VM + self.assertIsNotNone(nova_utils.detach_volume( + self.nova, neutron, self.instance_creator.get_vm_inst(), + self.volume_creator.get_volume())) + + vol_detach = cinder_utils.get_volume_by_id( + self.cinder, self.volume_creator.get_volume().id) + vm_detach = nova_utils.get_server_object_by_id( + self.nova, neutron, self.instance_creator.get_vm_inst().id) + # Validate Detachment self.assertIsNotNone(vol_detach) self.assertEqual(self.volume_creator.get_volume().id, vol_detach.id) + + if len(vol_detach.attachments) > 0: + vol_detach = cinder_utils.get_volume_by_id( + self.cinder, self.volume_creator.get_volume().id) + self.assertEqual(0, len(vol_detach.attachments)) self.assertEqual(0, len(vm_detach.volume_ids)) + + def test_attach_volume_nowait(self): + """ + Tests the nova_utils.attach_volume() with a timeout value that is too + small to have the volume attachment data to be included on the VmInst + object that was supposed to be returned + """ + + self.assertIsNotNone(self.volume_creator.get_volume()) + self.assertEqual(0, len(self.volume_creator.get_volume().attachments)) + + # Attach volume to VM + neutron = neutron_utils.neutron_client(self.os_creds) + with self.assertRaises(NovaException): + nova_utils.attach_volume( + self.nova, neutron, self.instance_creator.get_vm_inst(), + self.volume_creator.get_volume(), 0) + + def test_detach_volume_nowait(self): + """ + Tests the nova_utils.detach_volume() with a timeout value that is too + small to have the volume attachment data to be included on the VmInst + object that was supposed to be returned + """ + + self.assertIsNotNone(self.volume_creator.get_volume()) + self.assertEqual(0, len(self.volume_creator.get_volume().attachments)) + + # Attach volume to VM + neutron = neutron_utils.neutron_client(self.os_creds) + nova_utils.attach_volume( + self.nova, neutron, self.instance_creator.get_vm_inst(), + self.volume_creator.get_volume()) + + # Check VmInst for attachment + latest_vm = nova_utils.get_server_object_by_id( + self.nova, neutron, self.instance_creator.get_vm_inst().id) + self.assertEqual(1, len(latest_vm.volume_ids)) + + # Check Volume for attachment + vol_attach = None + attached = False + start_time = time.time() + while time.time() < start_time + 30: + vol_attach = cinder_utils.get_volume_by_id( + self.cinder, self.volume_creator.get_volume().id) + + if len(vol_attach.attachments) > 0: + attached = True + break + + time.sleep(3) + + self.assertTrue(attached) + self.assertIsNotNone(vol_attach) + + # Detach volume + with self.assertRaises(NovaException): + nova_utils.detach_volume( + self.nova, neutron, self.instance_creator.get_vm_inst(), + self.volume_creator.get_volume(), 0) -- cgit 1.2.3-korg