Allow deploy_stack scripts to invoke sudo for users who have not added their user to the docker group. This makes the deploy scripts more user friendly.
./deploy_stack.sh should work successfully for users who run docker commands with sudo
./deploy_stack.sh fails with permission denied error
Check if a user is in the docker group or if the user is root. If yes then invoke docker commands as usual. If not then prepend sudo.
I am one of those users that runs docker using sudo. This has caused some delay when running the OpenFaaS workshop.
Docker version 18.03.0-ce, build 0520e24
Using Docker swarm
Operating System: Fedora 27
haha! "one of those users..." :smile:
This sounds like a reasonable request. Perhaps add a check in the deploy script to check if the current user is in the docker group or already executing as sudo, and if not, request sudo access?
@nishakm Would you be interested in implementing this change? We are always welcome to pull requests! Also, if you're interested, join our Slack Community: https://docs.openfaas.com/community/
@BurtonR I can submit a PR. I have already sent a request to add me to the Slack Community.
Awesome! Be sure to have a look at the Contribution Guide and don't be afraid to ask questions!
@BurtonR please see my comments on #665.
In the same way that apt-get install fails to work with an unprivileged account I think the solution (without making the shell script more complicated) is to type in sudo ./deploy_stack.sh just like sudo apt-get install. This is explained in more detail on the PR.
As stated in the proposal, the motivation is to make them user friendly to users who do not add their user to the docker group or do not run as root. The obvious reason for not running docker commands as root or with root-like privileges is security. Docker's documentation specifically warns about this: https://docs.docker.com/install/linux/linux-postinstall/
Indeed it is a small annoyance to run a shell script, get permission denied error and then run it again especially when you are going through a workshop and would rather concentrate on getting OpenFaaS working so you can start writing functions, but it is an annoyance to users nonetheless. I will not argue about the impact to users though. I do not know how many users out there do not give their user access to the unix socket. The change has certainly made the workshop easier for me to go through.
@alexellis regarding your comment on #665, There's nothing in the Docker docs _recommending_ that your user be added to the group. As Nisha mentioned, there is a security risk in adding your user to that group.
The change is a fairly simple/straight-forward if check. But, I do see your point of just running the script as sudo...
Maybe rather than changing the script, update the workshop (and possibly the CLI readme) with a note: "the deploy_stack.sh uses the docker command and may require sudo"
I would like to point out that the change doesn't affect how the majority of users run the deployment scripts. They wouldn't see any difference. You also get an additional utility if in the future you would need to make a check for any reason :)
I want you to know that I hear your point. It's not invalid but it does have wider impact than a single script and we already have a valid way of running scripts with sudo.
Docker on Mac runs with elevated permissions already and the client on Windows needs an elevated command prompt so this would just benefit people on Linux (I think). This change if accepted affects more than just a single shell script - we have a dozen repositories with shell scripts - if it is implemented it needs to be done across the board and maintained. I'd rather we focus on the many open issues right now.
I am going to tag the issue #revisit and see what the demand is over time about running the command with sudo vs editing shell scripts. My suggestion for now is to run sudo ./deploy_stack.sh. We can also update to the docs with that note in the swarm documentation which seems to address the point too.
https://github.com/openfaas/docs/blob/master/docs/deployment/docker-swarm.md
Derek add label: revisit
The docs have been updated to mention use of sudo: https://github.com/openfaas/docs/commit/6bf7b681c9939b4ff29787c78040f0dc4bf0c889
I agree. Let's leave this around as an open issue and see if there is a larger demand for it.
I hadn't considered the numerous other shell scripts we have across the repos that utilize the docker command that would then also need updating.
Updating the docs, is good. This will at least remove the surprise of getting an error right away.
Derek close: inactive
Most helpful comment
I agree. Let's leave this around as an open issue and see if there is a larger demand for it.
I hadn't considered the numerous other shell scripts we have across the repos that utilize the
dockercommand that would then also need updating.Updating the docs, is good. This will at least remove the surprise of getting an error right away.