Merge "Cinder: Add support to extend attached volumes"4.3.0

11 files changed, 212 insertions, 50 deletions

diff --git a/glance_store/_drivers/cinder/base.py b/glance_store/_drivers/cinder/base.py
index 69bb5ee..20fb897 100644
--- a/glance_store/_drivers/cinder/base.py
+++ b/glance_store/_drivers/cinder/base.py
@@ -52,6 +52,9 @@ class BaseBrickConnectorInterface(object):
     def disconnect_volume(self, device):
         self.conn.disconnect_volume(self.connection_info, device)
 
+    def extend_volume(self):
+        self.conn.extend_volume(self.connection_info)
+
     def yield_path(self, volume, volume_path):
         """
         This method returns the volume file path.
diff --git a/glance_store/_drivers/cinder/nfs.py b/glance_store/_drivers/cinder/nfs.py
index 7ae17ab..0e092d2 100644
--- a/glance_store/_drivers/cinder/nfs.py
+++ b/glance_store/_drivers/cinder/nfs.py
@@ -101,3 +101,6 @@ class NfsBrickConnector(base.BaseBrickConnectorInterface):
             mount.umount(vol_name, path, self.host,
                          self.root_helper)
         disconnect_volume_nfs()
+
+    def extend_volume(self):
+        raise NotImplementedError
diff --git a/glance_store/_drivers/cinder/store.py b/glance_store/_drivers/cinder/store.py
index fae6600..17fb96b 100644
--- a/glance_store/_drivers/cinder/store.py
+++ b/glance_store/_drivers/cinder/store.py
@@ -404,6 +404,19 @@ Possible values:
 
 * A string representing absolute path of mount point.
 """),
+    cfg.BoolOpt('cinder_do_extend_attached',
+                default=False,
+                help="""
+If this is set to True, glance will perform an extend operation
+on the attached volume. Only enable this option if the cinder
+backend driver supports the functionality of extending online
+(in-use) volumes. Supported from cinder microversion 3.42 and
+onwards. By default, it is set to False.
+
+Possible values:
+    * True or False
+
+"""),
 ]
 
 CINDER_SESSION = None
@@ -483,6 +496,13 @@ class Store(glance_store.driver.Store):
             self.store_conf = self.conf.glance_store
         self.volume_api = cinder_utils.API()
         getattr(os_brick, 'setup', lambda x: None)(CONF)
+        # The purpose of this map is to store the connector object for a
+        # particular volume as we will need to call os-brick extend_volume
+        # method for the kernel to realize the new size change after cinder
+        # extends the volume
+        # We only use it when creating the image so a volume will only have
+        # one mapping to a particular connector
+        self.volume_connector_map = {}
 
     def _set_url_prefix(self):
         self._url_prefix = "cinder://"
@@ -730,6 +750,8 @@ class Store(glance_store.driver.Store):
             self.volume_api.attachment_complete(client, attachment.id)
             LOG.debug('Attachment %(attachment_id)s completed successfully.',
                       {'attachment_id': attachment.id})
+
+            self.volume_connector_map[volume.id] = conn
             if (connection_info['driver_volume_type'] == 'rbd' and
                not conn.conn.do_local_attach):
                 yield device['path']
@@ -750,7 +772,8 @@ class Store(glance_store.driver.Store):
                             connection_info, device)
                     else:
                         conn.disconnect_volume(device)
-
+                        if self.volume_connector_map.get(volume.id):
+                            del self.volume_connector_map[volume.id]
                 except Exception:
                     LOG.exception(_LE('Failed to disconnect volume '
                                       '%(volume_id)s.'),
@@ -848,6 +871,100 @@ class Store(glance_store.driver.Store):
                               "internal error."))
             return 0
 
