curtin-dev team mailing list archive

[Dan Bungert <daniel.bungert@xxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:fs_pass_one into curtin:master.

Commit message:
Output a 1 for fstab lines if not overridden                                                                                   
                                                                                                                               
Change the default passno to 1.                                                                                                
Also document that passno/freq can be set in the yaml.                                                                         
                                                                                                                               
LP: #1717584, LP: #1785354                                                                                                     

Requested reviews:
  Michael Hudson-Doyle (mwhudson)
  Ryan Harper (raharper)
  Server Team CI bot (server-team-bot): continuous-integration

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/400005

Assign a default fstab passno of 1.
-- 
Your team curtin developers is subscribed to branch curtin:master.

diff --git a/curtin/__init__.py b/curtin/__init__.py
index 0178a7b..ea7d1ee 100644
--- a/curtin/__init__.py
+++ b/curtin/__init__.py
@@ -36,6 +36,8 @@ FEATURES = [
     'HAS_VERSION_MODULE',
     # uefi_reoder has fallback support if BootCurrent is missing
     'UEFI_REORDER_FALLBACK_SUPPORT',
+    # fstabs by default are output with passno = 1
+    'FSTAB_DEFAULT_FSCK_ON'
 ]
 
 __version__ = "21.2"
diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
index 4dc2f0a..7e8e324 100644
--- a/curtin/block/schemas.py
+++ b/curtin/block/schemas.py
@@ -262,6 +262,10 @@ MOUNT = {
             ],
         },
         'spec': {'type': 'string'},  # XXX: Tighten this to fstab fs_spec
