pyinfra-dev / pyinfra

pyinfra turns Python code into shell commands and runs them on your servers. Execute ad-hoc commands and write declarative operations. Target SSH servers, local machine and Docker containers. Fast and scales from one server to thousands.
https://pyinfra.com
MIT License
3.91k stars 382 forks source link

Pyinfra tries to collect postgresql fact on wrong host #767

Closed PermanAtayev closed 2 years ago

PermanAtayev commented 2 years ago

Describe the bug

While running postgresql.PostgresqlDatabases fact, pyinfra does not take into consideration postgresql_host conditional logic.

To Reproduce

databases = host.get_fact(postgresql.PostgresqlDatabases, postgresql_user="admin", postgresql_password="admin",
                          postgresql_host="localhost" if host.data.node=="master" else "192.168.56.10")

Here is the error log:

psql: error: connection to server at "localhost" (127.0.0.1), port 5432 failed: Connection refused
    [192.168.56.11]     Is the server running on that host and accepting TCP/IP connections?

Expected behavior

With the fact collection above, pyinfra would not run the conditional logic for host selection, and default to host as localhost regardless of which server it is running on. Since one of the servers, I am targeting, is running the postgres its host needs to be localhost, while others are not running postgres and thus the host they use to connect to postgres needs to be different (ip in this case). Thus, I would expect that the conditional logic in postgresql_host field is respected when choosing a host to connect to.

Meta

Fizzadar commented 2 years ago

Hi @PermanAtayev - I believe the issue here is the way pyinfra (v1) collects facts - each fact is gathered on all hosts in parallel. That means that your example will effectively run both these facts on all hosts:

not_master_node_databases = host.get_fact(postgresql.PostgresqlDatabases, postgresql_user="admin", postgresql_password="admin",
                          postgresql_host="localhost")

master_node_databases = host.get_fact(postgresql.PostgresqlDatabases, postgresql_user="admin", postgresql_password="admin",
                          postgresql_host="192.168.56.10")

Currently, a workaround for this would be to do the following:

if host.data.node == "master":
    databases = host.get_fact(postgresql.PostgresqlDatabases, postgresql_user="admin", postgresql_password="admin",
                          postgresql_host="192.168.56.10")
else:
    databases = host.get_fact(postgresql.PostgresqlDatabases, postgresql_user="admin", postgresql_password="admin",
                          postgresql_host="localhost")

Version 2 (next few weeks hopefully!) will solve this issue by changing how operations are generated meaning each fact neednt be loaded for every host.

PermanAtayev commented 2 years ago

Hello @Fizzadar , thank you for getting back to me :)

I have originally started with your workaround, but fact collection does not respect the if condition and tries to connect to db in default port 5432 for all nodes. Thus, all slave nodes fail after that fact gathering part, since they don't have a pg cluster running on that port.

Here is the code chunk that I used, where is_master_node is a boolean flag that is True only for master node:

if host.data.is_master_node:
    databases = host.get_fact(postgresql.PostgresqlDatabases, su_user="postgres")
    if "desired_db" not in databases:
        python.call(name="Initialize db", function=init_db)

Error for slave node:

psql: error: connection to server on socket "/var/run/postgresql/.s.PGSQL.5432" failed: No such file or directory
Is the server running locally and accepting connections on that socket?
Error: could not load fact: postgresql_databases 
Fizzadar commented 2 years ago

@PermanAtayev my apologies - my workaround actually has exactly the same issue. The fact will always be executed on all hosts. A quick hack for now could be to ignore errors:

host.get_fact(postgresql.PostgresqlDatabases, su_user="postgres", ignore_errors=True)

This should get you going. Alternatively you could try out v2.0.dev0 which should properly resolve this issue. This is an unfortunate gotcha with the way v1 generates operations.

PermanAtayev commented 2 years ago

@PermanAtayev my apologies - my workaround actually has exactly the same issue. The fact will always be executed on all hosts. A quick hack for now could be to ignore errors:

host.get_fact(postgresql.PostgresqlDatabases, su_user="postgres", ignore_errors=True)

This should get you going. Alternatively you could try out v2.0.dev0 which should properly resolve this issue. This is an unfortunate gotcha with the way v1 generates operations.

I will check v2.0.dev0 because ignoring errors is not really an option. Thank you :)

Fizzadar commented 2 years ago

v2 is now out which should resolve this, please re-open or make a new issue if there are still problems :)