-
Notifications
You must be signed in to change notification settings - Fork 44
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
QPID-8352: [Broker-J] Official Docker image for Broker-J #225
Conversation
Considering we have never had a single image all this time, this feels over elaborate as a starting point. Are there seriously going to be 192 image variants with the different os/jvm/protocol/store options as described? That seems excessive. Which ones would actually be getting pushed and why? The 'protocol' images having amqp-all, amqp-0-8, amqp-0-10, amqp-1-0 means it doesnt even allow picking e.g just AMQP 0-9 or 0-9-1 (and doesnt make it obvious the 0-8 one would probably actually include them). The broker already allows selecting specific protocols through its configuration, so I dont see a good reason to complicate things by having 4 (or 6..so make that 288 variants?) different image variations for that. If someone wants a seriously customised image with different protocol deps in it then they can easily [/continue to] roll their own. For everything else they can just configure it. I also see no good reason to have a postgres specific image (or derby, or memory store either). Are we going to add one for every DB someone likes and explode the matrix of image options even further? We dont even test with postgres at all so its unclear why would we ever have that as a default image option to begin with. Having a single basic out-the-image setup that gets people started, like the regular broker distribution does itself, and then having ways for people to easily supply custom config (including users if appropriate, as opposed to the hard coded ones) to use instead to meet their specific needs, seem easier to justify and maintain later, while actually being more general for people to get whatever behaviour they want and without needing yet more image variants to be created later. Even the 4 OS and 3 JVM options seems a bit much. As comparison, some other Apache projects with brokers that also only started deploying images this year, currently have 1 or 2 images that they actually push, and perhaps a few for local dev that they dont. That seems like a more appropriate path to start with. |
Hi Robbie, Thank you for the feedback. Would it be OK from your point of view to provide following options by default: broker storage: memory / BDB Possibility for further customization could then be available in a separate "dev" profile. Best regards, |
I would personally pick a single setup to start with as I said. Probably something close to what users get with the regular assembly, i.e BDB. Allow its config to be changed out so that people can supply config to do whatever else it is they like, e.g if they want memory store, JDBC, protocol-specific setups, etc. I'd just pick 1 JVM. If people dont like whats picked, they can then roll their own. |
Hi Robbie, The module was reworked. There are two maven profiles now - the default one and the development one. The default one allows creation of broker images using BDB as a message storage based on java 11 with the possibility of operational system choice: qpid-broker-j-alpine Development profile provides wider configuration options for users who may need them. The README.md was updated with the examples of broker customization for preparing images having pre-declared queues, exchanges and other broker objects. Best regards, |
This still seems way over complicated to be starting with. In some ways actually more than it was first time round now there are 2 different mechanisms to use, and yet still the 192 options. Other broker projects here at the ASF, some that have done this very recently, ended up with 1 or 2 basic images being published, with configs that roughly match the existing release bundles. Built from the existing release bundle, using a basic script. For anything else configuration wise, users can then configure the broker. They are trivial by comparison, and look far easier to maintain later than this, and actually allow building from any release version as well which this approach by contrast doesnt look to. E.g I simply do not see a need for us to have, and need to maintain later, 16 new assembly variants inside this module to select protocols or store types from. Which as before, the protocol options are actually not even clear which protocols will be getting enabled and so will actually mislead, and you cant even select any different options if you want a combo. Users that want to configure protocols and store types can and should just configure that using the far more flexible existing configuration mechanisms for that, or just make their own images as they must have been doing for a decade already. All we need to do is make it easy for people to supply their own desired configuration (which its not clear this really does yet). If they want to customise the deps they based on their protocols and stores, can make their own images, as they have had to for a decade already. We can pick a JVM, we dont need to support them all. 17 or 21 at this point makes sense. We dont need 4 OS options. Particularly when one is going EOL in a bit over 6 months. 1 would be fine, 2 at a stretch if you want to e.g do alpine. |
Hi Robbie, The qpid-docker module was reworked (similar module of ActiveMQ Artemis was taken as an example). There is a shell script docker-build.sh provided, which prepares the folder containing files needed for docker images either from local distribution or from the Broker-J version specified. The README.md was updated with the examples of broker customization for preparing images having pre-declared users, queues, exchanges and other broker objects. Best regards, |
Hi Robbie, Thank you for the review and for the remarks and comments. The docker file and scripts were modified, the README.md and the broker book were updated. Best regards, |
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.
Looks good, just a few tiny niggles now.
Thinking ahead, what images are you thinking you/@vavrtom would actually be publishing in the end? Maybe apache/qpid-broker-j: and apache/qpid-broker-j:latest as alias to the newest version? Or also the alpine ones too, e.g also adding apache/qpid-broker-j:-alpine with apache/qpid-broker-j:latest-alpine as alias?
Hi Robbie, Thank you for the remarks, the scripts and the documentation were updated. We were thinking about starting with the ubuntu images - apache/qpid-broker-j:{version} and apache/qpid-broker-j:latest. Alpine based images could be added later if there will be a demand for it. Best regards, |
Sounds good |
This PR addresses JIRA QPID-8352, adding functionality for building broker-j container images