[159039271]
System daemons should not expect to write to /etc. This causes issues in environments like docker where /etc is frequently on a read-only FS.
https://groups.google.com/d/topic/rabbitmq-users/DuxRP0zrMdA/discussion
It's unusual for an application to write to /etc, and as the above mailing list discussion shows, /etc is sometimes mounted read-only.
We should investigate supporting the plugins file first in /var/lib/rabbitmq, falling back to /etc/rabbitmq.
Makes sense 馃憤
@lukebakken @michaelklishin I believe the broker won't start without a writable /var/lib/rabbitmq, so when we say fallback, do we mean to first look for /var/lib/rabbitmq/enabled_plugins, and if it does not exist look for it at the current location? I presume we would also want to log a warning if it is in /etc that the default has now changed (not to mention update all the docs).
So the behavior that I now have in mind is:
IF /var/lib/rabbitmq/enabled_plugins is present
we use it
ELSE
IF /etc/rabbitmq/enabled_plugins is present and writable
we use the /etc location, but log a console message that the location is deprecated
ELSE
we use the new /var location, but log a message that the file was ignored because it was not writable.
I believe this should be safe for upgrades, as with an existing read and writable file in /etc, the behavior is unchanged aside from the log messages.
That sounds reasonable to me.
OK!
Work in progress is here: https://github.com/rabbitmq/rabbitmq-common/pull/369
In short, I want to validate it's effects on the docker images before calling the PR ready.
After some further consideration, if the file is present in /etc but not writable, I think rather than ignoring it, we should copy it to the new location. Since it seems possible that if one never changes the enabled plugins through the cli, the broker could have been working for them with that file not writable. Doing the copy means that we won't suddenly disable their plugins, and if they later enable/disable and restart, that state will be honored as the new location would be writable.
I have solved the docker issue that I was running into, and the fix is here https://github.com/rabbitmq/rabbitmq-server-release/pull/126
I believe these changes are now ready to go.
https://github.com/rabbitmq/rabbitmq-server/pull/2298 handles the change in the broker, but I'll need to make a corresponding change to the cli to handle use of the rabbitmq-plugins cli when used with --offline
Now addressed in the CLI with https://github.com/rabbitmq/rabbitmq-cli/pull/407
To elaborate on the CLI changes, based on @michaelklishin 's helpful input I decided to simplify the approach taken there. In the CLI, if the enabled_plugins file is present at the old location only, we use that location and let the CLI subsequently succeed or fail as it would normally, without a guard for read only access. Otherwise, we use the new location, again allowing the cli to fail normally based on that location. This should reduce the behavior change, but be compatible with the way things are now handled in the broker.
I looked at the patches from @pjk25 and they look good to me.
That said, I believe we still have a deeper issue with that file. And if users and us will have to go through some kind of migration plan, I would prefer we fix the actual problem.
To me, this file has two incompatible uses:
Nobody should consider it a configuration file, but its location on Unix says the opposite and there is no other way for configuration management tools to configure RabbitMQ plugins. I'm sure this leads to some surprises like "I disabled that plugin using rabbitmq-plugins(8) but the plugin is back! WTF!" if the configuration management tool "fixed" the configuration.
I think we should first split the two uses:
enabled_plugins to the data directory (i.e. this set of patches).Now that we (almost) support many small files (#2277), we should think about a solution to:
What do you think?
Assuming that configuration file would only need schema files from plugins, that could work. If we'd depend on schema files from enabled plugins, there's a chicken-and-egg problem.
Conflicts between explicit operator actions and what deployment tools do are not specific to this file or RabbitMQ in general. I think this is more or less obvious to operators.
If plugins can be enabled/disabled with the rabbitmq-plugins cli, management ui, _or_ a config file, then there is always potential for discrepancies of state that surprise the user. But, I believe @michaelklishin is correct in that operators are not too surprised by it (or at least they haven't been complaining in this case).
Configuring plugins via file rather than additional cli commands will be the most common path in the k8s world in my opinion, so I think it deserves first class support of some kind.
What currently feels most "right" to me as a goal would be that rabbitmq.conf exposes a plugins.default_enabled option, to signify that the list of plugins may be overridden later at runtime, or may have no effect if plugins have already been enabled via other means.
The currently enabled plugins would continue to be written to /var/lib/rabbitmq/enabled_plugins and persist across restarts of the broker. I'd prefer that we stop exposing the list of currently enabled plugins through the filesystem at all, and instead force users to query the broker, but I also prefer to avoid any breaking changes whenever possible.
Assuming that configuration file would only need schema files from plugins, that could work. If we'd depend on schema files from enabled plugins, there's a chicken-and-egg problem.
Plugins are in the code path, enabled or not, and rabbitmq_prelaunch_conf passes all schemas to Cuttlefish, regardless of the plugins' status. So I believe we don't have this issue.
I'd prefer that we stop exposing the list of currently enabled plugins through the filesystem at all, and instead force users to query the broker (...)
We have to persist that list somewhere, but I see what you mean. That's exactly my point: we should decouple the state recording from the configuration part.
I suggest that we adopt a solution like this and consider changing the way plugin state is stored and preconfigured for 4.0.
Before we get too fancy, just remember the original problem is that /etc is not writable all the time. That's the only problem we need to solve right now 馃槃
I just discovered that the management plugin expects to be able to determine the location from the env, and crashes with the current code. Looks like I have more to do before this is ready to go.
My comment above can be disregarded. I was running mismatched branches in my build of the broker.
Given https://github.com/docker-library/rabbitmq/issues/410#issuecomment-630907597, I'm now tempted to say that the file should stay put for now.
Basically, in the docker context, we should not assume /etc writable, nor should we assume that initial config can be kept in /var/lib/rabbitmq. Since the enabled_plugins file is both config and state, neither place is an appropriate home for it. At least it's configurable with RABBITMQ_ENABLED_PLUGINS_FILE so while it might not work out of the box, a user has options.
What does everyone think about me closing this issue and related PRs? I think the rabbitmq-plugins cli is basically incompatible with a read-only config scenario. What I might propose instead is a new flag for that tool, maybe rabbitmq-plugins enable_plugin rabbitmq_tracing --transient, where the enabling is not written to disk, and will revert on restart. It might address the debugging scenario from the mailing list that brought this up more directly.
Why not take these changes "all the way" at this point in time and separate config vs state correctly? This is the correct fix since we can't assume /etc is writable.
The VOLUME issue shouldn't affect our decisions as the solution is already outlined in https://github.com/docker-library/rabbitmq/issues/410#issuecomment-630907597 - drop VOLUME altogther.
My impression was that VOLUME wouldn't get dropped. It servers as a hint to docker compose to retain data. So I think we have to account for it. Maybe not after https://github.com/moby/moby/issues/3465 allows UNVOLUME or something similar.
To fix this properly, I think we have to assume that the config files aren't writable, or at least offer a mode of behavior that supports it. That's the real k8s pattern as I see it - config files get injected read only. https://github.com/kubernetes/kubernetes/issues/62099#issuecomment-378809922 & https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/#add-configmap-data-to-a-specific-path-in-the-volume
I don't know if that would be too big a change for 3.8.5. But, what if we
.conf/.config filesrabbitmq-plugins --offline will fail, as the file won't be modifiedrabbitmq-plugins will also fail, unless invoked with --transient, where the plugin will activate, but only until the next restart.conf, honor the enabled_plugins file with the current behavior, in the current locationOnce the public docker takes the new approach (it relies on --offline), these users should not run into this problem, but everyone else can opt-in to the new option on their own time.
Let's think of this from an operator's point of view.
Ensure that /etc/rabbitmq/enabled_plugins is available when RabbitMQ starts. Supported today and with the changes in these PRs.
_Or_, create /var/lib/rabbitmq/enabled_plugins, which is supported by these PRs.
rabbitmq-plugins at some point to enable or disable plugins."Use the CLI command. Changes to the running set of plugins are saved in /var/lib/rabbitmq, which will be writable. RabbitMQ will always consult this file first when it starts to determine the list of plugins to start.
I suspect this is a very rare scenario. Modify /var/lib/rabbitmq/enabled_plugins. If the user modifies the file in /etc/rabbitmq the changes won't be applied BUT I expect that RabbitMQ will log loudly that files exist in /etc/rabbitmq and /var/lib/rabbitmq and that the latter file is used.
So having said all that I think the current set of changes suffice.
The latter scenario is how most CI scripts work. They start a node and then use rabbitmq-plugins enable or rabbitmq-plugins set. As long as those are affective immediately and in case of a node restart, whether /var/lib/rabbitmq/enabled_plugins is used won't matter to almost anyone.
It's not yet clear to me where we landed on this. If we apply the change, it breaks the management variants of the public docker image. They would have to make some changes to the Dockerfiles, to support it. Maybe we are okay with that, but it wouldn't be my first choice, especially because there would still be some limitations for child docker files.
If not for the above, I'd be in favor of merging these changes now.
Maybe a suitable option is to contribute the necessary fix in advance to https://github.com/docker-library/rabbitmq. I'll take a look at what that would entail, and open an appropriate PR.
I have opened https://github.com/docker-library/rabbitmq/pull/416
Most helpful comment
I looked at the patches from @pjk25 and they look good to me.
That said, I believe we still have a deeper issue with that file. And if users and us will have to go through some kind of migration plan, I would prefer we fix the actual problem.
To me, this file has two incompatible uses:
Nobody should consider it a configuration file, but its location on Unix says the opposite and there is no other way for configuration management tools to configure RabbitMQ plugins. I'm sure this leads to some surprises like "I disabled that plugin using rabbitmq-plugins(8) but the plugin is back! WTF!" if the configuration management tool "fixed" the configuration.
I think we should first split the two uses:
enabled_pluginsto the data directory (i.e. this set of patches).Now that we (almost) support many small files (#2277), we should think about a solution to:
What do you think?