NAS-130638 / 25.04 / add more default zfs props for apps internal dat…

…aset (#14247)

* more zfs properties on apps dataset

* convert to immutable global type

* fix usages

* move all logic to DATASET_DEFAULTS class

* use update_only() attribute in migrate.py

* optimize, simplify, and improve create_update_docker_datasets()

* Adapt dataset defaults class name with python conventions

* Fix dict conversion endpoint

* Fix parameters

* Fix quotes usages

* Update state utils to account for custom props

* Make sure create time props are appropriately applied

* Make sure we get update time props for datasets properly

* Handle update props when migrating k8s to docker

* Minor bug fix

---------

Co-authored-by: Waqar Ahmed <[email protected]>

src/middlewared/middlewared/plugins/apps/schema_action_context.py

@@ -3,7 +3,7 @@
 from middlewared.service import CallError, Service
 
 from .ix_apps.path import get_app_parent_volume_ds_name
-from .utils import DATASET_DEFAULTS
+from .utils import DatasetDefaults
 
 
 class AppSchemaActions(Service):
@@ -27,7 +27,7 @@ async def update_volumes(self, app_name, volumes):
         for create_ds in sorted(set(user_wants) - existing_datasets):
             await self.middleware.call(
                 'zfs.dataset.create', {
-                    'properties': user_wants[create_ds]['properties'] | DATASET_DEFAULTS,
+                    'properties': user_wants[create_ds]['properties'] | DatasetDefaults.to_dict(),
                     'name': create_ds, 'type': 'FILESYSTEM',
                 }
             )

src/middlewared/middlewared/plugins/apps/utils.py

@@ -1,7 +1,7 @@
 import os
 import subprocess
 
-from middlewared.plugins.docker.state_utils import DATASET_DEFAULTS, IX_APPS_MOUNT_PATH  # noqa
+from middlewared.plugins.docker.state_utils import DatasetDefaults, IX_APPS_MOUNT_PATH  # noqa
 
 
 PROJECT_PREFIX = 'ix-'

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

@@ -8,7 +8,7 @@
 from middlewared.service import CallError, private, Service
 
 from .state_utils import (
-    DATASET_DEFAULTS, docker_datasets, docker_dataset_custom_props, docker_dataset_update_props, IX_APPS_MOUNT_PATH,
+    DatasetDefaults, DOCKER_DATASET_NAME, docker_datasets, IX_APPS_MOUNT_PATH, IX_APPS_DIR_NAME,
     missing_required_datasets,
 )
 
@@ -61,41 +61,59 @@ async def status_change(self):
         await self.middleware.call('docker.state.start_service')
 
     @private
-    async def create_update_docker_datasets(self, docker_ds):
-        create_props_default = DATASET_DEFAULTS.copy()
-        for dataset_name in docker_datasets(docker_ds):
-            custom_props = docker_dataset_custom_props(dataset_name.split('/', 1)[-1])
-            # got custom properties, need to re-calculate
-            # the update and create props.
-            create_props = dict(create_props_default, **custom_props) if custom_props else create_props_default
-            update_props = docker_dataset_update_props(create_props)
-
-            dataset = await self.middleware.call(
-                'zfs.dataset.query', [['id', '=', dataset_name]], {
+    def move_conflicting_dir(self, ds_name):
+        base_ds_name = os.path.basename(ds_name)
+        from_path = os.path.join(IX_APPS_MOUNT_PATH, base_ds_name)
+        if ds_name == DOCKER_DATASET_NAME:
+            from_path = IX_APPS_MOUNT_PATH
+
+        with contextlib.suppress(FileNotFoundError):
+            # can't stop someone from manually creating same name
+            # directories on disk so we'll just move them
+            shutil.move(from_path, f'{from_path}-{str(uuid.uuid4())[:4]}-{datetime.now().isoformat()}')
+
+    @private
+    def create_update_docker_datasets_impl(self, docker_ds):
+        expected_docker_datasets = docker_datasets(docker_ds)
+        actual_docker_datasets = {
+            k['id']: k['properties'] for k in self.middleware.call_sync(
+                'zfs.dataset.query', [['id', 'in', expected_docker_datasets]], {
                     'extra': {
-                        'properties': list(update_props),
+                        'properties': list(DatasetDefaults.update_only(skip_ds_name_check=True).keys()),
                         'retrieve_children': False,
                         'user_properties': False,
                     }
                 }
             )
-            if not dataset:
-                base_ds_name = os.path.basename(dataset_name)
-                test_path = IX_APPS_MOUNT_PATH if base_ds_name == 'ix-apps' else os.path.join(
-                    IX_APPS_MOUNT_PATH, base_ds_name
-                )
-                with contextlib.suppress(FileNotFoundError):
-                    await self.middleware.run_in_thread(
-                        shutil.move, test_path, f'{test_path}-{str(uuid.uuid4())[:4]}-{datetime.now().isoformat()}',
+        }
+        for dataset_name in expected_docker_datasets:
+            if existing_dataset := actual_docker_datasets.get(dataset_name):
+                update_props = DatasetDefaults.update_only(os.path.basename(dataset_name))
+                if any(val['value'] != update_props[name] for name, val in existing_dataset.items()):
+                    # if any of the zfs properties don't match what we expect we'll update all properties
+                    self.middleware.call_sync(
+                        'zfs.dataset.update', dataset_name, {
+                            'properties': {k: {'value': v} for k, v in update_props.items()}
+                        }
                     )
-                await self.middleware.call(
-                    'zfs.dataset.create', {
-                        'name': dataset_name, 'type': 'FILESYSTEM', 'properties': create_props,
-                    }
-                )
-            elif any(val['value'] != update_props[name] for name, val in dataset[0]['properties'].items()):
-                await self.middleware.call(
-                    'zfs.dataset.update', dataset_name, {
-                        'properties': {k: {'value': v} for k, v in update_props.items()}
-                    }
-                )
+            else:
+                self.move_conflicting_dir(dataset_name)
+                self.middleware.call_sync('zfs.dataset.create', {
+                    'name': dataset_name, 'type': 'FILESYSTEM', 'properties': DatasetDefaults.create_time_only(
+                        os.path.basename(dataset_name)
+                    ),
+                })
+
+    @private
+    async def create_update_docker_datasets(self, docker_ds):
+        """The following logic applies:
+
+            1. create the docker datasets fresh (if they dont exist)
+            2. OR update the docker datasets zfs properties if they
+                don't match reality.
+
+            NOTE: this method needs to be optimized as much as possible
+            since this is called on docker state change for each docker
+            dataset
+        """
+        await self.middleware.run_in_thread(self.create_update_docker_datasets_impl, docker_ds)

src/middlewared/middlewared/plugins/docker/state_utils.py

@@ -1,3 +1,4 @@
+import dataclasses
 import collections
 import enum
 import os
@@ -6,18 +7,52 @@
 
 APPS_STATUS: collections.namedtuple = collections.namedtuple('Status', ['status', 'description'])
 CATALOG_DATASET_NAME: str = 'truenas_catalog'
+DOCKER_DATASET_NAME: str = 'ix-apps'
 IX_APPS_DIR_NAME = '.ix-apps'
 IX_APPS_MOUNT_PATH: str = os.path.join('/mnt', IX_APPS_DIR_NAME)
 
-DATASET_DEFAULTS: dict = {
-    'aclmode': 'discard',
-    'acltype': 'posix',
-    'exec': 'on',
-    'setuid': 'on',
-    'casesensitivity': 'sensitive',
-    'atime': 'off',
-    'canmount': 'noauto',
-}
+
+@dataclasses.dataclass(slots=True, frozen=True)
+class DatasetProp:
+    value: str
+    create_time_only: bool
+    ds_name: str | None = None
+
+
+@dataclasses.dataclass(slots=True, frozen=True)
+class DatasetDefaults:
+    aclmode: DatasetProp = DatasetProp('discard', False)
+    acltype: DatasetProp = DatasetProp('posix', False)
+    atime: DatasetProp = DatasetProp('off', False)
+    casesensitivity: DatasetProp = DatasetProp('sensitive', True)
+    canmount: DatasetProp = DatasetProp('noauto', False)
+    dedup: DatasetProp = DatasetProp('off', False)
+    encryption: DatasetProp = DatasetProp('off', True, DOCKER_DATASET_NAME)
+    exec: DatasetProp = DatasetProp('on', False)
+    mountpoint: DatasetProp = DatasetProp(f'/{IX_APPS_DIR_NAME}', True, DOCKER_DATASET_NAME)
+    normalization: DatasetProp = DatasetProp('none', True)
+    overlay: DatasetProp = DatasetProp('on', False)
+    setuid: DatasetProp = DatasetProp('on', False)
+    snapdir: DatasetProp = DatasetProp('hidden', False)
+    xattr: DatasetProp = DatasetProp('sa', False)
+
+    @classmethod
+    def to_dict(cls):
+        return {k: v['value'] for k, v in dataclasses.asdict(cls()).items()}
+
+    @classmethod
+    def create_time_only(cls, ds_name: str | None = None):
+        return {
+            k: v['value'] for k, v in dataclasses.asdict(cls()).items()
+            if v['create_time_only'] and v['ds_name'] in (ds_name, None)
+        }
+
+    @classmethod
+    def update_only(cls, ds_name: str | None = None, skip_ds_name_check: bool = False):
+        return {
+            k: v['value'] for k, v in dataclasses.asdict(cls()).items()
+            if v['create_time_only'] is False and (skip_ds_name_check or v['ds_name'] in (ds_name, None))
+        }
 
 
 class Status(enum.Enum):

src/middlewared/middlewared/plugins/kubernetes_to_docker/migrate.py

@@ -2,7 +2,7 @@
 import shutil
 
 from middlewared.plugins.apps.ix_apps.path import get_app_parent_volume_ds_name, get_installed_app_path
-from middlewared.plugins.docker.state_utils import DATASET_DEFAULTS
+from middlewared.plugins.docker.state_utils import DatasetDefaults
 from middlewared.schema import accepts, Bool, Dict, List, returns, Str
 from middlewared.service import CallError, job, Service
 
@@ -181,9 +181,7 @@ def migrate(self, job, kubernetes_pool, options):
                     self.middleware.call_sync('zfs.snapshot.clone', {
                         'snapshot': snapshot,
                         'dataset_dst': destination_ds,
-                        'dataset_properties': {
-                            k: v for k, v in DATASET_DEFAULTS.items() if k not in ['casesensitivity']
-                        },
+                        'dataset_properties': DatasetDefaults.update_only(os.path.basename(destination_ds)),
                     })
                     self.middleware.call_sync('zfs.dataset.promote', destination_ds)
                     self.middleware.call_sync('zfs.dataset.mount', destination_ds)