NAS-131208 / 25.04 / Improvements to mounting/umounting docker datase…

…ts (#14525)

* Introduce a simple service to manage docker dataset mounts

* Use docker.fs_manage service for mounting/umounting datasets

* Raise proper exception with code if ds does not exist

* Add a util method to see if some ds is mounted on ix-apps

* Umount docker datasets if docker pool is changed

* Mount docker datasets if pool is changed

* Add a sanity test to ensure correct ix-apps is mounted before trying to start docker service

* Mount docker datasets on system boot if docker is configured

* Do not mount/umount docker datasets on docker service start/stop

* Minor fixes

src/middlewared/middlewared/plugins/docker/fs_manage.py

@@ -0,0 +1,52 @@
+import errno
+
+from middlewared.service import CallError, Service
+
+from .state_utils import IX_APPS_MOUNT_PATH, Status
+
+
+class DockerFilesystemManageService(Service):
+
+    class Config:
+        namespace = 'docker.fs_manage'
+        private = True
+
+    async def common_func(self, mount):
+        if docker_ds := (await self.middleware.call('docker.config'))['dataset']:
+            try:
+                if mount:
+                    await self.middleware.call('zfs.dataset.mount', docker_ds, {'recursive': True, 'force_mount': True})
+                else:
+                    await self.middleware.call('zfs.dataset.umount', docker_ds, {'force': True})
+                await self.middleware.call('catalog.sync')
+            except Exception as e:
+                await self.middleware.call(
+                    'docker.state.set_status', Status.FAILED.value,
+                    f'Failed to {"mount" if mount else "umount"} {docker_ds!r}: {e}',
+                )
+                raise
+
+    async def mount(self):
+        await self.common_func(True)
+
+    async def umount(self):
+        await self.common_func(False)
+
+    async def ix_apps_is_mounted(self, dataset_to_check=None):
+        """
+        This will tell us if some dataset is mounted on /mnt/.ix-apps or not.
+        """
+        try:
+            fs_details = await self.middleware.call('filesystem.statfs', IX_APPS_MOUNT_PATH)
+        except CallError as e:
+            if e.errno == errno.ENOENT:
+                return False
+            raise
+
+        if fs_details['source'].startswith('boot-pool/'):
+            return False
+
+        if dataset_to_check:
+            return fs_details['source'] == dataset_to_check
+
+        return True

src/middlewared/middlewared/plugins/docker/state_management.py

@@ -106,19 +106,16 @@ async def _event_system_ready(middleware, event_type, args):
     if await middleware.call('failover.licensed'):
         return
 
-    docker_pool_configured = bool((await middleware.call('docker.config'))['pool'])
-    start_docker = True
     if (
         (await middleware.call('docker.config'))['nvidia'] and
         await middleware.call('nvidia.present') and
         not await middleware.call('nvidia.installed')
     ):
-        await middleware.call('nvidia.install', docker_pool_configured)
-        start_docker = False
+        await middleware.call('nvidia.install', False)
 
-    if docker_pool_configured:
-        if start_docker:
-            middleware.create_task(middleware.call('docker.state.start_service'))
+    if (await middleware.call('docker.config'))['pool']:
+        await middleware.call('docker.fs_manage.mount')
+        middleware.create_task(middleware.call('docker.state.start_service'))
     else:
         await middleware.call('docker.state.set_status', Status.UNCONFIGURED.value)
 

src/middlewared/middlewared/plugins/docker/state_setup.py

@@ -54,6 +54,10 @@ async def validate_fs(self):
         # This is problematic for bridge interfaces which can or cannot come up in time
         await self.validate_interfaces()
 
+        # Make sure correct ix-apps dataset is mounted
+        if not await self.middleware.call('docker.fs_manage.ix_apps_is_mounted', config['dataset']):
+            raise CallError(f'{config["dataset"]!r} dataset is not mounted on {IX_APPS_MOUNT_PATH!r}')
+
     @private
     async def validate_interfaces(self):
         default_iface, success = await self.middleware.run_in_thread(wait_for_default_interface_link_state_up)
@@ -70,6 +74,8 @@ async def status_change(self):
             return
 
         await self.create_update_docker_datasets(config['dataset'])
+        # Docker dataset would not be mounted at this point, so we will explicitly mount them now
+        await self.middleware.call('docker.fs_manage.mount')
         await self.middleware.call('docker.state.start_service')
         self.middleware.create_task(self.middleware.call('docker.state.periodic_check'))
 

src/middlewared/middlewared/plugins/docker/update.py

@@ -1,3 +1,5 @@
+import errno
+
 import middlewared.sqlalchemy as sa
 
 from middlewared.schema import accepts, Bool, Dict, Int, IPAddr, List, Patch, Str, ValidationErrors
@@ -94,6 +96,16 @@ async def do_update(self, job, data):
                 except Exception as e:
                     raise CallError(f'Failed to stop docker service: {e}')
 
+                try:
+                    await self.middleware.call('docker.fs_manage.umount')
+                except CallError as e:
+                    # We handle this specially, if for whatever reason ix-apps dataset is not there,
+                    # we don't make it fatal to change pools etc - however if some dataset other then
+                    # boot pool is mounted at ix-apps dir, then we will error out as it's a problem
+                    # and needs to be fixed before we can proceed
+                    if e.errno != errno.ENOENT or await self.middleware.call('docker.fs_manage.ix_apps_is_mounted'):
+                        raise
+
                 await self.middleware.call('docker.state.set_status', Status.UNCONFIGURED.value)
 
             await self.middleware.call('datastore.update', self._config.datastore, old_config['id'], config)

src/middlewared/middlewared/plugins/service_/services/docker.py

@@ -10,25 +10,9 @@ class DockerService(SimpleService):
     etc = ['docker']
     systemd_unit = 'docker'
 
-    async def mount_umount_ix_apps(self, mount):
-        if docker_ds := (await self.middleware.call('docker.config'))['dataset']:
-            try:
-                if mount:
-                    await self.middleware.call('zfs.dataset.mount', docker_ds, {'recursive': True, 'force_mount': True})
-                else:
-                    await self.middleware.call('zfs.dataset.umount', docker_ds, {'force': True})
-                await self.middleware.call('catalog.sync')
-            except Exception as e:
-                await self.middleware.call(
-                    'docker.state.set_status', Status.FAILED.value,
-                    f'Failed to {"mount" if mount else "umount"} {docker_ds!r}: {e}',
-                )
-                raise
-
     async def before_start(self):
         await self.middleware.call('docker.state.set_status', Status.INITIALIZING.value)
         await self.middleware.call('docker.state.before_start_check')
-        await self.mount_umount_ix_apps(True)
         await self.middleware.call('catalog.sync')
         for key, value in (
             ('vm.panic_on_oom', 0),
@@ -69,6 +53,5 @@ async def before_stop(self):
         await self.middleware.call('docker.state.set_status', Status.STOPPING.value)
 
     async def after_stop(self):
-        await self.mount_umount_ix_apps(False)
         await self.middleware.call('docker.state.set_status', Status.STOPPED.value)
         await self.middleware.call('catalog.sync')

src/middlewared/middlewared/plugins/zfs_/dataset_actions.py

@@ -8,6 +8,11 @@
 from middlewared.utils.path import is_child
 
 
+def handle_ds_not_found(error_code: int, ds_name: str):
+    if error_code == libzfs.Error.NOENT.value:
+        raise CallError(f'Dataset {ds_name!r} not found', errno.ENOENT)
+
+
 class ZFSDatasetService(Service):
 
     class Config:
@@ -69,6 +74,7 @@ def mount(self, name, options):
                     dataset.mount()
         except libzfs.ZFSException as e:
             self.logger.error('Failed to mount dataset', exc_info=True)
+            handle_ds_not_found(e.code, name)
             raise CallError(f'Failed to mount dataset: {e}')
 
     @accepts(Str('name'), Dict('options', Bool('force', default=False)))
@@ -79,6 +85,7 @@ def umount(self, name, options):
                 dataset.umount(force=options['force'])
         except libzfs.ZFSException as e:
             self.logger.error('Failed to umount dataset', exc_info=True)
+            handle_ds_not_found(e.code, name)
             raise CallError(f'Failed to umount dataset: {e}')
 
     @accepts(
@@ -96,6 +103,7 @@ def rename(self, name, options):
                 dataset.rename(options['new_name'], recursive=options['recursive'])
         except libzfs.ZFSException as e:
             self.logger.error('Failed to rename dataset', exc_info=True)
+            handle_ds_not_found(e.code, name)
             raise CallError(f'Failed to rename dataset: {e}')
 
     def promote(self, name):
@@ -105,6 +113,7 @@ def promote(self, name):
                 dataset.promote()
         except libzfs.ZFSException as e:
             self.logger.error('Failed to promote dataset', exc_info=True)
+            handle_ds_not_found(e.code, name)
             raise CallError(f'Failed to promote dataset: {e}')
 
     def inherit(self, name, prop, recursive=False):
@@ -116,6 +125,8 @@ def inherit(self, name, prop, recursive=False):
                     raise CallError(f'Property {prop!r} not found.', errno.ENOENT)
                 zprop.inherit(recursive=recursive)
         except libzfs.ZFSException as e:
+            handle_ds_not_found(e.code, name)
+
             if prop != 'mountpoint':
                 raise CallError(str(e))