Add the os-extend_volume_completion volume action

Split off the finalization part of the volume manager's
extend_volume method and make it externally callable as the new
os-extend_volume_completion admin volume action.

This is the first part of a feature that will allow volume drivers
to rely on feedback from Nova when extending attached volumes,
allowing e.g. NFS-based drivers to support online extend.

See the linked blueprint for details.

Implements: bp extend-volume-completion-action
Change-Id: I4aaa5da1ad67a948102c498483de318bd245d86b

api-ref/source/v3/parameters.yaml

@@ -1400,6 +1400,12 @@ event_id:
   in: body
   required: true
   type: string
+extend_completion_error:
+  description: |
+    Used to indicate that the extend operation has failed outside of cinder.
+  in: body
+  required: false
+  type: boolean
 extra_info:
   description: |
     More information about the resource.
@@ -2358,6 +2364,12 @@ os-extend:
   in: body
   required: true
   type: object
+os-extend_volume_completion:
+  description: |
+    The ``os-extend_volume_completion`` action.
+  in: body
+  required: true
+  type: object
 os-force_delete:
   description: |
     The ``os-force_delete`` action.

api-ref/source/v3/samples/versions/version-show-response.json

@@ -21,8 +21,8 @@
             ],
             "min_version": "3.0",
             "status": "CURRENT",
-            "updated": "2022-08-31T00:00:00Z",
+            "updated": "2023-08-31T00:00:00Z",
-            "version": "3.70"
+            "version": "3.71"
         }
     ]
 }

api-ref/source/v3/samples/versions/versions-response.json

@@ -22,7 +22,7 @@
             "min_version": "3.0",
             "status": "CURRENT",
             "updated": "2022-08-31T00:00:00Z",
-            "version": "3.70"
+            "version": "3.71"
         }
     ]
 }

api-ref/source/v3/samples/volume-os-extend_volume_completion-request.json

@@ -0,0 +1,5 @@
+{
+    "os-extend_volume_completion": {
+        "error": false
+    }
+}

api-ref/source/v3/volumes-v3-volumes-actions.inc

@@ -77,6 +77,55 @@ Request Example
    :language: javascript
 
 
+Complete extending a volume
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. rest_method::  POST /v3/{project_id}/volumes/{volume_id}/action
+
+Specify the ``os-extend_volume_completion`` action in the request body.
+
+Complete extending an attached volume that has been left in status
+``extending`` after notifying the compute agent.
+Depending on the value of the ``error`` parameter, the extend operation
+will be either rolled back or finalized.
+
+**Preconditions**
+
+* The volume must have the status ``extending``.
+* The volume's admin metadata must contain a set of keys indicating that
+  Cinder was waiting for external feedback on the success of the operation.
+
+**Asynchronous Postconditions**
+
+If the ``error`` parameter is ``false`` or missing, and the extend operation
+was successfully finalized, the volume status will be ``in-use``.
+Otherwise, the volume status will be ``error_extending``.
+
+Response codes
+--------------
+
+.. rest_status_code:: success ../status.yaml
+
+   - 202
+
+
+Request
+-------
+
+.. rest_parameters:: parameters.yaml
+
+   - volume_id: volume_id_path
+   - project_id: project_id_path
+   - os-extend_volume_completion: os-extend_volume_completion
+   - error: extend_completion_error
+
+Request Example
+---------------
+
+.. literalinclude:: ./samples/volume-os-extend_volume_completion-request.json
+   :language: javascript
+
+
 Reset a volume's statuses
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 

cinder/api/contrib/admin_actions.py

@@ -283,6 +283,19 @@ class VolumeAdminController(AdminController):
                                                         new_volume, error)
         return {'save_volume_id': ret}
 
+    @wsgi.response(HTTPStatus.ACCEPTED)
+    @wsgi.action('os-extend_volume_completion')
+    @validation.schema(admin_actions.extend_volume_completion)
+    def _extend_volume_completion(self, req, id, body):
+        """Complete an in-progress extend operation."""
+        context = req.environ['cinder.context']
+        # Not found exception will be handled at the wsgi level
+        volume = self._get(context, id)
+        self.authorize(context, 'extend_volume_completion', target_obj=volume)
+        params = body['os-extend_volume_completion']
+        error = params.get('error', False)
+        self.volume_api.extend_volume_completion(context, volume, error)
+
 
 class SnapshotAdminController(AdminController):
     """AdminController for Snapshots."""

cinder/api/microversions.py

@@ -179,6 +179,8 @@ SHARED_TARGETS_TRISTATE = '3.69'
 
 TRANSFER_ENCRYPTED_VOLUME = '3.70'
 
