Apply @deploy wrapped func default arguments to global arguments

[romanchyla]

Describe the bug

A changed behaviour in a new release - context (some shared state) seems to be leaking or getting modified between calls.

To Reproduce

This test fails (see below info about version info).

It passes if I call get_fact before calling the nested deployment; it also passes if I set the global _sudo

from pyinfra import host
from pyinfra.api import deploy
from pyinfra.facts.server import Home


@deploy("")
def do(_sudo=True):
    # (1)
    # uncomment this to make the test to pass
    # assert host.get_fact(Home) != "/root"
    # (2) set _sudo=False (above) to make this
    # test to pass
    extensions(_sudo=_sudo)


@deploy("")
def extensions(_sudo=True):
    assert host.get_fact(Home) != "/root"

Expected behavior

host.get_fact(Home) should return the home of the user who is executing the deployment (which used to be the default behaviour). It should not return different results if (some) prior call is made -- it should not share environment/context with the parent. If it does, it should do it consistently.

Meta

(.venv) rchyla@u0b1b660032e456:~/BoringStuffAutomated$ 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.15.0-56-generic-x86_64-with-glibc2.35
      Release: 5.15.0-56-generic
      Machine: x86_64
    pyinfra: v2.5.1
    Executable: /home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/bin/pyinfra
    Python: 3.10.6 (CPython, GCC 11.3.0)

pyinfra @local bsa.setup.ubuntu.bug.do -vv --debug

--> Loading config...

--> Loading inventory...
    [pyinfra_cli.inventory] Creating fake inventory...
    [pyinfra_cli.inventory] Checking possible group_data directory: /home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated
    [pyinfra_cli.inventory] Looking for group data in: /home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/group_data/all.py

--> Connecting to hosts...
    [@local] Connected
INFO:pyinfra:[@local] Connected
    [pyinfra.api.state] Activating host: @local
DEBUG:pyinfra:Activating host: @local

--> Preparing operation...
    [pyinfra.api.host] Starting deploy  (args={'sudo': False, 'sudo_user': None, 'use_sudo_login': False, 'use_sudo_password': False, 'preserve_sudo_env': False, 'su_user': None, 'use_su_login': False, 'preserve_su_env': False, 'su_shell': None, 'doas': False, 'doas_user': None, 'shell_executable': 'sh', 'chdir': None, 'env': {}, 'success_exit_codes': [0], 'timeout': None, 'get_pty': None, 'stdin': None, 'name': None, 'ignore_errors': False, 'continue_on_error': False, 'precondition': None, 'postcondition': None, 'on_success': None, 'on_error': None, 'parallel': 1, 'run_once': False, 'serial': False}, data=None)
DEBUG:pyinfra:Starting deploy  (args={'sudo': False, 'sudo_user': None, 'use_sudo_login': False, 'use_sudo_password': False, 'preserve_sudo_env': False, 'su_user': None, 'use_su_login': False, 'preserve_su_env': False, 'su_shell': None, 'doas': False, 'doas_user': None, 'shell_executable': 'sh', 'chdir': None, 'env': {}, 'success_exit_codes': [0], 'timeout': None, 'get_pty': None, 'stdin': None, 'name': None, 'ignore_errors': False, 'continue_on_error': False, 'precondition': None, 'postcondition': None, 'on_success': None, 'on_error': None, 'parallel': 1, 'run_once': False, 'serial': False}, data=None)
    [pyinfra.api.host] Starting deploy  (args={'sudo': True, 'sudo_user': None, 'use_sudo_login': False, 'use_sudo_password': False, 'preserve_sudo_env': False, 'su_user': None, 'use_su_login': False, 'preserve_su_env': False, 'su_shell': None, 'doas': False, 'doas_user': None, 'shell_executable': 'sh', 'chdir': None, 'env': {}, 'success_exit_codes': [0], 'timeout': None, 'get_pty': None, 'stdin': None, 'name': None, 'ignore_errors': False, 'continue_on_error': False, 'precondition': None, 'postcondition': None, 'on_success': None, 'on_error': None, 'parallel': 1, 'run_once': False, 'serial': False}, data=None)
