From cfaa79ecd2c1423b723df73c96e52b2cfc0fe1f2 Mon Sep 17 00:00:00 2001
From: GomathiselviS <gomathiselvi@gmail.com>
Date: Mon, 28 Sep 2020 14:34:08 -0400
Subject: [PATCH] eos_acls: Fix to maintain idempotent state for overridden
 operation (#116)

eos_acls: Fix to maintain idempotent state for overridden operation

Reviewed-by: Rohit Thakur <rohitthakur2590@outlook.com>
             https://github.com/rohitthakur2590
---
 .../116-acls-overridden-idempotent.yaml       |  3 ++
 .../network/eos/config/acls/acls.py           | 42 +++++++++++++++----
 .../eos_acls/tests/common/overridden.yaml     |  2 +-
 .../unit/modules/network/eos/test_eos_acls.py |  1 -
 4 files changed, 39 insertions(+), 9 deletions(-)
 create mode 100644 changelogs/fragments/116-acls-overridden-idempotent.yaml

diff --git a/changelogs/fragments/116-acls-overridden-idempotent.yaml b/changelogs/fragments/116-acls-overridden-idempotent.yaml
new file mode 100644
index 000000000..ef9765e74
--- /dev/null
+++ b/changelogs/fragments/116-acls-overridden-idempotent.yaml
@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - Added fix to maintain the idempotency while using overridden operation.
diff --git a/plugins/module_utils/network/eos/config/acls/acls.py b/plugins/module_utils/network/eos/config/acls/acls.py
index 6d872940b..e82821e8b 100644
--- a/plugins/module_utils/network/eos/config/acls/acls.py
+++ b/plugins/module_utils/network/eos/config/acls/acls.py
@@ -260,7 +260,6 @@ def _state_overridden(self, want, have):
         ace_diff = {}
         h_afi_list = []
         w_afi_list = []
-        diff = False
         for h in have:
             h_afi_list.append(h["afi"])
         for w in want:
@@ -285,7 +284,6 @@ def _state_overridden(self, want, have):
                                         w_ace, h_acl["aces"]
                                     )
                                     if ace_diff:
-                                        diff = True
                                         h = {
                                             "afi": h["afi"],
                                             "acls": [
@@ -297,6 +295,25 @@ def _state_overridden(self, want, have):
                                         }
                                         remove_cmds = del_commands(h, have)
                                         commands.append(remove_cmds)
+                                        w_diff = [
+                                            {
+                                                "afi": h["afi"],
+                                                "acls": [
+                                                    {
+                                                        "name": h_acl["name"],
+                                                        "aces": [w_ace],
+                                                    }
+                                                ],
+                                            }
+                                        ]
+                                        config_cmds = set_commands(
+                                            w_diff, have
+                                        )
+                                        config_cmds = list(
+                                            itertools.chain(*config_cmds)
+                                        )
+                                        commands.append(config_cmds)
+
                     for hname in h_names:
                         if hname not in w_names:
                             h = {"afi": h["afi"], "acls": [{"name": hname}]}
@@ -304,13 +321,22 @@ def _state_overridden(self, want, have):
                             if remove_cmds not in commands:
                                 commands.append(remove_cmds)
 
-        if diff:
-            config_cmds = set_commands(want, have)
-            config_cmds = list(itertools.chain(*config_cmds))
-            commands.append(config_cmds)
         if commands:
             commands = list(itertools.chain(*commands))
-        return commands
+        commandset = []
+        for c in commands:
+            access_list = re.findall(r"(ip.*) access-list (.*)", c)
+            if access_list and "no" in c:
+                commandset.append(c)
+                continue
+            if access_list:
+                acl_index = commands.index(c)
+            else:
+                if commands[acl_index] not in commandset:
+                    commandset.append(commands[acl_index])
+                commandset.append(c)
+
+        return commandset
 
     def _state_merged(self, want, have):
         """ The command generator when state is merged
@@ -382,6 +408,8 @@ def set_commands(want, have):
 
 def get_updated_ace(w, h):
     # gives the ace to be updated in case of merge update.
+    if not dict_diff(w, h):
+        return
     w_updated = w.copy()
     for hkey in h.keys():
         if hkey not in w.keys():
diff --git a/tests/integration/targets/eos_acls/tests/common/overridden.yaml b/tests/integration/targets/eos_acls/tests/common/overridden.yaml
index 025036cf9..0114000bf 100644
--- a/tests/integration/targets/eos_acls/tests/common/overridden.yaml
+++ b/tests/integration/targets/eos_acls/tests/common/overridden.yaml
@@ -53,7 +53,7 @@
 
     - assert:
         that:
-          - result.commands|length == 8
+          - result.commands|length == 7
           - result.changed == true
           - "'ip access-list test1' in result.commands"
           - "'10 permit ospf any any log' in result.commands"
diff --git a/tests/unit/modules/network/eos/test_eos_acls.py b/tests/unit/modules/network/eos/test_eos_acls.py
index e8ba97790..a34eeba64 100644
--- a/tests/unit/modules/network/eos/test_eos_acls.py
+++ b/tests/unit/modules/network/eos/test_eos_acls.py
@@ -254,7 +254,6 @@ def test_eos_acls_overridden(self):
             "ip access-list test1",
             "no 35",
             "no 45",
-            "ip access-list test1",
             "10 permit ospf 30.2.0.0/8 any log",
         ]
         self.execute_module(changed=True, commands=commands)