refactored few test cases

Author: Locharla, Sandeep

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java

@@ -457,11 +457,8 @@ public boolean deleteStoragePool(StoragePoolType type, String uuid) {
 
     public boolean deleteStoragePool(StoragePoolType type, String uuid, Map<String, String> details) {
         StorageAdaptor adaptor = getStorageAdaptor(type);
-        logger.debug("[deleteStoragePool] calling adaptor.deleteStoragePool for pool {} (type={})", uuid, type);
         boolean deleteStatus = adaptor.deleteStoragePool(uuid, details);
-        logger.debug("[deleteStoragePool] adaptor.deleteStoragePool returned {} for pool {}", deleteStatus, uuid);
         if (type == StoragePoolType.NetworkFilesystem) {
-            logger.debug("[deleteStoragePool] calling haMonitor.removeStoragePool for NFS pool {}", uuid);
             _haMonitor.removeStoragePool(uuid);
         }
         synchronized (_storagePools) {

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

@@ -950,8 +950,6 @@ private boolean destroyStoragePoolHandleException(Connect conn, String uuid) {
 
     @Override
     public boolean deleteStoragePool(String uuid, Map<String, String> details) {
-        logger.debug("[deleteStoragePool] details overload called for pool {}, delegating to deleteStoragePool(uuid)",
-                uuid);
         return deleteStoragePool(uuid);
     }
 

test/integration/plugins/ontap/nfs3/instance/test_vm_volume_attach.py

@@ -491,6 +491,7 @@ def test_04_attach_volume_to_vm(self):
             (Note: on ONTAP/NFS shared storage the volume state remains 'Ready';
             attachment is signalled by virtualmachineid being populated)
           - ONTAP: FlexVol remains online
+          - ONTAP: NFS3 volume data file created in FlexVol after attach (lazy creation)
           - VM remains 'Running'
         """
         if self.__class__.vm is None:
@@ -539,6 +540,17 @@ def test_04_attach_volume_to_vm(self):
             "ONTAP FlexVol should be 'online' after attach"
         )
 
+        # ONTAP: NFS3 uses lazy file creation — the volume data file is
+        # materialised on the FlexVol only when CloudStack calls createAsync
+        # during attachVolume.  Verify that the file now exists.
+        files = self.ontap.list_files_in_volume(pool.name)
+        vol_file = next((f for f in files if vol.id in f), None)
+        self.assertIsNotNone(
+            vol_file,
+            "No data file matching volume UUID '%s' found in FlexVol '%s' "
+            "after attach; files present: %s" % (vol.id, pool.name, files)
+        )
+
     # ------------------------------------------------------------------
     # Step 05 - Stop VM — export policy must be retained
     # ------------------------------------------------------------------
@@ -666,6 +678,7 @@ def test_07_detach_volume_from_vm(self):
           - Volume no longer lists the VM's ID
           - VM remains 'Running'
           - ONTAP: FlexVol remains online
+          - ONTAP: NFS3 volume data file persists in FlexVol after detach
         """
         if self.__class__.vm is None:
             self.skipTest("VM not deployed — test_03 was skipped (no ready template)")
@@ -723,6 +736,16 @@ def test_07_detach_volume_from_vm(self):
             "ONTAP FlexVol should be 'online' after detach"
         )
 
+        # ONTAP: NFS3 volume data file must still exist after detach — the file
+        # is only removed when deleteVolume is called, not on detach.
+        files = self.ontap.list_files_in_volume(pool.name)
+        vol_file = next((f for f in files if vol.id in f), None)
+        self.assertIsNotNone(
+            vol_file,
+            "Volume data file for '%s' should persist in FlexVol '%s' after "
+            "detach; files present: %s" % (vol.id, pool.name, files)
+        )
+
     # ------------------------------------------------------------------
     # Step 08 - Destroy VM, delete volume, delete pool
     # ------------------------------------------------------------------
@@ -735,6 +758,7 @@ def test_08_destroy_vm_and_cleanup(self):
         Verifies:
           - VM is destroyed/expunged from CloudStack
           - Volume is deleted from CloudStack
+          - ONTAP: NFS3 volume data file removed from FlexVol after deleteVolume
           - Pool is removed from CloudStack
           - ONTAP: FlexVol is deleted after pool removal
           - ONTAP: Export policy is removed after pool removal
@@ -766,11 +790,24 @@ def test_08_destroy_vm_and_cleanup(self):
 
         # Delete the ONTAP data volume
         if vol is not None:
+            vol_id = vol.id
             cmd = deleteVolumeAPI.deleteVolumeCmd()
-            cmd.id = vol.id
+            cmd.id = vol_id
             self.apiClient.deleteVolume(cmd)
             self.__class__.volume = None
 
+            # ONTAP: NFS3 volume data file must be removed from the FlexVol
+            # after deleteVolume (CloudStack/libvirt deletes the file from the
+            # NFS mount as part of the destroy workflow).
+            files = self.ontap.list_files_in_volume(pool_name)
+            vol_file = next((f for f in files if vol_id in f), None)
+            self.assertIsNone(
+                vol_file,
+                "Volume data file for '%s' should be gone from FlexVol '%s' "
+                "after deleteVolume; files still present: %s"
+                % (vol_id, pool_name, files)
+            )
+
         # Enter maintenance then delete the pool
         maint_cmd = enableStorageMaintenance.enableStorageMaintenanceCmd()
         maint_cmd.id = pool.id

test/integration/plugins/ontap/nfs3/pool/test_pool_lifecycle.py

@@ -40,7 +40,7 @@
 Running:
   nosetests --with-marvin \\
       --marvin-config=test/integration/plugins/ontap/ontap.cfg \\
-      test/integration/plugins/ontap/test_ontap_create_primary_storage_nfs3.py -v
+      test/integration/plugins/ontap/nfs3/pool/test_pool_lifecycle.py -v
 
 Note: Tests 01-06 share class-level state (sequential).  Running a single test
 with -m "test_NN" will invoke setUpClass but the guard assertion will fail
@@ -477,18 +477,6 @@ def test_05_cancel_maintenance_mode(self):
         """
         Cancel maintenance mode and verify the pool returns to Up.
 
-        cancelStorageMaintenance sends ModifyStoragePoolCommand(add=True) to the
-        KVM agent, which calls createStoragePool() with details that include
-        nfsMountOptions=vers=3.  The agent rebuilds the libvirt pool XML with the
-        xmlns:fs namespace extension and mounts the NFS share with vers=3.
-
-        Fix confirmed — LibvirtStorageAdaptor now correctly handles the case
-        where a stale-active libvirt pool entry lingers at the mount point after
-        sp.destroy() during enter-maintenance.  The fix:
-          1. Detects a stale-active pool (isActive==1 but mountpoint -q fails)
-             and destroys it before re-creating.
-          2. Retries createNetfsStoragePool once after 5 s on LibvirtException.
-
         Verifies:
           - CloudStack reports pool state Up
           - ONTAP: FlexVol is still online

test/integration/plugins/ontap/nfs3/pool/test_pool_with_volumes.py

@@ -18,18 +18,6 @@
 """
 NFS3 pool lifecycle tests with a CloudStack volume present throughout.
 
-Covers the TDS (section 10) scenarios that require a data volume to already
-exist on the pool during pool state transitions:
-
-  TDS Approach-1 SN 11-12 — Disable pool WITH volumes
-  TDS Approach-1 SN 15-16 — Enable pool WITH volumes
-  TDS Approach-1 SN 19-20 — Enter maintenance WITH volumes
-  TDS Approach-1 SN 21-22 — Cancel maintenance WITH volumes (fix confirmed)
-  TDS Negative   SN 5-6   — Delete pool that has volumes; forced=False rejected
-
-Note: TDS SN 7-8 (force-delete NFS3 pool after manual volume deletion) is
-already covered by test_ontap_create_primary_storage_nfs3.py test_07/test_08.
-
 Tests are numbered test_01 ... test_07 and must run in that order.  Each step
 builds on the shared state established by the previous step.
 
@@ -360,8 +348,7 @@ def test_01_create_pool_and_volume(self):
     @attr(tags=["nfs3_with_volumes"], required_hardware=True)
     def test_02_disable_pool_volume_survives(self):
         """
-        Disable the pool while a CloudStack data volume exists on it.
-        Covers TDS Approach-1 SN 11 (iSCSI) and SN 12 (NFS3):
+        Disable the pool while a CloudStack data volume exists on it:
           - Pool should no longer be available for scheduling new CS volumes
           - The existing CS volume should continue to exist (not deleted)
           - ONTAP: FlexVol remains online; export policy unchanged
@@ -412,8 +399,7 @@ def test_02_disable_pool_volume_survives(self):
     @attr(tags=["nfs3_with_volumes"], required_hardware=True)
     def test_03_enable_pool_volume_intact(self):
         """
-        Re-enable the pool while a CloudStack data volume exists on it.
-        Covers TDS Approach-1 SN 15 (iSCSI) and SN 16 (NFS3):
+        Re-enable the pool while a CloudStack data volume exists on it:
           - Pool state transitions back to Up
           - The existing CS volume is still accessible
           - ONTAP: FlexVol remains online; export policy unchanged
@@ -464,8 +450,7 @@ def test_03_enable_pool_volume_intact(self):
     @attr(tags=["nfs3_with_volumes"], required_hardware=True)
     def test_04_enter_maintenance_volume_present(self):
         """
-        Enter maintenance mode while a CloudStack data volume exists on the pool.
-        Covers TDS Approach-1 SN 19 (iSCSI) and SN 20 (NFS3):
+        Enter maintenance mode while a CloudStack data volume exists on the pool:
           - Pool transitions to Maintenance state
           - Existing CS volume remains in CloudStack
           - ONTAP: FlexVol stays online (maintenance is a CS-only state)
@@ -517,8 +502,7 @@ def test_04_enter_maintenance_volume_present(self):
     @attr(tags=["nfs3_with_volumes"], required_hardware=True)
     def test_05_cancel_maintenance_with_volume(self):
         """
-        Cancel maintenance mode while a CloudStack data volume exists on the pool.
-        Covers TDS Approach-1 SN 21 (iSCSI) and SN 22 (NFS3):
+        Cancel maintenance mode while a CloudStack data volume exists on the pool:
           - cancelStorageMaintenance succeeds (KVM/NFS3 fix confirmed)
           - Pool returns to Up state
           - Existing CS volume is still present in CloudStack
@@ -578,8 +562,7 @@ def test_06_forced_false_delete_rejected(self):
         """
         Enter maintenance mode then attempt to delete the pool (forced=False)
         while a CloudStack volume still exists on it.  The operation must be
-        rejected.
-        Covers TDS Negative Scenarios SN 5 (iSCSI) and SN 6 (NFS3):
+        rejected:
           - CloudstackAPIException is raised with an appropriate error
           - Pool remains in Maintenance state
           - CS volume still exists

test/integration/plugins/ontap/ontap_test_base.py

@@ -197,6 +197,39 @@ def list_lun_maps_for_volume(self, svm_name, vol_name):
         return [r for r in data.get("records", [])
                 if r.get("lun", {}).get("name", "").startswith(prefix)]
 
+    # -- NFS file helpers ----------------------------------------------------
+
+    def list_files_in_volume(self, vol_name, path="/"):
+        """Return a list of file names at ``path`` inside the named FlexVol.
+
+        Uses the ONTAP REST file-system API:
+          GET /api/storage/volumes/{uuid}/files/{url_encoded_path}
+
+        The path must appear in the URL (not as a query parameter).  The root
+        directory is represented as ``%2F``.
+
+        Returns an empty list if the volume does not exist, the path is empty,
+        or the request fails.
+        """
+        vol = self.get_volume(vol_name)
+        if not vol:
+            return []
+        vol_uuid = vol.get("uuid", "")
+        if not vol_uuid:
+            return []
+        # URL-encode the path component (/ → %2F) and embed it in the URL.
+        from urllib.parse import quote
+        encoded_path = quote(path, safe="")
+        try:
+            resp = self._get(
+                "/storage/volumes/%s/files/%s" % (vol_uuid, encoded_path),
+                params={"fields": "name,type", "max_records": "500"}
+            )
+        except Exception:
+            return []
+        return [r.get("name", "") for r in resp.get("records", [])
+                if r.get("name") not in (".", "..")]
+
 
 # ---------------------------------------------------------------------------
 # Base test class