DEBUG:pyinfra:Starting deploy  (args={'sudo': True, 'sudo_user': None, 'use_sudo_login': False, 'use_sudo_password': False, 'preserve_sudo_env': False, 'su_user': None, 'use_su_login': False, 'preserve_su_env': False, 'su_shell': None, 'doas': False, 'doas_user': None, 'shell_executable': 'sh', 'chdir': None, 'env': {}, 'success_exit_codes': [0], 'timeout': None, 'get_pty': None, 'stdin': None, 'name': None, 'ignore_errors': False, 'continue_on_error': False, 'precondition': None, 'postcondition': None, 'on_success': None, 'on_error': None, 'parallel': 1, 'run_once': False, 'serial': False}, data=None)
    [pyinfra.api.facts] Getting fact: server.Home () (ensure_hosts: None)
DEBUG:pyinfra:Getting fact: server.Home () (ensure_hosts: None)
[pyinfra.connectors.local] --> Running command on localhost: sudo -H -n sh -c 'echo $HOME'
DEBUG:pyinfra:--> Running command on localhost: sudo -H -n sh -c 'echo $HOME'
[@local] >>> sudo -H -n sh -c 'echo $HOME'
[pyinfra.connectors.util] --> Waiting for exit status...
DEBUG:pyinfra:--> Waiting for exit status...
[pyinfra.connectors.util] --> Command exit status: 1
DEBUG:pyinfra:--> Command exit status: 1
[pyinfra.connectors.local] --> Running command on localhost: sh -c '
temp=$(mktemp /tmp/pyinfra-sudo-askpass-XXXXXXXXXXXX)
cat >"$temp"<<'"'"'__EOF__'"'"'
#!/bin/sh
printf '"'"'%s\n'"'"' "$PYINFRA_SUDO_PASSWORD"
__EOF__
chmod 755 "$temp"
echo "$temp"
'
DEBUG:pyinfra:--> Running command on localhost: sh -c '
temp=$(mktemp /tmp/pyinfra-sudo-askpass-XXXXXXXXXXXX)
cat >"$temp"<<'"'"'__EOF__'"'"'
#!/bin/sh
printf '"'"'%s\n'"'"' "$PYINFRA_SUDO_PASSWORD"
__EOF__
chmod 755 "$temp"
echo "$temp"
'
[pyinfra.connectors.util] --> Waiting for exit status...
DEBUG:pyinfra:--> Waiting for exit status...
[pyinfra.connectors.util] --> Command exit status: 0
DEBUG:pyinfra:--> Command exit status: 0
[@local] sudo password: 
[pyinfra.connectors.local] --> Running command on localhost: env SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-YEg7LT5KJxOX *** sudo -H -A -k sh -c 'echo $HOME'
DEBUG:pyinfra:--> Running command on localhost: env SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-YEg7LT5KJxOX *** sudo -H -A -k sh -c 'echo $HOME'
[@local] >>> env SUDO_ASKPASS=/tmp/pyinfra-sudo-askpass-YEg7LT5KJxOX *** sudo -H -A -k sh -c 'echo $HOME'
Authenticated with cached credentials.
[pyinfra.connectors.util] --> Waiting for exit status...
DEBUG:pyinfra:--> Waiting for exit status...
[pyinfra.connectors.util] --> Command exit status: 0
DEBUG:pyinfra:--> Command exit status: 0
    [@local] Loaded fact server.Home
INFO:pyinfra:[@local] Loaded fact server.Home
[pyinfra.connectors.local] --> Running command on localhost: sh -c 'rm -f /tmp/pyinfra-sudo-askpass-YEg7LT5KJxOX'
DEBUG:pyinfra:--> Running command on localhost: sh -c 'rm -f /tmp/pyinfra-sudo-askpass-YEg7LT5KJxOX'
[pyinfra.connectors.util] --> Waiting for exit status...
DEBUG:pyinfra:--> Waiting for exit status...
[pyinfra.connectors.util] --> Command exit status: 0
DEBUG:pyinfra:--> Command exit status: 0
--> An unexpected internal exception occurred:

  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/bsa/setup/ubuntu/bug.py", line 18, in extensions
    assert host.get_fact(Home) != "/root"
