cc_disk_setup.py: fix MBR single partition creation (#1932)

Fixes the creation of single partitions on MBR devices. Currently this
fails with the following debug output:

cc_disk_setup.py[DEBUG]: Calculating partition layout
cc_disk_setup.py[DEBUG]:    Layout is: 0,
cc_disk_setup.py[DEBUG]: Creating partition table on /dev/sdb
subp.py[DEBUG]: Running command ['/sbin/sfdisk', '--Linux', '--unit=S',
'--force', '/dev/sdb'] with allowed return codes [0] (shell=False,
capture=True)
util.py[DEBUG]: Creating partition on /dev/sdb took 0.237 seconds
util.py[WARNING]: Failed partitioning operation
Failed to partition device /dev/sdb
Unexpected error while running command.
Command: ['/sbin/sfdisk/', '--Linux', '--unit=S', '--force', '/dev/sdb']
Exit code: 1
Reason: -
Stdout: Checking that no-one is using this disk right now ... OK

        Disk /dev/sdb: 16 MiB, 16777216 bytes, 32768 sectors
        Disk model: HARDDISK
        Units: sectors of 1 * 512 = 512 bytes
        Sector size (logical/physical): 512 bytes / 512 bytes
        I/O size (minimum/optimal): 512 bytes / 512 bytes

        >>> Created a new DOS disklabel with disk identifier 0xb3604c9a.
        /dev/sdb1: Leaving.
Stderr: sfdisk: --Linux option is unnecessary and deprecated
        Start sector 0 out of range.
        Failed to add #1 partition: Result not representable
util.py[DEBUG]: Failed partitioning operation

On a BIOS/MBR partitioned device the 1st partition cannot start at sector
0 as this is reserved for the MBR.

Documentation clarifications/corrections and additional examples added.

Also remove "--Linux" and "--unit=S" options from sfdisk calls, these
options have been deprecated since October 2014.

Note: This is not a change of behavior because the change provoking
the error was introduced in util-linux 2.26 in Xenial. Thus, every
supported cloud-init version fails.

LP: #1851438

4 files changed, 39 insertions, 31 deletions

diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py
index 71d52d3d..5ec5c793 100644
--- a/cloudinit/config/cc_disk_setup.py
+++ b/cloudinit/config/cc_disk_setup.py
@@ -80,6 +80,10 @@ meta: MetaSchema = {
                 table_type: gpt
                 layout: [[100, 82]]
                 overwrite: true
+              /dev/sdd:
+                table_type: mbr
+                layout: true
+                overwrite: true
             fs_setup:
             - label: fs1
               filesystem: ext4
@@ -91,10 +95,14 @@ meta: MetaSchema = {
             - label: swap
               device: swap_disk.1
               filesystem: swap
+            - label: fs3
+              device: /dev/sdd1
+              filesystem: ext4
             mounts:
             - ["my_alias.1", "/mnt1"]
             - ["my_alias.2", "/mnt2"]
             - ["swap_disk.1", "none", "swap", "sw", "0", "0"]
+            - ["/dev/sdd1", "/mnt3"]
             """
         )
     ],
@@ -605,8 +613,8 @@ def get_partition_mbr_layout(size, layout):
     """
 
     if not isinstance(layout, list) and isinstance(layout, bool):
-        # Create a single partition
-        return "0,"
+        # Create a single partition, default to Linux
+        return ",,83"
 
     if (len(layout) == 0 and isinstance(layout, list)) or not isinstance(
         layout, list
@@ -741,7 +749,7 @@ def exec_mkpart_mbr(device, layout):
     types, i.e. gpt
     """
     # Create the partitions
-    prt_cmd = [SFDISK_CMD, "--Linux", "--unit=S", "--force", device]
+    prt_cmd = [SFDISK_CMD, "--force", device]
     try:
         subp.subp(prt_cmd, data="%s\n" % layout)
     except Exception as e:
@@ -968,7 +976,7 @@ def mkfs(fs_cfg):
         odevice = device
         LOG.debug("Identifying device to create %s filesytem on", label)
 
-        # any mean pick the first match on the device with matching fs_type
+        # 'any' means pick the first match on the device with matching fs_type
         label_match = True
         if partition.lower() == "any":
             label_match = False
diff --git a/cloudinit/config/schemas/schema-cloud-config-v1.json b/cloudinit/config/schemas/schema-cloud-config-v1.json
index 7ff80ce3..42745b28 100644
--- a/cloudinit/config/schemas/schema-cloud-config-v1.json
+++ b/cloudinit/config/schemas/schema-cloud-config-v1.json
@@ -1180,7 +1180,7 @@
               },
               "device": {
                 "type": "string",
-                "description": "Specified either as a path or as an alias in the format ``<alias name>.<y>`` where ``<y>`` denotes the partition number on the device. If specifying device using the ``<device name>.<partition number>`` format, the value of ``partition`` will be overwritten."
+                "description": "Specified either as a path or as an alias in the format ``<alias name>.<y>`` where ``<y>`` denotes the partition number on the device. If specifying device using the ``<alias name>.<partition number>`` format, the value of ``partition`` will be overwritten."
               },
               "partition": {
                 "type": [
@@ -1197,7 +1197,7 @@
                     ]
                   }
                 ],
-                "description": "The partition can be specified by setting ``partition`` to the desired partition number. The ``partition`` option may also be set to ``auto``, in which this module will search for the existence of a filesystem matching the ``label``, ``type`` and ``device`` of the ``fs_setup`` entry and will skip creating the filesystem if one is found. The ``partition`` option may also be set to ``any``, in which case any file system that matches ``type`` and ``device`` will cause this module to skip filesystem creation for the ``fs_setup`` entry, regardless of ``label`` matching or not. To write a filesystem directly to a device, use ``partition: none``. ``partition: none`` will **always** write the filesystem, even when the ``label`` and ``filesystem`` are matched, and ``overwrite`` is ``false``."
+                "description": "The partition can be specified by setting ``partition`` to the desired partition number. The ``partition`` option may also be set to ``auto``, in which this module will search for the existence of a filesystem matching the ``label``, ``filesystem`` and ``device`` of the ``fs_setup`` entry and will skip creating the filesystem if one is found. The ``partition`` option may also be set to ``any``, in which case any filesystem that matches ``filesystem`` and ``device`` will cause this module to skip filesystem creation for the ``fs_setup`` entry, regardless of ``label`` matching or not. To write a filesystem directly to a device, use ``partition: none``. ``partition: none`` will **always** write the filesystem, even when the ``label`` and ``filesystem`` are matched, and ``overwrite`` is ``false``."
               },
               "overwrite": {
                 "type": "boolean",
diff --git a/doc/examples/cloud-config-disk-setup.txt b/doc/examples/cloud-config-disk-setup.txt
index cdd176d3..3c8fc36c 100644
--- a/doc/examples/cloud-config-disk-setup.txt
+++ b/doc/examples/cloud-config-disk-setup.txt
@@ -1,5 +1,5 @@
 #cloud-config
-# Cloud-init supports the creation of simple partition tables and file systems
+# Cloud-init supports the creation of simple partition tables and filesystems
 # on devices.
 
 # Default disk definitions for AWS
@@ -147,13 +147,13 @@ disk_setup:
 #               If layout is set to "true" and overwrite is set to "false",
 #               it will skip partitioning the device without a failure.
 #
-#   overwrite=<BOOL>: This describes whether to ride with saftey's on and
+#   overwrite=<BOOL>: This describes whether to ride with safetys on and
 #               everything holstered.
 #
 #               'false' is the default, which means that:
 #                   1. The device will be checked for a partition table
-#                   2. The device will be checked for a file system
-#                   3. If either a partition of file system is found, then
+#                   2. The device will be checked for a filesystem
+#                   3. If either a partition of filesystem is found, then
 #                       the operation will be _skipped_.
 #
 #               'true' is cowboy mode. There are no checks and things are
@@ -161,10 +161,10 @@ disk_setup:
 #                   really, really don't want to do.
 #
 #
-# fs_setup: Setup the file system
-# -------------------------------
+# fs_setup: Setup the filesystem
+# ------------------------------
 #
-# fs_setup describes the how the file systems are supposed to look.
+# fs_setup describes the how the filesystems are supposed to look.
 
 fs_setup:
   - label: ephemeral0
@@ -189,10 +189,10 @@ fs_setup:
 #       replace_fs: <FS_TYPE>
 #
 # Where:
-#   <LABEL>: The file system label to be used. If set to None, no label is
+#   <LABEL>: The filesystem label to be used. If set to None, no label is
 #     used.
 #
-#   <FS_TYPE>: The file system type. It is assumed that the there
+#   <FS_TYPE>: The filesystem type. It is assumed that the there
 #     will be a "mkfs.<FS_TYPE>" that behaves likes "mkfs". On a standard
 #     Ubuntu Cloud Image, this means that you have the option of ext{2,3,4},
 #     and vfat by default.
@@ -212,15 +212,15 @@ fs_setup:
 #     The valid options are:
 #     "auto|any": tell cloud-init not to care whether there is a partition
 #       or not. Auto will use the first partition that does not contain a
-#       file system already. In the absence of a partition table, it will
+#       filesystem already. In the absence of a partition table, it will
 #       put it directly on the disk.
 #
-#       "auto": If a file system that matches the specification in terms of
-#       label, type and device, then cloud-init will skip the creation of
-#       the file system.
+#       "auto": If a filesystem that matches the specification in terms of
+#       label, filesystem and device, then cloud-init will skip the creation
+#       of the filesystem.
 #
-#       "any": If a file system that matches the file system type and device,
-#       then cloud-init will skip the creation of the file system.
+#       "any": If a filesystem that matches the filesystem type and device,
+#       then cloud-init will skip the creation of the filesystem.
 #
 #       Devices are selected based on first-detected, starting with partitions
 #       and then the raw disk. Consider the following:
@@ -231,7 +231,7 @@ fs_setup:
 #           |-xvdb3  btrfs  test
 #           \-xvdb4  ext4   test
 #
-#         If you ask for 'auto', label of 'test, and file system of 'ext4'
+#         If you ask for 'auto', label of 'test, and filesystem of 'ext4'
 #         then cloud-init will select the 2nd partition, even though there
 #         is a partition match at the 4th partition.
 #
@@ -243,25 +243,25 @@ fs_setup:
 #
 #         In general, if you have a specific partition configuration in mind,
 #         you should define either the device or the partition number. 'auto'
-#         and 'any' are specifically intended for formatting ephemeral storage or
-#         for simple schemes.
+#         and 'any' are specifically intended for formatting ephemeral storage
+#         or for simple schemes.
 #
-#       "none": Put the file system directly on the device.
+#       "none": Put the filesystem directly on the device.
 #
 #       <NUM>: where NUM is the actual partition number.
 #
 #   <OVERWRITE>: Defines whether or not to overwrite any existing
 #     filesystem.
 #
-#     "true": Indiscriminately destroy any pre-existing file system. Use at
+#     "true": Indiscriminately destroy any pre-existing filesystem. Use at
 #         your own peril.
 #
-#     "false": If an existing file system exists, skip the creation.
+#     "false": If an existing filesystem exists, skip the creation.
 #
 #   <REPLACE_FS>: This is a special directive, used for Microsoft Azure that
-#     instructs cloud-init to replace a file system of <FS_TYPE>. NOTE:
+#     instructs cloud-init to replace a filesystem of <FS_TYPE>. NOTE:
 #     unless you define a label, this requires the use of the 'any' partition
 #     directive.
 #
-# Behavior Caveat: The default behavior is to _check_ if the file system exists.
-#   If a file system matches the specification, then the operation is a no-op.
+# Behavior Caveat: The default behavior is to _check_ if the filesystem exists.
+#   If a filesystem matches the specification, then the operation is a no-op.
diff --git a/tests/unittests/config/test_cc_disk_setup.py b/tests/unittests/config/test_cc_disk_setup.py
index 9c02a651..70085314 100644
--- a/tests/unittests/config/test_cc_disk_setup.py
+++ b/tests/unittests/config/test_cc_disk_setup.py
@@ -102,7 +102,7 @@ class TestGetMbrHddSize(TestCase):
 class TestGetPartitionMbrLayout(TestCase):
     def test_single_partition_using_boolean(self):
         self.assertEqual(
-            "0,", cc_disk_setup.get_partition_mbr_layout(1000, True)
+            ",,83", cc_disk_setup.get_partition_mbr_layout(1000, True)
         )
 
     def test_single_partition_using_list(self):