From 6f989af09b58274b9e8979ceb1df52e724b590a5 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Tue, 22 Oct 2019 19:40:35 +0200 Subject: [PATCH] Enhance extend functionality Cinderlib's volume extend functionality would extend the volume in the backend yet ignore any refresh of the view of the volume if it was attached. It also didn't provide a way to refresh the view of an extended volume given a Connection instance. This patch adds the funtionality to automatically refresh the view when extending a locally attached volume as well as to do the refresh using a Connection instance. The refresh is done making the appropriate extend_volume call to os-brick's connector. The OS-Brick RBD connector doesn't have the method because there's nothing for it to do, but to provide a consistent API we add the method to our NOS-Brick to return the new size like other connectors do. Change-Id: I04a29597e7515526fb8505c76c3d7e3026b2d9c8 --- cinderlib/nos_brick.py | 26 +++++++-- cinderlib/objects.py | 9 +++ cinderlib/tests/functional/test_basic.py | 22 ++++++++ .../tests/unit/objects/test_connection.py | 7 +++ cinderlib/tests/unit/objects/test_volume.py | 15 ++++- cinderlib/tests/unit/test_nos_brick.py | 16 ++++++ doc/source/topics/connections.rst | 55 +++++++++++++++++++ doc/source/topics/volumes.rst | 10 +++- .../enhance-extend-687d8e9c4a58e517.yaml | 8 +++ 9 files changed, 162 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/enhance-extend-687d8e9c4a58e517.yaml diff --git a/cinderlib/nos_brick.py b/cinderlib/nos_brick.py index 747b7d8..bc1076d 100644 --- a/cinderlib/nos_brick.py +++ b/cinderlib/nos_brick.py @@ -148,6 +148,13 @@ class RBDConnector(connectors.rbd.RBDConnector): return False return True + def _get_vol_data(self, connection_properties): + self._setup_rbd_class() + pool, volume = connection_properties['name'].split('/') + link_name = self.get_rbd_device_name(pool, volume) + real_dev_path = os.path.realpath(link_name) + return link_name, real_dev_path + def _unmap(self, real_dev_path, conf_file, connection_properties): if os.path.exists(real_dev_path): cmd = ['rbd', 'unmap', real_dev_path, '--conf', conf_file] @@ -157,11 +164,8 @@ class RBDConnector(connectors.rbd.RBDConnector): def disconnect_volume(self, connection_properties, device_info, force=False, ignore_errors=False): - self._setup_rbd_class() - pool, volume = connection_properties['name'].split('/') conf_file = device_info['conf'] - link_name = self.get_rbd_device_name(pool, volume) - real_dev_path = os.path.realpath(link_name) + link_name, real_dev_path = self._get_vol_data(connection_properties) self._unmap(real_dev_path, conf_file, connection_properties) if self.containerized: @@ -193,6 +197,20 @@ class RBDConnector(connectors.rbd.RBDConnector): # Don't check again to speed things on following connections RBDConnector._setup_rbd_class = lambda *args: None + def extend_volume(self, connection_properties): + """Refresh local volume view and return current size in bytes.""" + # Nothing to do, RBD attached volumes are automatically refreshed, but + # we need to return the new size for compatibility + link_name, real_dev_path = self._get_vol_data(connection_properties) + + device_name = os.path.basename(real_dev_path) # ie: rbd0 + device_number = device_name[3:] # ie: 0 + # Get size from /sys/devices/rbd/0/size instead of + # /sys/class/block/rbd0/size because the latter isn't updated + with open('/sys/devices/rbd/' + device_number + '/size') as f: + size_bytes = f.read().strip() + return int(size_bytes) + _setup_rbd_class = _setup_class diff --git a/cinderlib/objects.py b/cinderlib/objects.py index 2586954..86c9c82 100644 --- a/cinderlib/objects.py +++ b/cinderlib/objects.py @@ -39,6 +39,7 @@ DEFAULT_PROJECT_ID = 'cinderlib' DEFAULT_USER_ID = 'cinderlib' BACKEND_NAME_SNAPSHOT_FIELD = 'progress' CONNECTIONS_OVO_FIELD = 'volume_attachment' +GB = 1024 ** 3 # This cannot go in the setup method because cinderlib objects need them to # be setup to set OVO_CLASS @@ -500,6 +501,11 @@ class Volume(NamedObject): finally: self.save() + if volume.status == 'in-use' and self.local_attach: + return self.local_attach.extend() + # Must return size in bytes + return size * GB + def clone(self, **new_vol_attrs): new_vol_attrs['source_vol_id'] = self.id new_vol = Volume(self, **new_vol_attrs) @@ -867,6 +873,9 @@ class Connection(Object, LazyVolumeAttr): def save(self): self.persistence.set_connection(self) + def extend(self): + return self.connector.extend_volume(self.conn_info['data']) + class Snapshot(NamedObject, LazyVolumeAttr): OVO_CLASS = cinder_objs.Snapshot diff --git a/cinderlib/tests/functional/test_basic.py b/cinderlib/tests/functional/test_basic.py index e65f994..a20a9de 100644 --- a/cinderlib/tests/functional/test_basic.py +++ b/cinderlib/tests/functional/test_basic.py @@ -179,6 +179,28 @@ class BackendFunctBasic(base_tests.BaseFunctTestCase): result_new_size = self._get_vol_size(vol) self.assertSize(new_size, result_new_size) + def test_extend_attached(self): + vol = self._create_vol(self.backend) + original_size = vol.size + # Attach, get size, and leave volume attached + result_original_size = self._get_vol_size(vol, do_detach=False) + self.assertSize(original_size, result_original_size) + + new_size = vol.size + 1 + # Extending the volume should also extend the local view of the volume + reported_size = vol.extend(new_size) + + # The instance size must have been updated + self.assertEqual(new_size, vol.size) + self.assertEqual(new_size, vol._ovo.size) + + # Returned size must match the requested one + self.assertEqual(new_size * (1024 ** 3), reported_size) + + # Get size of attached volume on the host and detach it + result_new_size = self._get_vol_size(vol) + self.assertSize(new_size, result_new_size) + def test_clone(self): vol = self._create_vol(self.backend) original_size = self._get_vol_size(vol, do_detach=False) diff --git a/cinderlib/tests/unit/objects/test_connection.py b/cinderlib/tests/unit/objects/test_connection.py index f3cea04..d7b3107 100644 --- a/cinderlib/tests/unit/objects/test_connection.py +++ b/cinderlib/tests/unit/objects/test_connection.py @@ -282,3 +282,10 @@ class TestConnection(base.BaseTest): def test_connected(self, value): with mock.patch('cinderlib.objects.Connection.conn_info', value): self.assertEqual(value, self.conn.connected) + + def test_extend(self): + self.conn._ovo.connection_info['conn'] = {'data': mock.sentinel.data} + with mock.patch('cinderlib.objects.Connection.connector') as mock_conn: + res = self.conn.extend() + mock_conn.extend_volume.assert_called_once_with(mock.sentinel.data) + self.assertEqual(mock_conn.extend_volume.return_value, res) diff --git a/cinderlib/tests/unit/objects/test_volume.py b/cinderlib/tests/unit/objects/test_volume.py index 5616ba8..c177299 100644 --- a/cinderlib/tests/unit/objects/test_volume.py +++ b/cinderlib/tests/unit/objects/test_volume.py @@ -173,13 +173,26 @@ class TestVolume(base.BaseTest): def test_extend(self): vol = objects.Volume(self.backend_name, status='available', size=10) - vol.extend(11) + res = vol.extend(11) + self.assertEqual(11 * (1024 ** 3), res) # size is in bytes not GBi self.backend.driver.extend_volume.assert_called_once_with(vol._ovo, 11) self.persistence.set_volume.assert_called_once_with(vol) self.assertEqual('available', vol.status) self.assertEqual(11, vol.size) + def test_extend_attached(self): + vol = objects.Volume(self.backend_name, status='in-use', size=10) + vol.local_attach = mock.Mock() + res = vol.extend(11) + + self.assertEqual(vol.local_attach.extend.return_value, res) + self.backend.driver.extend_volume.assert_called_once_with(vol._ovo, 11) + vol.local_attach.extend.assert_called_once_with() + self.persistence.set_volume.assert_called_once_with(vol) + self.assertEqual('in-use', vol.status) + self.assertEqual(11, vol.size) + def test_extend_error(self): vol = objects.Volume(self.backend_name, status='available', size=10) self.backend.driver.extend_volume.side_effect = exception.NotFound diff --git a/cinderlib/tests/unit/test_nos_brick.py b/cinderlib/tests/unit/test_nos_brick.py index e6c4629..cb03cef 100644 --- a/cinderlib/tests/unit/test_nos_brick.py +++ b/cinderlib/tests/unit/test_nos_brick.py @@ -363,3 +363,19 @@ class TestRBDConnector(base.BaseTest): exists_mock.assert_called_once_with(link) remove_mock.assert_called_once_with(link) link_mock.assert_called_once_with(source, link) + + @mock.patch('six.moves.builtins.open') + @mock.patch.object(nos_brick.RBDConnector, '_get_vol_data') + def test_extend_volume(self, get_data_mock, open_mock): + get_data_mock.return_value = ( + '/dev/rbd/rbd/volume-56539d26-2b78-49b8-8b96-160a62b0831f', + '/dev/rbd10') + + cm_open = open_mock.return_value.__enter__.return_value + cm_open.read.return_value = '5368709120' + res = self.connector.extend_volume(mock.sentinel.connector_properties) + + self.assertEqual(5 * (1024 ** 3), res) # 5 GBi + get_data_mock.assert_called_once_with( + mock.sentinel.connector_properties) + open_mock.assert_called_once_with('/sys/devices/rbd/10/size') diff --git a/doc/source/topics/connections.rst b/doc/source/topics/connections.rst index 43a05a8..5df7daf 100644 --- a/doc/source/topics/connections.rst +++ b/doc/source/topics/connections.rst @@ -256,6 +256,61 @@ know when instantiating the driver by passing the use_multipath_for_image_xfer=True, ) +Extend +------ + +The `Connection` object has an `extend` method that will refresh the host's +view of an attached volume to reflect the latest size of the volume and return +the new size in bytes. + +There is no need to manually call this method for volumes that are locally +attached to the node that calls the `Volume`'s `extend` method, since that call +takes care of it. + +When extending volumes that are attached to nodes other than the one calling +the `Volume`'s `extend` method we will need to either detach and re-attach the +volume on the host following the mechanisms explained above, or refresh the +current view of the volume. + +How we refresh the host's view of an attached volume will depend on how we are +attaching the volumes. + +With access to the metadata persistence storage +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In this case things are easier, just like it was on the +`Remote connection`_. + +Assuming we have a `volume_id` variable with the volume, and `storage` has the +`Backend` instance, all we need to do is: + +.. code-block:: python + + vol = storage.Volume.get_by_id(volume_id) + vol.connections[0].extend() + +No access to the metadata persistence storage +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This is more inconvenient, as you'll have to handle the data exchange manually +as well as the *OS-Brick* library calls to do the extend. + +We'll need to get the connector information on the host that is going to do +the attach. Asuuming the dictionary is available in `connection_info` the +code would look like this: + +.. code-block:: python + + from os_brick.initiator import connector + + connector_dict = connection_info['connector'] + protocol = connection_info['conn']['driver_volume_type'] + + conn = connector.InitiatorConnector.factory( + protocol, 'sudo', user_multipath=True, + device_scan_attempts=3, conn=connector_dict) + conn.extend() + Multi attach ------------ diff --git a/doc/source/topics/volumes.rst b/doc/source/topics/volumes.rst index d55b2bc..6758fbe 100644 --- a/doc/source/topics/volumes.rst +++ b/doc/source/topics/volumes.rst @@ -224,16 +224,24 @@ The only parameter received by the `extend` method is the new size, and this must always be greater than the current value because *cinderlib* is not validating this at the moment. +The call will return the new size of the volume in bytes. + Example of creating, extending, and deleting a volume: .. code-block:: python vol = lvm.create_volume(size=1) print('Vol %s has %s GBi' % (vol.id, vol.size)) - vol.extend(2) + new_size = vol.extend(2) print('Extended vol %s has %s GBi' % (vol.id, vol.size)) + print('Detected new size is %s bytes' % new_size) vol.delete() +A call to `extend` on a locally attached volume will automatically update the +host's view of the volume to reflect the new size. For non locally attached +volumes please refer to the `extend section in the connections +`_ section. + Other methods ------------- diff --git a/releasenotes/notes/enhance-extend-687d8e9c4a58e517.yaml b/releasenotes/notes/enhance-extend-687d8e9c4a58e517.yaml new file mode 100644 index 0000000..e3ef243 --- /dev/null +++ b/releasenotes/notes/enhance-extend-687d8e9c4a58e517.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Enhance volume extend functionality in cinderlib by supporting the refresh + of the host's view of an attached volume that has been extended in the + backend to reflect the new size. A call to volume.extend will + automatically extend the view if the volume is locally attached and + connection.extend will do the same when run on a non controller host.