+    def _call_offline_extend(self, volume, size_gb):
+        size_gb += 1
+        LOG.debug("Extending (offline) volume %(volume_id)s to %(size)s GB.",
+                  {'volume_id': volume.id, 'size': size_gb})
+        volume.extend(volume, size_gb)
+        try:
+            volume = self._wait_volume_status(volume,
+                                              'extending',
+                                              'available')
+            size_gb = volume.size
+            return size_gb
+        except exceptions.BackendException:
+            raise exceptions.StorageFull()
+
+    def _call_online_extend(self, client, volume, size_gb):
+        size_gb += 1
+        LOG.debug("Extending (online) volume %(volume_id)s to %(size)s GB.",
+                  {'volume_id': volume.id, 'size': size_gb})
+        self.volume_api.extend_volume(client, volume, size_gb)
+        try:
+            volume = self._wait_volume_status(volume,
+                                              'extending',
+                                              'in-use')
+            size_gb = volume.size
+            return size_gb
+        except exceptions.BackendException:
+            raise exceptions.StorageFull()
+
+    def _write_data(self, f, write_props):
+        LOG.debug('Writing data to volume with write properties: '
+                  'bytes_written: %s, size_gb: %s, need_extend: %s, '
+                  'image_size: %s' %
+                  (write_props.bytes_written, write_props.size_gb,
+                   write_props.need_extend, write_props.image_size))
+        f.seek(write_props.bytes_written)
+        if write_props.buf:
+            f.write(write_props.buf)
+            write_props.bytes_written += len(write_props.buf)
+        while True:
+            write_props.buf = write_props.image_file.read(
+                self.WRITE_CHUNKSIZE)
+            if not write_props.buf:
+                write_props.need_extend = False
+                return
+            write_props.os_hash_value.update(write_props.buf)
+            write_props.checksum.update(write_props.buf)
+            if write_props.verifier:
+                write_props.verifier.update(write_props.buf)
+            if ((write_props.bytes_written + len(write_props.buf)) > (
+                write_props.size_gb * units.Gi) and
+                    (write_props.image_size == 0)):
+                return
+            f.write(write_props.buf)
+            write_props.bytes_written += len(write_props.buf)
+
+    def _offline_extend(self, client, volume, write_props):
+        while write_props.need_extend:
+            with self._open_cinder_volume(client, volume, 'wb') as f:
+                self._write_data(f, write_props)
+            if write_props.need_extend:
+                write_props.size_gb = self._call_offline_extend(
+                    volume, write_props.size_gb)
+
+    def _online_extend(self, client, volume, write_props):
+        with self._open_cinder_volume(client, volume, 'wb') as f:
+            # Th connector is initialized in _open_cinder_volume method
+            # and by mapping it with the volume ID, we are able to fetch
+            # it here
+            conn = self.volume_connector_map[volume.id]
+            while write_props.need_extend:
+                self._write_data(f, write_props)
+                if write_props.need_extend:
+                    # we already initialize a client with MV 3.54 and
+                    # we require 3.42 for online extend so we should
+                    # be good here.
+                    write_props.size_gb = self._call_online_extend(
+                        client, volume, write_props.size_gb)
+                    # Call os-brick to resize the LUN on the host
+                    conn.extend_volume()
+
+    # WriteProperties class is useful to allow us to modify immutable
+    # objects in the called methods
+    class WriteProperties:
+        def __init__(self, *args, **kwargs):
+            self.bytes_written = kwargs.get('bytes_written')
+            self.size_gb = kwargs.get('size_gb')
+            self.buf = kwargs.get('buf')
+            self.image_file = kwargs.get('image_file')
+            self.need_extend = kwargs.get('need_extend')
+            self.image_size = kwargs.get('image_size')
+            self.verifier = kwargs.get('verifier')
+            self.checksum = kwargs.get('checksum')
+            self.os_hash_value = kwargs.get('os_hash_value')
+
     @glance_store.driver.back_compat_add
     @capabilities.check
     def add(self, image_id, image_file, image_size, hashing_algo, context=None,
@@ -904,41 +1021,20 @@ class Store(glance_store.driver.Store):
         failed = True
         need_extend = True
         buf = None
+        online_extend = self.store_conf.cinder_do_extend_attached
+        write_props = self.WriteProperties(
+            bytes_written=bytes_written, size_gb=size_gb, buf=buf,
+            image_file=image_file, need_extend=need_extend,
+            image_size=image_size, verifier=verifier, checksum=checksum,
+            os_hash_value=os_hash_value)
         try:
-            while need_extend:
-                with self._open_cinder_volume(client, volume, 'wb') as f:
-                    f.seek(bytes_written)
-                    if buf:
-                        f.write(buf)
-                        bytes_written += len(buf)
-                    while True:
-                        buf = image_file.read(self.WRITE_CHUNKSIZE)
-                        if not buf:
-                            need_extend = False
-                            break
-                        os_hash_value.update(buf)
-                        checksum.update(buf)
-                        if verifier:
-                            verifier.update(buf)
-                        if (bytes_written + len(buf) > size_gb * units.Gi and
-                                image_size == 0):
-                            break
-                        f.write(buf)
-                        bytes_written += len(buf)
-
-                if need_extend:
-                    size_gb += 1
-                    LOG.debug("Extending volume %(volume_id)s to %(size)s GB.",
-                              {'volume_id': volume.id, 'size': size_gb})
-                    volume.extend(volume, size_gb)
-                    try:
-                        volume = self._wait_volume_status(volume,
-                                                          'extending',
-                                                          'available')
-                        size_gb = volume.size
-                    except exceptions.BackendException:
-                        raise exceptions.StorageFull()
-
+            if online_extend:
+                # we already initialize a client with MV 3.54 and
+                # we require 3.42 for online extend so we should
+                # be good here.
+                self._online_extend(client, volume, write_props)
+            else:
+                self._offline_extend(client, volume, write_props)
             failed = False
         except IOError as e:
             # Convert IOError reasons to Glance Store exceptions
@@ -957,17 +1053,17 @@ class Store(glance_store.driver.Store):
                                       '%(volume_id)s.'),
                                   {'volume_id': volume.id})
 
