cc_disk_setup: pass options in correct order to utils (#1829)

When testing cc_disk_setup it failed with the following error:

  Unexpected error while running command.
  Command: ['/sbin/mkfs.ext4', '/dev/sdc1', '-L', 'disk3-fs2']
  Exit code: 1
  Reason: -
  Stdout:
  Stderr: mke2fs 1.46.5 (30-Dec-2021)
          mkfs.ext4: invalid blocks '-L' on device '/dev/sdc1'

The manpages for mkfs.ext4, mkfs.xfs, and mkswap all indicate that
options should be passed *before* the device name but cc_disk_setup
passed them after the device name - in the case of mkfx.ext4 a
"fs-size" can be passed after the device and that is what the
"-L disk3-fs2" option is being misintepreted as.

This PR ensures that the device name is passed last. The underlying
issue appears to be due to a different in behaviour between glibc and
musl where glibc "helps" applications by re-ordered command-line
parameters by musl does not[1] as it sticks to POSIX spec.

This PR also modifies 2 testcases to cater for this change in the code,
adds a note to disk_setup to clarify that when creating a swap partition
a fs_entry also needs to be specified so that mkswap is run, adds to the
examples how to specify a non-default partition type (i.e. for swap),
and modifies the description for disk_setup to clarify this.

[1] https://wiki.musl-libc.org/functional-differences-from-glibc.html#Miscellaneous_functions_with_GNU_quirks

3 files changed, 21 insertions, 4 deletions

diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py
index 1a8dd257..71d52d3d 100644
--- a/cloudinit/config/cc_disk_setup.py
+++ b/cloudinit/config/cc_disk_setup.py
@@ -39,6 +39,11 @@ This module is able to configure simple partition tables and filesystems.
     for more detail about configuration options for disk setup, see the disk
     setup example
 
+.. note::
+    if a swap partition is being created via ``disk_setup`` then a ``fs_entry``
+    entry is also needed in order for mkswap to be run, otherwise when swap
+    activation is later attempted it will fail.
+
 For convenience, aliases can be specified for disks using the
 ``device_aliases`` config key, which takes a dictionary of alias: path
 mappings. There are automatic aliases for ``swap`` and ``ephemeral<X>``, where
@@ -65,11 +70,16 @@ meta: MetaSchema = {
             """\
             device_aliases:
               my_alias: /dev/sdb
+              swap_disk: /dev/sdc
             disk_setup:
               my_alias:
                 table_type: gpt
                 layout: [50, 50]
                 overwrite: true
+              swap_disk:
+                table_type: gpt
+                layout: [[100, 82]]
+                overwrite: true
             fs_setup:
             - label: fs1
               filesystem: ext4
@@ -78,9 +88,13 @@ meta: MetaSchema = {
             - label: fs2
               device: my_alias.2
               filesystem: ext4
+            - label: swap
+              device: swap_disk.1
+              filesystem: swap
             mounts:
             - ["my_alias.1", "/mnt1"]
             - ["my_alias.2", "/mnt2"]
+            - ["swap_disk.1", "none", "swap", "sw", "0", "0"]
             """
         )
     ],
@@ -1032,6 +1046,7 @@ def mkfs(fs_cfg):
         # Find the mkfs command
         mkfs_cmd = subp.which("mkfs.%s" % fs_type)
         if not mkfs_cmd:
+            # for "mkswap"
             mkfs_cmd = subp.which("mk%s" % fs_type)
 
         if not mkfs_cmd:
@@ -1042,7 +1057,7 @@ def mkfs(fs_cfg):
             )
             return
 
-        fs_cmd = [mkfs_cmd, device]
+        fs_cmd = [mkfs_cmd]
 
         if label:
             fs_cmd.extend(["-L", label])
@@ -1057,6 +1072,8 @@ def mkfs(fs_cfg):
         if fs_opts:
             fs_cmd.extend(fs_opts)
 
+        fs_cmd.append(device)
+
     LOG.debug("Creating file system %s on %s", label, device)
     LOG.debug("     Using cmd: %s", str(fs_cmd))
     try:
diff --git a/cloudinit/config/schemas/schema-cloud-config-v1.json b/cloudinit/config/schemas/schema-cloud-config-v1.json
index e4074306..a91dc482 100644
--- a/cloudinit/config/schemas/schema-cloud-config-v1.json
+++ b/cloudinit/config/schemas/schema-cloud-config-v1.json
@@ -1153,7 +1153,7 @@
                       }
                     }
                   ],
-                  "description": "If set to ``true``, a single partition using all the space on the device will be created. If set to ``false``, no partitions will be created. If set to ``remove``, any existing partition table will be purged. Partitions can be specified by providing a list to ``layout``, where each entry in the list is either a size or a list containing a size and the numerical value for a partition type. The size for partitions is specified in **percentage** of disk space, not in bytes (e.g. a size of 33 would take up 1/3 of the disk space). Default: ``false``."
+                  "description": "If set to ``true``, a single partition using all the space on the device will be created. If set to ``false``, no partitions will be created. If set to ``remove``, any existing partition table will be purged. Partitions can be specified by providing a list to ``layout``, where each entry in the list is either a size or a list containing a size and the numerical value for a partition type. The size for partitions is specified in **percentage** of disk space, not in bytes (e.g. a size of 33 would take up 1/3 of the disk space). The partition type defaults to '83' (Linux partition), for other types of partition, such as Linux swap, the type must be passed as part of a list along with the size. Default: ``false``."
                 },
                 "overwrite": {
                   "type": "boolean",
diff --git a/tests/unittests/config/test_cc_disk_setup.py b/tests/unittests/config/test_cc_disk_setup.py
index c61a26f3..9c02a651 100644
--- a/tests/unittests/config/test_cc_disk_setup.py
+++ b/tests/unittests/config/test_cc_disk_setup.py
@@ -263,12 +263,12 @@ class TestMkfsCommandHandling(CiTestCase):
         subp.assert_called_once_with(
             [
                 "/sbin/mkfs.ext4",
-                "/dev/xdb1",
                 "-L",
                 "without_cmd",
                 "-F",
                 "are",
                 "added",
+                "/dev/xdb1",
             ],
             shell=False,
         )
@@ -292,7 +292,7 @@ class TestMkfsCommandHandling(CiTestCase):
             m_which.call_args_list,
         )
         subp.assert_called_once_with(
-            ["/sbin/mkswap", "/dev/xdb1", "-L", "swap", "-f"], shell=False
+            ["/sbin/mkswap", "-L", "swap", "-f", "/dev/xdb1"], shell=False
         )