Check task_state of instance before volume actions

Check task_state before verify resize or before detachment.

We need to make sure the task_state moved out from resize_finish before
we trigger some action like attach/detach volume.

So when we update flvor and volume at the samethime, the resize action
will not affect volume attachment/detachment.

Depends-On: https://review.opendev.org/#/c/700512/

Change-Id: I64033d5a0a8fea5c4fd93b1deb111d3d8fba0cf7
Story: #2007042
Task: #37854
Task: #37855
Task: #37869

heat/engine/clients/os/nova.py

@@ -167,6 +167,11 @@ class NovaClientPlugin(microversion_mixin.MicroversionMixin,
                 raise
         return server
 
+    def fetch_server_attr(self, server_id, attr):
+        server = self.fetch_server(server_id)
+        fetched_attr = getattr(server, attr, None)
+        return fetched_attr
+
     def refresh_server(self, server):
         """Refresh server's attributes.
 
@@ -560,6 +565,10 @@ echo -e '%s\tALL=(ALL)\tNOPASSWD: ALL' >> /etc/sudoers
             return True
         if status == 'VERIFY_RESIZE':
             return False
+        task_state_in_nova = getattr(server, 'OS-EXT-STS:task_state', None)
+        # Wait till move out from any resize steps (including resize_finish).
+        if task_state_in_nova is not None and 'resize' in task_state_in_nova:
+            return False
         else:
             msg = _("Confirm resize for server %s failed") % server_id
             raise exception.ResourceUnknownStatus(

heat/engine/resources/openstack/cinder/volume.py

@@ -507,9 +507,17 @@ class CinderVolume(vb.BaseVolume, sh.SchedulerHintsMixin):
 
     def _detach_volume_to_complete(self, prg_detach):
         if not prg_detach.called:
-            self.client_plugin('nova').detach_volume(prg_detach.srv_id,
-                                                     prg_detach.attach_id)
-            prg_detach.called = True
+            # 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
             return False
         if not prg_detach.cinder_complete:
             prg_detach.cinder_complete = self.client_plugin(
@@ -524,8 +532,16 @@ class CinderVolume(vb.BaseVolume, sh.SchedulerHintsMixin):
 
     def _attach_volume_to_complete(self, prg_attach):
         if not prg_attach.called:
-            prg_attach.called = self.client_plugin('nova').attach_volume(
-                prg_attach.srv_id, prg_attach.vol_id, prg_attach.device)
+            # 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)
             return False
         if not prg_attach.complete:
             prg_attach.complete = self.client_plugin(
@@ -747,11 +763,21 @@ class CinderVolumeAttachment(vb.BaseVolumeAttachment):
             # self.resource_id is not replaced prematurely
             volume_id = self.properties[self.VOLUME_ID]
             server_id = self.properties[self.INSTANCE_ID]
-            self.client_plugin('nova').detach_volume(server_id,
-                                                     self.resource_id)
+
             prg_detach = progress.VolumeDetachProgress(
                 server_id, volume_id, self.resource_id)
-            prg_detach.called = True
+
+            # 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
 
             if self.VOLUME_ID in prop_diff:
                 volume_id = prop_diff.get(self.VOLUME_ID)
@@ -784,8 +810,16 @@ class CinderVolumeAttachment(vb.BaseVolumeAttachment):
                                                      self.resource_id)
             return False
         if not prg_attach.called:
-            prg_attach.called = self.client_plugin('nova').attach_volume(
-                prg_attach.srv_id, prg_attach.vol_id, prg_attach.device)
+            # 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)
             return False
         if not prg_attach.complete:
             prg_attach.complete = self.client_plugin(

heat/engine/resources/openstack/heat/remote_stack.py

@@ -467,7 +467,7 @@ class RemoteStack(resource.Resource):
             after_props.get(self.CONTEXT).get(
                 'region_name') != before_props.get(self.CONTEXT).get(
                     'region_name')):
-                return True
+            return True
         return False
 
 

heat/tests/clients/test_nova_client.py

@@ -181,6 +181,26 @@ class NovaClientPluginTest(NovaClientPluginTestCase):
         observed = self.nova_plugin.get_status(server)
         self.assertEqual('ACTIVE', observed)
 
+    def test_check_verify_resize_task_state(self):
+        """Tests the check_verify_resize function with resize task_state."""
+        my_server = mock.MagicMock(status='Foo')
+        setattr(my_server, 'OS-EXT-STS:task_state', 'resize_finish')
+        self.nova_client.servers.get.side_effect = [my_server]
+
+        self.assertEqual(
+            False, self.nova_plugin.check_verify_resize('my_server'))
+
+    def test_check_verify_resize_error(self):
+        """Tests the check_verify_resize function with unknown status."""
+        my_server = mock.MagicMock(status='Foo')
+        setattr(my_server, 'OS-EXT-STS:task_state', 'active')
+        self.nova_client.servers.get.side_effect = [my_server]
+
+        self.assertRaises(
+            exception.ResourceUnknownStatus,
+            self.nova_plugin.check_verify_resize,
+            'my_server')
+
     def _absolute_limits(self):
         max_personality = mock.Mock()
         max_personality.name = 'maxPersonality'

heat/tests/openstack/cinder/test_volume.py

@@ -893,6 +893,37 @@ class CinderVolumeTest(vt_base.VolumeTestCase):
         self.fc.volumes.delete_server_volume.assert_called_with(
             'WikiDatabase', 'vol-123')
 
+    def test_cinder_volume_attachment_with_serv_resize_task_state(self):
+        self.stack_name = 'test_cvolume_attach_usrv_resize_task_state_stack'
+
+        fv1 = self._mock_create_server_volume_script(
+            vt_base.FakeVolume('attaching'))
+        fva = vt_base.FakeVolume('in-use')
+        fv2 = self._mock_create_server_volume_script(
+            vt_base.FakeVolume('attaching'), update=True)
+        self._mock_create_volume(vt_base.FakeVolume('creating'),
+                                 self.stack_name,
+                                 extra_get_mocks=[
+                                     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()]
+        self.fc.volumes.delete_server_volume.return_value = None
+
+        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.assertEqual(False,
+                         rsrc.check_update_complete((prg_detach, prg_attach)))
+        self.assertEqual(False, prg_attach.called)
+
     def test_delete_attachment_has_not_been_created(self):
         self.stack_name = 'test_delete_attachment_has_not_been_created'
         stack = utils.parse_stack(self.t, stack_name=self.stack_name)
@@ -1234,3 +1265,87 @@ class CinderVolumeTest(vt_base.VolumeTestCase):
         }
 
         self.assertEqual(expected, reality)
+
+    def test_detach_volume_to_complete_with_resize_task_state(self):
+        fv = vt_base.FakeVolume('creating')
+        self.stack_name = 'test_cvolume_detach_with_resize_task_state_stack'
+
+        self.stub_SnapshotConstraint_validate()
+        self.stub_VolumeConstraint_validate()
+        self.stub_VolumeTypeConstraint_validate()
+        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.t['resources']['volume']['properties'].update({
+            'volume_type': 'lvm',
+        })
+        stack = utils.parse_stack(self.t, stack_name=self.stack_name)
+        rsrc = self.create_volume(self.t, stack, 'volume')
+        prg_detach = mock.MagicMock(called=False, srv_id='InstanceInResize')
+        self.assertEqual(False, rsrc._detach_volume_to_complete(prg_detach))
+        self.assertEqual(False, prg_detach.called)
+
+    def test_detach_volume_to_complete_with_active_task_state(self):
+        fv = vt_base.FakeVolume('creating')
+        self.stack_name = 'test_cvolume_detach_with_active_task_state_stack'
+
+        self.stub_SnapshotConstraint_validate()
+        self.stub_VolumeConstraint_validate()
+        self.stub_VolumeTypeConstraint_validate()
+        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.t['resources']['volume']['properties'].update({
+            'volume_type': 'lvm',
+        })
+        stack = utils.parse_stack(self.t, stack_name=self.stack_name)
+        rsrc = self.create_volume(self.t, stack, 'volume')
+        prg_detach = mock.MagicMock(called=False, srv_id='InstanceInActive')
+        self.assertEqual(False, rsrc._detach_volume_to_complete(prg_detach))
+        self.assertEqual(True, prg_detach.called)
+
+    def test_attach_volume_to_complete_with_resize_task_state(self):
+        fv = vt_base.FakeVolume('creating')
+        self.stack_name = 'test_cvolume_attach_with_resize_task_state_stack'
+
+        self.stub_SnapshotConstraint_validate()
+        self.stub_VolumeConstraint_validate()
+        self.stub_VolumeTypeConstraint_validate()
+        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.t['resources']['volume']['properties'].update({
+            'volume_type': 'lvm',
+        })
+        stack = utils.parse_stack(self.t, stack_name=self.stack_name)
+        rsrc = self.create_volume(self.t, stack, 'volume')
+        prg_attach = mock.MagicMock(called=False, srv_id='InstanceInResize')
+        self.assertEqual(False, rsrc._attach_volume_to_complete(prg_attach))
+        self.assertEqual(False, prg_attach.called)
+
+    def test_attach_volume_to_complete_with_active_task_state(self):
+        fv = vt_base.FakeVolume('creating')
+        self.stack_name = 'test_cvolume_attach_with_active_task_state_stack'
+
+        self.stub_SnapshotConstraint_validate()
+        self.stub_VolumeConstraint_validate()
+        self.stub_VolumeTypeConstraint_validate()
+        self.cinder_fc.volumes.create.return_value = fv
+        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.t['resources']['volume']['properties'].update({
+            'volume_type': 'lvm',
+        })
+        stack = utils.parse_stack(self.t, stack_name=self.stack_name)
+        rsrc = self.create_volume(self.t, stack, 'volume')
+        self._mock_create_server_volume_script(
+            vt_base.FakeVolume('attaching'))
+
+        prg_attach = mock.MagicMock(called=False, srv_id='InstanceInActive')
+        self.assertEqual(False, rsrc._attach_volume_to_complete(prg_attach))
+        self.assertEqual('vol-123', prg_attach.called)

heat/tests/openstack/nova/fakes.py

@@ -113,6 +113,8 @@ class FakeSessionClient(base_client.SessionClient):
                           "accessIPv6": "",
                           "metadata": {"Server Label": "Web Head 1",
                                        "Image Version": "2.1"}},
+
+                         # 1
                          {"id": "5678",
                           "name": "sample-server2",
                           "OS-EXT-AZ:availability_zone": "nova2",
@@ -137,6 +139,7 @@ class FakeSessionClient(base_client.SessionClient):
                                                      "OS-EXT-IPS-MAC:mac_addr":
                                                      "fa:16:3e:8c:44:cc"}]},
                           "metadata": {}},
+                         # 2
                          {"id": "9101",
                           "name": "hard-reboot",
                           "OS-EXT-SRV-ATTR:instance_name":
@@ -154,6 +157,7 @@ class FakeSessionClient(base_client.SessionClient):
                                         "private": [{"version": 4,
                                                      "addr": "10.13.12.13"}]},
                           "metadata": {"Server Label": "DB 1"}},
+                         # 3
                          {"id": "9102",
                           "name": "server-with-no-ip",
                           "OS-EXT-SRV-ATTR:instance_name":
@@ -166,6 +170,7 @@ class FakeSessionClient(base_client.SessionClient):
                           "accessIPv6": "",
                           "addresses": {"empty_net": []},
                           "metadata": {"Server Label": "DB 1"}},
+                         # 4
                          {"id": "9999",
                           "name": "sample-server3",
                           "OS-EXT-SRV-ATTR:instance_name":
@@ -186,6 +191,7 @@ class FakeSessionClient(base_client.SessionClient):
                           "os-extended-volumes:volumes_attached":
                               [{"id":
                                     "66359157-dace-43ab-a7ed-a7e7cd7be59d"}]},
+                         # 5
                          {"id": 56789,
                           "name": "server-with-metadata",
                           "OS-EXT-SRV-ATTR:instance_name":
@@ -196,6 +202,74 @@ class FakeSessionClient(base_client.SessionClient):
                           "status": "ACTIVE",
                           "accessIPv4": "192.0.2.0",
                           "accessIPv6": "::babe:4317:0A83",
+                          "addresses": {"public": [{"version": 4,
+                                                    "addr": "4.5.6.7"},
+                                                   {"version": 4,
+                                                    "addr": "5.6.9.8"}],
+                                        "private": [{"version": 4,
+                                                     "addr": "10.13.12.13"}]},
+                          "metadata": {'test': '123', 'this': 'that'}},
+                         # 6
+                         {"id": "WikiDatabase",
+                          "name": "server-with-metadata",
+                          "OS-EXT-STS:task_state": None,
+                          "image": {"id": 2, "name": "sample image"},
+                          "flavor": {"id": 1, "name": "256 MB Server"},
+                          "hostId": "9e107d9d372bb6826bd81d3542a419d6",
+                          "status": "ACTIVE",
+                          "accessIPv4": "192.0.2.0",
+                          "accessIPv6": "::babe:4317:0A83",
+                          "addresses": {"public": [{"version": 4,
+                                                    "addr": "4.5.6.7"},
+                                                   {"version": 4,
+                                                    "addr": "5.6.9.8"}],
+                                        "private": [{"version": 4,
+                                                     "addr": "10.13.12.13"}]},
+                          "metadata": {'test': '123', 'this': 'that'}},
+                         # 7
+                         {"id": "InstanceInResize",
+                          "name": "server-with-metadata",
+                          "OS-EXT-STS:task_state": 'resize_finish',
+                          "image": {"id": 2, "name": "sample image"},
+                          "flavor": {"id": 1, "name": "256 MB Server"},
+                          "hostId": "9e107d9d372bb6826bd81d3542a419d6",
+                          "status": "ACTIVE",
+                          "accessIPv4": "192.0.2.0",
+                          "accessIPv6": "::babe:4317:0A83",
+                          "addresses": {"public": [{"version": 4,
+                                                    "addr": "4.5.6.7"},
+                                                   {"version": 4,
+                                                    "addr": "5.6.9.8"}],
+                                        "private": [{"version": 4,
+                                                     "addr": "10.13.12.13"}]},
+                          "metadata": {'test': '123', 'this': 'that'}},
+                         # 8
+                         {"id": "InstanceInActive",
+                          "name": "server-with-metadata",
+                          "OS-EXT-STS:task_state": 'active',
+                          "image": {"id": 2, "name": "sample image"},
+                          "flavor": {"id": 1, "name": "256 MB Server"},
+                          "hostId": "9e107d9d372bb6826bd81d3542a419d6",
+                          "status": "ACTIVE",
+                          "accessIPv4": "192.0.2.0",
+                          "accessIPv6": "::babe:4317:0A83",
+                          "addresses": {"public": [{"version": 4,
+                                                    "addr": "4.5.6.7"},
+                                                   {"version": 4,
+                                                    "addr": "5.6.9.8"}],
+                                        "private": [{"version": 4,
+                                                     "addr": "10.13.12.13"}]},
+                          "metadata": {'test': '123', 'this': 'that'}},
+                         # 9
+                         {"id": "AnotherServer",
+                          "name": "server-with-metadata",
+                          "OS-EXT-STS:task_state": 'active',
+                          "image": {"id": 2, "name": "sample image"},
+                          "flavor": {"id": 1, "name": "256 MB Server"},
+                          "hostId": "9e107d9d372bb6826bd81d3542a419d6",
+                          "status": "ACTIVE",
+                          "accessIPv4": "192.0.2.0",
+                          "accessIPv6": "::babe:4317:0A83",
                           "addresses": {"public": [{"version": 4,
                                                     "addr": "4.5.6.7"},
                                                    {"version": 4,
@@ -216,6 +290,22 @@ class FakeSessionClient(base_client.SessionClient):
         r = {'server': self.get_servers_detail()[1]['servers'][0]}
         return (200, r)
 
+    def get_servers_WikiDatabase(self, **kw):
+        r = {'server': self.get_servers_detail()[1]['servers'][6]}
+        return (200, r)
+
+    def get_servers_InstanceInResize(self, **kw):
+        r = {'server': self.get_servers_detail()[1]['servers'][7]}
+        return (200, r)
+
+    def get_servers_InstanceInActive(self, **kw):
+        r = {'server': self.get_servers_detail()[1]['servers'][8]}
+        return (200, r)
+
+    def get_servers_AnotherServer(self, **kw):
+        r = {'server': self.get_servers_detail()[1]['servers'][9]}
+        return (200, r)
+
     def get_servers_WikiServerOne1(self, **kw):
         r = {'server': self.get_servers_detail()[1]['servers'][0]}
         return (200, r)

heat_integrationtests/functional/test_create_update.py

@@ -62,15 +62,54 @@ test_template_two_resource = {
     }
 }
 
+test_template_updatae_flavor_and_volume_size = '''
+
+heat_template_version: 2013-05-23
+
+parameters:
+  volume_size:
+    default: 10
+    type: number
+  flavor:
+    type: string
+  network:
+    type: string
+  image:
+    type: string
+
+resources:
+  my_instance:
+    type: OS::Nova::Server
+    properties:
+      image:  {get_param: image}
+      flavor: {get_param: flavor}
+      admin_pass: 1
+      networks:
+        - network: {get_param: network}
+  data_volume_attachment:
+    depends_on: my_instance
+    type: 'OS::Cinder::VolumeAttachment'
+    properties:
+      instance_uuid:
+        get_resource: my_instance
+      volume_id:
+        get_resource: data_volume
+  data_volume:
+    type: 'OS::Cinder::Volume'
+    properties:
+      name: myvolume
+      size: {get_param: volume_size}
+'''
+
 
 def _change_rsrc_properties(template, rsrcs, values):
-        modified_template = copy.deepcopy(template)
-        for rsrc_name in rsrcs:
-            rsrc_prop = modified_template['resources'][
-                rsrc_name]['properties']
-            for prop, new_val in values.items():
-                rsrc_prop[prop] = new_val
-        return modified_template
+    modified_template = copy.deepcopy(template)
+    for rsrc_name in rsrcs:
+        rsrc_prop = modified_template['resources'][
+            rsrc_name]['properties']
+        for prop, new_val in values.items():
+            rsrc_prop[prop] = new_val
+    return modified_template
 
 
 class CreateStackTest(functional_base.FunctionalTestsBase):
@@ -165,6 +204,26 @@ resources:
         self.assertEqual(expected_resources,
                          self.list_resources(stack_identifier))
 
+    def test_stack_update_flavor_volume(self):
+
+        parms = {'flavor': self.conf.minimal_instance_type,
+                 'volume_size': 10,
+                 'image': self.conf.minimal_image_ref,
+                 'network': self.conf.fixed_network_name}
+
+        stack_identifier = self.stack_create(
+            template=test_template_updatae_flavor_and_volume_size,
+            parameters=parms
+        )
+
+        parms_updated = parms
+        parms_updated['volume_size'] = 20
+        parms_updated['flavor'] = self.conf.instance_type
+        self.update_stack(
+            stack_identifier,
+            template=test_template_updatae_flavor_and_volume_size,
+            parameters=parms_updated)
+
     def test_stack_in_place_update(self):
         template = _change_rsrc_properties(test_template_one_resource,
                                            ['test1'],