Skip to content

Commit

Permalink
Fix new Proxmox Volume handling (#8646)
Browse files Browse the repository at this point in the history
* proxmox(fix): volume string builder

Half of the string was incorrectly discarded

* proxmox(fix): remove string conversion of values

 - Also converted `None` values into strings
 - Clashed with non-`str` values in documentation

* proxmox: add changelog fragment

* proxmox(fix): remove old & unused imports

* proxmox(fix): correctly turn maps into lists

* Update changelogs/fragments/8646-fix-bug-in-proxmox-volumes.yml

Co-authored-by: Felix Fontein <[email protected]>

* Update plugins/modules/proxmox.py

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
  • Loading branch information
Lithimlin and felixfontein authored Jul 23, 2024
1 parent c0fd10e commit e1148e6
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 10 deletions.
4 changes: 4 additions & 0 deletions changelogs/fragments/8646-fix-bug-in-proxmox-volumes.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
bugfixes:
- proxmox - removed the forced conversion of non-string values to strings to be consistent with the module documentation (https://github.com/ansible-collections/community.general/pull/8646).
- proxmox - fixed an issue where the new volume handling incorrectly converted ``null`` values into ``"None"`` strings (https://github.com/ansible-collections/community.general/pull/8646).
- proxmox - fixed an issue where volume strings where overwritten instead of appended to in the new ``build_volume()`` method (https://github.com/ansible-collections/community.general/pull/8646).
14 changes: 4 additions & 10 deletions plugins/modules/proxmox.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,7 @@
from ansible_collections.community.general.plugins.module_utils.version import LooseVersion

from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.six import string_types
from ansible.module_utils.common.text.converters import to_native, to_text
from ansible.module_utils.common.text.converters import to_native


from ansible_collections.community.general.plugins.module_utils.proxmox import (
Expand Down Expand Up @@ -727,10 +726,11 @@ def build_volume(

# 1.3 If we have a host_path, we don't have storage, a volume, or a size
vol_string = ",".join(
[vol_string] +
([] if host_path is None else [host_path]) +
([] if mountpoint is None else ["mp={0}".format(mountpoint)]) +
([] if options is None else [map("=".join, options.items())]) +
([] if not kwargs else [map("=".join, kwargs.items())])
([] if options is None else ["{0}={1}".format(k, v) for k, v in options.items()]) +
([] if not kwargs else ["{0}={1}".format(k, v) for k, v in kwargs.items()])
)

return {key: vol_string}
Expand Down Expand Up @@ -759,9 +759,6 @@ def build_volume(
if disk is not None:
kwargs["disk_volume"] = parse_disk_string(disk)
if "disk_volume" in kwargs:
if not all(isinstance(val, string_types) for val in kwargs["disk_volume"].values()):
self.module.warn("All disk_volume values must be strings. Converting non-string values to strings.")
kwargs["disk_volume"] = {key: to_text(val) for key, val in kwargs["disk_volume"].items()}
disk_dict = build_volume(key="rootfs", **kwargs.pop("disk_volume"))
kwargs.update(disk_dict)
if memory is not None:
Expand All @@ -775,9 +772,6 @@ def build_volume(
if "mount_volumes" in kwargs:
mounts_list = kwargs.pop("mount_volumes")
for mount_config in mounts_list:
if not all(isinstance(val, string_types) for val in mount_config.values()):
self.module.warn("All mount_volumes values must be strings. Converting non-string values to strings.")
mount_config = {key: to_text(val) for key, val in mount_config.items()}
key = mount_config.pop("id")
mount_dict = build_volume(key=key, **mount_config)
kwargs.update(mount_dict)
Expand Down

0 comments on commit e1148e6

Please sign in to comment.