NFS Module Enhancement: diff support (#132)

* NFS Module Enhacement: diff support Signed-off-by: sharmb5 <[email protected]> * Updating rst file Signed-off-by: sharmb5 <[email protected]> --------- Signed-off-by: sharmb5 <[email protected]>
dell · Nov 19, 2024 · f380887 · f380887
1 parent cd0414f
commit f380887
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 25 deletions.
diff --git a/docs/modules/nfs.rst b/docs/modules/nfs.rst
@@ -40,7 +40,7 @@ Parameters
 
     This list can be changed anytime during the lifetime of the NFS export.
 
-    If the clients list needs to be replaced, \ :literal:`client\_state`\  should not be provided.
+    \ :emphasis:`client\_state`\  is not provided, then the host machine will replicate the values provided in the \ :emphasis:`clients`\ .
 
 
   client_state (optional, str, None)
@@ -150,23 +150,23 @@ Parameters
 
     This list can be changed anytime during the lifetime of the NFS export.
 
-    If the read\_only\_clients list needs to be replaced, \ :literal:`client\_state`\  should not be provided.
+    \ :emphasis:`client\_state`\  is not provided, then the host machine will replicate the values provided in the \ :emphasis:`read\_only\_clients`\ .
 
 
   read_write_clients (optional, list, None)
     Specifies the clients with both read and write access to the export, even when the export is set to read-only.
 
     This list can be changed anytime during the lifetime of the NFS export.
 
-    If the read\_write\_clients list needs to be replaced, \ :literal:`client\_state`\  should not be provided.
+    \ :emphasis:`client\_state`\  is not provided, then the host machine will replicate the values provided in the \ :emphasis:`read\_write\_clients`\ .
 
 
   root_clients (optional, list, None)
     Specifies the clients with root access to the export.
 
     This list can be changed anytime during the lifetime of the NFS export.
 
-    If the root\_clients list needs to be replaced, \ :literal:`client\_state`\  should not be provided.
+    \ :emphasis:`client\_state`\  is not provided, then the host machine will replicate the values provided in the \ :emphasis:`root\_clients`\ .
 
 
   security_flavors (optional, list, None)
@@ -218,7 +218,7 @@ Notes
 -----
 
 .. note::
-   - The \ :emphasis:`check\_mode`\  is supported.
+   - As \ :emphasis:`ignore\_unresolvable\_hosts`\  is input only parameter, therefore idempotency is not supported for it.
    - The modules present in this collection named as 'dellemc.powerscale' are built to support the Dell PowerScale storage platform.
 
 

diff --git a/plugins/modules/nfs.py b/plugins/modules/nfs.py
@@ -200,9 +200,17 @@
       set, sub-directories will not be mountable.
     - This setting can be modified any time.
     type: bool
-
+attributes:
+  check_mode:
+    description:
+    - Runs task to validate without performing action on the target machine.
+    support: full
+  diff_mode:
+    description:
+    - Runs the task to report the changes made or to be made.
+    support: full
 notes:
-  - The I(check_mode) is supported.
+  - As I(ignore_unresolvable_hosts) is input only parameter, therefore idempotency is not supported for it.
 '''
 
 EXAMPLES = r'''
@@ -509,7 +517,8 @@ def __init__(self):
         # details
         self.result = {
             "changed": False,
-            "NFS_export_details": {}
+            "NFS_export_details": {},
+            "diff": {}
         }
 
     def get_zone_base_path(self, access_zone):
@@ -625,6 +634,8 @@ def create_nfs_export(self, path, access_zone, ignore_unresolvable_hosts):
         nfs_map_non_root = set_nfs_map(self.module.params.get('map_non_root'), 'map_non_root')
         if nfs_map_non_root:
             nfs_export.map_non_root = nfs_map_non_root
+        if self.module._diff:
+            self.result.update({"diff": {"before": {}, "after": nfs_export.to_dict()}})
         try:
             if not self.module.check_mode:
                 msg = ("Creating NFS export with parameters:nfs_export=%s",
@@ -834,7 +845,7 @@ def modify_nfs_export(self, path, access_zone, ignore_unresolvable_hosts):
         if all(
             field_mod_flag is False for field_mod_flag in [
                 client_flag, read_only_flag, all_dirs_flag, description_flag, map_root_flag,
-                map_non_root_flag, security_flag]) and self.module.params['ignore_unresolvable_hosts'] is not True:
+                map_non_root_flag, security_flag]):
             LOG.info(
                 'No change detected for the NFS Export, returning changed = False')
             return False
