alangecker / bigbluebutton-docker

merged into https://github.com/bigbluebutton/docker
GNU Lesser General Public License v3.0
99 stars 33 forks source link

Avoid using script/compose #42

Closed trickert76 closed 3 years ago

trickert76 commented 4 years ago

I think, your project is a much better approach to dockerize BBB or wait for a new BBB version that supports Ubuntu 18.04 or 20.04.

I think, the way, you integrate the optional nginx and coturn isn't ideal. I think, it would be "nicer" to have only the docker-compose.yml and (which is supported out of the box from docker-compose) an additional docker-compose.override.yml (see https://devilbox.readthedocs.io/en/latest/configuration-files/docker-compose-override-yml.html).

I would change the setup script in a way, that it cats/echos/pipes the content of the docker-compose.*.yml into the docker-compose.override.yml, when the user says 'y' in the script. So, instead of

if [ ! "$https_proxy" == "y" ]
then
    sed -i "s/ENABLE_HTTPS_PROXY.*/#ENABLE_HTTPS_PROXY=true/" .env
fi
...

do a:

rm docker-compose.override.yml && touch docker-compose.override.yml
if [ "$https_proxy" == "y" ]
then
    cat docker-compose.https.yml >> docker-compose.override.yml
fi

if [ "$greenlight" == "y" ]
then
    cat docker-compose.greenlight.yml >> docker-compose.override.yml
fi

if [ "$coturn" == "y" ]
then
    cat docker-compose.coturn.yml >> docker-compose.override.yml
fi

In that case a user can just use docker-compose in the root directory of the project.

After reviewing the code, there are several other things to do:

I'm switching (personally) to a Jinja template together with Ansible. That works too (because the script cannot be used easy in Ansible because of the question-answer-format without params).

alangecker commented 4 years ago

I think, your project is a much better approach to dockerize BBB or wait for a new BBB version that supports Ubuntu 18.04 or 20.04.

thanks a lot for appreciating it!

I think, the way, you integrate the optional nginx and coturn isn't ideal. I think, it would be "nicer" to have only the docker-compose.yml and (which is supported out of the box from docker-compose) an additional docker-compose.override.yml

hmm, I don't really understand the disadvantages of the current solution? :D

I personally think it is "nicer" like it is now, but with that we are so far just expressing our taste^^

Arguments, that I see, for keeping it like it is:

trickert76 commented 4 years ago

I've two disadventages for you:

As an adventage

I'll make a proposal. I'm not sure, if I can do that this week, but I try.

trickert76 commented 4 years ago

Can you have a look at: https://github.com/trickert76/bigbluebutton-docker

Of course it doesn't contain your latest changes (ipv6, prometheus).

staukini commented 4 years ago

I would second this design change.

Coming from the docker environment it feels a bit hacky. You could still use a setup script, where you generate the .env file (including passwords) and generate one clean docker-compose.override.yml file.

I think with this step you could easily upload your bbb containers to docker hub where a bigger audience will notice this project.

trickert76 commented 4 years ago

Of course. I‘m not sure, why the images arent hosted on Docker Hub. But maybe with the new abilities here on Github (Actions and Hub) this could be easier. I‘d like to try that in my repo and give some feedback.

alangecker commented 4 years ago

@trickert76 thanks a lot for your push in that direction!

As I wrote in https://github.com/alangecker/bigbluebutton-docker/pull/47#issuecomment-687743566 already, I think we should include this in the upcoming v2.3 release, so that these breaking changes are introduced in a moment where people are aware about it. This gives us the opportunity to even further completely rethink the way the tooling is done. Unfortunately this would abandon the work you've done already :/

My Proposal

would be happy about any feedback! :)

clawoflight commented 3 years ago

As someone who is used to docker and docker-compose, I fully agree with the points made by @trickert76; the current design is both unidiomatic and more complex than necessary. Thank you for your work :)

alangecker commented 3 years ago

I worked now on a new approach: https://github.com/alangecker/bigbluebutton-docker/pull/71 @clawoflight maybe you have some feedback on that? :)

alangecker commented 3 years ago

this is now finally done in the v2.3.x branch! :) see https://github.com/alangecker/bigbluebutton-docker/pull/71#issuecomment-777506431