-
-
Notifications
You must be signed in to change notification settings - Fork 2k
change plugin install doc to show one-step method #1504
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
Conversation
docs/plugins.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention that this requires root access?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure. Any idea where folks got the idea they didn't need root though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean why would you need root for a directory that you'd expect to be owned by root?
8bc5aa6 to
afd549e
Compare
|
I added a note underneath. Let me know what you think. |
docs/plugins.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you still need root to clone the plugin in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily. You could clone it if you run plugn install as the dokku user. However plugn trigger install will fail with the reported error.
root@dokku:/var/lib/dokku# ls -lad ./*/{available,enabled}
drwxr-xr-x 21 dokku dokku 4096 Sep 19 00:27 ./core-plugins/available
drwxr-xr-x 2 dokku dokku 4096 Sep 19 00:27 ./core-plugins/enabled
drwxr-xr-x 2 dokku dokku 4096 Sep 22 16:01 ./plugins/available
drwxr-xr-x 2 dokku dokku 4096 Sep 22 16:01 ./plugins/enabled
EDIT: We no longer sudo -u dokku for plugin:*install* so we can do root things like apt-get.
https://github.com/progrium/dokku/blob/f8c9d9b8239cf5b20020366ecec2e2c799185718/dokku#L44-L47
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, my dokku dir is owned by root...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/var/lib/dokku is owned by root. However we don't install plugins in that directory anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically I think this comment outside of the codeblock should be inside the codeblock and we should have sudo prefix on the command. Something like this:
# This command requires `root` permissions as the `install` and `install-dependencies`
# plugin triggers may utilize commands such as `apt-get`. For non-core plugins, please
# inspect those plugins before running the following command as `root` user.
sudo plugin:install <git_url>
# previous versions (0.3.x and below) of dokku require a manual process to install plugins
cd /var/lib/dokku/plugins/available
sudo git clone <git url>
sudo dokku plugin:installDoes that make sense? Keeping the root notes outside of the codeblock is an easy way for users to get confused and complain that the docs are bad because they can't read ;)
afd549e to
a798347
Compare
change plugin install doc to show one-step method
closes #1503