-        if image_size == 0:
-            metadata.update({'image_size': str(bytes_written)})
+        if write_props.image_size == 0:
+            metadata.update({'image_size': str(write_props.bytes_written)})
             volume.update_all_metadata(metadata)
         volume.update_readonly_flag(volume, True)
 
-        hash_hex = os_hash_value.hexdigest()
-        checksum_hex = checksum.hexdigest()
+        hash_hex = write_props.os_hash_value.hexdigest()
+        checksum_hex = write_props.checksum.hexdigest()
 
         LOG.debug("Wrote %(bytes_written)d bytes to volume %(volume_id)s "
                   "with checksum %(checksum_hex)s.",
-                  {'bytes_written': bytes_written,
+                  {'bytes_written': write_props.bytes_written,
                    'volume_id': volume.id,
                    'checksum_hex': checksum_hex})
 
@@ -979,7 +1075,7 @@ class Store(glance_store.driver.Store):
                                                volume.id)
 
         return (location_url,
-                bytes_written,
+                write_props.bytes_written,
                 checksum_hex,
                 hash_hex,
                 image_metadata)
diff --git a/glance_store/common/cinder_utils.py b/glance_store/common/cinder_utils.py
index b14aa23..c1c4186 100644
--- a/glance_store/common/cinder_utils.py
+++ b/glance_store/common/cinder_utils.py
@@ -208,3 +208,21 @@ class API(object):
                           {'id': attachment_id,
                            'msg': str(ex),
                            'code': getattr(ex, 'code', None)})
