NAS-128399 / 24.10 / More nfs failover fixes

src/freenas/etc/systemd/system/nfsdcld.service.d/override.conf

@@ -0,0 +1,3 @@
+[Unit]
+BindTo=nfs-server.service
+

src/middlewared/middlewared/etc_files/nfs.conf.d/local.conf.mako

@@ -1,4 +1,8 @@
 <%
+    from middlewared.plugins.nfs import NFSPath
+    state_path = NFSPath.STATEDIR.platform()
+    cld_storedir = NFSPath.CLDDIR.platform()
+    cltrack_storedir = NFSPath.CLDTRKDIR.platform()
     config = render_ctx["nfs.config"]
 
     # Fail-safe setting is two nfsd
@@ -28,14 +32,23 @@ vers4 = n
 host = ${','.join(config['bindip'])}
 % endif
 
+[exportd]
+state-directory-path = ${state_path}
+[nfsdcld]
+storagedir = ${cld_storedir}
+[nfsdcltrack]
+storagedir = ${cltrack_storedir}
+
 [mountd]
+state-directory-path = ${state_path}
 threads = ${num_mountd}
 % if config['mountd_port']:
 port = ${config['mountd_port']}
 % endif
 manage-gids = ${manage_gids}
 
 [statd]
+state-directory-path = ${state_path}
 % if config['rpcstatd_port']:
 port = ${config['rpcstatd_port']}
 % endif

src/middlewared/middlewared/plugins/nfs.py

@@ -2,8 +2,9 @@
 import errno
 import ipaddress
 import itertools
-import os.path
+import os
 import pathlib
+import shutil
 
 from middlewared.common.attachment import LockableFSAttachmentDelegate
 from middlewared.common.listen import SystemServiceListenMultipleDelegate
@@ -15,6 +16,30 @@
 import middlewared.sqlalchemy as sa
 from middlewared.utils.asyncio_ import asyncio_map
 from middlewared.plugins.nfs_.utils import get_domain, leftmost_has_wildcards, get_wildcard_domain
+from middlewared.plugins.system_dataset.utils import SYSDATASET_PATH
+
+
+class NFSPath(enum.Enum):
+    # nfs conf sections that use STATEDIR: exportd, mountd, statd
+    STATEDIR = (os.path.join(SYSDATASET_PATH, 'nfs'), 0o755, True, {'uid': 0, 'gid': 0})
+    CLDDIR = (os.path.join(SYSDATASET_PATH, 'nfs', 'nfsdcld'), 0o700, True, {'uid': 0, 'gid': 0})
+    CLDTRKDIR = (os.path.join(SYSDATASET_PATH, 'nfs', 'nfsdcltrack'), 0o700, True, {'uid': 0, 'gid': 0})
+    # Fix up the uid and gid in setup_directories
+    SMDIR = (os.path.join(SYSDATASET_PATH, 'nfs', 'sm'), 0o755, True, {'uid': 'statd', 'gid': 'nogroup'})
+    SMBAKDIR = (os.path.join(SYSDATASET_PATH, 'nfs', 'sm.bak'), 0o755, True, {'uid': 'statd', 'gid': 'nogroup'})
+    V4RECOVERYDIR = (os.path.join(SYSDATASET_PATH, 'nfs', 'v4recovery'), 0o755, True, {'uid': 0, 'gid': 0})
+
+    def platform(self):
+        return self.value[0]
+
+    def mode(self):
+        return self.value[1]
+
+    def is_dir(self):
+        return self.value[2]
+
+    def owner(self):
+        return self.value[3]
 
 
 class NFSProtocol(str, enum.Enum):
@@ -77,6 +102,67 @@ class Config:
         Bool('managed_nfsd', default=True),
     )
 