+EXTEND_VOLUME_COMPLETION = '3.71'
+
 
 def get_mv_header(version):
     """Gets a formatted HTTP microversion header.

cinder/api/openstack/api_version_request.py

@@ -156,14 +156,15 @@ REST_API_VERSION_HISTORY = """
     * 3.68 - Support re-image volume
     * 3.69 - Allow null value for shared_targets
     * 3.70 - Support encrypted volume transfers
+    * 3.71 - Support 'os-extend_volume_completion' volume action
 """
 
 # The minimum and maximum versions of the API supported
 # The default api version request is defined to be the
 # minimum version of the API supported.
 _MIN_API_VERSION = "3.0"
-_MAX_API_VERSION = "3.70"
+_MAX_API_VERSION = "3.71"
-UPDATED = "2022-08-31T00:00:00Z"
+UPDATED = "2023-08-31T00:00:00Z"
 
 
 # NOTE(cyeoh): min and max versions declared as functions so we can

cinder/api/openstack/rest_api_version_history.rst

@@ -535,3 +535,9 @@ following meanings:
 Add the ability to transfer encrypted volumes and their snapshots. The feature
 removes a prior restriction on transferring encrypted volumes. Otherwise, the
 API request and response schema are unchanged.
+
+3.71
+----
+Add the ``os-extend_volume_completion`` volume action, which Nova can use
+to notify Cinder of success and error when handling a ``volume-extended``
+external server event.

cinder/api/schemas/admin_actions.py

@@ -119,6 +119,22 @@ migrate_volume_completion = {
 }
 
 
+extend_volume_completion = {
+    'type': 'object',
+    'properties': {
+        'os-extend_volume_completion': {
+            'type': 'object',
+            'properties': {
+                'error': {'type': ['string', 'null', 'boolean']},
+            },
+            'additionalProperties': False,
+        },
+    },
+    'required': ['os-extend_volume_completion'],
+    'additionalProperties': False,
+}
+
+
 reset_status_backup = {
     'type': 'object',
     'properties': {

cinder/policies/volume_actions.py

@@ -20,6 +20,8 @@ from cinder.policies import base
 
 EXTEND_POLICY = "volume:extend"
 EXTEND_ATTACHED_POLICY = "volume:extend_attached_volume"
+EXTEND_COMPLETE_POLICY = \
+    "volume_extension:volume_admin_actions:extend_volume_completion"
 REVERT_POLICY = "volume:revert_to_snapshot"
 RESET_STATUS = "volume_extension:volume_admin_actions:reset_status"
 RETYPE_POLICY = "volume:retype"
@@ -124,6 +126,16 @@ volume_action_policies = [
         ],
         deprecated_rule=deprecated_extend_attached_policy,
     ),
+    policy.DocumentedRuleDefault(
+        name=EXTEND_COMPLETE_POLICY,
+        check_str=base.RULE_ADMIN_API,
+        description="Complete a volume extend operation.",
+        operations=[{
+            'method': 'POST',
+            'path':
+                '/volumes/{volume_id}/action (os-extend_volume_completion)'}
+        ],
+    ),
     policy.DocumentedRuleDefault(
         name=REVERT_POLICY,
         check_str=base.SYSTEM_ADMIN_OR_PROJECT_MEMBER,

cinder/tests/unit/api/contrib/test_admin_actions.py

@@ -1031,6 +1031,88 @@ class AdminActionsTest(BaseAdminTest):
         res = req.get_response(app())
         self.assertEqual(HTTPStatus.METHOD_NOT_ALLOWED, res.status_int)
 
+    def _extend_volume_comp_exec(self, ctx, volume, error, expected_status):
+        req = webob.Request.blank('/v3/%s/volumes/%s/action' % (
+            fake.PROJECT_ID, volume['id']))
+        req.method = 'POST'
+        req.headers['content-type'] = 'application/json'
+        body = {'os-extend_volume_completion': {'error': error}}
+        req.body = jsonutils.dump_as_bytes(body)
+        req.environ['cinder.context'] = ctx
+        resp = req.get_response(app())
+        # verify status
+        self.assertEqual(expected_status, resp.status_int)
+
+    def test_extend_volume_comp_accepted_success(self):
+        volume = self._create_volume(
+            self.ctx,
+            {'size': 1,
+             'status': 'extending',
+             'admin_metadata': {
+                 'extend_new_size': '2',
+                 'extend_reservations':
+                     '["563e9e70-5f46-4265-9a92-f7bca28d896c"]'
+             }})
+
+        expected_status = HTTPStatus.ACCEPTED
+        self._extend_volume_comp_exec(self.ctx, volume, False,
+                                      expected_status)
+
+    def test_extend_volume_comp_accepted_failure(self):
+        volume = self._create_volume(
+            self.ctx,
+            {'size': 1,
+             'status': 'extending',
+             'admin_metadata': {
+                 'extend_new_size': '2',
+                 'extend_reservations':
+                     '["563e9e70-5f46-4265-9a92-f7bca28d896c"]'
+             }})
+
+        expected_status = HTTPStatus.ACCEPTED
+        self._extend_volume_comp_exec(self.ctx, volume, True,
+                                      expected_status)
+
+    def test_extend_volume_comp_wrong_status(self):
+        volume = self._create_volume(
+            self.ctx,
+            {'size': 1,
+             'status': 'in-use',
+             'admin_metadata': {
+                 'extend_new_size': '2',
+                 'extend_reservations':
+                     '["563e9e70-5f46-4265-9a92-f7bca28d896c"]'
+             }})
+
+        expected_status = HTTPStatus.BAD_REQUEST
+        self._extend_volume_comp_exec(self.ctx, volume, False,
+                                      expected_status)
+
+    def test_extend_volume_comp_missing_metadata(self):
+        volume = self._create_volume(
+            self.ctx,
+            {'size': 1,
+             'status': 'extending'})
+
+        expected_status = HTTPStatus.BAD_REQUEST
+        self._extend_volume_comp_exec(self.ctx, volume, False,
+                                      expected_status)
+
+    def test_extend_volume_comp_wrong_size(self):
+        volume = self._create_volume(
+            self.ctx,
+            {'size': 2,
+             'status': 'extending',
+             'admin_metadata': {
+                 'extend_new_size': '1',
+                 'extend_reservations':
+                     '["563e9e70-5f46-4265-9a92-f7bca28d896c"]'
+             }})
+
+        expected_status = HTTPStatus.BAD_REQUEST
+        self._extend_volume_comp_exec(self.ctx, volume, False,
+                                      expected_status)
+
 
 class AdminActionsAttachDetachTest(BaseAdminTest):
     def setUp(self):

cinder/tests/unit/volume/test_volume.py

@@ -29,6 +29,7 @@ import eventlet
 import os_brick.initiator.connectors.iscsi
 from oslo_concurrency import processutils
 from oslo_config import cfg
+from oslo_utils.fixture import uuidsentinel as uuids
 from oslo_utils import imageutils
 from taskflow.engines.action_engine import engine
 
@@ -2804,7 +2805,7 @@ class VolumeTestCase(base.BaseVolumeTestCase):
             with mock.patch.object(
                     self.volume.message_api, 'create') as mock_create:
                 volume['status'] = 'extending'
-                self.volume.extend_volume(self.context, volume, '4',
+                self.volume.extend_volume(self.context, volume, 4,
                                           fake_reservations)
                 volume.refresh()
                 self.assertEqual(2, volume.size)
@@ -2832,7 +2833,7 @@ class VolumeTestCase(base.BaseVolumeTestCase):
             with mock.patch.object(QUOTAS, 'commit') as quotas_commit:
                 extend_volume.return_value = fake_extend
                 volume.status = 'extending'
-                self.volume.extend_volume(self.context, volume, '4',
+                self.volume.extend_volume(self.context, volume, 4,
                                           fake_reservations)
                 volume.refresh()
                 self.assertEqual(4, volume.size)
@@ -2946,6 +2947,77 @@ class VolumeTestCase(base.BaseVolumeTestCase):
 
         self.assertEqual(100, volumes_reserved)
 
+    @mock.patch('cinder.compute.nova.API.extend_volume')
+    @mock.patch('cinder.volume.manager.VolumeManager.'
+                'extend_volume_completion')
+    def test_extend_volume_no_wait_for_nova_available(self,
+                                                      extend_completion,
+                                                      nova_extend):
+        volume = tests_utils.create_volume(self.context, size=2,
+                                           status='extending')
+
+        with mock.patch.object(self.volume.driver, 'extend_volume'):
+            self.volume.extend_volume(self.context, volume, 4,
+                                      [uuids.reservation])
+
+            extend_completion.assert_called_once_with(self.context,
+                                                      volume,
+                                                      4,
+                                                      [uuids.reservation],
+                                                      error=False)
+            nova_extend.assert_not_called()
+            self.assertNotIn('extend_new_size', volume.admin_metadata)
+            self.assertNotIn('extend_reservations', volume.admin_metadata)
+
+    @mock.patch('cinder.compute.nova.API.extend_volume')
+    @mock.patch('cinder.volume.manager.VolumeManager.'
+                'extend_volume_completion')
+    def test_extend_volume_no_wait_for_nova_attached(self,
+                                                     extend_completion,
+                                                     nova_extend):
+        volume = tests_utils.create_volume(self.context, size=2)
+        tests_utils.attach_volume(self.context, volume.id, uuids.instance,
+                                  'fake-host', '/dev/vda')
+        db.volume_update(self.context, volume.id, {'status': 'extending'})
+        volume.refresh()
+
+        with mock.patch.object(self.volume.driver, 'extend_volume'):
+            self.volume.extend_volume(self.context, volume, 4,
+                                      [uuids.reservation])
+
+            extend_completion.assert_called_once_with(self.context,
+                                                      volume,
+                                                      4,
+                                                      [uuids.reservation],
+                                                      error=False)
+            nova_extend.assert_called_once_with(self.context,
+                                                [uuids.instance],
+                                                volume.id)
+            self.assertNotIn('extend_new_size', volume.admin_metadata)
+            self.assertNotIn('extend_reservations', volume.admin_metadata)
+
+    @mock.patch('cinder.compute.nova.API.extend_volume', return_value=False)
+    @mock.patch('cinder.volume.manager.VolumeManager.'
+                'extend_volume_completion')
+    def test_extend_volume_no_wait_for_nova_fail_to_send(self,
+                                                         extend_completion,
+                                                         nova_extend):
+        volume = tests_utils.create_volume(self.context, size=2)
+        tests_utils.attach_volume(self.context, volume.id, uuids.instance,
+                                  'fake-host', '/dev/vda')
+        db.volume_update(self.context, volume.id, {'status': 'extending'})
+        volume.refresh()
+
+        with mock.patch.object(self.volume.driver, 'extend_volume'):
+            self.volume.extend_volume(self.context, volume, 4,
+                                      [uuids.reservation])
+
+            extend_completion.assert_called_once_with(self.context,
+                                                      volume,
+                                                      4,
+                                                      [uuids.reservation],
+                                                      error=False)
+
     def test_create_volume_from_sourcevol(self):
         """Test volume can be created from a source volume."""
         def fake_create_cloned_volume(volume, src_vref):

cinder/volume/api.py

@@ -26,6 +26,7 @@ from typing import (Any, DefaultDict, Iterable, Optional, Union)
 from castellan import key_manager
 from oslo_config import cfg
 from oslo_log import log as logging
+from oslo_serialization import jsonutils
 from oslo_utils import excutils
 from oslo_utils import strutils
 from oslo_utils import timeutils
@@ -1675,6 +1676,48 @@ class API(base.Base):
                           target_obj=volume)
         self._extend(context, volume, new_size, attached=True)
 
+    def extend_volume_completion(self,
+                                 context: context.RequestContext,
+                                 volume: objects.Volume,
+                                 error: bool):
+        context.authorize(vol_action_policy.EXTEND_COMPLETE_POLICY,
+                          target_obj=volume)
+
+        if volume.status != 'extending':
+            msg = _('Volume is not being extended.')
+            raise exception.InvalidVolume(reason=msg)
+
+        try:
+            with volume.obj_as_admin():
+                new_size = int(volume.admin_metadata['extend_new_size'])
+                reservations = jsonutils.loads(
+                    volume.admin_metadata['extend_reservations'])
+        except (KeyError, ValueError, jsonutils.json.decoder.JSONDecodeError):
+            msg = _('Required volume admin metadata is malformed or missing.')
+            raise exception.InvalidVolume(reason=msg)
+
+        if new_size <= volume.size:
+            msg = _('The target volume size provided in volume admin metadata '
+                    '%(size)s is smaller or equal to the current volume size.'
+                    % volume.admin_metadata["extend_new_size"])
+            raise exception.InvalidVolume(reason=msg)
+
+        if type(reservations) is not list:
+            msg = _('The stored quota reservations for extending the volume '
+                    'must be in a list format.')
+            raise exception.InvalidVolume(reason=msg)
+
+        with volume.obj_as_admin():
+            del volume.admin_metadata['extend_new_size']
+            del volume.admin_metadata['extend_reservations']
+            volume.save()
+
+        self.volume_rpcapi.extend_volume_completion(context, volume, new_size,
+                                                    reservations, error)
+
+        LOG.info("Extend volume completion issued successfully.",
+                 resource=volume)
+
     def migrate_volume(self,
                        context: context.RequestContext,
                        volume: objects.Volume,

cinder/volume/manager.py

@@ -2910,8 +2910,6 @@ class VolumeManager(manager.CleanableManager,
                 volume.status = 'error_extending'
                 volume.save()
 
-        project_id = volume.project_id
-        size_increase = (int(new_size)) - volume.size
         self._notify_about_volume_usage(context, volume, "resize.start")
         try:
             self.driver.extend_volume(volume, new_size)
@@ -2921,6 +2919,38 @@ class VolumeManager(manager.CleanableManager,
         except Exception:
             LOG.exception("Extend volume failed.",
                           resource=volume)
+            self.extend_volume_completion(context, volume, new_size,
+                                          reservations, error=True)
+            return
+
+        self.extend_volume_completion(context, volume, new_size,
+                                      reservations, error=False)
+
+        attachments = volume.volume_attachment or []
+        # If instance_uuid field is None on attachment, it means that the
+        # volume is used by Glance Cinder store
+        instance_uuids = [attachment.instance_uuid
+                          for attachment in attachments
+                          if attachment.instance_uuid]
+
+        # If the volume is not attached to any instances, we should not send
+        # external events to Nova
+        if instance_uuids:
+            nova_api = compute.API()
+            nova_api.extend_volume(context, instance_uuids, volume.id)
+
+    def extend_volume_completion(self,
+                                 context: context.RequestContext,
+                                 volume: objects.Volume,
+                                 new_size: int,
+                                 reservations: list[str],
+                                 error: bool) -> None:
+
+        project_id = volume.project_id
+        size_increase = new_size - volume.size
+
+        if error:
+            LOG.error("Failed to extend volume.", resource=volume)
             self.message_api.create(
                 context,
                 message_field.Action.EXTEND_VOLUME,
@@ -2938,8 +2968,7 @@ class VolumeManager(manager.CleanableManager,
 
         QUOTAS.commit(context, reservations, project_id=project_id)
 
-        attachments = volume.volume_attachment
+        if not volume.volume_attachment:
-        if not attachments:
             orig_volume_status = 'available'
         else:
             orig_volume_status = 'in-use'
@@ -2947,18 +2976,6 @@ class VolumeManager(manager.CleanableManager,
         volume.update({'size': int(new_size), 'status': orig_volume_status})
         volume.save()
 
-        if orig_volume_status == 'in-use':
-            nova_api = compute.API()
-            # If instance_uuid field is None on attachment, it means the
-            # request is coming from glance and not nova
-            instance_uuids = [attachment.instance_uuid
-                              for attachment in attachments
-                              if attachment.instance_uuid]
-            # If we are using glance cinder store, we should not send any
-            # external events to nova
-            if instance_uuids:
-                nova_api.extend_volume(context, instance_uuids, volume.id)
-
         pool = volume_utils.extract_host(volume.host, 'pool')
         if pool is None:
             # Legacy volume, put them into default pool

cinder/volume/rpcapi.py

@@ -140,9 +140,10 @@ class VolumeAPI(rpc.RPCAPI):
         3.16 - Add no_snapshots to accept_transfer method
         3.17 - Make get_backup_device a cast (async)
         3.18 - Add reimage method
+        3.19 - Add extend_volume_completion method
     """
 