+        'freq': {'type': ['integer', 'string'],
+                 'pattern': r'[0-9]'},
+        'passno': {'type': ['integer', 'string'],
+                   'pattern': r'[0-9]'},
     },
 }
 PARTITION = {
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index cf6bc02..f419826 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -27,7 +27,7 @@ import time
 FstabData = namedtuple(
     "FstabData", ('spec', 'path', 'fstype', 'options', 'freq', 'passno',
                   'device'))
-FstabData.__new__.__defaults__ = (None, None, None, "", "0", "0", None)
+FstabData.__new__.__defaults__ = (None, None, None, "", "0", "1", None)
 
 
 SIMPLE = 'simple'
@@ -1020,7 +1020,7 @@ def mount_data(info, storage_config):
     fstype = info.get('fstype')
     path = info.get('path')
     freq = str(info.get('freq', 0))
-    passno = str(info.get('passno', 0))
+    passno = str(info.get('passno', 1))
 
     # turn empty options into "defaults", which works in fstab and mount -o.
     if not info.get('options'):
diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
index 75c5537..11db7a5 100644
--- a/doc/topics/storage.rst
+++ b/doc/topics/storage.rst
@@ -542,6 +542,11 @@ One of ``device`` or ``spec`` must be present.
   fstab entry will contain ``_netdev`` to indicate networking is
   required to mount this filesystem.
 
+**freq**: *<dump(8) integer from 0-9 inclusive>*
+
+The ``freq`` key refers to the freq as defined in dump(8).
+Defaults to ``0`` if unspecified.
+
 **fstype**: *<fileystem type>*
 
 ``fstype`` is only required if ``device`` is not present.  It indicates
@@ -552,7 +557,7 @@ to ``/etc/fstab``
 
 The ``options`` key will replace the default options value of ``defaults``.
 
-.. warning:: 
+.. warning::
   The kernel and user-space utilities may differ between the install
   environment and the runtime environment.  Not all kernels and user-space
   combinations will support all options.  Providing options for a mount point
@@ -565,6 +570,13 @@ The ``options`` key will replace the default options value of ``defaults``.
   If either of the environments (install or target) do not have support for
   the provided options, the behavior is undefined.
 
+**passno**: *<fsck(8) non-negative integer, typically 0-2>*
+
+The ``passno`` key refers to the fs_passno as defined in fsck(8).
+If unspecified, ``curtin`` will default to 1.
+Note that per systemd-fstab-generator(8), systemd interprets passno as a
+boolean.
+
 **spec**: *<fs_spec>*
 
 The ``spec`` attribute defines the fsspec as defined in fstab(5).
@@ -596,7 +608,7 @@ Below is an example of configuring a bind mount.
 
 That would result in a fstab entry like::
 
-  /my/bind-over-var-lib /var/lib none bind 0 0
+  /my/bind-over-var-lib /var/lib none bind 0 1
 
 **Tmpfs Mount**
 
@@ -613,7 +625,7 @@ Below is an example of configuring a tmpfsbind mount.
 
 That would result in a fstab entry like::
 
-  none /my/tmpfs tmpfs size=4194304 0 0
+  none /my/tmpfs tmpfs size=4194304 0 1
 
 
 Lvm Volgroup Command
diff --git a/examples/tests/filesystem_battery.yaml b/examples/tests/filesystem_battery.yaml
index 8166360..406a087 100644
--- a/examples/tests/filesystem_battery.yaml
+++ b/examples/tests/filesystem_battery.yaml
@@ -111,15 +111,18 @@ storage:
       spec: "none"
       path: "/my/ramfs"
       fstype: "ramfs"
+      passno: 0
     - id: bind1
       fstype: "none"
       options: "bind"
       path: "/var/cache"
       spec: "/my/bind-over-var-cache"
       type: mount
+      freq: 3
     - id: bind2
       fstype: "none"
       options: "bind,ro"
       path: "/my/bind-ro-etc"
       spec: "/etc"
       type: mount
+      freq: 1
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 8cfd6af..534cce5 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -361,7 +361,7 @@ class TestBlockMeta(CiTestCase):
         block_meta.mount_handler(mount_info, self.storage_config)
         options = 'defaults'
         comment = "# / was on /wark/xxx during curtin installation"
-        expected = "%s\n%s %s %s %s 0 0\n" % (comment,
+        expected = "%s\n%s %s %s %s 0 1\n" % (comment,
                                               disk_info['path'],
                                               mount_info['path'],
                                               fs_info['fstype'], options)
@@ -389,7 +389,7 @@ class TestBlockMeta(CiTestCase):
         block_meta.mount_handler(mount_info, self.storage_config)
         options = 'ro'
         comment = "# /readonly was on /wark/xxx during curtin installation"
-        expected = "%s\n%s %s %s %s 0 0\n" % (comment,
+        expected = "%s\n%s %s %s %s 0 1\n" % (comment,
                                               disk_info['path'],
                                               mount_info['path'],
                                               fs_info['fstype'], options)
@@ -418,7 +418,7 @@ class TestBlockMeta(CiTestCase):
         block_meta.mount_handler(mount_info, self.storage_config)
         options = 'defaults'
         comment = "# /readonly was on /wark/xxx during curtin installation"
-        expected = "%s\n%s %s %s %s 0 0\n" % (comment,
+        expected = "%s\n%s %s %s %s 0 1\n" % (comment,
                                               disk_info['path'],
                                               mount_info['path'],
                                               fs_info['fstype'], options)
@@ -449,7 +449,7 @@ class TestBlockMeta(CiTestCase):
         block_meta.mount_handler(mount_info, self.storage_config)
         options = 'defaults'
         comment = "# /readonly was on /wark/xxx during curtin installation"
-        expected = "#curtin-test\n%s\n%s %s %s %s 0 0\n" % (comment,
+        expected = "#curtin-test\n%s\n%s %s %s %s 0 1\n" % (comment,
                                                             disk_info['path'],
                                                             mount_info['path'],
                                                             fs_info['fstype'],
@@ -610,7 +610,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec="none", fstype="tmpfs", path="/tmpfs",
-                options="defaults", freq="0", passno="0", device=None),
+                options="defaults", freq="0", device=None),
             block_meta.mount_data(info, {'xm1': info}))
 
     @patch('curtin.block.iscsi.volpath_is_iscsi')
@@ -625,7 +625,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec=None, fstype="ext4", path="/",
-                options="noatime", freq="0", passno="0", device="/dev/xda1"),
+                options="noatime", freq="0", device="/dev/xda1"),
             block_meta.mount_data(scfg['m1'], scfg))
 
     @patch('curtin.block.iscsi.volpath_is_iscsi', return_value=False)
