mirror of
https://github.com/ansible-collections/community.general.git
synced 2026-04-20 18:59:08 +00:00
[PR #11813/edf8f249 backport][stable-12] parted: add unit_preserve_case option to fix unit case in return value (#11875)
parted: add unit_preserve_case option to fix unit case in return value (#11813)
* parted: add unit_preserve_case option to fix unit case in return value
Adds O(unit_preserve_case) feature flag (bool, default None) to control
the case of the ``unit`` field in the module return value.
Previously the unit was always lowercased (e.g. ``kib``), making it
impossible to feed ``disk.unit`` back as the ``unit`` parameter without
a validation error. With O(unit_preserve_case=true) the unit is returned
in its original mixed case (e.g. ``KiB``), matching the accepted input
values.
The default (None) emits a deprecation notice; the default will become
V(true) in community.general 14.0.0.
Fixes #1860
* parted: add changelog fragment for PR #11813
* adjustments from review
* Comment 15.0.0 deprecation in option decription.
* parted: fix unit test calls to parse_partition_info after signature change
* parted: fix unit_preserve_case - parted outputs lowercase units in machine mode
Parted's machine-parseable output always uses lowercase unit suffixes
(e.g. ``kib``, ``mib``) regardless of what was passed to the ``unit``
parameter. Removing the explicit ``.lower()`` call was therefore not
enough to preserve case.
Add a ``canonical_unit()`` helper that maps a unit string to its canonical
mixed-case form using ``parted_units`` as the reference, and use it
instead of a bare identity when ``unit_preserve_case=true``.
Also fix a yamllint violation in the DOCUMENTATION block (missing space
after ``#`` in inline comments).
* Update plugins/modules/parted.py
* Update plugins/modules/parted.py
---------
(cherry picked from commit edf8f24959)
Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
This commit is contained in:
parent
3867300eca
commit
27ca6be10a
4 changed files with 93 additions and 14 deletions
5
changelogs/fragments/11813-parted-unit-preserve-case.yml
Normal file
5
changelogs/fragments/11813-parted-unit-preserve-case.yml
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
minor_changes:
|
||||
- parted - add ``unit_preserve_case`` option to control the case of the ``unit`` field in the
|
||||
return value, fixing the round-trip use case where the returned unit is fed back as input
|
||||
(https://github.com/ansible-collections/community.general/issues/1860,
|
||||
https://github.com/ansible-collections/community.general/pull/11813).
|
||||
|
|
@ -50,6 +50,8 @@ options:
|
|||
- Selects the current default unit that Parted uses to display locations and capacities on the disk and to interpret
|
||||
those given by the user if they are not suffixed by an unit.
|
||||
- When fetching information about a disk, it is recommended to always specify a unit.
|
||||
- O(unit_preserve_case) controls the case of the units in the return values. It used to be all lower case, but using that option you
|
||||
can make it return the units textually equal to the choices used in O(unit).
|
||||
type: str
|
||||
choices: [s, B, KB, KiB, MB, MiB, GB, GiB, TB, TiB, '%', cyl, chs, compact]
|
||||
default: KiB
|
||||
|
|
@ -112,6 +114,17 @@ options:
|
|||
type: bool
|
||||
default: false
|
||||
version_added: '1.3.0'
|
||||
unit_preserve_case:
|
||||
description:
|
||||
- Controls the case of the V(unit) field in the module output (RV(ignore:partition_info.disk.unit), RV(ignore:partition_info.partitions[].unit)).
|
||||
- When V(true), the unit is returned in its original mixed case, for example V(KiB) or V(MiB). This matches the values
|
||||
accepted by O(unit), making it safe to feed the output back as input.
|
||||
- When V(false), the unit is returned in lowercase (legacy behavior), for example V(kib) or V(mib).
|
||||
# - When not set, the module uses the legacy lowercase behavior and emits a deprecation warning. The default will change
|
||||
# to V(true) in community.general 15.0.0.
|
||||
type: bool
|
||||
default: false
|
||||
version_added: '12.6.0'
|
||||
|
||||
notes:
|
||||
- When fetching information about a new disk and when the version of parted installed on the system is before version 3.1,
|
||||
|
|
@ -144,7 +157,7 @@ partition_info:
|
|||
"physical_block": 512
|
||||
"size": 5.0
|
||||
"table": "msdos"
|
||||
"unit": "gib"
|
||||
"unit": "GiB"
|
||||
"partitions":
|
||||
- "begin": 0.0
|
||||
"end": 1.0
|
||||
|
|
@ -232,6 +245,12 @@ from ansible.module_utils.basic import AnsibleModule
|
|||
units_si = ["B", "KB", "MB", "GB", "TB"]
|
||||
units_iec = ["KiB", "MiB", "GiB", "TiB"]
|
||||
parted_units = units_si + units_iec + ["s", "%", "cyl", "chs", "compact"]
|
||||
_parted_units_lower = {u.lower(): u for u in parted_units}
|
||||
|
||||
|
||||
def canonical_unit(unit):
|
||||
"""Return the canonical mixed-case form of a parted unit string."""
|
||||
return _parted_units_lower.get(unit.lower(), unit)
|
||||
|
||||
|
||||
def parse_unit(size_str, unit=""):
|
||||
|
|
@ -258,7 +277,7 @@ def parse_unit(size_str, unit=""):
|
|||
return size, unit
|
||||
|
||||
|
||||
def parse_partition_info(parted_output, unit):
|
||||
def parse_partition_info(parted_output, unit, unit_preserve_case):
|
||||
"""
|
||||
Parses the output of parted and transforms the data into
|
||||
a dictionary.
|
||||
|
|
@ -289,10 +308,11 @@ def parse_partition_info(parted_output, unit):
|
|||
# The unit is read once, because parted always returns the same unit
|
||||
size, unit = parse_unit(generic_params[1], unit)
|
||||
|
||||
unit_output = canonical_unit(unit) if unit_preserve_case else unit.lower()
|
||||
generic = {
|
||||
"dev": generic_params[0],
|
||||
"size": size,
|
||||
"unit": unit.lower(),
|
||||
"unit": unit_output,
|
||||
"table": generic_params[5],
|
||||
"model": generic_params[6],
|
||||
"logical_block": int(generic_params[3]),
|
||||
|
|
@ -308,7 +328,7 @@ def parse_partition_info(parted_output, unit):
|
|||
"heads": int(chs_info[1]),
|
||||
"sectors": int(chs_info[2]),
|
||||
"cyl_size": cyl_size,
|
||||
"cyl_size_unit": cyl_unit.lower(),
|
||||
"cyl_size_unit": canonical_unit(cyl_unit) if unit_preserve_case else cyl_unit.lower(),
|
||||
}
|
||||
lines = lines[1:]
|
||||
|
||||
|
|
@ -341,7 +361,7 @@ def parse_partition_info(parted_output, unit):
|
|||
"fstype": fstype,
|
||||
"name": name,
|
||||
"flags": [f.strip() for f in flags.split(", ") if f != ""],
|
||||
"unit": unit.lower(),
|
||||
"unit": unit_output,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -413,7 +433,7 @@ def convert_to_bytes(size_str, unit):
|
|||
return int(output)
|
||||
|
||||
|
||||
def get_unlabeled_device_info(device, unit):
|
||||
def get_unlabeled_device_info(device, unit, unit_preserve_case):
|
||||
"""
|
||||
Fetches device information directly from the kernel and it is used when
|
||||
parted cannot work because of a missing label. It always returns a 'unknown'
|
||||
|
|
@ -429,6 +449,8 @@ def get_unlabeled_device_info(device, unit):
|
|||
size_bytes = int(read_record(f"{base}/size", 0)) * logic_block
|
||||
|
||||
size, unit = format_disk_size(size_bytes, unit)
|
||||
if unit_preserve_case:
|
||||
unit = canonical_unit(unit)
|
||||
|
||||
return {
|
||||
"generic": {
|
||||
|
|
@ -444,7 +466,7 @@ def get_unlabeled_device_info(device, unit):
|
|||
}
|
||||
|
||||
|
||||
def get_device_info(device, unit):
|
||||
def get_device_info(device, unit, unit_preserve_case):
|
||||
"""
|
||||
Fetches information about a disk and its partitions and it returns a
|
||||
dictionary.
|
||||
|
|
@ -456,7 +478,7 @@ def get_device_info(device, unit):
|
|||
# parted formats for the unit.
|
||||
label_needed = check_parted_label(device)
|
||||
if label_needed:
|
||||
return get_unlabeled_device_info(device, unit)
|
||||
return get_unlabeled_device_info(device, unit, unit_preserve_case)
|
||||
|
||||
command = [parted_exec, "-s", "-m", device, "--", "unit", unit, "print"]
|
||||
rc, out, err = module.run_command(command)
|
||||
|
|
@ -468,7 +490,7 @@ def get_device_info(device, unit):
|
|||
err=err,
|
||||
)
|
||||
|
||||
return parse_partition_info(out, unit)
|
||||
return parse_partition_info(out, unit, unit_preserve_case)
|
||||
|
||||
|
||||
def check_parted_label(device):
|
||||
|
|
@ -622,6 +644,7 @@ def main():
|
|||
state=dict(type="str", default="info", choices=["absent", "info", "present"]),
|
||||
# resize part
|
||||
resize=dict(type="bool", default=False),
|
||||
unit_preserve_case=dict(type="bool", default=False),
|
||||
),
|
||||
required_if=[
|
||||
["state", "present", ["number"]],
|
||||
|
|
@ -645,6 +668,20 @@ def main():
|
|||
flags = module.params["flags"]
|
||||
fs_type = module.params["fs_type"]
|
||||
resize = module.params["resize"]
|
||||
unit_preserve_case = module.params["unit_preserve_case"]
|
||||
if unit_preserve_case is None:
|
||||
# UNCOMMENT when 13.0.0 is released
|
||||
#
|
||||
# module.deprecate(
|
||||
# "The unit field in the return value of community.general.parted is currently lowercased "
|
||||
# "(for example ``kib``) when its original has mixed case (for example ``KiB``). "
|
||||
# "To suppress this warning message, set the parameter ``unit_preserve_case`` either to ``false``, "
|
||||
# "for preserving the current behavior, or to ``true``, for using the new behavior immediately. "
|
||||
# "In community.general 15.0.0, the default value of this option will be set to ``true``.",
|
||||
# version="15.0.0",
|
||||
# collection_name="community.general",
|
||||
# )
|
||||
unit_preserve_case = False
|
||||
|
||||
# Parted executable
|
||||
parted_exec = module.get_bin_path("parted", True)
|
||||
|
|
@ -664,7 +701,7 @@ def main():
|
|||
)
|
||||
|
||||
# Read the current disk information
|
||||
current_device = get_device_info(device, unit)
|
||||
current_device = get_device_info(device, unit, unit_preserve_case)
|
||||
current_parts = current_device["partitions"]
|
||||
|
||||
if state == "present":
|
||||
|
|
@ -710,7 +747,7 @@ def main():
|
|||
script = []
|
||||
|
||||
if not module.check_mode:
|
||||
current_parts = get_device_info(device, unit)["partitions"]
|
||||
current_parts = get_device_info(device, unit, unit_preserve_case)["partitions"]
|
||||
|
||||
if part_exists(current_parts, "num", number) or module.check_mode:
|
||||
if changed and module.check_mode:
|
||||
|
|
@ -761,7 +798,7 @@ def main():
|
|||
elif state == "info":
|
||||
output_script = ["unit", unit, "print"]
|
||||
# Final status of the device
|
||||
final_device_status = get_device_info(device, unit)
|
||||
final_device_status = get_device_info(device, unit, unit_preserve_case)
|
||||
module.exit_json(
|
||||
changed=changed,
|
||||
disk=final_device_status["generic"],
|
||||
|
|
|
|||
|
|
@ -84,3 +84,40 @@
|
|||
- fs2_succ is changed
|
||||
- partition_rem1 is changed
|
||||
- partition_rem1.partitions | length == 1
|
||||
|
||||
- name: Get partition info with unit_preserve_case=true
|
||||
community.general.parted:
|
||||
device: "{{ losetup_name.stdout }}"
|
||||
state: info
|
||||
unit: KiB
|
||||
unit_preserve_case: true
|
||||
register: info_preserve_true
|
||||
|
||||
- name: Get partition info with unit_preserve_case=false
|
||||
community.general.parted:
|
||||
device: "{{ losetup_name.stdout }}"
|
||||
state: info
|
||||
unit: KiB
|
||||
unit_preserve_case: false
|
||||
register: info_preserve_false
|
||||
|
||||
- name: Assert unit_preserve_case results
|
||||
assert:
|
||||
that:
|
||||
- info_preserve_true.disk.unit == "KiB"
|
||||
- info_preserve_true.partitions[0].unit == "KiB"
|
||||
- info_preserve_false.disk.unit == "kib"
|
||||
- info_preserve_false.partitions[0].unit == "kib"
|
||||
|
||||
- name: Use returned unit as input (round-trip) - regression test for issue 1860
|
||||
community.general.parted:
|
||||
device: "{{ losetup_name.stdout }}"
|
||||
state: info
|
||||
unit: "{{ info_preserve_true.disk.unit }}"
|
||||
unit_preserve_case: true
|
||||
register: info_roundtrip
|
||||
|
||||
- name: Assert round-trip succeeds and unit is consistent
|
||||
assert:
|
||||
that:
|
||||
- info_roundtrip.disk.unit == info_preserve_true.disk.unit
|
||||
|
|
|
|||
|
|
@ -200,8 +200,8 @@ class TestParted(ModuleTestCase):
|
|||
|
||||
def test_parse_partition_info(self):
|
||||
"""Test that the parse_partition_info returns the expected dictionary"""
|
||||
self.assertEqual(parse_partition_info(parted_output1, "MB"), parted_dict1)
|
||||
self.assertEqual(parse_partition_info(parted_output2, "MB"), parted_dict2)
|
||||
self.assertEqual(parse_partition_info(parted_output1, "MB", False), parted_dict1)
|
||||
self.assertEqual(parse_partition_info(parted_output2, "MB", False), parted_dict2)
|
||||
|
||||
def test_partition_already_exists(self):
|
||||
with set_module_args(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue