-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Change the default location of the 'enabled_plugins' file #2298
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
Conversation
bb604c8
to
9927b60
Compare
This PR should fix this issue - |
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.
I didn't test the patch locally yet, but beside my comment, it looks good to me.
However I'm still not sure about the approach, but I will add a comment in the #2234 issue.
rabbit_env:log_context(Context2), | ||
|
||
%% Migrate the enabled_plugins file to the new location if necessary | ||
Context3 = rabbit_plugins:maybe_migrate_enabled_plugins_file(Context2), |
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.
The rabbitmq_prelaunch
application can't depend on a module from rabbit
. The dependency is the other way around.
You could move the code you added to rabbit_plugins
to a new rabbitmq_prelaunch_plugins
module in the rabbitmq_prelaunch
application for instance.
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.
Ok, that makes sense.
I see we don't appear to separate the prelaunch app's tests into their own directory? So I will leave the rabbit_plugins_SUITE
in test/
as rabbit_prelaunch_plugins_SUITE
instead of placing it in apps/rabbit_prelaunch/test
.
During prelaunch, if an explicit plugins file location has not been specified via the RABBITMQ_ENABLED_PLUGINS_FILE env var, adopt a new location of /var/lib/rabbitmq/enabled_plugins if possible, but fallback to the original location if there is no file at the new location, and the file at the old location is both readable and writable. No change is applied for Windows.
apps/rabbit_prelaunch should not depend on rabbit, since rabbit already depends on apps/rabbit_prelaunch. So, rabbit_plugins:maybe_migrate_enabled_plugins_file/1 becomes rabbit_prelaunch_plugins:maybe_migrate_enabled_plugins_file/1.
3852035
to
fc18ca9
Compare
My recollection was that there were some additional unwanted side effects stemming from these changes. Since has been sitting for quite some time, I think the issue is best closed. |
During prelaunch, if an explicit plugins file location has not been specified via the
RABBITMQ_ENABLED_PLUGINS_FILE
env var, adopt a new location of/var/lib/rabbitmq/enabled_plugins
if possible, but fallback to the original location if there is no file at the new location, and the file at the old location is both readable and writable. It the file at the old location is strictly readable, we copy it to the new location and log a message.No change is applied for Windows, since the Windows
config_base_dir
anddata_dir
default to the same location and there is effectively no change.