diff --git a/changelogs/fragments/2454-detect_zfs_changed.yml b/changelogs/fragments/2454-detect_zfs_changed.yml new file mode 100644 index 0000000000..0604278f6b --- /dev/null +++ b/changelogs/fragments/2454-detect_zfs_changed.yml @@ -0,0 +1,2 @@ +bugfixes: + - zfs - certain ZFS properties, especially sizes, would lead to a task being falsely marked as "changed" even when no actual change was made (https://github.com/ansible-collections/community.general/issues/975, https://github.com/ansible-collections/community.general/pull/2454). diff --git a/plugins/modules/storage/zfs/zfs.py b/plugins/modules/storage/zfs/zfs.py index fe693a5045..2d5d4487dd 100644 --- a/plugins/modules/storage/zfs/zfs.py +++ b/plugins/modules/storage/zfs/zfs.py @@ -37,6 +37,12 @@ options: - A dictionary of zfs properties to be set. - See the zfs(8) man page for more information. type: dict +notes: + - C(check_mode) is supported, but in certain situations it may report a task + as changed that will not be reported as changed when C(check_mode) is disabled. + For example, this might occur when the zpool C(altroot) option is set or when + a size is written using human-readable notation, such as C(1M) or C(1024K), + instead of as an unqualified byte count, such as C(1048576). author: - Johan Wiren (@johanwiren) ''' @@ -184,9 +190,7 @@ class Zfs(object): return cmd = [self.zfs_cmd, 'set', prop + '=' + str(value), self.name] (rc, out, err) = self.module.run_command(cmd) - if rc == 0: - self.changed = True - else: + if rc != 0: self.module.fail_json(msg=err) def set_properties_if_changed(self): @@ -194,15 +198,25 @@ class Zfs(object): for prop, value in self.properties.items(): if current_properties.get(prop, None) != value: self.set_property(prop, value) + if self.module.check_mode: + return + updated_properties = self.get_current_properties() + for prop in self.properties: + value = updated_properties.get(prop, None) + if value is None: + self.module.fail_json(msg="zfsprop was not present after being successfully set: %s" % prop) + if current_properties.get(prop, None) != value: + self.changed = True def get_current_properties(self): - cmd = [self.zfs_cmd, 'get', '-H'] + cmd = [self.zfs_cmd, 'get', '-H', '-p', '-o', "property,value,source"] if self.enhanced_sharing: cmd += ['-e'] cmd += ['all', self.name] rc, out, err = self.module.run_command(" ".join(cmd)) properties = dict() - for prop, value, source in [l.split('\t')[1:4] for l in out.splitlines()]: + for line in out.splitlines(): + prop, value, source = line.split('\t') # include source '-' so that creation-only properties are not removed # to avoids errors when the dataset already exists and the property is not changed # this scenario is most likely when the same playbook is run more than once