zfs.py: treated received properties as local and added diff mode support (#502) (#3304)

* zfs.py: treated received properties as local and added diff mode support

If you use "zfs set" to explicitly set ZFS properties, they are marked as from source "local". If ZFS properties are implicitly set by using "zfs send" and "zfs receive", for example as part of a template based installation, they are marked as from source "received". But as there is no technical difference between both types of them, the “received” ZFS properties should also be considered “local”. Otherwise Ansible would detect changes, which aren’t actual changes. Therefore I changed line 202/207 to reflect this.

For us it’s quite important, that Ansible modules support the diff mode in order to qualify changes. Therefore I added some code lines to address this.

* added changelog fragment for PR #502

* fixed typos in changelog fragment for PR #502

* minor changes in changelog fragment for PR #502

* added link to pull request in changelog fragment for PR #502

* extended the diff data structure to always include the name of the zfs filesystem

* added code to also maintain the diff data structure after a change

* reverted back some code lines for better code readability

* added an extra dict in the diff data structure to hold the zfs properties

(cherry picked from commit baa721ac22)

Co-authored-by: froebela <32922546+froebela@users.noreply.github.com>

changelogs/fragments/502-zfs_bugfix_and_diff_mode_support.yaml

@@ -0,0 +1,4 @@
+bugfixes:
+  - zfs - treated received properties as local (https://github.com/ansible-collections/community.general/pull/502).
+minor_changes:
+  - zfs - added diff mode support (https://github.com/ansible-collections/community.general/pull/502).

plugins/modules/storage/zfs/zfs.py

@@ -194,12 +194,16 @@ class Zfs(object):
             self.module.fail_json(msg=err)
 
     def set_properties_if_changed(self):
+        diff = {'before': {'extra_zfs_properties': {}}, 'after': {'extra_zfs_properties': {}}}
         current_properties = self.get_current_properties()
         for prop, value in self.properties.items():
-            if current_properties.get(prop, None) != value:
+            current_value = current_properties.get(prop, None)
+            if current_value != value:
                 self.set_property(prop, value)
+                diff['before']['extra_zfs_properties'][prop] = current_value
+                diff['after']['extra_zfs_properties'][prop] = value
         if self.module.check_mode:
-            return
+            return diff
         updated_properties = self.get_current_properties()
         for prop in self.properties:
             value = updated_properties.get(prop, None)
@@ -207,6 +211,9 @@ class Zfs(object):
                 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
+            if prop in diff['after']['extra_zfs_properties']:
+                diff['after']['extra_zfs_properties'][prop] = value
+        return diff
 
     def get_current_properties(self):
         cmd = [self.zfs_cmd, 'get', '-H', '-p', '-o', "property,value,source"]
@@ -220,7 +227,7 @@ class Zfs(object):
             # 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
-            if source == 'local' or source == '-':
+            if source == 'local' or source == 'received' or source == '-':
                 properties[prop] = value
         # Add alias for enhanced sharing properties
         if self.enhanced_sharing:
@@ -266,13 +273,20 @@ def main():
 
     if state == 'present':
         if zfs.exists():
-            zfs.set_properties_if_changed()
+            result['diff'] = zfs.set_properties_if_changed()
         else:
             zfs.create()
+            result['diff'] = {'before': {'state': 'absent'}, 'after': {'state': state}}
 
     elif state == 'absent':
         if zfs.exists():
             zfs.destroy()
+            result['diff'] = {'before': {'state': 'present'}, 'after': {'state': state}}
+        else:
+            result['diff'] = {}
+
+    result['diff']['before_header'] = name
+    result['diff']['after_header'] = name
 
     result.update(zfs.properties)
     result['changed'] = zfs.changed