From 1fd411bc37a30d03515beda505a4cd12154f2b08 Mon Sep 17 00:00:00 2001 From: Gabriel Cuba Date: Wed, 14 Jun 2023 00:50:57 -0500 Subject: [PATCH] Fixes multiattach issues in attaching and deletion Change-Id: I17ecb0418a3c366ea66e822ec34dbd4c6a9d335a Signed-off-by: Gabriel Cuba --- NG-RO/osm_ng_ro/ns.py | 12 ++++- NG-RO/osm_ng_ro/ns_thread.py | 16 ++++-- NG-RO/osm_ng_ro/tests/test_ns.py | 14 ++++- NG-RO/osm_ng_ro/tests/test_ns_thread.py | 2 +- .../tests/test_vimconn_openstack.py | 3 +- .../osm_rovim_openstack/vimconn_openstack.py | 53 ++++++++++++++----- ...me_multiattach_fixes-0dc3ccda8e1b01a2.yaml | 21 ++++++++ 7 files changed, 100 insertions(+), 21 deletions(-) create mode 100644 releasenotes/notes/volume_multiattach_fixes-0dc3ccda8e1b01a2.yaml diff --git a/NG-RO/osm_ng_ro/ns.py b/NG-RO/osm_ng_ro/ns.py index cd750251..ba5582ed 100644 --- a/NG-RO/osm_ng_ro/ns.py +++ b/NG-RO/osm_ng_ro/ns.py @@ -1422,7 +1422,9 @@ class Ns(object): persistent_root_disk: dict, persistent_ordinary_disk: dict, disk_list: list, + extra_dict: dict, vnf_id: str = None, + nsr_id: str = None, ) -> None: """Fill the disk list by adding persistent ordinary disks. @@ -1448,6 +1450,10 @@ class Ns(object): "multiattach": multiattach, } disk_list.append(persistent_ordinary_disk[disk["id"]]) + if multiattach: # VDU creation has to wait for shared volumes + extra_dict["depends_on"].append( + f"nsrs:{nsr_id}:shared-volumes.{name}" + ) @staticmethod def _prepare_vdu_affinity_group_list( @@ -1598,7 +1604,9 @@ class Ns(object): persistent_root_disk, persistent_ordinary_disk, disk_list, + extra_dict, vnfd["id"], + nsr_id, ) affinity_group_list = Ns._prepare_vdu_affinity_group_list( @@ -1638,6 +1646,7 @@ class Ns(object): "size": target_shared_volume["size-of-storage"], "name": target_shared_volume["id"], "type": target_shared_volume["type-of-storage"], + "keep": Ns.is_volume_keeping_required(target_shared_volume), } extra_dict["params"] = shared_volume_data return extra_dict @@ -2121,7 +2130,6 @@ class Ns(object): "image", "flavor", "affinity-or-anti-affinity-group", - "shared-volumes", ]: self.logger.debug("process NS={} {}".format(nsr_id, item)) diff_items, task_index = self.calculate_diff_items( @@ -2141,7 +2149,7 @@ class Ns(object): # VNF vlds and vdus for vnfr_id, vnfr in db_vnfrs.items(): # vnfr_id need to be set as global variable for among others nested method _process_vdu_params - for item in ["net", "vdu"]: + for item in ["net", "vdu", "shared-volumes"]: self.logger.debug("process VNF={} {}".format(vnfr_id, item)) diff_items, task_index = self.calculate_diff_items( indata=indata, diff --git a/NG-RO/osm_ng_ro/ns_thread.py b/NG-RO/osm_ng_ro/ns_thread.py index 03255e3f..3c567124 100644 --- a/NG-RO/osm_ng_ro/ns_thread.py +++ b/NG-RO/osm_ng_ro/ns_thread.py @@ -694,12 +694,20 @@ class VimInteractionSharedVolume(VimInteractionBase): task = ro_task["tasks"][task_index] task_id = task["task_id"] shared_volume_vim_id = ro_task["vim_info"]["vim_id"] + created_items = ro_task["vim_info"]["created_items"] ro_vim_item_update_ok = { "vim_status": "DELETED", "created": False, "vim_message": "DELETED", "vim_id": None, } + if created_items and created_items.get(shared_volume_vim_id).get("keep"): + ro_vim_item_update_ok = { + "vim_status": "ACTIVE", + "created": False, + "vim_message": None, + } + return "DONE", ro_vim_item_update_ok try: if shared_volume_vim_id: target_vim = self.my_vims[ro_task["target_id"]] @@ -738,7 +746,6 @@ class VimInteractionSharedVolume(VimInteractionBase): target_vim = self.my_vims[ro_task["target_id"]] try: - shared_volume_name = None shared_volume_vim_id = None shared_volume_data = None @@ -754,11 +761,14 @@ class VimInteractionSharedVolume(VimInteractionBase): shared_volume_vim_id, ) = target_vim.new_shared_volumes(shared_volume_data) created = True - created_items[shared_volume_vim_id] = shared_volume_name + created_items[shared_volume_vim_id] = { + "name": shared_volume_name, + "keep": shared_volume_data.get("keep"), + } ro_vim_item_update = { "vim_id": shared_volume_vim_id, - "vim_status": "DONE", + "vim_status": "ACTIVE", "created": created, "created_items": created_items, "vim_details": None, diff --git a/NG-RO/osm_ng_ro/tests/test_ns.py b/NG-RO/osm_ng_ro/tests/test_ns.py index a2368195..79e98c38 100644 --- a/NG-RO/osm_ng_ro/tests/test_ns.py +++ b/NG-RO/osm_ng_ro/tests/test_ns.py @@ -4543,6 +4543,7 @@ class TestProcessVduParams(unittest.TestCase): } persistent_ordinary_disk = {} disk_list = [] + extra_dict = {} expected_disk_list = [ { "size": "10", @@ -4552,7 +4553,11 @@ class TestProcessVduParams(unittest.TestCase): } ] self.ns._add_persistent_ordinary_disks_to_disk_list( - target_vdu, persistent_root_disk, persistent_ordinary_disk, disk_list + target_vdu, + persistent_root_disk, + persistent_ordinary_disk, + disk_list, + extra_dict, ) self.assertEqual(disk_list, expected_disk_list) mock_volume_keeping_required.assert_called_once_with(ordinary_disk) @@ -4576,9 +4581,14 @@ class TestProcessVduParams(unittest.TestCase): } persistent_ordinary_disk = {} disk_list = [] + extra_dict = {} self.ns._add_persistent_ordinary_disks_to_disk_list( - target_vdu, persistent_root_disk, persistent_ordinary_disk, disk_list + target_vdu, + persistent_root_disk, + persistent_ordinary_disk, + disk_list, + extra_dict, ) self.assertEqual(disk_list, []) mock_volume_keeping_required.assert_not_called() diff --git a/NG-RO/osm_ng_ro/tests/test_ns_thread.py b/NG-RO/osm_ng_ro/tests/test_ns_thread.py index 4e42b4f3..2fac22a1 100644 --- a/NG-RO/osm_ng_ro/tests/test_ns_thread.py +++ b/NG-RO/osm_ng_ro/tests/test_ns_thread.py @@ -1513,7 +1513,7 @@ class TestVimInteractionSharedVolume(unittest.TestCase): self.assertEqual(result[0], "DONE") self.assertEqual(result[1].get("vim_id"), "shared-volume") self.assertEqual(result[1].get("created"), True) - self.assertEqual(result[1].get("vim_status"), "DONE") + self.assertEqual(result[1].get("vim_status"), "ACTIVE") def test__new_shared_volume_failed(self): """ diff --git a/RO-VIM-openstack/osm_rovim_openstack/tests/test_vimconn_openstack.py b/RO-VIM-openstack/osm_rovim_openstack/tests/test_vimconn_openstack.py index f022e7a5..08f73bc9 100644 --- a/RO-VIM-openstack/osm_rovim_openstack/tests/test_vimconn_openstack.py +++ b/RO-VIM-openstack/osm_rovim_openstack/tests/test_vimconn_openstack.py @@ -1344,9 +1344,10 @@ class TestNewVmInstance(unittest.TestCase): created_items = {} expected_block_device_mapping = {} self.vimconn.cinder.volumes.list.return_value = [ - Volume("avaible", "multiattach", "shared-volume", volume_id4) + Volume("available", "multiattach", "shared-volume", volume_id4) ] self.vimconn.cinder.volumes.get.return_value.id = volume_id4 + self.vimconn.cinder.volumes.get.return_value.status = "available" self.vimconn._prepare_shared_volumes( name, disk, diff --git a/RO-VIM-openstack/osm_rovim_openstack/vimconn_openstack.py b/RO-VIM-openstack/osm_rovim_openstack/vimconn_openstack.py index 7ce0ff78..8de65341 100644 --- a/RO-VIM-openstack/osm_rovim_openstack/vimconn_openstack.py +++ b/RO-VIM-openstack/osm_rovim_openstack/vimconn_openstack.py @@ -2170,11 +2170,13 @@ class vimconnector(vimconn.VimConnector): "Created volume is not valid, does not have id attribute." ) + block_device_mapping["vd" + chr(base_disk_index)] = volume.id + if disk.get("multiattach"): # multiattach volumes do not belong to VDUs + return volume_txt = "volume:" + str(volume.id) if disk.get("keep"): volume_txt += ":keep" created_items[volume_txt] = True - block_device_mapping["vd" + chr(base_disk_index)] = volume.id def new_shared_volumes(self, shared_volume_data) -> (str, str): try: @@ -2199,13 +2201,27 @@ class vimconnector(vimconn.VimConnector): volumes = {volume.name: volume.id for volume in self.cinder.volumes.list()} if volumes.get(disk["name"]): sv_id = volumes[disk["name"]] - volume = self.cinder.volumes.get(sv_id) - self.update_block_device_mapping( - volume=volume, - block_device_mapping=block_device_mapping, - base_disk_index=base_disk_index, - disk=disk, - created_items=created_items, + max_retries = 3 + vol_status = "" + # If this is not the first VM to attach the volume, volume status may be "reserved" for a short time + while max_retries: + max_retries -= 1 + volume = self.cinder.volumes.get(sv_id) + vol_status = volume.status + if volume.status not in ("in-use", "available"): + time.sleep(5) + continue + self.update_block_device_mapping( + volume=volume, + block_device_mapping=block_device_mapping, + base_disk_index=base_disk_index, + disk=disk, + created_items=created_items, + ) + return + raise vimconn.VimConnException( + "Shared volume is not prepared, status is: {}".format(vol_status), + http_code=vimconn.HTTP_Internal_Server_Error, ) def _prepare_non_root_persistent_volumes( @@ -2980,17 +2996,30 @@ class vimconnector(vimconn.VimConnector): Args: shared_volume_vim_id (str): ID of shared volume in VIM """ + elapsed_time = 0 try: - if self.cinder.volumes.get(shared_volume_vim_id).status != "available": - return True + while elapsed_time < server_timeout: + vol_status = self.cinder.volumes.get(shared_volume_vim_id).status + if vol_status == "available": + self.cinder.volumes.delete(shared_volume_vim_id) + return True - else: - self.cinder.volumes.delete(shared_volume_vim_id) + time.sleep(5) + elapsed_time += 5 + + if elapsed_time >= server_timeout: + raise vimconn.VimConnException( + "Timeout waiting for volume " + + shared_volume_vim_id + + " to be available", + http_code=vimconn.HTTP_Request_Timeout, + ) except Exception as e: self.logger.error( "Error deleting volume: {}: {}".format(type(e).__name__, e) ) + self._format_exception(e) def _delete_volumes_by_id_wth_cinder( self, k: str, k_id: str, volumes_to_hold: list, created_items: dict diff --git a/releasenotes/notes/volume_multiattach_fixes-0dc3ccda8e1b01a2.yaml b/releasenotes/notes/volume_multiattach_fixes-0dc3ccda8e1b01a2.yaml new file mode 100644 index 00000000..71d35b23 --- /dev/null +++ b/releasenotes/notes/volume_multiattach_fixes-0dc3ccda8e1b01a2.yaml @@ -0,0 +1,21 @@ +####################################################################################### +# Copyright ETSI Contributors and Others. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. +####################################################################################### +--- +fixes: + - | + Feature 10972: Fixes blocking when deleting shared volumes and failure when attaching to VDUs in Openstack. + Enables the use of keep-volume and multiattach in the same volume. \ No newline at end of file -- 2.17.1