ByteInternet / hypernode-docker

Fast and easy Docker for Magento development
https://community.hypernode.io/hypernode-docker
35 stars 8 forks source link

Add EXPOSE to Dockerfile #13

Closed markvds closed 5 years ago

markvds commented 5 years ago

Wouldn't it be good to document the exposed ports in the Dockerfile?

See here: https://docs.docker.com/engine/reference/builder/#expose

It would look something like (I must admit I didn't check if all these ports are open for external connections):

# Web
EXPOSE 80/tcp
# Mailhog
EXPOSE 8025/tcp
# MySQL
EXPOSE 3306/tcp
# Redis
EXPOSE 6379/tcp

My use case is that I use docker-hostmanager, which adds entries for all running Docker containers to the host's /etc/hosts. But before it does so, it checks if a running Docker container has exposed ports. Which seems a smart thing to do, but now it doesn't add my Hypernode containers because they don't document any exposed ports.

From what I understand, adding the exposed ports does not do anything and causes no incompatibilities whatsoever. It just adds some meta data to an image.

vdloo commented 5 years ago

sounds like a great idea @markvds!

Lambdara commented 5 years ago

If I recall correctly, this would also expose those ports by default (to the host, not publicly) which it does not seem to do now (but it should).

markvds commented 5 years ago

@Uwila that's not what Docker's docs say:

The EXPOSE instruction does not actually publish the port. It functions as a type of documentation between the person who builds the image and the person who runs the container, about which ports are intended to be published.

Lambdara commented 5 years ago

Then I don't really know what's going on, because when I create a container from a simple Dockerfile like this:

FROM ubuntu:18.04

EXPOSE 200

and then run docker ps I can actually see 200/tcp listed under ports. Maybe I'm not understanding it correctly and this is just a reminder, but even if it isn't listed it's reachable? Or does it do more than the documentation says? Or perhaps by publish the port they mean "make public" (0.0.0.0:200->200/tcp vs. just 200/tcp).

But perhaps that's an issue for Docker rather than Hypernode-Docker?

markvds commented 5 years ago

The ports you see listed in docker ps are all EXPOSEd ports together with ports that have been mapped. But exposing them doesn't do anything; it only tells the outside world that certain ports should do something. The services running in your container actually define which ports accept connections, but there is no way you can tell which ports that are from outside the container.

So: even if it's not listed it's reachable and vice versa: you could list ports that do nothing.

From what I understand they indeed mean 'make public' by publishing the port.

@vdloo Apart from thinking it's a great idea, will you do anything with it? :)

vdloo commented 5 years ago

The services running in your container actually define which ports accept connections, but there is no way you can tell which ports that are from outside the container.

Am I wrong to think that you can just run netstat -tulpn and fine all those ports or is that not what you mean?

app@a009c7ebde94:~$ ss -tulpn | grep tcp | awk '{print$5}' | grep -v 127
*:8888
*:8443
*:443
*:6081
*:3306
*:8880
*:80
*:22
:::8025
:::6081
:::1025

will you do anything with it

not until I have a better understanding of what this will entail for other people using the image and how that might affect their environments. If we expose 80 by default for example and they already have another service on 80 on their local host, would that then mean there is a collision and things break? in that case I think it would be better that everyone who wants these ports exposed by default to inherit from the Dockerfile and put the EXPOSES in there instead

Lambdara commented 5 years ago

If we expose 80 by default for example and they already have another service on 80 on their local host, would that then mean there is a collision and things break?

The container has its own IP so that should not be a problem. I run a setup with over 30 containers that all have port 80 exposed, with no problems.

markvds commented 5 years ago

@vdloo Of course you can netstat (but in this case you use it from the inside, not from the outside). And you are right, that was not what I meant :)

And about doing something with it: as the documentation says, EXPOSEing ports does not do anything. It's just a way to tell the outside world what ports they can use on the container.

As I quoted a few hours ago (copied from the official documentation):

The EXPOSE instruction does not actually publish the port. It functions as a type of documentation between the person who builds the image and the person who runs the container, about which ports are intended to be published.

Note the use of the words person (it's documentation, to be interpreted by the person that wants to use the image) and intended (they are not actually published, you could publish them if you want to but nothing stops you from using undocumented ports nor does EXPOSEing ports means that ports actually do anything).

FYI: We did a test with a custom Dockerfile inheriting from the Hypernode image and it works like intended: docker ps says the ports are available, but apart from that everything functions exactly like it did without EXPOSE.

Also, remember that an image has very little (to no) control over networking outside the container. Publishing ports (mapping them to the host's ports that is) is not something an image can do.

vdloo commented 5 years ago

Alright which ports do you want to have exposed? Just all those that I listed above? Or perhaps only a subset? Would there be any reason to leave any out?

markvds commented 5 years ago

I actually don't really know what's best practice for a 'fat container'. I think it's best to document all ports that are logical to use from the outside:

vdloo commented 5 years ago
EXPOSE 22/tcp
EXPOSE 80/tcp
EXPOSE 443/tcp
EXPOSE 3306/tcp
EXPOSE 8800/tcp
EXPOSE 8843/tcp
EXPOSE 8888/tcp
EXPOSE 6081/tcp
EXPOSE 1025/tcp

has been added to the Dockerfile used to build hypernode-docker and the image will be built with that from release 5​52​8 on

edit: also see this changelog