@@ -643,7 +643,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec=None, fstype="vfat", path="/boot/efi",
-                options="defaults", freq="0", passno="0", device="/dev/xda1"),
+                options="defaults", freq="0", device="/dev/xda1"),
             block_meta.mount_data(scfg['m1'], scfg))
 
     @patch('curtin.block.iscsi.volpath_is_iscsi')
@@ -657,7 +657,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec=None, fstype="ext4", path="/",
-                options="noatime,_netdev", freq="0", passno="0",
+                options="noatime,_netdev", freq="0",
                 device="/dev/xda1"),
             block_meta.mount_data(scfg['m1'], scfg))
 
@@ -683,7 +683,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec=myspec, fstype="ext3", path="/",
-                options="noatime", freq="0", passno="0",
+                options="noatime", freq="0",
                 device=None),
             block_meta.mount_data(mnt, scfg))
 
@@ -737,7 +737,7 @@ class TestFstabData(CiTestCase):
         fdata = block_meta.FstabData(
             spec="/dev/disk2", path="none", fstype='swap')
         self.assertEqual(
-            ["/dev/disk2", "none", "swap", "sw", "0", "0"],
+            ["/dev/disk2", "none", "swap", "sw", "0", "1"],
             block_meta.fstab_line_for_data(fdata).split())
 
     def test_fstab_line_for_data_swap_no_path(self):
@@ -745,7 +745,7 @@ class TestFstabData(CiTestCase):
         fdata = block_meta.FstabData(
             spec="/dev/disk2", path=None, fstype='swap')
         self.assertEqual(
-            ["/dev/disk2", "none", "swap", "sw", "0", "0"],
+            ["/dev/disk2", "none", "swap", "sw", "0", "1"],
             block_meta.fstab_line_for_data(fdata).split())
 
     def test_fstab_line_for_data_not_swap_and_no_path(self):
@@ -761,15 +761,53 @@ class TestFstabData(CiTestCase):
             spec="/dev/disk2", path="/mnt", fstype='btrfs', options='noatime')
         lines = block_meta.fstab_line_for_data(fdata).splitlines()
         self.assertEqual(
-            ["/dev/disk2", "/mnt", "btrfs", "noatime", "0", "0"],
+            ["/dev/disk2", "/mnt", "btrfs", "noatime", "0", "1"],
+            lines[1].split())
+
+    def test_fstab_line_root_and_no_passno(self):
+        """fstab_line_for_data passno autoselect for /."""
+        fdata = block_meta.FstabData(
+            spec="/dev/disk2", path="/", fstype='btrfs', options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/", "btrfs", "noatime", "0", "1"],
+            lines[1].split())
+
+    def test_fstab_line_boot_and_no_passno(self):
+        """fstab_line_for_data passno autoselect for /boot."""
+        fdata = block_meta.FstabData(
+            spec="/dev/disk2", path="/boot", fstype='btrfs', options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/boot", "btrfs", "noatime", "0", "1"],
+            lines[1].split())
+
+    def test_fstab_line_boot_efi_and_no_passno(self):
+        """fstab_line_for_data passno autoselect for /boot/efi."""
+        fdata = block_meta.FstabData(
+            spec="/dev/disk2", path="/boot/efi", fstype='btrfs',
+            options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/boot/efi", "btrfs", "noatime", "0", "1"],
+            lines[1].split())
+
+    def test_fstab_line_almost_boot(self):
+        """fstab_line_for_data passno that pretends to be /boot."""
+        fdata = block_meta.FstabData(
+            spec="/dev/disk2", path="/boots", fstype='btrfs',
+            options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/boots", "btrfs", "noatime", "0", "1"],
             lines[1].split())
 
     def test_fstab_line_for_data_with_passno_and_freq(self):
         """fstab_line_for_data should respect passno and freq."""
         fdata = block_meta.FstabData(
-            spec="/dev/d1", path="/mnt", fstype='ext4', freq="1", passno="2")
+            spec="/dev/d1", path="/mnt", fstype='ext4', freq="1", passno="1")
         lines = block_meta.fstab_line_for_data(fdata).splitlines()
