Skip to content

cc_disk_setup.py: fix MBR single partition creation #1932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aciba90 merged 1 commit into from
Jan 5, 2023
Merged

cc_disk_setup.py: fix MBR single partition creation #1932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions cloudinit/config/cc_disk_setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@
table_type: gpt
layout: [[100, 82]]
overwrite: true
/dev/sdd:
table_type: mbr
layout: true
overwrite: true
fs_setup:
- label: fs1
filesystem: ext4
Expand All @@ -91,10 +95,14 @@
- 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"]
"""
)
],
Expand Down Expand Up @@ -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"
Copy link
Copy Markdown
Contributor

@aciba90 aciba90 Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

83 / Linux is already the default in sfdisk, so we are not changing behavior in that regard.

Copy link
Copy Markdown
Contributor Author

@dermotbradley dermotbradley Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd added the "83" to align/be consistent with the existing reference on line 631 (to show that "layout: true" and "layout: [100]" are equivalent).

I can change this but it would make sense for the 2 places to be consistent.


if (len(layout) == 0 and isinstance(layout, list)) or not isinstance(
layout, list
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions cloudinit/config/schemas/schema-cloud-config-v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand All @@ -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",
Expand Down
48 changes: 24 additions & 24 deletions doc/examples/cloud-config-disk-setup.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -147,24 +147,24 @@ 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
# done blindly. USE with caution, you can do things you
# 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
Expand All @@ -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.
Expand All @@ -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:
Expand All @@ -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.
#
Expand All @@ -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.
2 changes: 1 addition & 1 deletion tests/unittests/config/test_cc_disk_setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def test_size_for_4096_byte_sectors(self):
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):
Expand Down