diff --git a/changelogs/fragments/11813-parted-unit-preserve-case.yml b/changelogs/fragments/11813-parted-unit-preserve-case.yml new file mode 100644 index 0000000000..9ff0f3eef7 --- /dev/null +++ b/changelogs/fragments/11813-parted-unit-preserve-case.yml @@ -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). diff --git a/plugins/modules/parted.py b/plugins/modules/parted.py index e34141ee65..2ff73a56b1 100644 --- a/plugins/modules/parted.py +++ b/plugins/modules/parted.py @@ -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"], diff --git a/tests/integration/targets/parted/tasks/main.yml b/tests/integration/targets/parted/tasks/main.yml index dbd916486e..2069a98aab 100644 --- a/tests/integration/targets/parted/tasks/main.yml +++ b/tests/integration/targets/parted/tasks/main.yml @@ -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 diff --git a/tests/unit/plugins/modules/test_parted.py b/tests/unit/plugins/modules/test_parted.py index 13d1c05f34..466cc1851f 100644 --- a/tests/unit/plugins/modules/test_parted.py +++ b/tests/unit/plugins/modules/test_parted.py @@ -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(