Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdlib.run: Fix missing executable #836

Merged
merged 1 commit into from
Aug 14, 2023
Merged

Commits on Aug 14, 2023

  1. stdlib.run: Fix problems when an executable is missing

    Currently, if the executable required in `run` does not exist (or
    is not executable), the child process's code is not replaced by
    `os.execvpe` function and it raises the OSError instead. However,
    the parent process does not get this OSError. It consumes exit code,
    stderr, ... of the child process. So in case the code does something
    like this:
        ```
        try:
            result = run(['non-executable'])
        except OSError:
            pass
        except CalledProcessError:
            # do something..
        ```
    the child process passes and do whatever.. In case it ends with
    zero exit code, the obtained result is usually something totally
    different than expected in actors. Also there could be problems
    with non-idempotent code, when some actions could be done twice
    (once executed by the child, send time executed by the parent
    [current] process).
    
    We have realized that number of existing leapp actors for in-place
    upgrades already count with the raise of OSError when executable
    cannot be used. So we choosed for now to check whether executables
    are present and raise OSError if not, so we are sure that only
    one process leave the function really.
    
    Also applied another seatbelt into the child process - if the
    OSError is raised anyway despite our checks (e.g. SELinux prevents
    the execution) let's just kill the process instead giving
    it a possibility to continue. In such a case, always print a msg to
    stderr of the child process and exit with ecode 1.
    
    Note: To check an executable we use `distutils.spawn.find_executable`
    which is deprecated in Python 3 and will be dropped in Python 3.12.
    However it exists now for Python 2 & 3, so we use this one for now
    and will replace it in future by `shutils.which` when the time comes.
    
    Additional changes:
    * Make pylint happy (set noqa for E721 for the check the Fields is not
    created directly). The "isinstance" function has a different behaviour
    in this case than we want.
    
    * Fix imports to make pylint happy
    
    * Set `result` to empty dict instead of None: in case a ValueError
      or TypeError is raised inside the _call function, it's expected
      to copy the `result` content inside the final block, however in
      case the results is None, it fails and raise additional exception
      that covers the original one.
    
    * Update unit tests to cover issues with missing executable.
    pirat89 committed Aug 14, 2023
    Configuration menu
    Copy the full SHA
    84ab1e4 View commit details
    Browse the repository at this point in the history