Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Move the VOLUME declaration to the end of the Dockerfile #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HoloRin
Copy link
Contributor

@HoloRin HoloRin commented Apr 1, 2020

One a VOLUME has been declared, changes to that location are subsequently dropped during the building of the Dockerfile. With the changes in rabbitmq/rabbitmq-common#369 applied, this meant that the enabling of plugins no longer worked correctly. We now make the VOLUME declaration the last line of the Dockerfile, assuming that we would not want to throw out any changes.

Additionally, we use gosu to execute plugin enablement, so that the 'enabled_plugins' file is owned by the rabbitmq user when the broker goes to update it.

One a VOLUME has been declared, changes to that location are subsequently dropped during the building of the Dockerfile. With the changes, in rabbitmq/rabbitmq-common#369 applied, this meant that the enabling of plugins no longer worked. We now make the VOLUME declaration the last line of the Dockerfile, assuming that we would not want to throw out any changes.

Additionally, we use gosu to execute plugin enablement, so that the 'enabled_plugins' file is owned by the rabbitmq user when the broker goes to update it.
@gerhard
Copy link
Contributor

gerhard commented May 18, 2020

I have only just noticed this. It's a good change!

Since this Dockerfile is tracking https://github.com/docker-library/rabbitmq, do you want to contribute this PR there instead?

@HoloRin
Copy link
Contributor Author

HoloRin commented May 19, 2020

@gerhard They declare the VOLUME at the end of the base Dockerfile, then the management image inherits from that, so it's kind of already at the end. Still, I think I will raise an issue to see if there is a way of addressing it that I haven't thought of.

@HoloRin
Copy link
Contributor Author

HoloRin commented May 19, 2020

@gerhard I've created docker-library/rabbitmq#410, but it's not clear to me what a good solution would be. Drop the volume declaration entirely? Unfortunately the ignore changes effect cascades to inheriting Dockerfiles, so this is not an artifact of this repo flattening two Dockerfiles into 1.

@HoloRin
Copy link
Contributor Author

HoloRin commented May 22, 2020

Based on docker-library/rabbitmq#410 (comment), I think it's unlikely the declaration is removed upstream.

I think this supports @dumbbell 's point in rabbitmq/rabbitmq-server#2234, that the file being both current state and configuration is not ideal and has drawbacks. I would say that the list of enabled plugins on startup should be taken from the .conf/.config file, and that runtime changes of enabled plugins could be reflected in this file in the data dir, but that presents a bit of a problem with the --offline switch for rabbitmq-plugins.

@HoloRin
Copy link
Contributor Author

HoloRin commented May 25, 2020

@gerhard I think I've ultimately decided that I would recommend dropping this change: rabbitmq/rabbitmq-server#2234 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants