Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

Avoid using script/compose #42

Closed
trickert76 opened this issue Jul 29, 2020 · 9 comments
Closed

Avoid using script/compose #42

trickert76 opened this issue Jul 29, 2020 · 9 comments
Milestone

Comments

@trickert76
Copy link

trickert76 commented Jul 29, 2020

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:

  • there is no setup question for webhooks and demo but a docker-compose.*.yml exists. The compose looks for the webhooks but that is only enabled manually.
  • also cat'ing the corresponding YAML file into the override may end with a wrong syntax, because of multiple main-nodes 'version', 'services' etc. so, the compose files should only contain the diff or use somekind of patch-mechanism.

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
Copy link
Owner

alangecker commented Jul 29, 2020

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:

  • You can't add/remove features easily anymore. It would always require to rerun the setup script from scratch (including throwing away all customizations)
  • There are not only components which get enabled/disabled, even some strings are getting replaced by conditions (https://github.com/alangecker/bigbluebutton-docker/blob/develop/scripts/compose). This would also be necessary to be handled by diff's during "compile time" (=on setup run), since we aren't able to set them in the compose script anymore.
  • diff/patch-files are more difficult to read compared to standard docker-compose YAML files
  • it is quite a huge rework, for which someone needs to volunteer. I personally can't imagine doing this currently - especially because I still can't see the disadvantage (:

@trickert76
Copy link
Author

I've two disadventages for you:

  • you need a script to start docker-compose, which is a script itself that could do everything you try to wrap with your script (merging multiple compose-files based on a basic system plus some overrides) - have look at 'mailcow' project as an example
  • as a user which uses only docker-compose environments it is a "break" that I need to run a script for bbb instead of docker-compose. I've a lot of docker-compose environments and I need to make an exception in my Ansible environment for new pulls etc. for bbb-docker.

As an adventage

  • of course, I can rewrite that setup script for you, I first wanted you opinion. Otherwise I would change my Ansible playbook.
  • of course, I can see, that the setup script also makes some useful other thinks (generating passwords, etc). That's fine and I wouldn't change that.
  • your first argument is partly "wrong", because, when I try to add a coturn server, I need to configure the secret and that is easier with the setup script. Also I could parse the existing .env file and don't need to stop the script.

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

@trickert76
Copy link
Author

trickert76 commented Jul 30, 2020

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

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

  • setup runs now under MacOS too (problems with sed and tr)
  • setup reads .env if available and uses the values as default

@staukini
Copy link

staukini commented Sep 4, 2020

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
Copy link
Author

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 alangecker mentioned this issue Sep 6, 2020
7 tasks
@alangecker
Copy link
Owner

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

As I wrote in #47 (comment) 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

  • ./scripts/setup

    • asks questions
    • generates a ./conf/config file which is Tcl compatible so that it can be used as source ./conf/config (current .env has some syntax issues).
    • asks at the end whether it should run ./scripts/generate
  • ./scripts/generate

    • generates a single docker-compose.yml only based on ./conf/config and a docker-compose.yml.tmpl
    • can be run by ansible (no interaction)
    • a templating language give us a much more beautiful and readable method to generate a file as cat and sed commands can.
    • since we use go-templates with dockerize already a lot, I think it makes sense to use the same templating language here. We could leverage docker run jwilder/dockerize to avoid any new dependencies on the host.

would be happy about any feedback! :)

@alangecker alangecker added this to the Release v2.3 milestone Oct 5, 2020
@alangecker alangecker mentioned this issue Oct 6, 2020
@clawoflight
Copy link

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
Copy link
Owner

I worked now on a new approach: #71
@clawoflight maybe you have some feedback on that? :)

@alangecker
Copy link
Owner

this is now finally done in the v2.3.x branch! :)
see #71 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants