From 7b04985553ed8b0bc29574608e261ee42280f4d0 Mon Sep 17 00:00:00 2001
From: dermotbradley <dermot_bradley@yahoo.com>
Date: Mon, 14 Nov 2022 23:22:34 +0000
Subject: [PATCH] cc_disk_setup: pass options in correct order to utils (#1829)

Reference:https://github.com/canonical/cloud-init/commit/7b04985553ed8b0bc29574608e261ee42280f4d0
Conflict:(1)schema-cloud-config-v1.json not change.
(2)not change meta, no meta.
(3)test format

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
---
 cloudinit/config/cc_disk_setup.py                      | 10 +++++++++-
 .../unittests/test_handler/test_handler_disk_setup.py  |  6 +++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py
index abdc111..f2ba9c6 100644
--- a/cloudinit/config/cc_disk_setup.py
+++ b/cloudinit/config/cc_disk_setup.py
@@ -16,6 +16,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
@@ -993,6 +998,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:
@@ -1000,7 +1006,7 @@ def mkfs(fs_cfg):
                         fs_type, fs_type)
             return
 
-        fs_cmd = [mkfs_cmd, device]
+        fs_cmd = [mkfs_cmd]
 
         if label:
             fs_cmd.extend(["-L", label])
@@ -1015,6 +1021,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/tests/unittests/test_handler/test_handler_disk_setup.py b/tests/unittests/test_handler/test_handler_disk_setup.py
index 2f3a8df..69c7a0d 100644
--- a/tests/unittests/test_handler/test_handler_disk_setup.py
+++ b/tests/unittests/test_handler/test_handler_disk_setup.py
@@ -235,8 +235,8 @@ class TestMkfsCommandHandling(CiTestCase):
         })
 
         subp.assert_called_once_with(
-            ['/sbin/mkfs.ext4', '/dev/xdb1',
-             '-L', 'without_cmd', '-F', 'are', 'added'],
+            ['/sbin/mkfs.ext4', 
+             '-L', 'without_cmd', '-F', 'are', 'added', '/dev/xdb1'],
             shell=False)
 
     @mock.patch('cloudinit.config.cc_disk_setup.subp.which')
@@ -254,7 +254,7 @@ class TestMkfsCommandHandling(CiTestCase):
         self.assertEqual([mock.call('mkfs.swap'), mock.call('mkswap')],
                          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)
 
 #
 # vi: ts=4 expandtab
-- 
2.33.0