Merge "Fix generic driver NFS share deletion 'target is busy' error" into stable/2025.1

manila/share/drivers/generic.py

@@ -400,7 +400,18 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
                           "'%(server)s'.", log_data)
                 unmount_cmd = ['sudo', 'umount', mount_path, '&&', 'sudo',
                                'rmdir', mount_path]
-                self._ssh_exec(server_details, unmount_cmd)
+                try:
+                    self._ssh_exec(server_details, unmount_cmd)
+                except exception.ProcessExecutionError as e:
+                    if e.stderr and 'is busy' in e.stderr.lower():
+                        LOG.warning("Device '%(path)s' is busy on server "
+                                    "'%(server)s', retrying with lazy "
+                                    "unmount.", log_data)
+                        lazy_cmd = ['sudo', 'umount', '-l', mount_path,
+                                    '&&', 'sudo', 'rmdir', mount_path]
+                        self._ssh_exec(server_details, lazy_cmd)
+                    else:
+                        raise
                 self._remove_mount_permanently(share.id, server_details)
             else:
                 LOG.warning("Mount point '%(path)s' does not exist on "

manila/share/drivers/helpers.py

@@ -219,7 +219,33 @@ class NFSHelper(NASHelperBase):
             LOG.error(e.stderr)
 
     def remove_exports(self, server, share_name):
-        """Remove exports."""
+        """Remove NFS exports for the given share.
+
+        Unexports all clients from the share path using exportfs -u before
+        the share is unmounted. This prevents 'device busy' errors during
+        unmount by ensuring no NFS clients hold references to the export.
+        """
+        local_path = os.path.join(self.configuration.share_mount_path,
+                                  share_name)
+        out, __ = self._ssh_exec(server, ['sudo', 'exportfs'])
+        hosts = self.get_host_list(out, local_path)
+        for host in hosts:
+            parsed_host = self._get_parsed_address_or_cidr(host)
+            try:
+                self._ssh_exec(server, ['sudo', 'exportfs', '-u',
+                                        ':'.join((parsed_host, local_path))])
+            except exception.ProcessExecutionError as e:
+                if "could not find" in e.stderr.lower():
+                    LOG.debug("Export %(host)s:%(path)s was already "
+                              "removed.", {'host': parsed_host,
+                                           'path': local_path})
+                else:
+                    LOG.warning("Failed to unexport %(host)s:%(path)s: "
+                                "%(error)s",
+                                {'host': parsed_host, 'path': local_path,
+                                 'error': e.stderr})
+        if hosts:
+            self._sync_nfs_temp_and_perm_files(server)
 
     @nfs_synchronized
     def update_access(self, server, share_name, access_rules, add_rules,

manila/tests/share/drivers/test_generic.py

@@ -434,6 +434,37 @@ class GenericShareDriverTestCase(test.TestCase):
             mount_path, self.server)
         generic.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY)
 
+    def test_unmount_device_busy_lazy_fallback(self):
+        mount_path = '/fake/mount/path'
+        self.mock_object(self._driver, '_is_device_mounted',
+                         mock.Mock(return_value=True))
+        self.mock_object(self._driver, '_remove_mount_permanently')
+        self.mock_object(self._driver, '_get_mount_path',
+                         mock.Mock(return_value=mount_path))
+
+        def fake_ssh_exec(server, cmd):
+            if cmd == ['sudo', 'umount', mount_path, '&&',
+                       'sudo', 'rmdir', mount_path]:
+                raise exception.ProcessExecutionError(
+                    stderr='umount: /fake/mount/path: target is busy')
+            return ('', '')
+
+        self.mock_object(self._driver, '_ssh_exec',
+                         mock.Mock(side_effect=fake_ssh_exec))
+
+        self._driver._unmount_device(self.share, self.server)
+
+        self._driver._ssh_exec.assert_has_calls([
+            mock.call(self.server,
+                      ['sudo', 'umount', mount_path,
+                       '&&', 'sudo', 'rmdir', mount_path]),
+            mock.call(self.server,
+                      ['sudo', 'umount', '-l', mount_path,
+                       '&&', 'sudo', 'rmdir', mount_path]),
+        ])
+        self._driver._remove_mount_permanently.assert_called_once_with(
+            self.share.id, self.server)
+
     def test_is_device_mounted_true(self):
         volume = {'mountpoint': 'fake_mount_point', 'id': 'fake_id'}
         mount_path = '/fake/mount/path'

manila/tests/share/drivers/test_helpers.py

@@ -433,6 +433,64 @@ class NFSHelperTestCase(test.TestCase):
              fake_maintenance_path]
         )
 
+    def test_remove_exports(self):
+        local_path = os.path.join(CONF.share_mount_path, self.share_name)
+        exec_result = (
+            '%(path)s\n\t\t1.1.1.1\n'
+            '%(path)s\n\t\t2.2.2.2' % {'path': local_path}
+        )
+
+        def fake_ssh_exec(*args, **kwargs):
+            if args[1] == ['sudo', 'exportfs']:
+                return (exec_result, '')
+            return ('', '')
+
+        self.mock_object(self._helper, '_ssh_exec',
+                         mock.Mock(side_effect=fake_ssh_exec))
+        self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files')
+
+        self._helper.remove_exports(self.server, self.share_name)
+
+        self._helper._ssh_exec.assert_has_calls([
+            mock.call(self.server, ['sudo', 'exportfs']),
+            mock.call(self.server, ['sudo', 'exportfs', '-u',
+                                    ':'.join(['1.1.1.1', local_path])]),
+            mock.call(self.server, ['sudo', 'exportfs', '-u',
+                                    ':'.join(['2.2.2.2', local_path])]),
+        ])
+        self._helper._sync_nfs_temp_and_perm_files.assert_called_once_with(
+            self.server)
+
+    def test_remove_exports_no_active_exports(self):
+        self.mock_object(self._helper, '_ssh_exec',
+                         mock.Mock(return_value=('', '')))
+        self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files')
+
+        self._helper.remove_exports(self.server, self.share_name)
+
+        self._helper._ssh_exec.assert_called_once_with(
+            self.server, ['sudo', 'exportfs'])
+        self._helper._sync_nfs_temp_and_perm_files.assert_not_called()
+
+    def test_remove_exports_already_removed(self):
+        local_path = os.path.join(CONF.share_mount_path, self.share_name)
+        exec_result = '%s\n\t\t1.1.1.1' % local_path
+
+        def fake_ssh_exec(*args, **kwargs):
+            if args[1] == ['sudo', 'exportfs']:
+                return (exec_result, '')
+            raise exception.ProcessExecutionError(
+                stderr='could not find export')
+
+        self.mock_object(self._helper, '_ssh_exec',
+                         mock.Mock(side_effect=fake_ssh_exec))
+        self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files')
+
+        self._helper.remove_exports(self.server, self.share_name)
+
+        self._helper._sync_nfs_temp_and_perm_files.assert_called_once_with(
+            self.server)
+
 
 @ddt.ddt
 class CIFSHelperIPAccessTestCase(test.TestCase):

releasenotes/notes/fix-generic-driver-nfs-share-deletion-busy-error-4cee8e87e98fa73b.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixed a bug in the generic share driver where deleting NFS shares
+    from service VMs could fail with a 'target is busy' error. The
+    NFS exports are now properly unexported via ``exportfs -u`` before
+    the share is unmounted. Additionally, a lazy unmount fallback
+    (``umount -l``) has been added to handle cases where the NFS
+    kernel state has not fully drained at unmount time.