ansible-collections / community.general

Ansible Community General Collection
https://galaxy.ansible.com/ui/repo/published/community/general/
GNU General Public License v3.0
830 stars 1.53k forks source link

community.general.zfs tries to reset volblocksize thus breaking idempotency #4699

Open rbadass opened 2 years ago

rbadass commented 2 years ago

Summary

When running the following: community.general.zfs: name: pool/vol1 state: present extra_zfs_properties: volsize: 4G volblocksize: 4k

the initial run succeeds and creates the zvol, however subsequent runs fail with: FAILED! => {"changed": false, "msg": "cannot set property for 'pool/vol1': 'volblocksize' is readonly\n"}

it should simply skip over and not try to set volblocksize as it's already at the correct volblocksize.

Issue Type

Bug Report

Component Name

zfs

Ansible Version

ansible [core 2.12.3]

Community.general Version

community.general 1.3.0
community.general 4.6.0

Configuration

$ ansible-config dump --only-changed

OS / Environment

No response

Steps to Reproduce

community.general.zfs: name: pool/vol1 state: present extra_zfs_properties: volsize: 4G volblocksize: 4k

Expected Results

I expected it to skip over a property that was already set correctly.

Actual Results

It tried to re-set the volblocksize property, which is readonly and thus throws an error.

Code of Conduct

ansibullbot commented 2 years ago

Files identified in the description:

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 commented 2 years ago

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

felixfontein commented 2 years ago

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

ansibullbot commented 2 years ago

Files identified in the description:

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 commented 2 years ago

cc @johanwiren click here for bot help

felixfontein commented 2 years ago

This should have been fixed in #1833. Are you sure you tried this also with community.general 4.6.0, and not just with community.general 1.3.0 (which is EOL btw)?

rbadass commented 2 years ago

This should have been fixed in #1833. Are you sure you tried this also with community.general 4.6.0, and not just with community.general 1.3.0 (which is EOL btw)?

i just updated and tried again, the problem persists

$ ansible-galaxy collection list community.general community.general 4.8.1

cola-zero commented 1 year ago

When I change volblocksize = 4k to vloblocksize = 4096, task become "ok". I don't know why volsize = 4G is good but volblocksize = 4k is not.

Ramblurr commented 1 year ago

Same thing with the encryption property:

The following can only one once, every other time it errors:

- name: create encrypted guest zfs volume
  community.general.zfs:
    name: "{{ zfs_encrypted_guest_dataset }}"
    state: present
    extra_zfs_properties:
      encryption: on
      keyformat: hex
      keylocation: file:///root/.rpool-data-encrypted.key
      compression: zstd-3
TASK [rmblr.proxmox_setup : create encrypted guest zfs volume ] ********************
fatal: [mill]: FAILED! => changed=false 
  cmd: /usr/sbin/zfs set encryption=on rpool/data/encrypted
  msg: 'cannot set property for ''rpool/data/encrypted'': ''encryption'' is readonly'
  rc: 1
  stderr: |-
    cannot set property for 'rpool/data/encrypted': 'encryption' is readonly
  stderr_lines: <omitted>
  stdout: ''
  stdout_lines: <omitted>

Using community.general==6.3.0

Analysis

Looking at the code, the module is parsing the result of essentially this (filtered down to the encryption property):

$ zfs get -H -p -o property,value,source all rpool/data/encrypted|grep encryption
encryption  aes-256-gcm -
encryptionroot  rpool/data/encrypted    -

Note that the playbook called for encryption=on. In modern ZFS (2.1.9) this is equivalent to aes-256-gcm in previous versions the default encryption value was different.

Changing the playbook to use encryption: aes-256-gcm makes it idempotent.

Tualua commented 1 month ago

Hello!

Any plans on fixing this issue?

felixfontein commented 1 month ago

I'm not aware of anyone working on this.