nso_config break cycles in dependency sorting (#36828)

False assumption that values can not have cyclic dependencies. Fix by
removing dependency on self and look for cycles, if found remove
dependency to get a partial sort done.

lib/ansible/module_utils/network/nso/nso.py

@@ -285,6 +285,10 @@ class ValueBuilder(object):
             self.value = value
             self.deps = deps
 
+            # nodes can depend on themselves
+            if self.tag_path in self.deps:
+                self.deps.remove(self.tag_path)
+
         def __lt__(self, rhs):
             l_len = len(self.path.split('/'))
             r_len = len(rhs.path.split('/'))
@@ -348,12 +352,13 @@ class ValueBuilder(object):
     @property
     def values(self):
         if self._values_dirty:
-            self._values = self._sort_values(sorted(self._values))
+            self._values = ValueBuilder.sort_values(self._values)
             self._values_dirty = False
 
         return self._values
 
-    def _sort_values(self, values):
+    @staticmethod
+    def sort_values(values):
         class N(object):
             def __init__(self, v):
                 self.tmp_mark = False
@@ -361,7 +366,32 @@ class ValueBuilder(object):
                 self.v = v
 
         sorted_values = []
-        nodes = [N(v) for v in values]
+        nodes = [N(v) for v in sorted(values)]
+
+        def get_node(tag_path):
+            return next((m for m in nodes
+                         if m.v.tag_path == tag_path), None)
+
+        def is_cycle(n, dep, visited):
+            visited.add(n.v.tag_path)
+            if dep in visited:
+                return True
+
+            dep_n = get_node(dep)
+            if dep_n is not None:
+                for sub_dep in dep_n.v.deps:
+                    if is_cycle(dep_n, sub_dep, visited):
+                        return True
+
+            return False
+
+        # check for dependency cycles, remove if detected. sort will
+        # not be 100% but allows for a best-effort to work around
+        # issue in NSO.
+        for n in nodes:
+            for dep in n.v.deps:
+                if is_cycle(n, dep, set()):
+                    n.v.deps.remove(dep)
 
         def visit(n):
             if n.tmp_mark:
@@ -380,9 +410,10 @@ class ValueBuilder(object):
 
             return True
 
-        while any((not n.mark for n in nodes)):
-            n = next((node for node in nodes if not node.mark))
+        n = next((n for n in nodes if not n.mark), None)
+        while n is not None:
             visit(n)
+            n = next((n for n in nodes if not n.mark), None)
 
         return sorted_values[::-1]
 

test/units/module_utils/network/nso/test_nso.py

@@ -600,3 +600,48 @@ class TestVerifyVersion(unittest.TestCase):
         self.assertFalse(nso.verify_version_str('4.4.1', [(4, 6), (4, 5, 1)]))
         self.assertFalse(nso.verify_version_str('4.4.1.2', [(4, 6), (4, 5, 1)]))
         self.assertFalse(nso.verify_version_str('4.5.0', [(4, 6), (4, 5, 1)]))
+
+
+class TestValueSort(unittest.TestCase):
+    def test_sort_parent_depend(self):
+        values = [
+            nso.ValueBuilder.Value('/test/list{entry}', '/test/list', 'CREATE', ['']),
+            nso.ValueBuilder.Value('/test/list{entry}/description', '/test/list/description', 'TEST', ['']),
+            nso.ValueBuilder.Value('/test/entry', '/test/entry', 'VALUE', ['/test/list', '/test/list/name'])
+        ]
+
+        result = [v.path for v in nso.ValueBuilder.sort_values(values)]
+
+        self.assertEquals(['/test/list{entry}', '/test/entry', '/test/list{entry}/description'], result)
+
+    def test_sort_break_direct_cycle(self):
+        values = [
+            nso.ValueBuilder.Value('/test/a', '/test/a', 'VALUE', ['/test/c']),
+            nso.ValueBuilder.Value('/test/b', '/test/b', 'VALUE', ['/test/a']),
+            nso.ValueBuilder.Value('/test/c', '/test/c', 'VALUE', ['/test/a'])
+        ]
+
+        result = [v.path for v in nso.ValueBuilder.sort_values(values)]
+
+        self.assertEquals(['/test/a', '/test/b', '/test/c'], result)
+
+    def test_sort_break_indirect_cycle(self):
+        values = [
+            nso.ValueBuilder.Value('/test/c', '/test/c', 'VALUE', ['/test/a']),
+            nso.ValueBuilder.Value('/test/a', '/test/a', 'VALUE', ['/test/b']),
+            nso.ValueBuilder.Value('/test/b', '/test/b', 'VALUE', ['/test/c'])
+        ]
+
+        result = [v.path for v in nso.ValueBuilder.sort_values(values)]
+
+        self.assertEquals(['/test/a', '/test/c', '/test/b'], result)
+
+    def test_sort_depend_on_self(self):
+        values = [
+            nso.ValueBuilder.Value('/test/a', '/test/a', 'VALUE', ['/test/a']),
+            nso.ValueBuilder.Value('/test/b', '/test/b', 'VALUE', [])
+        ]
+
+        result = [v.path for v in nso.ValueBuilder.sort_values(values)]
+
+        self.assertEqual(['/test/a', '/test/b'], result)