mirror of
https://github.com/ansible-collections/community.general.git
synced 2024-09-14 20:13:21 +02:00
Avoid incorrectly marking zfs tasks as changed (#2454)
* Avoid incorrectly marking zfs tasks as changed The zfs module will incorrectly mark certain tasks as having been changed. For example, if a dataset has a quota of "1G" and the user changes it to "1024M", the actual quota vale has not changed, but since the module is doing a simple string comparison between "1G" and "1024M", it marks the step as "changed". Instead of trying to handle all the corner cases of zfs (another example is when the zpool "altroot" property has been set), this change simply compares the output of "zfs-get" from before and after "zfs-set" is called * update changelog format * Update changelogs/fragments/2454-detect_zfs_changed.yml Co-authored-by: Felix Fontein <felix@fontein.de> * add note about check_mode * Update plugins/modules/storage/zfs/zfs.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/storage/zfs/zfs.py Co-authored-by: Felix Fontein <felix@fontein.de> * clarify check mode qualifications * rephrase to avoid hypothetical Co-authored-by: Felix Fontein <felix@fontein.de>
This commit is contained in:
parent
2e58dfe52a
commit
8e7aff00b5
2 changed files with 21 additions and 5 deletions
2
changelogs/fragments/2454-detect_zfs_changed.yml
Normal file
2
changelogs/fragments/2454-detect_zfs_changed.yml
Normal file
|
@ -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).
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue