New ZFS create fails when you specify mount point

[ankycooper]

Summary

New ZFS create fails when you specify mount point, all the existing are not affected.
This is because the quotes added to the cmd

'mountpoint="/mnt/spin/test"'

if it

Issue Type

Bug Report

Component Name

plugins/modules/storage/zfs/zfs.py

Ansible Version

$ ansible --version
ansible [core 2.12.5]
  config file = /Users/<some-path>>/ansible/ansible.cfg
  configured module search path = ['/Users/<username>/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/ansible/5.8.0/libexec/lib/python3.10/site-packages/ansible
  ansible collection location = /Users/<some-path>>/ansible/ansible-tmp/.ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.10.4 (main, Apr 26 2022, 19:42:59) [Clang 13.1.6 (clang-1316.0.21.2)]
  jinja version = 3.1.2
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general
# /Users/<some file path>/ansible/ansible-tmp/.ansible/collections/ansible_collections
Collection        Version
----------------- -------
community.general 5.0.0  

# /usr/local/Cellar/ansible/5.8.0/libexec/lib/python3.10/site-packages/ansible_collections
Collection        Version
----------------- -------
community.general 4.8.1  

Configuration

$ ansible-config dump --only-changed
COLLECTIONS_PATHS(/Users/<some-path>/ansible/ansible.cfg) = ['/Users/<some-path>/ansible/ansible-tmp/.ansible/collections']
DEFAULT_HOST_LIST(/Users/<some-path>/ansible/ansible.cfg) = ['/Users/<some-path>/ansible/hosts']
DEFAULT_LOCAL_TMP(/Users/<some-path>/ansible/ansible.cfg) = /Users/<some-path>/ansible/ansible-tmp/.ansible/tmp/ansible-local-63125l1qzq99v
DEFAULT_ROLES_PATH(/Users/<some-path>/ansible/ansible.cfg) = ['/Users/<some-path>/ansible/ansible-tmp/.ansible/roles']
HOST_KEY_CHECKING(/Users/<some-path>/ansible/ansible.cfg) = False
INTERPRETER_PYTHON(/Users/<some-path>/ansible/ansible.cfg) = auto_silent

OS / Environment

tried on mac and linux

Steps to Reproduce

Task.yaml

---

• name: Create a new new ZFS DS {{item}}
  community.general.zfs:
  name: "{{item}}"
  state: present
  extra_zfs_properties:
  mountpoint={{zfs_base_mount}}{{item}}
  loop: "{{ zfsds }}"

Vars.yaml

zfs_base_mount: /mnt/ #keep trailing slash
zfsds:

• spin/data #this is existing so no issue
• spin/data/name1 #this is existing so no issue
• spin/data/name2/something #this is existing so no issue
• spin/test #this is new so it fails

Expected Results

create and mount zfs

Actual Results

failed: [host.example.com] (item=spin/test) => {
    "ansible_loop_var": "item",
    "changed": false,
    "cmd": "/usr/sbin/zfs create -p -o 'mountpoint=\"/mnt/spin/test\"' spin/test",
    "invocation": {
        "module_args": {
            "extra_zfs_properties": {
                "mountpoint": "/mnt/spin/test"
            },
            "name": "spin/test",
            "origin": null,
            "state": "present"
        }
    },
    "item": "spin/test",
    "msg": "cannot create 'spin/test': 'mountpoint' must be an absolute path, 'none', or 'legacy'",
    "rc": 1,
    "stderr": "cannot create 'spin/test': 'mountpoint' must be an absolute path, 'none', or 'legacy'\n",
    "stderr_lines": [
        "cannot create 'spin/test': 'mountpoint' must be an absolute path, 'none', or 'legacy'"
    ],
    "stdout": "",
    "stdout_lines": []
}

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/packaging/os/pkg5

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @bcoca @fishman @jasperla @jpdasma @mator @mavit @scathatheworm @troy2914 @xen0l
click here for bot help

[felixfontein]

!component =plugins/modules/storage/zfs/zfs.py

[ansibullbot]

Files identified in the description:

• plugins/modules/storage/zfs/zfs.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @johanwiren
click here for bot help

[sw104]

I am also affected by this problem. It is not just limited to the mountpoint option, you also now get an error when using the quota option (and also a number of other options).

This problem was not present with release 4.8.1, and was introduced in b9266c3. Specifically this problem comes from the change to line 170 (pre-commit)/167(post-commit):

  if properties:
            for prop, value in properties.items():
                if prop == 'volsize':
                    cmd += ['-V', value]
                elif prop == 'volblocksize':
                    cmd += ['-b', value]
                else:
                    cmd += ['-o', '%s="%s"' % (prop, value)]
        if origin and action == 'clone':
            cmd.append(origin)
        cmd.append(self.name)
-        (rc, out, err) = self.module.run_command(' '.join(cmd))
+      self.module.run_command(cmd, check_rc=True)

(Note that I've only highlighted the relevant line change in this diff.)

Either this single line change can be reverted, or line 163 can be altered to remove the explicit double quotes there, e.g.:

  if properties:
            for prop, value in properties.items():
                if prop == 'volsize':
                    cmd += ['-V', value]
                elif prop == 'volblocksize':
                    cmd += ['-b', value]
                else:
-                   cmd += ['-o', '%s="%s"' % (prop, value)]
+                   cmd += ['-o', '%s=%s' % (prop, value)]
        if origin and action == 'clone':
            cmd.append(origin)
        cmd.append(self.name)
       self.module.run_command(cmd, check_rc=True)

I suspect that the latter change would be more desirable, but I am unfamiliar with this project and unsure about any side-effects that this might have.

Hopefully this is somewhat helpful, and sorry for not being confident enough about it to submit a proper PR!

[felixfontein]

We should keep the list variant for the arguments, but indeed the quotes need to go. They were needed for the shell to interpret/remove. I unfortunately missed that during the review, sorry for that!

[felixfontein]

I've created a PR to fix this in #4726.