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

Added documentation on docker usage #1865

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

DrPyser
Copy link

@DrPyser DrPyser commented Apr 29, 2019

Closes #1855 .

This is an invitation to add more documentation related to docker usage.

A new Docker section is added to the readme, with subsections for describing exposed ports, volumes, environment variables and commands.

I only added the information I could gather from reading the dockerfile(https://github.com/prometheus/alertmanager/blob/master/Dockerfile). I invite contributors, members and people with better knowledge and experience on this software to help add relevant information relative to usage of the docker image.

@@ -50,6 +50,32 @@ You can also build just one of the binaries in this repo by passing a name to th
$ make build BINARIES=amtool
```

## Docker

<!--- example of complete docker run command to run a container --->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about linking to the Procfile of this repository, as it gives a user a full setup on their local machine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm talking about an actual invokation of docker run ..., using volumes(-v) and port(-p) mapping as appropriate, then the command arguments. The Procfile contains example invokations of the alertmanager binary, but it will look different when using docker run, so it can be confusing to people less familiar with docker and unix.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
These paths can also be mapped to volumes:
* `/etc/alertmanager/alertmanager.yml`: default configuration file path

### Environment variables
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not aware of any. Configuration is limited to the config file and the command line flags.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for me. I'd remove this section.

removed space

Co-Authored-By: DrPyser <schok53@gmail.com>
@stuartnelson3
Copy link
Contributor

other than the DCO, is there anything preventing this PR from being merged?

README.md Outdated

### Ports

* `9093`: main api port
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/main api/API/

README.md Outdated
@@ -50,6 +50,33 @@ You can also build just one of the binaries in this repo by passing a name to th
$ make build BINARIES=amtool
```

## Docker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "Container image" rather than "Docker ™️"?

<!--- example of complete docker run command to run a container --->
<!--- example of a docker-compose.yml service snippet --->

### Ports
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposed ports?

README.md Outdated
These paths can also be mapped to volumes:
* `/etc/alertmanager/alertmanager.yml`: default configuration file path

### Environment variables
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for me. I'd remove this section.


<!--- Add documentation here --->

### Command and entrypoint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Entrypoint" alone?

### Command and entrypoint
The container entrypoint is the [`alertmanager` command](https://prometheus.io/docs/alerting/configuration/)<!---More complete documentation of the command would be useful here--->.

The default docker command passed to the entrypoint is `--config.file=/etc/alertmanager/alertmanager.yml --storage.path=/alertmanager`. Specifying the docker command(using `docker run quay.io/prometheus/alertmanager $COMMAND`) thus override those default arguments.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding this paragraph, I'm not sure we want to get into this level of details as 1) the arguments may change over time and 2) it is more a matter of the container runtime.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, we could link to a documentation page for the binary.

@bploetz
Copy link

bploetz commented Dec 26, 2019

A lost afternoon and some Googling lead me here. I had been trying to mount the configuration for alertmanager via a volume, and the Dockerfile shown on Docker Hub (https://hub.docker.com/r/prom/alertmanager/dockerfile) shows the config file location used by alertmanager as /etc/alertmanager/config.yml. While trying to figure out why my config wasn't being honored I stumbled on this ticket to discover the config file location is actually /etc/alertmanager/alertmanager.yml.

Will the changes noted in this ticket also update the Dockerfile shown on Docker Hub?

@simonpasquier
Copy link
Member

@bploetz the inaccuracy on Docker Hub has already been reported but we need someone from Docker Hub to clean up things (see docker/hub-feedback#1847).

@stale stale bot added the stale label Mar 2, 2020
@fredericrous
Copy link

the issue on docker hub does not prevent this project to get a proper readme on github documenting the docker image.
Having not found a documentation on docker, people turn to github.

@stale stale bot removed the stale label Jun 2, 2020
@stale stale bot added the stale label Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for docker usage
6 participants