+    @private
+    def setup_directories(self):
+        '''
+        We are moving the NFS state directory from /var/lib/nfs to
+        the system dataset: /var/db/system/nfs.
+        When setup_directories is called /var/db/system/nfs is expected to exist.
+
+        If STATEDIR is empty, then this might be an initialization
+        and there might be current info in /var/lib/nfs.
+
+        We always make sure the expected directories are present
+        '''
+        def name_to_id_conversion(usrgrp):
+            ''' Accept [usr,grp], return [usr,grp] as IDs '''
+            if any(isinstance(owner, str) for owner in usrgrp):
+                # Convert from name to ID, then apply
+                try:
+                    usrgrp[0] = self.middleware.call_sync('user.get_builtin_user_id', usrgrp[0])
+                    usrgrp[1] = self.middleware.call_sync('group.get_builtin_group_id', usrgrp[1])
+                except Exception as e:
+                    self.logger.error(
+                        "%s: Failed to apply ownership. usr=%r, grp=%r %r", usrgrp[0], usrgrp[1], e
+                    )
+            return usrgrp
+
+        # Initialize the system dataset NFS state directory
+        try:
+            if not any(os.scandir(NFSPath.STATEDIR.platform())):
+                # System db is empty, populate it with contents of /var/lib/nfs
+                # Going forward, the system dataset should hold the NFS state data
+                if not os.path.isdir('/var/lib/nfs'):
+                    try:
+                        shutil.copytree('/var/lib/nfs', NFSPath.STATEDIR.platform())
+                    except Exception as e:
+                        self.logger.error('Failed to initialize NFS state from /var/lib/nfs: %r', e)
+                    # Continue anyway.
+        except Exception as e:
+            self.logger.error("Could not find required path: %r", e)
+
+        # Make sure we have the necessary directories
+        for p in NFSPath:
+            path = p.platform()
+            usrgrp = name_to_id_conversion([p.owner().get('uid'), p.owner().get('gid')])
+            try:
+                os.chmod(path, p.mode())
+                stat = os.stat(path)
+                if (stat.st_uid != usrgrp[0]) or (stat.st_gid != usrgrp[1]):
+                    self.logger.info("%s: Fix up owner for path", path)
+                    os.chown(path, usrgrp[0], usrgrp[1])
+            except FileNotFoundError:
+                if p.is_dir():
+                    os.mkdir(path, p.mode())
+                    os.chown(path, usrgrp[0], usrgrp[1])
+
+        if os.path.exists('/proc/fs/nfsd/nfsv4recoverydir'):
+            try:
+                with open('/proc/fs/nfsd/nfsv4recoverydir', 'r+') as fp:
+                    fp.write(NFSPath.V4RECOVERYDIR.platform() + '\n')
+            except Exception as e:
+                self.logger.error("Failed to update nfsv4recoverydir: %r", str(e))
+
     @private
     async def nfs_extend(self, nfs):
         keytab_has_nfs = await self.middleware.call("kerberos.keytab.has_nfs_principal")

src/middlewared/middlewared/plugins/nfs_/status.py

@@ -1,3 +1,4 @@
+from middlewared.plugins.nfs import NFSPath
 from middlewared.schema import accepts, Int, returns, Str, Dict
 from middlewared.service import Service, private, filterable, filterable_returns
 from middlewared.utils import filter_list