AssertionError

    [pyinfra_cli.exceptions]   File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra_cli/main.py", line 226, in cli
    _main(*args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra_cli/main.py", line 357, in _main
    state, config = _handle_commands(
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra_cli/main.py", line 621, in _handle_commands
    state, kwargs = _run_op_operations(
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra_cli/main.py", line 699, in _run_op_operations
    add_op(state, op, *args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra/api/operation.py", line 113, in add_op
    results[op_host] = op_func(*args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra/api/deploy.py", line 116, in decorated_func
    return func(*args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/bsa/setup/ubuntu/bug.py", line 13, in do
    extensions(_sudo=_sudo)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra/api/deploy.py", line 116, in decorated_func
    return func(*args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/bsa/setup/ubuntu/bug.py", line 18, in extensions
    assert host.get_fact(Home) != "/root"

DEBUG:pyinfra:  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra_cli/main.py", line 226, in cli
    _main(*args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra_cli/main.py", line 357, in _main
    state, config = _handle_commands(
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra_cli/main.py", line 621, in _handle_commands
    state, kwargs = _run_op_operations(
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra_cli/main.py", line 699, in _run_op_operations
    add_op(state, op, *args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra/api/operation.py", line 113, in add_op
    results[op_host] = op_func(*args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra/api/deploy.py", line 116, in decorated_func
    return func(*args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/bsa/setup/ubuntu/bug.py", line 13, in do
    extensions(_sudo=_sudo)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/.venv/lib/python3.10/site-packages/pyinfra/api/deploy.py", line 116, in decorated_func
    return func(*args, **kwargs)
  File "/home/ANT.AMAZON.COM/rchyla/BoringStuffAutomated/bsa/setup/ubuntu/bug.py", line 18, in extensions
    assert host.get_fact(Home) != "/root"

    [pyinfra_cli.exceptions] AssertionError

DEBUG:pyinfra:AssertionError

--> The full traceback has been written to pyinfra-debug.log
--> If this is unexpected please consider submitting a bug report on GitHub, for more information run `pyinfra --support`.

[romanchyla]

I tried testing it against previous releases (2.0--2.5) and got the same results. Maybe the behaviour was always there -- however, I only discovered it while debugging a deployment, which previously worked. And I was very surprised to see that host.get_fact runs with sudo

[Fizzadar]

Hi @romanchyla this is actually the correct behaviour, _sudo is passed between nested operation and deploy function calls (so a caller can expect all nested operations to share the same context). There was a bugfix related in 2.5.3 which may have accidentally hidden this.

For full context here's a slightly expanded example:

from pyinfra import host
from pyinfra.api import deploy
from pyinfra.facts.server import Home


@deploy("")
def do():
    # This should be SSH user's home unless do is called with _sudo, in which case root's home
    print("DO", host.get_fact(Home))
    # This should always be SSH user's home, whether do is called with sudo or not
    print("DO", host.get_fact(Home, _sudo=False))

    # Will show extension with root home (because sudo)
    extensions(_sudo=True)
    # Will show extension with same home as caller to do
    extensions()
    # Will always use SSH user as _sudo here overrides any passed into do
    extensions(_sudo=False)


@deploy("")
def extensions():
    print("EXTENSIONS", host.get_fact(Home))


do()
do(_sudo=True)

If I run this locally I get:

DO /Users/nick
DO /Users/nick
EXTENSIONS /var/root
EXTENSIONS /Users/nick
EXTENSIONS /Users/nick

DO /var/root
DO /Users/nick
EXTENSIONS /var/root
EXTENSIONS /var/root
EXTENSIONS /Users/nick

This is the intended behaviour as above, with the possibility of explicitly disabling sudo available as well.

[romanchyla]

Thank you @Fizzadar for the detailed example, it helps a lot.

Though what do you think about this?

@deploy("")
def do(_sudo=True):
    click.secho(host.get_fact(Home), fg="yellow")
    extensions()


@deploy("")
def extensions():
    click.secho(host.get_fact(Home), fg="yellow")


def doit():
    do()
    do(_sudo=False)
    do(_sudo=True)

doit()

will print:

/home/ANT.AMAZON.COM/rchyla
/home/ANT.AMAZON.COM/rchyla
/home/ANT.AMAZON.COM/rchyla
/home/ANT.AMAZON.COM/rchyla
[@local] sudo password: 
/root
/root

shouldn't the first call (with implicit _sudo=True) also produce /root?

Seems bit inconsistent still.

The changed behaviour, based on a different context in which it gets executed, is rather unobvious - while reading the code. It may make up for some interesting debugging :)

[Fizzadar]

@romanchyla agreed, I think this case should behave at you specify; quick explanation below on why that doesn't currently and a possible solution.

@deploy("")
def do(_sudo=True):
    click.secho(host.get_fact(Home), fg="yellow")
    extensions()

In this case the _sudo is ignored because the global arguments are collected by the @deploy decorator which happens before any call to do. It should be possible to support this though by inspecting the default arguments of do in the decorator and applying those accordingly.

The deploy decorator is found here and the global arguments are selected from the caller of do here. The above could work by providing defaults to the kwargs passed into pop_global_arguments based on the default arguments in the decorated deploy function.