grocy-docker
grocy-docker copied to clipboard
Configurable Makefile
While working on #141 I had some ideas for further changes. But I didn't want to bloat the scope of that pull request further. So here they are (or will be, when I'm finished).
The Makefile uses various names or values that don't affect the inner workings of the grocy containers. We can make them configurable without much trouble, by using overridable variables for them. The cases I identified that should be configurable are:
- [x] Container names (currently
frontend
andbackend
) - [x] The name of the pod (currently
grocy-pod
) - [x] The name of the volume (currently
app-db
) - [x] The endpoint for the published port (currently
127.0.0.1:80808
)
I also noticed that some of the current names don't convey any relation to grocy, e.g. fontend
could be a frontend for anything. Unfortunately podman doesn't namespace containers or volumes by the pod they belong to, so these names all end up in a namespace global to the whole podman instance. I would like to change the defaults to convey the relation to grocy:
-
frontend
->grocy-frontend
-
backend
->grocy-backend
-
app-db
->grocy-app-db
Ideally, when these changes are merged, you should be able to run multiple grocy instances on the same podman instance without collisions by using different names for all instances.
Hey @Kritzefitz, thanks! - the use case to run multiple instances of the application makes sense.
I'm a little unsure about changing the names of the containers, though. As a bit of history, what is now the frontend
container was previously named nginx
, and the change was made to clarify that it's not a 'plain' nginx container -- because it also hosts frontend assets (JS/CSS/HTML/etc) for the grocy application.
Doesn't the IMAGE_PREFIX
(current value: docker.io/grocy
) already disambiguate the fact that frontend
and backend
are part of grocy?
Hi @jayaddison, my main concern is that our container names might clash with containers of completely unrelated applications. Note that container and pod names are not implicitly namespaced or prefixed by podman (note: this is different from docker-compose, which does prefix container names). The IMAGE_PREFIX
is only applied to image names (which I haven't changed), not the names of the containers created by them. Since frontend
and backend
are rather generic names, it wouldn't be unreasonable to assume that some other project might use those names for their own containers. If they do, you would run into naming conflicts when trying to run grocy and that other application on the same podman instance.
But I'm not especially attached to the change of the default names. If you feel more comfortable leaving the names as is, just say so and I will remove the change from the PR.
Ok, thanks @Kritzefitz - I didn't understand that limitation / risk for podman
-based running containers previously.
Should we consider omitting the --name
argument entirely and allowing podman
to autogenerate unassociated names?
@jayaddison, omitting the names seems reasonable, since most operations on the containers can (and in most cases should) be run on the pod, which will keep its name. I will make the necessary changes and push them when ready.
I removed the container names.
It's been a while since I looked at this branch, but I thought I should mention that container_name
entries have been re-added to the docker-compose.yml
file in fde4b1a084d182a4fcd01e15a1795e9070ce8946, since that is potentially relevant to some previous discussion here.
This was done with a short-term aim of achieving network connectivity between the containers within a grocy
pod run using podman-compose
(something I'm evaluating as a potential migration path away from docker-compose
-- and that could also potentially simplify some of the Makefile
).
I'm not certain yet whether it's a good idea or not to provide static container names, to be completely honest; I can see it introducing some namespace collision concerns (even within a single deployment, when scaling horizontally).
It's been a while since I looked at this branch, but I thought I should mention that container_name entries have been re-added to the docker-compose.yml file in https://github.com/grocy/grocy-docker/commit/fde4b1a084d182a4fcd01e15a1795e9070ce8946, since that is potentially relevant to some previous discussion here.
I've changed my mind about this - I don't think that we should be using specific container names in our docker-compose.yml
file. Change reverted in 0da7afbe8fad91061d553020a04720095f4c8304.