@@ -846,6 +857,13 @@ def modify_nfs_export(self, path, access_zone, ignore_unresolvable_hosts):
             return self.perform_modify_nfs_export(nfs_export, path, access_zone, ignore_unresolvable_hosts)
 
     def perform_modify_nfs_export(self, nfs_export, path, access_zone, ignore_unresolvable_hosts):
+        '''
+        Modify NFS export in PowerScale system
+        '''
+
+        if self.module._diff:
+            self.result.update({"diff": {"before": self.result.get("NFS_export_details"), "after": nfs_export.to_dict()}})
+
         try:
             if not self.module.check_mode:
                 if ignore_unresolvable_hosts is not True:
@@ -876,6 +894,8 @@ def delete_nfs_export(self):
         Delete NFS export from system
         '''
         nfs_export = self.result['NFS_export_details']
+        if self.module._diff:
+            self.result.update({"diff": {"before": nfs_export, "after": {}}})
         try:
             if not self.module.check_mode:
                 msg = ('Deleting NFS export with path: {0}, zone: {1} and ID: {2}'.format(

diff --git a/tests/unit/plugins/module_utils/mock_nfs_export_api.py b/tests/unit/plugins/module_utils/mock_nfs_export_api.py
@@ -17,6 +17,9 @@
 NFS_ID_2 = "206"
 SYS_ZONE = "system"
 PATH_1 = "/ifs/test_sample_nfs"
+SAMPLE_ZONE = "sample_zone"
+STATE_P = "present"
+STATE_A = "absent"
 
 NFS_COMMON_ARGS = {
     "path": None,
@@ -243,3 +246,12 @@ def modify_nfs_failed_msg():
 
 def delete_nfs_failed_msg():
     return 'Delete NFS export with path: V, zone: system, id: 205 failed with error'
+
+
+def get_failed_msgs(response_type):
+    err_msg_dict = {
+        "az_path_err": "Unable to fetch base path of Access Zone sample_zone failed with error: SDK Error message",
+        "id_err": "Got error Test Exception while getting NFS export details for ID: 123 and access zone: system",
+        "multiple_nfs_err": "Multiple NFS Exports found",
+    }
+    return err_msg_dict.get(response_type)
diff --git a/tests/unit/plugins/modules/test_nfs.py b/tests/unit/plugins/modules/test_nfs.py
@@ -16,7 +16,8 @@
 utils.get_nfs_map_object = MagicMock()
 
 
-from ansible_collections.dellemc.powerscale.plugins.modules.nfs import NfsExport, NFSHandler
+from ansible_collections.dellemc.powerscale.plugins.modules.nfs import NfsExport, NFSHandler, \
+    is_map_primary_group_modified, is_map_secondary_groups_modified, main
 from ansible_collections.dellemc.powerscale.tests.unit.plugins.\
     module_utils import mock_nfs_export_api as MockNFSApi
 from ansible_collections.dellemc.powerscale.tests.unit.plugins.\
@@ -47,7 +48,7 @@ def test_get_nfs_response(self, powerscale_module_mock):
             powerscale_module_mock, self.get_nfs_args,
             {"path": MockNFSApi.PATH_1,
              "access_zone": MockNFSApi.SYS_ZONE,
-             "state": "present"})
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.protocol_api.list_nfs_exports = MagicMock(
             return_value=NFSTestExport(1, MockNFSApi.NFS_1['exports']))
         NFSHandler().handle(powerscale_module_mock,
@@ -59,7 +60,7 @@ def test_get_nfs_root_response(self, powerscale_module_mock):
             powerscale_module_mock, self.get_nfs_args,
             {"path": "/",
              "access_zone": "sample-zone",
-             "state": "present"})
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.get_zone_base_path = MagicMock(
             return_value="/ifs/sample-zone")
         powerscale_module_mock.protocol_api.list_nfs_exports = MagicMock(
@@ -73,7 +74,7 @@ def test_get_nfs_non_system_az_response(self, powerscale_module_mock):
             powerscale_module_mock, self.get_nfs_args,
             {"path": "/sample_file_path1",
              "access_zone": "sample_zone",
-             "state": "present"})
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.zones_summary_api.get_zones_summary_zone.to_dict = MagicMock(
             return_value=MockNFSApi.ZONE)
@@ -89,7 +90,7 @@ def test_get_nfs_non_system_az_exception(self, powerscale_module_mock):
             powerscale_module_mock, self.get_nfs_args,
             {"path": "/sample_file_path1",
              "access_zone": "sample_zone",
-             "state": "present"})
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.module.params = self.get_nfs_args
         MockApiException.status = '404'
         powerscale_module_mock.protocol_api.list_nfs_exports = MagicMock(
@@ -111,11 +112,11 @@ def operation_before_create(self, powerscale_module_mock):
              "security_flavors": ["kerberos"],
              "map_root": {
                  "enabled": True, "user": "root", "primary_group": "root",
-                 "secondary_groups": [{"name": "group1", "state": "absent"}, {"name": "group2"}]},
+                 "secondary_groups": [{"name": "group1", "state": MockNFSApi.STATE_A}, {"name": "group2"}]},
              "map_non_root": {
                  "enabled": True, "user": "root", "primary_group": "root",
-                 "secondary_groups": [{"name": "group1"}, {"name": "group2", "state": "absent"}]},
-             "state": "present"})
+                 "secondary_groups": [{"name": "group1"}, {"name": "group2", "state": MockNFSApi.STATE_A}]},
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.protocol_api.list_nfs_exports = MagicMock(
             return_value=NFSTestExport()
@@ -154,7 +155,7 @@ def test_create_nfs_without_clients(self, powerscale_module_mock):
              "read_only": True,
              "client_state": "present-in-export",
              "security_flavors": ["kerberos"],
-             "state": "present"})
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.protocol_api.list_nfs_exports = MagicMock(
             return_value=NFSTestExport()
@@ -169,7 +170,7 @@ def test_create_nfs_without_client_state(self, powerscale_module_mock):
              "read_only": True,
              "read_only_clients": [MockNFSApi.SAMPLE_IP1],
              "clients": [MockNFSApi.SAMPLE_IP1],
-             "state": "present"})
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.protocol_api.list_nfs_exports = MagicMock(
             return_value=NFSTestExport()
@@ -192,8 +193,8 @@ def operation_before_modify(self, powerscale_module_mock):
              "security_flavors": ["unix", "kerberos_privacy"],
              "map_root": {
                  "enabled": True, "user": "root", "primary_group": "root",
-                 "secondary_groups": [{"name": "group1", "state": "absent"}, {"name": "group2"}]},
-             "map_non_root": {"enabled": False}, "state": "present"})
+                 "secondary_groups": [{"name": "group1", "state": MockNFSApi.STATE_A}, {"name": "group2"}]},
+             "map_non_root": {"enabled": False}, "state": MockNFSApi.STATE_P})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.get_nfs_export = MagicMock(
             return_value=MockNFSApi.NFS_1['exports'][0])
@@ -223,7 +224,7 @@ def test_remove_clients_nfs(self, powerscale_module_mock):
              "read_write_clients": [MockNFSApi.SAMPLE_IP2],
              "root_clients": [MockNFSApi.SAMPLE_IP2],
              "client_state": "absent-in-export",
-             "state": "present"})
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.get_nfs_export = MagicMock(
             return_value=MockNFSApi.NFS_2['exports'][0])
@@ -243,7 +244,7 @@ def test_replace_clients_nfs(self, powerscale_module_mock):
              "clients": [MockNFSApi.SAMPLE_IP2],
              "read_write_clients": [MockNFSApi.SAMPLE_IP3],
              "root_clients": [MockNFSApi.SAMPLE_IP1],
-             "state": "present"})
+             "state": MockNFSApi.STATE_P})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.get_nfs_export = MagicMock(
             return_value=MockNFSApi.NFS_2['exports'][0])
@@ -259,7 +260,7 @@ def test_delete_nfs_response(self, powerscale_module_mock):
             powerscale_module_mock, self.get_nfs_args,
             {"path": MockNFSApi.PATH_1,
              "access_zone": MockNFSApi.SYS_ZONE,
-             "state": "absent"})
+             "state": MockNFSApi.STATE_A})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.get_nfs_export = MagicMock(
             return_value=MockNFSApi.NFS_2['exports'][0])
@@ -273,10 +274,106 @@ def test_delete_nfs_response_exception(self, powerscale_module_mock):
             powerscale_module_mock, self.get_nfs_args,
             {"path": MockNFSApi.PATH_1,
              "access_zone": MockNFSApi.SYS_ZONE,
-             "state": "absent"})
+             "state": MockNFSApi.STATE_A})
         powerscale_module_mock.module.params = self.get_nfs_args
         powerscale_module_mock.get_nfs_export = MagicMock(
             return_value=MockNFSApi.NFS_2['exports'][0])
         powerscale_module_mock.protocol_api.delete_nfs_export = MagicMock(
             side_effect=utils.ApiException)
         self.capture_fail_json_call(MockNFSApi.delete_nfs_failed_msg(), powerscale_module_mock, NFSHandler)
+
+    def test_get_zone_base_path_exception(self, powerscale_module_mock):
+        self.set_module_params(
+            powerscale_module_mock, self.get_nfs_args,
+            {"path": "sample-path",
+             "access_zone": MockNFSApi.SAMPLE_ZONE,
+             "state": MockNFSApi.STATE_P})
+        powerscale_module_mock.zones_summary_api.get_zones_summary_zone = MagicMock(
+            side_effect=MockApiException)
+        self.capture_fail_json_call(MockNFSApi.get_failed_msgs("az_path_err"), powerscale_module_mock, NFSHandler)
+
+    def test_multiple_nfs_exception(self, powerscale_module_mock):
+        powerscale_module_mock.protocol_api.list_nfs_exports.return_value.total = 2
+        with pytest.raises(Exception) as e:
+            powerscale_module_mock.get_nfs_export('/ifs/test_nfs', MockNFSApi.SYS_ZONE)
+        assert MockNFSApi.get_failed_msgs("multiple_nfs_err") in str(e.value)
+
+    def test_nfs_by_id_exception(self, powerscale_module_mock):
+        powerscale_module_mock.protocol_api.get_nfs_export.side_effect = Exception('Test Exception')
+        with pytest.raises(Exception) as e:
+            powerscale_module_mock._get_nfs_export_from_id('123', MockNFSApi.SYS_ZONE)
+        assert str(e.value) == MockNFSApi.get_failed_msgs("id_err")
+
+    def test_invalid_path(self, powerscale_module_mock):
+        with pytest.raises(Exception) as e:
+            powerscale_module_mock.effective_path(MockNFSApi.SYS_ZONE, 'path')
+        assert str(e.value) == "Invalid path path, Path must start with '/'"
+
+    @pytest.mark.parametrize(
+        "nfs_map_params, nfs_export_details, nfs_export_map, map_type, expected_result",
+        [
+            (
+                {"primary_group": "some_group"},
+                {"map_root": {"primary_group": {"id": "domain:groupB"}}},
+                MagicMock(primary_group={"name": "groupA"}),
+                "map_root",
+                True,
+            ),
+        ]
+    )
+    def test_is_map_primary_group_modified(self, nfs_map_params, nfs_export_details, nfs_export_map, map_type, expected_result):
+        result = is_map_primary_group_modified(nfs_map_params, nfs_export_details, nfs_export_map, map_type)
+
+        assert result == expected_result
+
+    @pytest.mark.parametrize(
+        "nfs_map_params, nfs_export_details, nfs_export_map, map_type, expected_result",
+        [
+            (
+                {"secondary_groups": ["groupA"]},
+                {"export": {"secondary_groups": ["groupA"]}},
+                MagicMock(secondary_groups=[]),
+                "export",
+                True,
+            ),
+        ]
+    )
+    def test_is_map_secondary_groups_modified(self, nfs_map_params, nfs_export_details, nfs_export_map, map_type, expected_result):
+        result = is_map_secondary_groups_modified(nfs_map_params, nfs_export_details, nfs_export_map, map_type)
+
+        assert result == expected_result
+
+    def test_main(self, powerscale_module_mock):
+        main()
+
+    @pytest.mark.parametrize(
+        "playbook_client_dict, nfs_export, mod_flag, expected_result",
+        [
+            (
+                {"read_write_clients": None},
+                {"key": "value"},
+                False,
+                (False, {"key": "value"}),
+            )
+        ]
+    )
+    def test_check_read_write_clients_none(self, powerscale_module_mock, playbook_client_dict, nfs_export, mod_flag, expected_result):
+        result = powerscale_module_mock._check_read_write_clients(nfs_export, playbook_client_dict, {}, mod_flag)
+
+        assert result == expected_result
+
+    @pytest.mark.parametrize(
+        "playbook_client_dict, nfs_export, mod_flag, expected_result",
+        [
+            (
+                {"clients": None},
+                {"key": "value"},
+                False,
+                (False, {"key": "value"}),
+            )
+        ]
+    )
+    def test_chek_clients_none(self, powerscale_module_mock, playbook_client_dict, nfs_export, mod_flag, expected_result):
+        result = powerscale_module_mock._check_clients(nfs_export, playbook_client_dict, {}, mod_flag)
+
+        assert result == expected_result