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

Move both examples into single Dockerfile for Docker Hub (fixes #347) #424

Merged
merged 1 commit into from
Jul 10, 2018

Conversation

pedrosland
Copy link
Contributor

This PR should be the last step to get the examples on the Docker Hub. It:

  • merges the Dockerfile for the two examples into one
  • puts it in the root of the repo for Docker Hub
  • updates to Go 1.10.3 to build the examples

The two commands to run the examples with this change could be:

docker run -ti prom/golang-examples /simple
docker run -ti prom/golang-examples /random

I have tested this on the hub and it does build:

docker run -ti -p 8080:8080 pedrosland/prom-test:test /simple

See issue #347 and the discussion on #348.

Please let me know @beorn7 if you would like any changes. I can also make a PR to the docs repo to add this to the getting started guide.

@beorn7
Copy link
Member

beorn7 commented Jul 8, 2018

This appears to me exactly the thing we need. However, since my Docker foo is still mediocre, it would be great if somebody else could chime in for a sanity check, for example @discordianfish . 🙏

Dockerfile Outdated
@@ -4,17 +4,20 @@
# docker build -f examples/$name/Dockerfile -t prometheus/golang-example-$name .

# Builder image, where we build the example.
FROM golang:1.9.0 AS builder
FROM golang:1.10.3 AS builder
Copy link
Member

Choose a reason for hiding this comment

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

Golang 1 has a stable and guaranteed interface, using golang:1 would remove the need to constantly update this version with every new release.

Copy link
Member

Choose a reason for hiding this comment

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

If that works, great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. Sounds good. I've updated the PR.

@discordianfish
Copy link
Member

LGTM

@beorn7
Copy link
Member

beorn7 commented Jul 10, 2018

Thanks everybody.

@beorn7 beorn7 merged commit ee1c9d7 into prometheus:master Jul 10, 2018
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

Successfully merging this pull request may close these issues.

None yet

4 participants