-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support legacy RabbitMQ configuration format #1692
Conversation
Thanks @twillouer. I have to say, I'm a bit confused that this should make a difference! Is RabbitMQ appending It's also slightly worrying that we must not have test coverage for this, otherwise the bug would never have happened! Would you mind adding a test case that we can use to reproduce the problem and see that it's resolved? |
@rnorth ok, but I was not able to run graddle check on my computer 👍
so... I'm not very confident about adding a new test :) I try and I update the PR soon |
@rnorth PTAL |
Interestingly according to the docs:
Perhaps there's an undocumented behaviour whereby it automatically strips Will have a quick play to confirm my suspicions, but perhaps if we're doing it 'by the book' we should not have the |
@rnorth I try (without .config in the end), and it didn't works. tell me if you're more lucky |
Ohoh, this is a bit of a rabbit hole. I tried reverting the change to Then I found more information in the RabbitMQ docs:
I then tried changing the
It looks like 3.6 appends
Would I be correct in thinking that you are using a <3.7.0 RabbitMQ image for your tests? I was wondering why you had also modified the example config file to use the Erlang format, but it would make sense. I think that if we want to make this backwards compatible, our only option is to retain the extension of the file provided by the user, and let RabbitMQ figure it out: public RabbitMQContainer withRabbitMQConfig(MountableFile rabbitMQConf) {
// DON'T provide the extension, and allow RabbitMQ to look for config/conf/both
withEnv("RABBITMQ_CONFIG_FILE", "/etc/rabbitmq/rabbitmq-custom");
return withCopyFileToContainer(rabbitMQConf, "/etc/rabbitmq/rabbitmq-custom" + whateverExtensionTheUserProvided); // extract from rabbitMQConf object?
} or something like this, anyway. WDYT? |
yes, you're perfectly right, I use a 3.6.15 rabbitmq.. I think your idea might works, but when I try withEnv("RABBITMQ_CONFIG_FILE", "/etc/rabbitmq/rabbitmq-custom"); rabbitmq 3.6 doesn't work correctly... perhaps we can try to figure out the type of the file (erlang or sysctl format), or try to check version from image name (but not robust in my opinion, if you have a custom image without this name), or, more basicaly, provide two function /**
* Overwrites the default RabbitMQ configuration file with the supplied one.
*
* @param rabbitMQConf The rabbitmq.conf file to use (in sysctl format)
* @return This container.
*/
public RabbitMQContainer withRabbitMQConfig(MountableFile rabbitMQConf) {
return withRabbitMQConfigSysctl(rabbitMQConf);
}
/**
* Overwrites the default RabbitMQ configuration file with the supplied one.
*
* This function doesn't work with RabbitMQ < 3.7.
*
* This function and the Sysctl format is recommended for RabbitMQ >= 3.7
*
* @param rabbitMQConf The rabbitmq.config file to use (in sysctl format)
* @return This container.
*/
public RabbitMQContainer withRabbitMQConfigSysctl(MountableFile rabbitMQConf) {
withEnv("RABBITMQ_CONFIG_FILE", "/etc/rabbitmq/rabbitmq-custom");
return withCopyFileToContainer(rabbitMQConf, "/etc/rabbitmq/rabbitmq-custom.conf");
}
/**
* Overwrites the default RabbitMQ configuration file with the supplied one.
*
* @param rabbitMQConf The rabbitmq.config file to use (in erlang format)
* @return This container.
*/
public RabbitMQContainer withRabbitMQConfigErlang(MountableFile rabbitMQConf) {
withEnv("RABBITMQ_CONFIG_FILE", "/etc/rabbitmq/rabbitmq-custom.config");
return withCopyFileToContainer(rabbitMQConf, "/etc/rabbitmq/rabbitmq-custom.config");
} |
Hmm, my quick test has it the other way round:
Still, I think your suggested additions to the API make sense really. |
ok, I update the PR. Some remarks: withEnv("RABBITMQ_CONFIG_FILE", "/etc/rabbitmq/rabbitmq-custom.config"); works with 3.7 alpine, and if I omit the suffix ".config", it doesn't work. The file in sysctl format MUST have an empty line in the end, unless you will have:
in error. PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @twillouer - I'm happy with this!
see #1690