Skip to content

Conversation

@handisyde
Copy link

@handisyde handisyde commented Nov 17, 2025

SUMMARY

Fixes #11019
Fix incorrect changed reporting in the zfs module when updating ZFS properties using extra_zfs_properties.

Previously, if a property already had a non-default value (e.g. SOURCE=local), applying a new value via extra_zfs_properties would run zfs set correctly, but the module would still return: changed: false

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

zfs

ADDITIONAL INFORMATION
Root cause

Previously, the module used:

            if self.get_property(prop, current_properties) != value:
                self.changed = True

But get_property() always calls zfs get again, so at that point it reads the updated value, not the original one, and != is always false for a successful change. Storing previous_values fixes this.
Check mode behaviour is unchanged: in check mode, set_property() just toggles self.changed = True and returns without running any command, and we still return diff early.

Testing Done / Evidence

Manual functional validation on a real ZFS dataset with both:

  1. Local -> different local value
    • Previously: incorrectly changed: false
    • Now: changed: true ✔️
  2. Local -> same value
    • changed: false ✔️

You can see the exact output below:
Dataset compression:

root@myserver:~ $ zfs get compression nvme/test
NAME       PROPERTY     VALUE           SOURCE
nvme/test  compression  zstd            local

Ansible output, changed: true:

changed: [myserver] => (item={'name': 'nvme/test', 'properties': {'compression': 'lz4'}}) => 
    ansible_facts:
        discovered_interpreter_python: /usr/bin/python3.11
    ansible_loop_var: item
    changed: true
    compression: lz4
    item:
        name: nvme/test
        properties:
            compression: lz4
    name: nvme/test
    state: present

Dataset compression correctly changed to lz4:

root@myserver:~ $ zfs get compression nvme/test
NAME       PROPERTY     VALUE           SOURCE
nvme/test  compression  lz4             local

Second Ansible run output, changed: false:

ok: [myserver] => (item={'name': 'nvme/test', 'properties': {'compression': 'lz4'}}) => 
    ansible_facts:
        discovered_interpreter_python: /usr/bin/python3.11
    ansible_loop_var: item
    changed: false
    compression: lz4
    item:
        name: nvme/test
        properties:
            compression: lz4
    name: nvme/test
    state: present

…lue differs, even if they already have a non-default value (ansible-collections#11019).

Signed-off-by: handisyde <[email protected]>
@handisyde handisyde changed the title zfs: mark change correctly when updating properties whose current value differs, even if they already have a non-default value (Fixes https://github.com/ansible-collections/community.general/issues/11019) zfs: mark change correctly when updating properties whose current value differs, even if they already have a non-default value (#11019) Nov 17, 2025
@handisyde handisyde changed the title zfs: mark change correctly when updating properties whose current value differs, even if they already have a non-default value (#11019) zfs: mark change correctly when updating properties whose current value differs, even if they already have a non-default value (Fixes #11019) Nov 17, 2025
@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Nov 17, 2025
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-11 Automatically create a backport for the stable-10 branch backport-12 Automatically create a backport for the stable-12 branch labels Nov 17, 2025
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @handisyde thanks for your contribution!

LGTM from the Ansible perspective. Have you tested it locally? Since we do not have tests for that module in the collection, it would be nice if people who actually use the module could test it with these changes.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-11 Automatically create a backport for the stable-10 branch backport-12 Automatically create a backport for the stable-12 branch bug This issue/PR relates to a bug check-before-release PR will be looked at again shortly before release and merged if possible. module module new_contributor Help guide this first time contributor plugins plugin (any type)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

community.general.zfs: no changed status on extra_zfs_properties changes

4 participants