-    RPC_API_VERSION = '3.18'
+    RPC_API_VERSION = '3.19'
     RPC_DEFAULT_VERSION = '3.0'
     TOPIC = constants.VOLUME_TOPIC
     BINARY = constants.VOLUME_BINARY
@@ -279,6 +280,13 @@ class VolumeAPI(rpc.RPCAPI):
         cctxt.cast(ctxt, 'extend_volume', volume=volume, new_size=new_size,
                    reservations=reservations)
 
+    @rpc.assert_min_rpc_version('3.19')
+    def extend_volume_completion(self, ctxt, volume, new_size, reservations,
+                                 error):
+        cctxt = self._get_cctxt(volume.service_topic_queue, version='3.19')
+        cctxt.cast(ctxt, 'extend_volume_completion', volume=volume,
+                   new_size=new_size, reservations=reservations, error=error)
+
     def migrate_volume(self, ctxt, volume, dest_backend, force_host_copy):
         backend_p = {'host': dest_backend.host,
                      'cluster_name': dest_backend.cluster_name,

releasenotes/notes/extend-volume-completion-action-9bf6b0ed551a8e32.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Add the new ``os-extend_volume_completion`` volume action, which the Nova
+    compute agent can use to notify Cinder that it has finished handling the
+    ``volume-extended`` external server event.