fix(docker): remove ARG PORT #157
Conversation
Shell parameter expansion doesn't happen inside the CMD array, one either needs to pass an ENV var and use it in an entrypoint shell script or, if one doesn't want to pollute the container's env space, modify the argument (with sed) within the entrypoint itself. Also ignore a couple fat directories that complicate local development.
|
I guess it'll work. Because I live in K8s-world where I don't have to worry about any of this, I'd just go the other way and force it to 5000 always and rely on a service or an ingress to modify that for the caller if I had to. But maybe assuming you've got k8s around is not a good assumption. |
There was a problem hiding this comment.
It seems fine. My question is whether it's needed, because it is a little harder to read than just saying "Yes, this listens on port 5000". Since it's inherently a docker container, mapping that to something different outside shouldn't be a problem, should it?
|
I'm actually with you on this one. Driven by the status quo where there's an |
|
If you're in agreement, yeah, let's just fix PORT to 5000 in the container and be done with it. We can always change it back if anyone screams, and if you are for some reason running without containerization where you might not have as much control, you've still got the option to set the port. |
Making it configurable via a build arg complicates the whole dockerfile setup way too much for such a useless feature :)
vit-zikmund
changed the title
|
@athornton I'm done, please squash this. Thanks! |
EDIT: This OP no longer holds true, things got rather simplified instead. Follow the discussion below.
Yet another gratuitous fix nobody asked for :)
Shell parameter expansion doesn't happen inside the CMD array, so literal
${PORT}gets passed touwsgi, which it happily accepts, but, well, doesn't work as expected.One either needs to set an
ENVvar and use it in an entrypoint shell script or, if one doesn't want to pollute the container's env space, modify the argument (with sed) within the entrypoint itself. That's what I did.Also this change ignores a couple fat development directories that complicate local
make dockerruns.PS: I'm not a fan of docker entrypoints, but this couldn't be solved better, unless using custom variable expansion like this in the
ENTRYPOINT:which, although surely very beautiful, is also pretty obscure, rather unsafe and adds one more (unwanted) level of variable expansion to the arguments passed as the
CMD.