-        self.assertEqual(["1", "2"], lines[1].split()[4:6])
+        self.assertEqual(["1", "1"], lines[1].split()[4:6])
 
     def test_fstab_line_for_data_raises_error_without_spec_or_device(self):
         """fstab_line_for_data should raise ValueError if no spec or device."""
@@ -797,7 +835,7 @@ class TestFstabData(CiTestCase):
             "# /mnt was on /dev/disk2 during curtin installation",
             lines[0])
         self.assertEqual(
-            [by_uuid, "/mnt", "ext4", "defaults", "0", "0"],
+            [by_uuid, "/mnt", "ext4", "defaults", "0", "1"],
             lines[1].split())
         self.assertEqual(1, m_uinfo.call_count)
         self.assertEqual(1, m_vol_type.call_count)
@@ -819,7 +857,7 @@ class TestFstabData(CiTestCase):
             "# /mnt was on /dev/disk2 during curtin installation",
             lines[0])
         self.assertEqual(
-            ["/dev/disk2", "/mnt", "ext4", "defaults", "0", "0"],
+            ["/dev/disk2", "/mnt", "ext4", "defaults", "0", "1"],
             lines[1].split())
         self.assertEqual(1, m_uinfo.call_count)
         self.assertEqual(1, m_vol_type.call_count)
@@ -837,7 +875,7 @@ class TestFstabData(CiTestCase):
             '# /mnt was on /dev/xvda1 during curtin installation',
             lines[0])
         self.assertEqual(
-            ["/dev/xvda1", "/mnt", "ext4", "defaults", "0", "0"],
+            ["/dev/xvda1", "/mnt", "ext4", "defaults", "0", "1"],
             lines[1].split())
         self.assertEqual(0, m_get_uuid.call_count)
 
diff --git a/tests/unittests/test_feature.py b/tests/unittests/test_feature.py
index 8690ad8..963082b 100644
--- a/tests/unittests/test_feature.py
+++ b/tests/unittests/test_feature.py
@@ -30,4 +30,7 @@ class TestExportsFeatures(CiTestCase):
     def test_has_uefi_reorder_fallback_support(self):
         self.assertIn('UEFI_REORDER_FALLBACK_SUPPORT', curtin.FEATURES)
 
+    def test_has_fstab_default_fsck_on(self):
+        self.assertIn('FSTAB_DEFAULT_FSCK_ON', curtin.FEATURES)
+
 # vi: ts=4 expandtab syntax=python
diff --git a/tests/vmtests/test_fs_battery.py b/tests/vmtests/test_fs_battery.py
index 7d7b494..16b8233 100644
--- a/tests/vmtests/test_fs_battery.py
+++ b/tests/vmtests/test_fs_battery.py
@@ -159,10 +159,10 @@ class TestFsBattery(VMBaseClass):
     def test_fstab_has_mounts(self):
         """Verify each of the expected "my" mounts got into fstab."""
         expected = [
-            "none /my/tmpfs tmpfs size=4194304 0 0".split(),
+            "none /my/tmpfs tmpfs size=4194304 0 1".split(),
             "none /my/ramfs ramfs defaults 0 0".split(),
-            "/my/bind-over-var-cache /var/cache none bind 0 0".split(),
-            "/etc /my/bind-ro-etc none bind,ro 0 0".split(),
+            "/my/bind-over-var-cache /var/cache none bind 3 1".split(),
+            "/etc /my/bind-ro-etc none bind,ro 1 1".split(),
         ]
         fstab_found = [
             line.split() for line in self.load_collect_file(