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

Docker: expose ports 14000 and 15000 in the pebble image #279

Merged
merged 1 commit into from
Sep 18, 2019
Merged

Docker: expose ports 14000 and 15000 in the pebble image #279

merged 1 commit into from
Sep 18, 2019

Conversation

sergioaugrod
Copy link
Contributor

Hello,

I exposed ports 14000 and 15000 in the pebble image. I believe this improves readability of Dockerfile and this will help me to run the image on GitlabCI.

Does that make sense to you?

Thank you!

@cpu
Copy link
Contributor

cpu commented Sep 16, 2019

Hi @sergioaugrod 👋 Thanks for opening a PR!

this will help me to run the image on GitlabCI.

Can you explain a little bit more about this part? My understanding was that the EXPOSE directive in a Dockerfile was strictly metadata and publishing the port requires additional configuration either way but I might be missing something about your use-case.

@sergioaugrod
Copy link
Contributor Author

Hi @cpu. You are right.

My problem is that Gitlab CI HealthCheck checks which ports are exposed from the container by default and If I don't expose any port in my Dockerfile, then the pipeline will fail. Currently, I can't expose ports in my gitlab-ci.yml.

There is a discussion about this here: https://gitlab.com/gitlab-org/gitlab-runner/issues/2460

Despite this very specific problem of mine, would it be nice to have it in Dockerfile to improve the documentation?

Thank you!

Copy link
Contributor

@cpu cpu left a comment

Choose a reason for hiding this comment

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

Hi @sergioaugrod

Thanks for the additional info. I think it makes sense to accept this PR. I think the additional metadata improves the clarity of the Docker environment. I hope Gitlab fixes their bug since it seems like something that shouldn't be required but I'm OK with landing this fix since it has benefits beyond just Gitlab CI.

Thanks again!

@cpu cpu merged commit 970dfd0 into letsencrypt:master Sep 18, 2019
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.

2 participants