CycleError with loop and more than 1 server

[julienlavergne]

Describe the bug

When running operations in a loop on more than 1 server, there is cases that creates a "graphlib.CycleError". This is due to the fact that we put every operations of every host in the same graph and check for cycle. The way operation hash is generated may create cycle.

To Reproduce

I went to great length to understand the issue and ended up with a minimal example. The random is here to represent the result of a get_fact that is different on each host. But simply putting a conditional get_fact in a loop can triggers the issue.

Run below example on 2 hosts or more:

import random

from pyinfra.operations import server

for i in range(0, 10):
    server.shell(name=f"ls A", commands="ls")

    if random.randint(0, 1) == 0:
        server.shell(name=f"ls B", commands="ls")

    server.shell(name=f"ls C", commands="ls")

Expected behavior

There is no cycle in the above example.

Meta

• Include output of pyinfra --support.

  --> Support information:
  
  If you are having issues with pyinfra or wish to make feature requests, please
  check out the GitHub issues at https://github.com/Fizzadar/pyinfra/issues .
  When adding an issue, be sure to include the following:
  
  System: Linux
    Platform: Linux-3.10.0-1160.71.1.el7.x86_64-x86_64-with-glibc2.17
    Release: 3.10.0-1160.71.1.el7.x86_64
    Machine: x86_64
  pyinfra: v2.3
  Executable: /opt/tools/python/bin/pyinfra
  Python: 3.9.4 (CPython, GCC 10.3.0)

[Fizzadar]

Hi @julienlavergne thank you for raising this and getting a reprodution example up!

The single DAG is a requirement for having all hosts execute in expected order, but loops once again break things!

[Fizzadar]

I have dug into this today and written up my thoughts here: https://github.com/Fizzadar/pyinfra/blob/850b443a95269df5758a0bd35e34f651b8b444ba/docs/deploy-process.rst#loops--cycle-errors

I have also PR'd a workaround for this in https://github.com/Fizzadar/pyinfra/pull/876.

Unfortunately I cannot think of any way to achieve this without an explicit workaround - there's just no way [I can think of] to pass through the loop position automatically...

[julienlavergne]

I added a comment to the PR

[Fizzadar]

This should now be resolved in v2.5 once pypi is back up to complete the release!