+
+    @handle_exceptions
+    def extend_volume(self, client, volume, new_size):
+        """Extend volume
+
+        :param client: cinderclient object
+        :param volume: UUID of the volume to extend
+        :param new_size: new size of the volume after extend
+        """
+        try:
+            client.volumes.extend(volume, new_size)
+        except cinder_exception.ClientException as ex:
+            with excutils.save_and_reraise_exception():
+                LOG.error(_LE('Extend volume failed for volume '
+                              '%(id)s. Error: %(msg)s Code: %(code)s'),
+                          {'id': volume.id,
+                           'msg': str(ex),
+                           'code': getattr(ex, 'code', None)})
diff --git a/glance_store/tests/unit/cinder/test_base.py b/glance_store/tests/unit/cinder/test_base.py
index cc57f56..488cf1a 100644
--- a/glance_store/tests/unit/cinder/test_base.py
+++ b/glance_store/tests/unit/cinder/test_base.py
@@ -104,6 +104,12 @@ class TestBaseBrickConnectorInterface(test_base.StoreBaseTest):
         self.connector.conn.disconnect_volume.assert_called_once_with(
             self.connection_info, fake_device)
 
+    def test_extend_volume(self):
+        self.mock_object(self.connector.conn, 'extend_volume')
+        self.connector.extend_volume()
+        self.connector.conn.extend_volume.assert_called_once_with(
+            self.connection_info)
+
     def test_yield_path(self):
         fake_vol = mock.MagicMock()
         fake_device = 'fake_dev_path'
diff --git a/glance_store/tests/unit/cinder/test_cinder_base.py b/glance_store/tests/unit/cinder/test_cinder_base.py
index d94782f..acdb70b 100644
--- a/glance_store/tests/unit/cinder/test_cinder_base.py
+++ b/glance_store/tests/unit/cinder/test_cinder_base.py
@@ -582,7 +582,7 @@ class TestCinderStoreBase(object):
                 fake_image_id, image_file, expected_size, self.hash_algo,
                 self.context, None)
 
-    def _test_cinder_add_extend(self, is_multi_store=False):
+    def _test_cinder_add_extend(self, is_multi_store=False, online=False):
 
         expected_volume_size = 2 * units.Gi
         expected_multihash = 'fake_hash'
@@ -619,6 +619,11 @@ class TestCinderStoreBase(object):
             backend = 'cinder1'
             expected_location = 'cinder://%s/%s' % (backend, fake_volume.id)
         self.config(cinder_volume_type='some_type', group=backend)
+        if online:
+            self.config(cinder_do_extend_attached=True, group=backend)
+            fake_connector = mock.MagicMock()
+            fake_vol_connector_map = {expected_volume_id: fake_connector}
+            self.store.volume_connector_map = fake_vol_connector_map
 
         fake_client = mock.MagicMock(auth_token=None, management_url=None)
         fake_volume.manager.get.return_value = fake_volume
@@ -635,9 +640,12 @@ class TestCinderStoreBase(object):
                                   side_effect=fake_open), \
                 mock.patch.object(cinder.utils, 'get_hasher') as fake_hasher, \
                 mock.patch.object(cinder.Store, '_wait_volume_status',
-                                  return_value=fake_volume) as mock_wait:
-            mock_cc.return_value = mock.MagicMock(client=fake_client,
-                                                  volumes=fake_volumes)
+                                  return_value=fake_volume) as mock_wait, \
+                mock.patch.object(cinder_utils.API,
+                                  'extend_volume') as extend_vol:
+            mock_cc_return_val = mock.MagicMock(client=fake_client,
+                                                volumes=fake_volumes)
+            mock_cc.return_value = mock_cc_return_val
 
             fake_hasher.side_effect = get_fake_hash
             loc, size, checksum, multihash, metadata = self.store.add(
@@ -656,11 +664,19 @@ class TestCinderStoreBase(object):
                 volume_type='some_type')
             if is_multi_store:
                 self.assertEqual(backend, metadata["store"])
