Re-visit visited predecessors for react_on events

Events of dependency type by design cant insert tasks into
the graph during build, but if reaction is processed
after dependency, and they have same child - it is possible
that dependency wont be present in the resulting graph
which will lead to incorrect order

Closes-Bug #1553187

Change-Id: I97cae1be538df5dd8ccd8b8bfcfc5bb3541b6e98

solar/events/api.py

@@ -138,7 +138,6 @@ def build_edges(changes_graph, events):
     for res_evts in events.values():
         for ev in res_evts:
             events_graph.add_edge(ev.parent_node, ev.child_node, event=ev)
-
     stack = changes_graph.nodes()
     visited = set()
     while stack:
@@ -148,7 +147,8 @@ def build_edges(changes_graph, events):
                       events_graph.successors(event_name))
         else:
             log.debug('No outgoing events based on %s', event_name)
-        if event_name not in visited:
+        if event_name in visited:
+            continue
         for parent, child, data in events_graph.edges(event_name,
                                                       data=True):
             succ_ev = data['event']
@@ -158,5 +158,11 @@ def build_edges(changes_graph, events):
                 for ev in new_events:
                     events_graph.add_edge(
                         ev.parent_node, ev.child_node, event=ev)
+                # re-visit all possible predecessors that were
+                # already visited
+                for pred in events_graph.predecessors(succ_ev.child_node):
+                    if pred in visited and event_name != pred:
+                        stack.append(pred)
+                        visited.discard(pred)
         visited.add(event_name)
     return changes_graph

solar/events/controls.py

@@ -98,7 +98,7 @@ class React(Event):
     etype = 'react_on'
 
     def insert(self, changed_resources, changes_graph):
-
+        created = False
         if self.parent_node in changes_graph:
             if self.child_node not in changes_graph:
                 try:
@@ -111,11 +111,11 @@ class React(Event):
                     target=location_id,
                     errmsg='', type='solar_resource',
                     args=[self.child, self.child_action])
-
+                created = True
             changes_graph.add_edge(
                 self.parent_node, self.child_node, state=self.state)
             changed_resources.append(self.child_node)
-            return True
+        return created
 
 
 class StateChange(Event):

solar/test/test_events.py

@@ -81,6 +81,22 @@ def test_nova_api(nova_deps):
     assert changes_graph.successors('nova_api.update') == ['nova.reboot']
 
 
+def test_mandatory_revisit():
+    events = {
+        'e1': [evapi.Dep('e1', 'run', 'success', 'e2', 'run'),
+               evapi.React('e1', 'run', 'success', 'e2', 'start')],
+        'e2': [evapi.React('e2', 'start', 'success', 'e2', 'run')]}
+    for name in events:
+        r = Resource.from_dict(dict(key=name, name=name))
+        r.inputs.add_new('location_id', '1')
+        r.save()
+        evapi.add_events(name, events[name])
+    changes_graph = nx.DiGraph()
+    changes_graph.add_node('e1.run')
+    evapi.build_edges(changes_graph, events)
+    assert set(changes_graph.predecessors('e2.run')) == {'e1.run', 'e2.start'}
+
+
 @fixture
 def rmq_deps():
     """Example of a case when defaults are not good enough.
@@ -125,7 +141,8 @@ def test_riak():
     events = {
         'riak_service1': [
             evapi.React('riak_service1', 'run', 'success', 'riak_service2',
-                        'run'), evapi.React('riak_service1', 'run', 'success',
+                        'run'),
+            evapi.React('riak_service1', 'run', 'success',
                         'riak_service3', 'run')
         ],
         'riak_service3': [