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

"uses: sh" not working in dockerized Popper #956

Open
wtraylor opened this issue Dec 17, 2020 · 2 comments
Open

"uses: sh" not working in dockerized Popper #956

wtraylor opened this issue Dec 17, 2020 · 2 comments

Comments

@wtraylor
Copy link
Contributor

With Popper installed through the install.sh script, which runs Popper within Docker, workflow steps with uses: sh don’t execute on the host shell.

Steps to Reproduce

cd /tmp
# Install Popper as suggested in the README.
curl -sSfL https://raw.githubusercontent.com/getpopper/popper/master/install.sh | sh
cat > wf.yml << EOF
steps:
- uses: "sh"
  runs: ["docker"]
  args: ["--version"]
EOF
./popper run -f wf.yml

The last command fails with:

[1] ('docker', '--version')
Command raised non-SubprocessError error: [Errno 2] No such file or directory: 'docker': 'docker'
ERROR: Step '1' failed ('1') !

That is because Popper cannot execute docker --version on the host shell, but only in its own container.

Suggestion

My humble suggestion is to remove the install.sh feature.

A proper installation through PIP has all the features one would expect (including Singularity and host shell). Recommending different installation methods with different feature sets can lead to great confusion, especially for beginners who just want to reproduce a given workflow. This will only be aggravated when (hopefully some day) Popper will be packaged for Linux distributions, because then a /usr/local/bin/popper installed through install.sh could unexpectedly override a /usr/bin/popper installed through a package manager.

@ivotron
Copy link
Collaborator

ivotron commented Jan 5, 2021

thanks for reporting this @wtraylor! The motivation behind the install.sh script is to address problems that some users have when installing via pip. For users not familiar with python or how pip works, the curl | sh method is convenient, as they have already installed docker (whose installer has better UX than pip), so using install.sh is straight-forward in this scenario.

Regarding the problems with uses: sh inside a container, the specific example can be solved by doing:

steps:
- uses: docker://docker:19.03.10
  args: ["--version"]

More fundamentally, one of the main goals of having container-native pipelines is to make them reproducible. By using sh to invoke commands that are expected to be available in the host, the probability of having a pipeline fail is higher. This is why we note on the documentation that the use of uses: sh should be minimized. Other additional "best practices" for steps with uses: sh would be to 1) only reference paths that are within the workspace or bind-mounted folders (via the dir option); and 2) only invoke POSIX/UNIX commands. These don't guarantee reproducibility but minimize errors. Maybe we should add this to the docs as well.

does this make sense?

@wtraylor
Copy link
Contributor Author

wtraylor commented Jan 6, 2021

does this make sense?

Yes, that makes sense. I see that it is very important to provide an easy installation method through install.sh. On the other hand, as long as uses: sh is in Popper’s feature set (whether using it is good practice or not), it should just work, no matter by which method Popper has been installed. I think that is very important.

One way out of the dilemma would be to fail with an error message if Popper running in Docker enconunters uses: sh and kindly ask the user to install Popper through PIP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants