Skip to content

Allow for custom definitions file #112

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

Merged
merged 2 commits into from
Jan 11, 2017

Conversation

cavemandaveman
Copy link
Contributor

If a user mounts a definitions file to /etc/rabbitmq/definitons.json, then it will get loaded on startup. Takes care #107

@jonapich jonapich mentioned this pull request Oct 18, 2016
@jippi
Copy link

jippi commented Nov 6, 2016

Any love for this PR ? :)

@ghost
Copy link

ghost commented Jan 6, 2017

Would love to see some progress on this as well. With this, there is no reason to create users or vhosts in the scripts since the definitions file can contain all of this info. Would love to see this merge.

Alternatively, would love to see some way to extend the config with custom code in which case we can use the official image and use the definitions files like that.

@tianon
Copy link
Member

tianon commented Jan 9, 2017

@michaelklishin or @dumbbell, do you have any thoughts on this? (especially WRT whether it's reasonable to include by default, and whether the filename/path we're adding here as the default is a sane one?)

Would it be better to have something like a RABBITMQ_MANAGEMENT_DEFINITIONS_FILE environment variable to make a request for this explicit, or is the presence of the file in a (hopefully well-documented) location fine/sufficient?

@michaelklishin
Copy link
Collaborator

@tianon definitions are merged with what's already there and empty definitions should do no harm. However, not everything should be configured via definitions. Dynamic Shovels and other things that need to be started when a plugin is loaded will run into race conditions between definition loading and plugin startup, for example.

Users, vhosts, permissions, possibly even topologies can be defined via definitions.

I have a feeling that definitions will be extracted into a separate plugin or moved into the core because right now they are an afterthought in the management plugin (an alternative way of loading definitions that uses a local file system). It's not necessarily going to be a breaking change but I wouldn't say that this feature should be widely advertised and recommended to everyone.
It also means that I'd rename the file to /etc/rabbitmq/definitions.json.

@michaelklishin
Copy link
Collaborator

@cavemandaveman may I ask you to change the default path per my comment above? @tianon I don't have any other suggestions because this feature is opt-in.

@tianon
Copy link
Member

tianon commented Jan 11, 2017

@michaelklishin thanks for your thoughts! The path you suggest is actually what was originally proposed and I screwed it up before pinging you 😞

I'll update this later today and merge it. 👍

@cavemandaveman
Copy link
Contributor Author

@tianon Since you made a couple of other tweaks to the PR, I'll leave it up to you to change it.

…clude "load_definitions" in the proper place
@tianon
Copy link
Member

tianon commented Jan 11, 2017

I've swapped it back to the original filename in 3ba4ec4, and added a link to @michaelklishin's comment above for our future selves to reference back to more easily. 😅

@yosifkit yosifkit merged commit 30356ca into docker-library:master Jan 11, 2017
tianon added a commit to infosiftr/stackbrew that referenced this pull request Jan 11, 2017
- `cassandra`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/cassandra#91)
- `elasticsearch`: avoid bootstrap checks by using `http.host` instead of `network.host` (docker-library/elasticsearch#153), use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/elasticsearch#152)
- `golang`: 1.8rc1, explicit Alpine 3.5 variant of Go 1.7 (docker-library/golang#134)
- `kibana`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/kibana#69)
- `logstash`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/logstash#78)
- `mariadb`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (MariaDB/mariadb-docker#93)
- `memcached`: 1.4.34
- `mongo`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/mongo#132)
- `openjdk`: debian 9~b151-2
- `percona`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/percona#39)
- `piwik`: 3.0.1
- `postgres`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/postgres#246)
- `rabbitmq`: allow custom definitions file (docker-library/rabbitmq#112), use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/rabbitmq#133)
- `redmine`: 3.2.5, 3.3.2
- `tomcat`: improve OpenSSL comments (docker-library/tomcat#59)
- `wordpress`: 4.7.1, fix Windows-newlines `sed` (docker-library/wordpress#197)
@cavemandaveman cavemandaveman deleted the definitions branch January 27, 2017 19:54
@RyanSMurphyDMCo
Copy link

So how do we actually use this? It doesn't seem obvious to me. Do we have to have a particular version of the image? Is it an environment variable, a file put in the right place, named specifically?

@tianon
Copy link
Member

tianon commented Feb 13, 2018

Yep (from the OP):

If a user mounts a definitions file to /etc/rabbitmq/definitons.json, then it will get loaded on startup.

It should work on either 3.6 or 3.7.

@RyanSMurphyDMCo
Copy link

Tried on both 3.6.6 and 3.7.3.

It works on 3.7.3, but does not work on 3.6.6.

@tianon
Copy link
Member

tianon commented Feb 13, 2018

The only currently-supported versions are 3.7.3 and 3.6.15 (no other versions will receive any updates).

That being said, this should work on 3.6.6 and above since it was merged into this repository around the time of 3.6.6, and any version of 3.7 (since that entire series was added following this PR being merged).

I've verified that the necessary docker-entrypoint.sh lines exist in the latest rabbitmq:3.6.6 image:

$ docker pull rabbitmq:3.6.6
3.6.6: Pulling from library/rabbitmq
Digest: sha256:2fce9a0919e732a5108b3c6653ab7ae8b51fea0336de5ce9b841837962992af1
Status: Image is up to date for rabbitmq:3.6.6

$ docker run --rm rabbitmq:3.6.6 grep definitions /usr/local/bin/docker-entrypoint.sh
		# if definitions file exists, then load it
		# https://www.rabbitmq.com/management.html#load-definitions
		managementDefinitionsFile='/etc/rabbitmq/definitions.json'
				"{ load_definitions, \"$managementDefinitionsFile\" }"

@matthewreimer
Copy link

matthewreimer commented Dec 5, 2019

@tianon definitions are merged with what's already there and empty definitions should do no harm. However, not everything should be configured via definitions. Dynamic Shovels and other things that need to be started when a plugin is loaded will run into race conditions between definition loading and plugin startup, for example.

Users, vhosts, permissions, possibly even topologies can be defined via definitions.

I have a feeling that definitions will be extracted into a separate plugin or moved into the core because right now they are an afterthought in the management plugin (an alternative way of loading definitions that uses a local file system). It's not necessarily going to be a breaking change but I wouldn't say that this feature should be widely advertised and recommended to everyone.
It also means that I'd rename the file to /etc/rabbitmq/definitions.json.

It appears this has been done in rabbitmq/rabbitmq-management#749.

@yosifkit
Copy link
Member

yosifkit commented Dec 5, 2019

Moving the export/import to core, rather than a plugin, is in 3.8.2 (see CLI Tools -> Enhancements). Is there something we need to change in the image?

@matthewreimer
Copy link

I'm not sure. Maybe now definitions.json can contain e.g. shovel definitions with causing a crash, without any change to the image, but I haven't had a chance to test yet.

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.

7 participants