-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
docs: Update docker installation topic #12770
Conversation
This PR must be merged before a backport PR will be created. |
1 similar comment
This PR must be merged before a backport PR will be created. |
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.
Looks good, one small nit/question
|
||
## Prerequisites | ||
|
||
- [Docker](https://docs.docker.com/install) | ||
- [Docker Compose](https://docs.docker.com/compose/install) (optional, only needed for the Docker Compose install method) | ||
|
||
## Install with Docker | ||
## Install with Docker on Linux |
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.
What's specific about Linux here? Will these instructions not work on Mac/BSD/etc?
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.
Good question, I was trying to differentiate between this and the Windows installation. Any suggestions for a better heading?
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.
Actually, that was already the heading....
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.
The heading was previously Install with Docker not Install with Docker on Linux unless I'm missing something?
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.
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.
Oh yeah, the bold bit after. Makes sense to make that part of the heading instead.
Probably worth visiting if the instructions are suitable for other similar platforms later
(cherry picked from commit afbeedc)
Which issue(s) this PR fixes:
Fixes #12155
Updated the docker installation topic before I realized that the issue was for docker install for Promtail. So two updates for the price of one!
Splits out the commands into separate steps for downloading and running the files. Adds some validation steps (for Linux).