Fixes multiattach issues in attaching and deletion 28/13528/5
authorGabriel Cuba <gcuba@whitestack.com>
Wed, 14 Jun 2023 05:50:57 +0000 (00:50 -0500)
committeraticig <gulsum.atici@canonical.com>
Mon, 19 Jun 2023 06:23:25 +0000 (08:23 +0200)
Change-Id: I17ecb0418a3c366ea66e822ec34dbd4c6a9d335a
Signed-off-by: Gabriel Cuba <gcuba@whitestack.com>
NG-RO/osm_ng_ro/ns.py
NG-RO/osm_ng_ro/ns_thread.py
NG-RO/osm_ng_ro/tests/test_ns.py
NG-RO/osm_ng_ro/tests/test_ns_thread.py
RO-VIM-openstack/osm_rovim_openstack/tests/test_vimconn_openstack.py
RO-VIM-openstack/osm_rovim_openstack/vimconn_openstack.py
releasenotes/notes/volume_multiattach_fixes-0dc3ccda8e1b01a2.yaml [new file with mode: 0644]

index cd75025..ba5582e 100644 (file)
@@ -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,
index 03255e3..3c56712 100644 (file)
@@ -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,
index a236819..79e98c3 100644 (file)
@@ -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()
index 4e42b4f..2fac22a 100644 (file)
@@ -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):
         """
index f022e7a..08f73bc 100644 (file)
@@ -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,
index 7ce0ff7..8de6534 100644 (file)
@@ -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 (file)
index 0000000..71d35b2
--- /dev/null
@@ -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