Pyinfra: Interdependent operations that depends on each other

[FooBarQuaxx]

Describe the bug
Operations are planned according to the facts gathered at the start of the run.
Operations usually changes the state of the host, this means that the preconditions that exists during the call to the operation is different that what is expected when declaring it.

To Reproduce

$ cat Dockerfile
FROM fedora:32

RUN dnf install -y git

$ docker build . -t my-fedora-image

$ cat deploy.py
from pyinfra import host
from pyinfra.operations import dnf

dnf.packages(
    packages=["git"],
    present=False,
    sudo=True,
)

dnf.packages(
    packages=["git"],
    present=True,
    sudo=True,
)

print(host.state.ops)
$ pyinfra --dry @docker/my-fedora-image deploy.py
--> Loading config...
--> Loading inventory...
    The @docker connector is in beta!

--> Connecting to hosts...
    [@docker/my-fedora-image] Connected

--> Preparing operations...
    Loading: deploy.py
{@docker/my-fedora-image: {'18996c2767e7142774328ef0b335b99342919a9e': {'commands': [StringCommand(dnf remove -y git)]}, '3b2f90227d43a40a2288506c2a99bdc5e71f32b2': {'commands': []}}}
    [@docker/my-fedora-image] Ready: deploy.py

--> Proposed changes:
    Groups: @docker
    [@docker/my-fedora-image]   Operations: 2   Commands: 1   
    [@docker/my-fedora-image] docker build complete, image ID: 39b782da6859

Expected behavior

$ pyinfra --dry @docker/my-fedora-image deploy.py
--> Loading config...
--> Loading inventory...
    The @docker connector is in beta!

--> Connecting to hosts...
    [@docker/my-fedora-image] Connected

--> Preparing operations...
    Loading: deploy.py
{@docker/my-fedora-image: {'18996c2767e7142774328ef0b335b99342919a9e': {'commands': [StringCommand(dnf remove -y git)]}, '3b2f90227d43a40a2288506c2a99bdc5e71f32b2': {'commands': [StringCommand(dnf install -y git)]}}}

    [@docker/my-fedora-image] Ready: deploy.py

--> Proposed changes:
    Groups: @docker
    [@docker/my-fedora-image]   Operations: 2   Commands: 2   
    [@docker/my-fedora-image] docker build complete, image ID: 39b782da6859

Meta

• pyinfra --support output

--> 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-5.4.0-40-generic-x86_64-with-glibc2.29
      Release: 5.4.0-40-generic
      Machine: x86_64
    pyinfra: v1.0.3
    Executable: /home/FooBarQuaxx/Code/pyinfra/venv/bin/pyinfra
    Python: 3.8.2 (CPython, GCC 9.3.0)

• installed from source
• no pyinfra-debug.log

[Fizzadar]

This is a tough one! It's a long-standing limitation of pyinfra's two phase deploy (see: https://docs.pyinfra.com/en/latest/deploy_process.html#interdependent-operations).

Fix wise I've got two approaches:

• Hint based arguments (as documented above) - implemented for files/etc, could work for packages too (certainly the above example)
• Have operations modify facts to match the state they _expect to set_ - this is the ideal (IMO) but needs some serious thought. It also does not deal with things like scripts or server.shell commands/etc.

[FooBarQuaxx]

I hope we can do something about it because it's a fundamental limitation.

[Fizzadar]

Agreed - I'd like to solve this.

For the docs example - I don't think this will ever be possible, certainly not with the two-phase execution model. Even if pyinfra was to update facts as operations executed, it'd miss stuff like package config files/etc being added to the filesystem. In the case of files one option could be to check at runtime, ie stat {path} && rm -f {path}, but that means always outputting a command for that operation, defeating a major advantage of the two phase execution!

Of note here is also the files.line operation, which explicitly handles the "unknown" situation by outputting a small script (https://github.com/Fizzadar/pyinfra/blob/master/pyinfra/operations/files.py#L262). It could be possible to replicate this same idea to other operations.

I'm keen to hear more about your use-case here as well - particularly installing and later removing the same package. Normally I'd treat oprations like installing packages as things you'd only want to run once, but that's coming from my own experience only.

Sidenote: I played around a bit with a potential API for updating facts during phase 1, which if fully implemented would satisfy your original example and others: https://github.com/Fizzadar/pyinfra/compare/experiment-update-facts-in-operation.

[nadrimajstor]

For what its worth, I believe, I've just hit a similar case:

yum.packages(packages=['ssmtp'], present=False)
yum.packages(packages=['postfix', 'mailx']) #ssmtp has to be removed or postifix install will not succeed

[Fizzadar]

Another example from #394 via @wookayin:

apt.packages(                                                            
    name='Install Docker via apt.',                                      
    packages=['docker-ce', 'docker-ce-cli', 'containerd.io'],            
    update=_add_apt.changed,
)                                                                        

server.group(                                                            
    name='Create docker user group',                                     
    group='docker',                                                      
)                                                                        

will fail when docker gets installed, because

At the moment when facts are gathered, there is no docker installed and therefore no docker group
After docker installation is done, it tries to do add group, but fails with an error "the group already exists"

[Fizzadar]

I've made some progress on this! Based on my experiment branch and some extra work, I now have support for the packaging modules (dnf.packages and yum.packages examples) implemented + tested in: https://github.com/Fizzadar/pyinfra/pull/397.

This approach works really well for calls to the same operation. The current model is roughly:

operations are idempotent when considered individually

this PR/approach would increase this to

single operations are idempotent across multiple calls

Unfortunately this goes nowhere to fixing the docker example which would be a full:

all operations are idempotent, both individually and with each other

So far I can't see any suitable way to achieve this without modifying the execution model to evaluate facts at runtime (loosing dry-runs and so on). For this situation, it could be possible to fix this by outputting scripts that first check to see if the state has changed, ie groups | grep <group> > /dev/null || groupadd <group>.

---

Ultimately I think there will always be some kind of edge cases here, no matter the implementation. I think it's super important to be really upfront where these remain and heavily document them, as I do not wish to change the execution method (I strongly believe the two-phase deploy has more advantages that disadvantages, esp. for future work ie APIs/servers/etc). Based on the above I can definitely see a path forward here that should support almost all use-cases, which is excellent :)

[Fizzadar]

I am now confident that https://github.com/Fizzadar/pyinfra/pull/397 covers all of the cases listed here, which is excellent :)

Need to decide next steps in terms of documenting and keeping track of any similar future issues, as they can be extremely nuanced/difficult to identify.

[nikaro]

For reference, i have a similar case:

server.user(
    name='Create user',
    user='nicolas',
)

server.shell(
    name='Hello',
    commands=['echo "Hello $USER"'],
    su_user='nicolas',
)

Here is the real usecase: https://www.reddit.com/r/pyinfra/comments/i50n0f/create_an_user_and_then_use_it_for_next_tasks/

I will test the PR to see it is fixed by it.

Edit: actually it works fine with server.shell operation but not with (at least) files.download and git.repo.

[Fizzadar]

@nikaro I believe this is failing due to fact collection (which happens before the user is created) is trying to use that user. In effect it's not possible to create a user and then use them with the auth arguments sudo_user/su_user.

Just noticed your reddit reply which should indeed cover this, using the user/group arguments to files.* operations to avoid the need for su.

The auth arguments expect the users to already exist on the remote side. I need to a) document this better and b) replicate your issue and make it output something useful, because the error you're getting is an awful experience!

[Fizzadar]

This has now been released in v1.1 🎉!