From 57037378bec5cd326f4d1efad5b0d4ba76322cc3 Mon Sep 17 00:00:00 2001 From: mvpnitesh Date: Mon, 28 Jan 2019 09:12:21 +0000 Subject: [PATCH] Provides mount point as cinder requires it to attach volume This patch provides a mount point which is required by cinder to attach the volume to the baremetal, when the volume targets are created for any node. Story: 2004864 Task: 29107 Change-Id: Id2a2e071026b86a6fd586a998bf865b1ddb960d7 --- ironic/common/cinder.py | 8 ++++---- ironic/tests/unit/common/test_cinder.py | 16 ++++++++++------ .../provide_mountpoint-58cfd25b6dd4cfde.yaml | 7 +++++++ 3 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/provide_mountpoint-58cfd25b6dd4cfde.yaml diff --git a/ironic/common/cinder.py b/ironic/common/cinder.py index 2a91b2385b..5b229b05d9 100644 --- a/ironic/common/cinder.py +++ b/ironic/common/cinder.py @@ -307,10 +307,10 @@ def attach_volumes(task, volume_list, connector): # database record to indicate that the attachment has # been completed, which moves the volume to the # 'attached' state. This action also sets a mountpoint - # for the volume, if known. In our use case, there is - # no way for us to know what the mountpoint is inside of - # the operating system, thus we send None. - client.volumes.attach(volume_id, instance_uuid, None) + # for the volume, as cinder requires a mointpoint to + # attach the volume, thus we send 'mount_volume'. + client.volumes.attach(volume_id, instance_uuid, + 'ironic_mountpoint') except cinder_exceptions.ClientException as e: msg = (_('Failed to inform cinder that the attachment for volume ' diff --git a/ironic/tests/unit/common/test_cinder.py b/ironic/tests/unit/common/test_cinder.py index 0008275e1d..f289a0d828 100644 --- a/ironic/tests/unit/common/test_cinder.py +++ b/ironic/tests/unit/common/test_cinder.py @@ -195,6 +195,7 @@ class TestCinderActions(db_base.DbTestCase): self.node = object_utils.create_test_node( self.context, instance_uuid=uuidutils.generate_uuid()) + self.mount_point = 'ironic_mountpoint' @mock.patch.object(cinderclient.volumes.VolumeManager, 'attach', autospec=True) @@ -239,7 +240,8 @@ class TestCinderActions(db_base.DbTestCase): mock_reserve.assert_called_once_with(mock.ANY, volume_id) mock_init.assert_called_once_with(mock.ANY, volume_id, connector) mock_attach.assert_called_once_with(mock.ANY, volume_id, - self.node.instance_uuid, None) + self.node.instance_uuid, + self.mount_point) mock_set_meta.assert_called_once_with(mock.ANY, volume_id, {'bar': 'baz'}) mock_get.assert_called_once_with(mock.ANY, volume_id) @@ -271,7 +273,6 @@ class TestCinderActions(db_base.DbTestCase): 'ironic_volume_uuid': '000-001'}}] volumes = [volume_id, 'already_attached'] - connector = {'foo': 'bar'} mock_create_meta.return_value = {'bar': 'baz'} mock_get.side_effect = [ @@ -294,7 +295,8 @@ class TestCinderActions(db_base.DbTestCase): mock_reserve.assert_called_once_with(mock.ANY, volume_id) mock_init.assert_called_once_with(mock.ANY, volume_id, connector) mock_attach.assert_called_once_with(mock.ANY, volume_id, - self.node.instance_uuid, None) + self.node.instance_uuid, + self.mount_point) mock_set_meta.assert_called_once_with(mock.ANY, volume_id, {'bar': 'baz'}) @@ -355,7 +357,7 @@ class TestCinderActions(db_base.DbTestCase): mock.ANY, '111111111-0000-0000-0000-000000000003', connector) mock_attach.assert_called_once_with( mock.ANY, '111111111-0000-0000-0000-000000000003', - self.node.instance_uuid, None) + self.node.instance_uuid, self.mount_point) mock_set_meta.assert_called_once_with( mock.ANY, '111111111-0000-0000-0000-000000000003', {'bar': 'baz'}) @@ -446,7 +448,8 @@ class TestCinderActions(db_base.DbTestCase): mock_reserve.assert_called_once_with(mock.ANY, volume_id) mock_init.assert_called_once_with(mock.ANY, volume_id, connector) mock_attach.assert_called_once_with(mock.ANY, volume_id, - self.node.instance_uuid, None) + self.node.instance_uuid, + self.mount_point) mock_get.assert_called_once_with(mock.ANY, volume_id) mock_is_attached.assert_called_once_with(mock.ANY, mock_get.return_value) @@ -496,7 +499,8 @@ class TestCinderActions(db_base.DbTestCase): mock_reserve.assert_called_once_with(mock.ANY, volume_id) mock_init.assert_called_once_with(mock.ANY, volume_id, connector) mock_attach.assert_called_once_with(mock.ANY, volume_id, - self.node.instance_uuid, None) + self.node.instance_uuid, + self.mount_point) mock_set_meta.assert_called_once_with(mock.ANY, volume_id, {'bar': 'baz'}) mock_get.assert_called_once_with(mock.ANY, volume_id) diff --git a/releasenotes/notes/provide_mountpoint-58cfd25b6dd4cfde.yaml b/releasenotes/notes/provide_mountpoint-58cfd25b6dd4cfde.yaml new file mode 100644 index 0000000000..79384bc033 --- /dev/null +++ b/releasenotes/notes/provide_mountpoint-58cfd25b6dd4cfde.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes a bug where cinder block storage service volumes volume fail to attach expecting a + mountpoint to be a valid string. See `story 2004864 + `_ for additional + information.