Pyinfra: Fix support `use_sudo_password` in facts

[nardusg]

Describe the bug
If I do have user present it does not continue because the user already exist.

To Reproduce
server.user(
{'Ensure users is present'},
'someuser',
present=True,
)

Expected behavior
To not error and continue.

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-5.6.19-300.fc32.x86_64-x86_64-with-glibc2.2.5
  Release: 5.6.19-300.fc32.x86_64
  Machine: x86_64
pyinfra: v0.16
Executable: /home/nardusg/.local/bin/pyinfra
Python: 3.8.3 (CPython, GCC 10.1.1 20200507 (Red Hat 10.1.1-1))

• How was pyinfra installed (source/pip)?
  pip
• Include pyinfra-debug.log (if one was created)
• Consider including output with -vv and --debug.
  --> Starting operation: Ensure users is present
  [pyinfra.api.operations] Starting operation Ensure users is present on someserver
  [pyinfra.api.connectors.ssh] Running command on someserver: (pty=None) env SUDO_ASKPASS=/tmp/pyinfra-pyinfra-sudo-askpass * sudo -H -A -k sh -c 'useradd -d /home/someuser someuser'
  [narrem.narra.co.za] >>> env SUDO_ASKPASS=/tmp/pyinfra-pyinfra-sudo-askpass * sudo -H -A -k sh -c 'useradd -d /home/someuser someuser'
  [someserver] useradd: user 'someuser' already exists
  [pyinfra.api.connectors.ssh] Waiting for exit status...
  [pyinfra.api.connectors.ssh] Command exit status: 9
  [narrem.narra.co.za] Error
  [pyinfra.api.state] Failing hosts: someserver

[FooBarQuaxx]

Looks like something is wrong with facts gathering. I am curious to know if pyinfra INVENTORY fact users has someuser as part of its output?

[nardusg]

    [pyinfra.api.operation] Adding operation, {'Ensure users is present'}, called @ deploy.py:21, opLines=(0, 21), opHash=b1d1da4808a9765a47e35b9cfb13e271e108f5f0
    [pyinfra.api.facts] Getting fact: users (ensure_hosts: (someserver,))
    [pyinfra.api.connectors.ssh] Running command on someserver: (pty=False) sudo -H -n sh -c '
        for i in `cat /etc/passwd | cut -d: -f1`; do
            ID=`id $i`
            META=`cat /etc/passwd | grep ^$i: | cut -d: -f6-7`
            echo "$ID $META"
        done
    '
    [pyinfra.api.connectors.ssh] Waiting for exit status...
    [pyinfra.api.connectors.ssh] Command exit status: 1
    [pyinfra.api.facts] Loaded fact users
    [pyinfra.api.facts] Getting fact: directory (ensure_hosts: (someserver,))
    [pyinfra.api.connectors.ssh] Running command on someserver: (pty=False) sudo -H -n sh -c 'ls -ld --time-style=long-iso /home/someuser 2> /dev/null || ls -ldT /home/someuser'
    [pyinfra.api.connectors.ssh] Waiting for exit status...
    [pyinfra.api.connectors.ssh] Command exit status: 1

[FooBarQuaxx]

I am pretty confident that this can be treated as a bug. The root cause is that fact gathering does not bother digging deeper if the execution was not successful. Around Here we see that we just use the default value for the Fact class in case the fact gathering was not a success which in this case will cause the server.user operation to do something that it should not do which is trying to create a user that already exists.

[Fizzadar]

Interesting, so something in the little shell script there is failing. @nardusg what OS is the target host in this instance? My thinking is that the format of /etc/passwd combined with the grep/cut call is failing.

This should absolutely not fail so easily - one bad line in the file will currently cause it to fail.

[nardusg]

Centos 8 😱

[Fizzadar]

Annoyingly I can't replicate this using a CentOS 8 VM or Docker container! Does the following run OK if you execute it manually:

for i in `cat /etc/passwd | cut -d: -f1`; do
    ID=`id $i`
    META=`cat /etc/passwd | grep ^$i: | cut -d: -f6-7`
    echo "$ID $META"
done

I believe this may be similar to https://github.com/Fizzadar/pyinfra/issues/336, where the lack of semicolons causes issues. I have pushed https://github.com/Fizzadar/pyinfra/commit/8ec1d0dc06f8ef5edc8bd6d9cc538e788be54a5e which adds these. If possible would you be able to test from the master branch @nardusg?

[FooBarQuaxx]

I think it would be also helpful for meaningful error messages to be reported when gathering facts and not just fail silently.

[nardusg]

[nardusg]

Annoyingly I can't replicate this using a CentOS 8 VM or Docker container! Does the following run OK if you execute it manually:

for i in `cat /etc/passwd | cut -d: -f1`; do
    ID=`id $i`
    META=`cat /etc/passwd | grep ^$i: | cut -d: -f6-7`
    echo "$ID $META"
