Skip to content

Load additional Cuttlefish configuration files #2258

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 3 commits into from
Feb 28, 2020

Conversation

dumbbell
Copy link
Collaborator

@dumbbell dumbbell commented Feb 26, 2020

By default, in addition to $RABBITMQ_CONFIG_FILE, we also load all matching $RABBITMQ_CONFIG_FILES. It allows to split the configuration into many files for easier management.

Here is the behavior in more details:

  • If $RABBITMQ_CONFIG_FILES is a directory, all files directly inside it are considered.

  • If $RABBITMQ_CONFIG_FILES is a glob pattern, all files matching the pattern are considered.

  • $RABBITMQ_CONFIG_FILES is only relevant when the main configuration is either missing (the file does not exist or is empty) or uses the Cuttlefish format.

The default value of $RABBITMQ_CONFIG_FILES is:

  • /etc/rabbitmq/conf.d/*.conf on Unix
  • %APPDATA%\RabbitMQ\conf.d\*.conf on Windows

CAUTION: If we want to backport this feature, we have to backport #2180 first! The reason is that to make this possible, we have to use the Cuttlefish library, not the executable.

Depends on rabbitmq/rabbitmq-common#357.

@dumbbell
Copy link
Collaborator Author

Before marking this patch ready for review, I would love to get feedback on the behavior and the variable name.

Copy link
Contributor

@gerhard gerhard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pair-tested in dev, works as expected with:

  • globs
  • dirs
  • dirs with subdirs
  • config file
  • advanced config file

Great debug logging output 👍

@dumbbell dumbbell force-pushed the load-several-config-files branch from 911dc11 to d424977 Compare February 26, 2020 16:05
@lukebakken
Copy link
Collaborator

I'm sure I've seen this feature requested before. Nice work @dumbbell

@dumbbell dumbbell marked this pull request as ready for review February 26, 2020 16:24
@dumbbell dumbbell force-pushed the load-several-config-files branch from d424977 to a3764e5 Compare February 26, 2020 16:29
@dumbbell
Copy link
Collaborator Author

Thanks!

@dumbbell
Copy link
Collaborator Author

I'm going to add a testcase before we merge it.

@dumbbell dumbbell force-pushed the load-several-config-files branch from a3764e5 to 8043cd0 Compare February 26, 2020 16:49
@dumbbell
Copy link
Collaborator Author

I can't add a testsuite because our framework extension, rabbitmq-ct-helpers, already uses an Erlang-term-based configuration file to configure nodes and it can't be modified without a heavy redesign of our testsuites. Therefore, the additional configuration files added by a testcase would be ignored anyway because of the main configuration file format.

Let's continue without a testcase for this :-(

It was already loaded in `rabbit_prelaunch` (prelaunch phase 1).
We already try both formats when we parse the configuration.
By default, in addition to `$RABBITMQ_CONFIG_FILE`, we also load all
matching `$RABBITMQ_CONFIG_FILES`. It allows to split the configuration
into many files for easier management.

Here is the behavior in more details:

* If `$RABBITMQ_CONFIG_FILES` is a directory, all files directly inside
  it are considered.

* If `$RABBITMQ_CONFIG_FILES` is a glob pattern, all files matching the
  pattern are considered.

* In both cases, subdirectories (and whatever they contain) are ignored.

* `$RABBITMQ_CONFIG_FILES` is only relevant when the main configuration
  is either missing (the file does not exist or is empty) or uses the
  Cuttlefish format.

* Additional configuration files must use the Cuttlefish format.

The default value of `$RABBITMQ_CONFIG_FILES` is:
* `/etc/rabbitmq/conf.d/*.conf` on Unix
* `%APPDATA%\RabbitMQ\conf.d\*.conf` on Windows

Error messages related to Cuttlefish parsing were improved in the
process.

[#171491267]
@dumbbell dumbbell force-pushed the load-several-config-files branch from 8043cd0 to c1e891c Compare February 28, 2020 08:42
@dumbbell dumbbell merged commit eb06138 into master Feb 28, 2020
@dumbbell dumbbell deleted the load-several-config-files branch February 28, 2020 09:16
@dumbbell
Copy link
Collaborator Author

To make use of RABBITMQ_CONFIG_FILES in the context of make run-broker, RABBITMQ_CONFIG_FILE must be set to a regular empty file (/dev/null will not work because the code checks if it is a regular file). This is required because RabbitMQ assumes that the main configuration file is Erlang-term based, in which case additional config files will not be loaded. We force RabbitMQ to use Cuttlefish with this empty file which is a valid Cuttlefish file, but it is not a valid Erlang-term based config file. This way, $RABBITMQ_CONFIG_FILES are loaded.

To improve this, we can probably always take the Cuttlefish code path: if $RABBITMQ_CONFIG_FILE is an Erlang-term based file, we still use Cuttlefish and pass it as the advanced configuration file.

@michaelklishin
Copy link
Collaborator

FTR, this change first shipped in 3.9.0.

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.

4 participants