-            fake_volume.extend.assert_called_once_with(
-                fake_volume, expected_volume_size // units.Gi)
-            mock_wait.assert_has_calls(
-                [mock.call(fake_volume, 'creating', 'available'),
-                 mock.call(fake_volume, 'extending', 'available')])
+            if online:
+                extend_vol.assert_called_once_with(
+                    mock_cc_return_val, fake_volume,
+                    expected_volume_size // units.Gi)
+                mock_wait.assert_has_calls(
+                    [mock.call(fake_volume, 'creating', 'available'),
+                     mock.call(fake_volume, 'extending', 'in-use')])
+            else:
+                fake_volume.extend.assert_called_once_with(
+                    fake_volume, expected_volume_size // units.Gi)
+                mock_wait.assert_has_calls(
+                    [mock.call(fake_volume, 'creating', 'available'),
+                     mock.call(fake_volume, 'extending', 'available')])
 
     def test_cinder_add_extend_storage_full(self):
 
diff --git a/glance_store/tests/unit/cinder/test_cinder_store.py b/glance_store/tests/unit/cinder/test_cinder_store.py
index a2b5af7..184acde 100644
--- a/glance_store/tests/unit/cinder/test_cinder_store.py
+++ b/glance_store/tests/unit/cinder/test_cinder_store.py
@@ -138,6 +138,9 @@ class TestCinderStore(base.StoreBaseTest,
     def test_cinder_add_extend(self):
         self._test_cinder_add_extend()
 
+    def test_cinder_add_extend_online(self):
+        self._test_cinder_add_extend(online=True)
+
     def test_cinder_delete(self):
         self._test_cinder_delete()
 
diff --git a/glance_store/tests/unit/cinder/test_multistore_cinder.py b/glance_store/tests/unit/cinder/test_multistore_cinder.py
index 01b27f2..f40965b 100644
--- a/glance_store/tests/unit/cinder/test_multistore_cinder.py
+++ b/glance_store/tests/unit/cinder/test_multistore_cinder.py
@@ -276,6 +276,9 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
     def test_cinder_add_extend(self):
         self._test_cinder_add_extend(is_multi_store=True)
 
+    def test_cinder_add_extend_online(self):
+        self._test_cinder_add_extend(is_multi_store=True, online=True)
+
     def test_cinder_delete(self):
         self._test_cinder_delete(is_multi_store=True)
 
diff --git a/glance_store/tests/unit/cinder/test_nfs.py b/glance_store/tests/unit/cinder/test_nfs.py
index d0f7d91..aa040ce 100644
--- a/glance_store/tests/unit/cinder/test_nfs.py
+++ b/glance_store/tests/unit/cinder/test_nfs.py
@@ -91,3 +91,6 @@ class TestNfsBrickConnector(
         nfs.mount.umount.assert_called_once_with(
             vol_name, mount_path, self.connector.host,
             self.connector.root_helper)
+
+    def test_extend_volume(self):
+        self.assertRaises(NotImplementedError, self.connector.extend_volume)
diff --git a/glance_store/tests/unit/test_opts.py b/glance_store/tests/unit/test_opts.py
index bfb9bce..073b782 100644
--- a/glance_store/tests/unit/test_opts.py
+++ b/glance_store/tests/unit/test_opts.py
@@ -84,6 +84,7 @@ class OptsTestCase(base.StoreBaseTest):
             'cinder_volume_type',
             'cinder_use_multipath',
             'cinder_enforce_multipath',
+            'cinder_do_extend_attached',
             'default_swift_reference',
             'https_insecure',
             'filesystem_store_chunk_size',
diff --git a/releasenotes/notes/cinder-support-extend-in-use-volume-c6292f950ff75cca.yaml b/releasenotes/notes/cinder-support-extend-in-use-volume-c6292f950ff75cca.yaml
new file mode 100644
index 0000000..2b4a748
--- /dev/null
+++ b/releasenotes/notes/cinder-support-extend-in-use-volume-c6292f950ff75cca.yaml
@@ -0,0 +1,10 @@
+---
+features:
+  - |
+    Added support for extending in-use volumes in cinder store.
+    A new boolean config option ``cinder_do_extend_attached`` is
+    added which allows operators to enable/disable extending
+    in-use volume support when creating an image.
+    By default, ``cinder_do_extend_attached`` will be ``False``
+    i.e. old flow of detaching, extending and attaching will be
+    used.