done

I believe this may be similar to #336, where the lack of semicolons causes issues. I have pushed 8ec1d0d which adds these. If possible would you be able to test from the master branch @nardusg?

I can, will just need to find out how I can update the pip version to specific branch

[nardusg]

after_patch.txt

[nardusg]

Also notice all of the following lines, bit weird:

[nardusg]

System Users

server.user(
{'Ensure users is present'},
'narrem',
public_keys='ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQCna1qWLoJROMz9B379FQhCYs+I4s0ZodMzeaG6fPmF5EGrjgEEUtwFPa9zdgaI2JzjKOVvnqo5bJ6qfN0Ommwxd36cqPlojfGRL8XzilITkyjtgX19abqe4zuaDlz0DOnIEb/MUIhCmlKtAoEXSPLBZ30zGm8YCVflFRPJEk7LaWiMApI4hHvmWAjESIXpCwPx7bcxvrV13fZ1JeByEoLv9eoEQx4vn5qkzb543zkeFdkawVaaX/nYlLTLWDP5Utj9p+2nVsIu9HzmpNxTVeenPKL8Frk+rUSdD/1OOQScX0snk5b0ttyll2YJ12kkz2l48Jybbg5ZLps5FxET4WI/009muOREbsVHRM+2ATOgRi4ZsdK2vLpNt2FaPrjLt7wuSYD1531S3C7UtTT56FJzfUKYHRQeAhowPfFLrDf4fmVBgBjJ5MFrDD1KL9pyibILxllPBP4TSJ6JM7IGehVwrcNeWTKn/H4l/0/j2dEwt6vPU05QPEBG4AxJJXoJgy7JfzNxdzA8oC5RqGpkekDWiF5+N9CDGVvo3YVzON8ZkytkGFt38dvcQN+4pFhCYPqKTszQdqqMtu7IPPwgb1bi90/RpJnUUDciGxKhcUThXO/CxUlE6PeskplK7T3/TfKvi9j57H0N9NeWQQumWqiP8rfHI4ne7t2+LJSHdWKL1Q== root@lunarnix',
present=True,
# ensure_home=True,
)

[Fizzadar]

@nardusg public_keys here must be a list. But it looks like the change has fixed it :)

I'm also going to update public_keys to work with a string as this is confusing.

[nardusg]

It stills fail, but lets see with your code:

--> Starting operation: Ensure users is present
[pyinfra.api.operations] Starting operation Ensure users is present on narrem.narra.co.za
[pyinfra.api.connectors.ssh] Running command on narrem.narra.co.za: (pty=None) env SUDO_ASKPASS=/tmp/pyinfra-pyinfra-sudo-askpass * sudo -H -A -k sh -c 'useradd -d /home/narrem narrem'
[pyinfra.api.connectors.ssh] Waiting for exit status...
[pyinfra.api.connectors.ssh] Command exit status: 9
[narrem.narra.co.za] useradd: user 'narrem' already exists
[narrem.narra.co.za] Error
[pyinfra.api.state] Failing hosts: narrem.narra.co.za

Did put it in a list, works nicely

Thanks

Nar

[Fizzadar]

@FooBarQuaxx so the issue with this is facts are intentionally expected to fail at times, ie running a deploy over a mix of linux distributions you want host.fact.deb_packages to work on them all, even if dpkg doesn't actually exist.

BUT this absolutely hides underlying issues such as this one, which is no good. I am going to look at:

• Make facts fail when this happens (respecting the ongoing operation's ignore_errors)
• Add a fact class attribute to use_default_on_error which will support the above assumptions with apt/etc without just having || true everywhere

---

Still does not solve the underlying issue here. @nardusg could you re-run just pyinfra HOST fact users -v --debug? I need to see exactly what error was encountered during the fact gathering.

[nardusg]

facts_debug.txt

[Fizzadar]

It works! Which makes the original error all the more bizzare. I can't see why this would be any different when running in the context of an operation. The only difference here is the lack of sudo, but that should not change how the underlying command is executed. Not entirely sure how to proceed at this point.

[Fizzadar]

I think I've got it! Note how in the debug lugs there's no sudo password in the fact commands.

I believe there is a bug here where use_sudo_password isn't being respected in commands, hence them all failing with status 1 and now output.

[FooBarQuaxx]

@Fizzadar Yes, I get that. It is not necessary that if a system is missing a feature, that it means it's should be a show stopper. But not in this case though.

[Fizzadar]

@FooBarQuaxx 👍 agreed, I've added https://github.com/Fizzadar/pyinfra/issues/347!

[Fizzadar]

@nardusg I've just released 0.16.1 which should fix this!

[nardusg]

Awesome, it works :smiley:

Thanks for fixing