@@ -18,7 +19,7 @@ def get_rmtab(self):
         """
         entries = []
         with suppress(FileNotFoundError):
-            with open("/var/lib/nfs/rmtab", "r") as f:
+            with open(os.path.join(NFSPath.STATEDIR.platform(), "rmtab"), "r") as f:
                 for line in f:
                     ip, data = line.split(":", 1)
                     export, refcnt = line.rsplit(":", 1)

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

@@ -6,6 +6,7 @@
 class NFSService(SimpleService):
     name = "nfs"
     reloadable = True
+    systemd_unit_timeout = 10
 
     etc = ["nfsd"]
 

src/middlewared/middlewared/plugins/system_dataset/hierarchy.py

@@ -106,6 +106,25 @@ def get_system_dataset_spec(pool_name: str, uuid: str) -> list:
                 'mode': 0o775,
             },
         },
+        {
+            'name': os.path.join(pool_name, '.system/nfs'),
+            'props': {
+                'mountpoint': 'legacy',
+                'readonly': 'off',
+                'snapdir': 'hidden',
+            },
+            'chown_config': {
+                'uid': 0,
+                'gid': 0,
+                'mode': 0o755,
+            },
+            'post_mount_actions': [
+                {
+                    'method': 'nfs.setup_directories',
+                    'args': [],
+                }
+            ]
+        },
         {
             'name': os.path.join(pool_name, '.system/samba4'),
             'props': {

tests/api2/test_001_ssh.py

@@ -8,7 +8,7 @@
 
 from functions import if_key_listed, SSH_TEST
 from auto_config import ip, sshKey, user, password
-from middlewared.test.integration.utils import call, fail
+from middlewared.test.integration.utils import fail
 from middlewared.test.integration.utils.client import client
 
 
@@ -34,6 +34,7 @@ def test_002_firstboot_checks(ws_client):
     expected_ds = [
         'boot-pool/.system',
         'boot-pool/.system/cores',
+        'boot-pool/.system/nfs',
         'boot-pool/.system/samba4',
         'boot-pool/grub'
     ]
@@ -50,8 +51,8 @@ def test_002_firstboot_checks(ws_client):
     # always start in all circumstances (even if there is an invalid (or empty) config)
     ignore = ('smartd',)
     for srv in filter(lambda x: x['service'] not in ignore, ws_client.call('service.query')):
-        assert srv['enable'] is False
-        assert srv['state'] == 'STOPPED'
+        assert srv['enable'] is False, f"{srv['service']} service is unexpectedly enabled"
+        assert srv['state'] == 'STOPPED', f"{srv['service']} service expected STOPPED, but found {srv['state']}"
 
     # verify posix mode, uid and gid for standard users
     stat_info = {

tests/api2/test_300_nfs.py

@@ -38,7 +38,14 @@
 conf_file = {
     "nfs": {
         "pname": "/etc/nfs.conf.d/local.conf",
-        "sections": {'nfsd': {}, 'mountd': {}, 'statd': {}, 'lockd': {}}
+        "sections": {
+            'nfsd': {},
+            'exportd': {},
+            'nfsdcld': {},
+            'nfsdcltrack': {},
+            'mountd': {},
+            'statd': {},
+            'lockd': {}}
     },
     "idmapd": {
         "pname": "/etc/idmapd.conf",
@@ -134,6 +141,11 @@ def parse_rpcbind_config():
     return rv
 
 
+def get_nfs_service_state():
+    nfs_service = call('service.query', [['service', '=', 'nfs']], {'get': True})
+    return nfs_service['state']
+
+
 def set_nfs_service_state(do_what=None, expect_to_pass=True, fail_check=None):
     '''
     Start or Stop NFS service
@@ -321,21 +333,29 @@ def nfs_config(options=None):
 
 
 # Enable NFS server
-def test_01_creating_the_nfs_server():
+def test_01_init_the_nfs_config():
     # initialize default_nfs_config for later restore
     save_nfs_config()
 
+    # Confirm NFS is not running
+    nfs_state = get_nfs_service_state()
+    assert nfs_state == 'STOPPED', f'Before update, expected NFS to be STOPPED, but found {nfs_state}'
+
     payload = {
-        "servers": 10,
         "mountd_port": 618,
         "allow_nonroot": False,
         "rpcstatd_port": 871,
         "rpclockd_port": 32803,
         "protocols": ["NFSV3", "NFSV4"]
     }
-    results = PUT("/nfs/", payload)
-    assert results.status_code == 200, results.text
-    # The service is not yet enabled, so we cannot yet confirm the settings
+    nfs_conf = call("nfs.update", payload)
+    assert nfs_conf['mountd_port'] == 618
+    assert nfs_conf['rpcstatd_port'] == 871
+    assert nfs_conf['rpclockd_port'] == 32803
+
+    # Confirm NFS remains not running
+    nfs_state = get_nfs_service_state()
+    assert nfs_state == 'STOPPED', f'After update, xpected NFS to be STOPPED, but found {nfs_state}'
 
 
 @pytest.mark.dependency(name='NFS_DATASET_CREATED')
@@ -396,8 +416,33 @@ def test_09_checking_to_see_if_nfs_service_is_running(request):
     assert results.json()[0]["state"] == "RUNNING", results.text
 
 
+def test_10_confirm_state_directory(request):
+    """
+    By default, the NFS state directory is at /var/lib/nfs.
+    To support HA systems, we moved this to the system dataset
+    at /var/db/system/nfs.  In support of this we updated the
+    NFS conf file settings
+    """
+    depends(request, ["NFS_SERVICE_STARTED"], scope="session")
+
+    # Make sure the conf file has the expected settings
+    nfs_state_dir = '/var/db/system/nfs'
+    s = parse_server_config()
+    assert s['exportd']['state-directory-path'] == nfs_state_dir, str(s)
+    assert s['nfsdcld']['storagedir'] == os.path.join(nfs_state_dir, 'nfsdcld'), str(s)
+    assert s['nfsdcltrack']['storagedir'] == os.path.join(nfs_state_dir, 'nfsdcltrack'), str(s)
+    assert s['nfsdcld']['storagedir'] == os.path.join(nfs_state_dir, 'nfsdcld'), str(s)
+    assert s['mountd']['state-directory-path'] == nfs_state_dir, str(s)
+    assert s['statd']['state-directory-path'] == nfs_state_dir, str(s)
+    # Confirm we have the mount point in the system dataset
+    # ----------------------------------------------------------------------
+    # NOTE: Update test_001_ssh.py: test_002_first_boot_checks.
+    # NOTE: Test fresh-install and upgrade.
+    # ----------------------------------------------------------------------
+
+
 @pytest.mark.parametrize('vers', [3, 4])
-def test_10_perform_basic_nfs_ops(request, vers):
+def test_11_perform_basic_nfs_ops(request, vers):
     with SSH_NFS(ip, NFS_PATH, vers=vers, user=user, password=password, ip=ip) as n:
         n.create('testfile')
         n.mkdir('testdir')
@@ -412,7 +457,7 @@ def test_10_perform_basic_nfs_ops(request, vers):
         assert 'testfile' not in contents
 
 
-def test_11_perform_server_side_copy(request):
+def test_12_perform_server_side_copy(request):
     with SSH_NFS(ip, NFS_PATH, vers=4, user=user, password=password, ip=ip) as n:
         n.server_side_copy('ssc1', 'ssc2')
 
@@ -1208,6 +1253,7 @@ def test_43_check_nfsv4_acl_support(request):
     4) For NFSv4.1 or NFSv4.2, repeat same process for each of the
        supported acl_flags.
     """
+    depends(request, ["NFS_SERVICE_STARTED"], scope="session")
     acl_nfs_path = f'/mnt/{pool_name}/test_nfs4_acl'
     test_perms = {
         "READ_DATA": True,