From 991197e20ef54de4bba5acddedf420af5853be5b Mon Sep 17 00:00:00 2001 From: Rabi Mishra Date: Fri, 26 Jun 2020 12:48:00 +0530 Subject: [PATCH] Check for nova exception.Conflict rather than task_state When detaching and attaching volumes nova would raise an exception.Conflict, if attach and detach can't happen. Let's use that to retry rather than using task_state. Change-Id: I50904e4254568cd807b9ff18eef482cadb626ce5 Task: 40169 (cherry picked from commit 01c230e793043310614d5b88096476ce0eac2c78) --- heat/engine/clients/os/nova.py | 11 ++-- .../resources/openstack/cinder/volume.py | 51 ++++--------------- heat/tests/openstack/cinder/test_volume.py | 11 ++-- 3 files changed, 25 insertions(+), 48 deletions(-) diff --git a/heat/engine/clients/os/nova.py b/heat/engine/clients/os/nova.py index 78126dd3e2..228116ff36 100644 --- a/heat/engine/clients/os/nova.py +++ b/heat/engine/clients/os/nova.py @@ -696,7 +696,9 @@ echo -e '%s\tALL=(ALL)\tNOPASSWD: ALL' >> /etc/sudoers volume_id=volume_id, device=device) except Exception as ex: - if self.is_client_exception(ex): + if self.is_conflict(ex): + return False + elif self.is_client_exception(ex): raise exception.Error(_( "Failed to attach volume %(vol)s to server %(srv)s " "- %(err)s") % {'vol': volume_id, @@ -711,12 +713,15 @@ echo -e '%s\tALL=(ALL)\tNOPASSWD: ALL' >> /etc/sudoers try: self.client().volumes.delete_server_volume(server_id, attach_id) except Exception as ex: - if not (self.is_not_found(ex) - or self.is_bad_request(ex)): + if self.is_conflict(ex): + return False + elif not (self.is_not_found(ex) + or self.is_bad_request(ex)): raise exception.Error( _("Could not detach attachment %(att)s " "from server %(srv)s.") % {'srv': server_id, 'att': attach_id}) + return True def check_detach_volume_complete(self, server_id, attach_id): """Check that nova server lost attachment. diff --git a/heat/engine/resources/openstack/cinder/volume.py b/heat/engine/resources/openstack/cinder/volume.py index b4e1a84f83..a47efb0c0a 100644 --- a/heat/engine/resources/openstack/cinder/volume.py +++ b/heat/engine/resources/openstack/cinder/volume.py @@ -508,17 +508,9 @@ class CinderVolume(vb.BaseVolume, sh.SchedulerHintsMixin): def _detach_volume_to_complete(self, prg_detach): if not prg_detach.called: - # Waiting OS-EXT-STS:task_state in server to become available for - # detach - task_state = self.client_plugin('nova').fetch_server_attr( - prg_detach.srv_id, 'OS-EXT-STS:task_state') - # Wait till out of any resize steps (including resize_finish) - if task_state is not None and 'resize' in task_state: - prg_detach.called = False - else: - self.client_plugin('nova').detach_volume(prg_detach.srv_id, - prg_detach.attach_id) - prg_detach.called = True + prg_detach.called = self.client_plugin( + 'nova').detach_volume(prg_detach.srv_id, + prg_detach.attach_id) return False if not prg_detach.cinder_complete: prg_detach.cinder_complete = self.client_plugin( @@ -533,16 +525,8 @@ class CinderVolume(vb.BaseVolume, sh.SchedulerHintsMixin): def _attach_volume_to_complete(self, prg_attach): if not prg_attach.called: - # Waiting OS-EXT-STS:task_state in server to become available for - # attach - task_state = self.client_plugin('nova').fetch_server_attr( - prg_attach.srv_id, 'OS-EXT-STS:task_state') - # Wait till out of any resize steps (including resize_finish) - if task_state is not None and 'resize' in task_state: - prg_attach.called = False - else: - prg_attach.called = self.client_plugin('nova').attach_volume( - prg_attach.srv_id, prg_attach.vol_id, prg_attach.device) + prg_attach.called = self.client_plugin('nova').attach_volume( + prg_attach.srv_id, prg_attach.vol_id, prg_attach.device) return False if not prg_attach.complete: prg_attach.complete = self.client_plugin( @@ -768,17 +752,8 @@ class CinderVolumeAttachment(vb.BaseVolumeAttachment): prg_detach = progress.VolumeDetachProgress( server_id, volume_id, self.resource_id) - # Waiting OS-EXT-STS:task_state in server to become available for - # detach - server = self.client_plugin('nova').fetch_server(server_id) - task_state = getattr(server, 'OS-EXT-STS:task_state', None) - # Wait till out of any resize steps (including resize_finish) - if task_state is not None and 'resize' in task_state: - prg_detach.called = False - else: - self.client_plugin('nova').detach_volume(server_id, - self.resource_id) - prg_detach.called = True + prg_detach.called = self.client_plugin( + 'nova').detach_volume(server_id, self.resource_id) if self.VOLUME_ID in prop_diff: volume_id = prop_diff.get(self.VOLUME_ID) @@ -811,16 +786,8 @@ class CinderVolumeAttachment(vb.BaseVolumeAttachment): self.resource_id) return False if not prg_attach.called: - # Waiting OS-EXT-STS:task_state in server to become available for - # attach - server = self.client_plugin('nova').fetch_server(prg_attach.srv_id) - task_state = getattr(server, 'OS-EXT-STS:task_state', None) - # Wait till out of any resize steps (including resize_finish) - if task_state is not None and 'resize' in task_state: - prg_attach.called = False - else: - prg_attach.called = self.client_plugin('nova').attach_volume( - prg_attach.srv_id, prg_attach.vol_id, prg_attach.device) + prg_attach.called = self.client_plugin('nova').attach_volume( + prg_attach.srv_id, prg_attach.vol_id, prg_attach.device) return False if not prg_attach.complete: prg_attach.complete = self.client_plugin( diff --git a/heat/tests/openstack/cinder/test_volume.py b/heat/tests/openstack/cinder/test_volume.py index 221d6880d8..d55159869b 100644 --- a/heat/tests/openstack/cinder/test_volume.py +++ b/heat/tests/openstack/cinder/test_volume.py @@ -17,6 +17,7 @@ import json from cinderclient import exceptions as cinder_exp import mock +from novaclient import exceptions as nova_exp from oslo_config import cfg import six @@ -907,7 +908,6 @@ class CinderVolumeTest(vt_base.VolumeTestCase): fv1, fva, vt_base.FakeVolume('available'), fv2]) self.stub_VolumeConstraint_validate() - # delete script self.fc.volumes.get_server_volume.side_effect = [ fva, fva, fakes_nova.fake_exception()] @@ -916,10 +916,12 @@ class CinderVolumeTest(vt_base.VolumeTestCase): stack = utils.parse_stack(self.t, stack_name=self.stack_name) self.create_volume(self.t, stack, 'volume') - rsrc = self.create_attachment(self.t, stack, 'attachment') prg_detach = mock.MagicMock(cinder_complete=True, nova_complete=True) prg_attach = mock.MagicMock(called=False, srv_id='InstanceInResize') + self.fc.volumes.create_server_volume.side_effect = [ + nova_exp.Conflict('409')] + self.assertEqual(False, rsrc.check_update_complete((prg_detach, prg_attach))) self.assertEqual(False, prg_attach.called) @@ -1299,6 +1301,8 @@ class CinderVolumeTest(vt_base.VolumeTestCase): self.cinder_fc.volumes.create.return_value = fv fv_ready = vt_base.FakeVolume('available', id=fv.id) self.cinder_fc.volumes.get.side_effect = [fv, fv_ready] + self.fc.volumes.delete_server_volume.side_effect = [ + nova_exp.Conflict('409')] self.t['resources']['volume']['properties'].update({ 'volume_type': 'lvm', }) @@ -1338,7 +1342,8 @@ class CinderVolumeTest(vt_base.VolumeTestCase): self.cinder_fc.volumes.create.return_value = fv fv_ready = vt_base.FakeVolume('available', id=fv.id) self.cinder_fc.volumes.get.side_effect = [fv, fv_ready] - + self.fc.volumes.create_server_volume.side_effect = [ + nova_exp.Conflict('409')] self.t['resources']['volume']['properties'].update({ 'volume_type': 'lvm', })