Skip to content

[WIP] Add 3.7 #197

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
Dec 8, 2017
Merged

[WIP] Add 3.7 #197

merged 2 commits into from
Dec 8, 2017

Conversation

tianon
Copy link
Member

@tianon tianon commented Sep 25, 2017

Closes #183

This builds on #195 to start implementing support for 3.7. I haven't converted the entrypoint to use rabbitmq.conf yet, but I've looked at a lot of the documentation and am really pleased with how nice and simple everything looks. 😄

I'm casually pondering (as noted in #187 (comment)) whether we should use this major version bump as an opportunity to claw back our supported environment variables to something closer to what upstream natively supports out-of-the-box, but would appreciate if @michaelklishin (or anyone else from upstream) would be willing to provide some color on whether that's something we definitely should try do, definitely should not do, or something upstream is likely to be indifferent about. 🙏

It's definitely easier in Docker to provide environment variables than it is to provide a whole new config file, although only really for the one-off use case -- for a real production deployment, the difference is pretty negligible.

There's also the new auto-clustering behavior proposed in #183 that we ought to consider supporting somehow (which if we stick with environment variables will mean new ones to configure that, but if we ditch the environment variables will likely mean a total punt on any additional behavior to support that out-of-the-box due to strictly bring-your-own configuration).

@tianon
Copy link
Member Author

tianon commented Sep 25, 2017

For those in the peanut gallery wondering about what this new configuration is / where it's documented, see http://next.rabbitmq.com/configure.html.

@tianon
Copy link
Member Author

tianon commented Oct 11, 2017

Discussed this a little bit over in #199 (comment) -- current leaning is towards taking the version bump as an opportunity to slim things down considerably, suggesting that users instead use the newer (simpler) configuration file, or definitions import (which should be much more reliable than our fiddly shell scripting anyhow).

@JorisAndrade
Copy link

Since 3.7 has just been released. Any news on the publication ?

@tianon
Copy link
Member Author

tianon commented Dec 7, 2017

@michaelklishin looking at getting this in ASAP so we can get the new 3.7.0 GA release out -- I'm thinking we should gut docker-entrypoint.sh of so many supported environment variables, but I'm torn.

On the one hand, I'd like to be as close to only supporting what upstream does out-of-the-box as we can reasonably be, but on the other, Docker containers are much easier to configure via environment variables than files. 😞

Any suggestions, advice, or even just opinion you're willing to offer? 🙏 ❤️

@tianon
Copy link
Member Author

tianon commented Dec 7, 2017

For completeness, here's what I believe is the full list of docker-entrypoint.sh-supported environment variables:

  • RABBITMQ_DEFAULT_PASS
  • RABBITMQ_DEFAULT_USER
  • RABBITMQ_DEFAULT_VHOST
  • RABBITMQ_ERLANG_COOKIE
  • RABBITMQ_HIPE_COMPILE
  • RABBITMQ_MANAGEMENT_SSL_CACERTFILE
  • RABBITMQ_MANAGEMENT_SSL_CERTFILE
  • RABBITMQ_MANAGEMENT_SSL_DEPTH
  • RABBITMQ_MANAGEMENT_SSL_FAIL_IF_NO_PEER_CERT
  • RABBITMQ_MANAGEMENT_SSL_KEYFILE
  • RABBITMQ_MANAGEMENT_SSL_VERIFY
  • RABBITMQ_SSL_CACERTFILE
  • RABBITMQ_SSL_CERTFILE
  • RABBITMQ_SSL_DEPTH
  • RABBITMQ_SSL_FAIL_IF_NO_PEER_CERT
  • RABBITMQ_SSL_KEYFILE
  • RABBITMQ_SSL_VERIFY
  • RABBITMQ_VM_MEMORY_HIGH_WATERMARK

@tianon
Copy link
Member Author

tianon commented Dec 7, 2017

Two places we apply additional processing are around RABBITMQ_VM_MEMORY_HIGH_WATERMARK (and its interaction with cgroups) and the SSL-related configuration (with which we apply the appropriate port changes automatically).

@tianon
Copy link
Member Author

tianon commented Dec 7, 2017

Oh, and a couple of those support an _FILE variant for use with Docker Secrets (which can only seed secret values into files, not environment variables)...

See #203.

@tianon
Copy link
Member Author

tianon commented Dec 7, 2017

Perhaps we should simply adjust to the new configuration file format so that we're more respectful of bring-your-own-config users, since this currently-supported set doesn't seem all that large, IMO.

@yosifkit your thoughts would obviously be appreciated too 🙏

@tianon tianon force-pushed the 3.7 branch 7 times, most recently from c41391b to c775c5e Compare December 7, 2017 22:57
@tianon
Copy link
Member Author

tianon commented Dec 8, 2017

I've updated this to convert to the new format (useful commit for review is 946a6d2).

The only major gotcha I've found so far is rabbitmq/rabbitmq-server#1445, which I've left as a TODO comment and can be added back in 3.7.1. 🎉

This also allows folks to bring their own rabbitmq.conf and still use our environment variables to adjust their configuration file at runtime! 🎂

@yosifkit yosifkit merged commit 211af6e into docker-library:master Dec 8, 2017
@michaelklishin
Copy link
Collaborator

@tianon @yosifkit note that the old config format can be used in combination with the new one. Some plugins (e.g. LDAP) support configuration that is really hard to express without a language with "real" data structures (or a DSL). So the image should support the advanced config file (a yet another file, in the Erlang term format). I suspect that a massive % of users would prefer to "bring their own" advanced config file.

@tianon
Copy link
Member Author

tianon commented Dec 9, 2017 via email

@lukebakken
Copy link
Collaborator

Just "rabbitmq.conf" with "advanced.config" together?

Yep! Riak works the same way, too.

tianon added a commit to infosiftr/stackbrew that referenced this pull request Dec 12, 2017
- `bash`: apply Alpine patch to 4.4 for process substitution hang (tianon/docker-bash#4)
- `golang`: add `alpine3.7` variant of 1.9 (docker-library/golang#192)
- `rabbitmq`: add 3.7, which includes a new configuration file format (docker-library/rabbitmq#197)
- `ruby`: remove `2.5-rc-alpine3.6` variant (superseded by `2.5-rc-alpine3.7`)
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.

Add tag with autocluster plugin
6 participants