Docker container

[AaronDewes]

The docker images build, but haven't been tested yet.

---

This adds very early docker container support. Once you merge this PR, builds will automatically happen on every push and tag and will be pushed to GitHub container registry.

[junderw]

main branch doesn't exist in this repository, so anyone could make an unprotected branch with push perms.

It should be removed, or a main branch should be created and protected.

[AaronDewes]

@bumi I'm not sure about your branch protection rules, does anyone without direct access to the main branch still have push access to this repo?

[junderw]

I am not a member of this project, but you can just look at their project branches and see that main doesn't exist.

In Github, you can't protect a branch that doesn't exist.

[bumi]

secrets and such are not available to forks, so this will not be working when somebody forks the repo (which is what we want).

edit: but yeah, LnMe uses master and not main.

but I am also wondering if we need that on-push action? isn't it enough to do that for tags which then are the official releases?

[AaronDewes]

I'm using an on push action myself, it's free to run, and it allows testing a new version in Docker (and maybe fixing new bugs only appearing in Docker) before tagging it.

[bumi]

if we want to keep building on push, then we must change "main" to "master" (we still use master as the master/main branch)

[AaronDewes]

It builds on both master and main, so it still works if you rename

[bumi]

ah I missed the master in the line below. sorry. thanks.

[bumi]

(though I have no plans to rename the branch and potentially break things.)

[AaronDewes]

Actually, branch could be left out so it also builds for WIP feature branches for testing. What do you think?

[junderw]

LGTM

[junderw]

Actually, this build should be conditional on the test suite passing.

This will prevent broken builds from being published.

Edit: No tests exist. Might want to fix that first.

[bumi]

Yes, sadly that's a problem. at this point we do not have automated tests.
it's a tiny app but at least some basic integration specs must be added with adding more features.

[AaronDewes]

Tested & works.

[bumi]

How are configuration options passed in? that would be done when running the container? e.g. through docker-compose?

[AaronDewes]

Yes, options get passed in as env vars through docker-compose

[bumi]

the PORT can be configured... should this then also be configurable (based on the LNME_LISTEN env variable) or is it a practice to NOT change the default port when using docker?
(I am not very experienced with docker)

[junderw]

EXPOSE just exposes a port to other containers by default.

At runtime / in a higher level compose file you can expose and/or open-to-host any port you want.

This should be fine to use the default port. (It's semi-convention)

[bumi]

thanks! 👍

[AaronDewes]

From the docker documentation

The EXPOSE instruction does not actually publish the port. It functions as a type of documentation between the person who builds the image and the person who runs the container, about which ports are intended to be published.

[bumi]

OK, I think this is cool.

I can then also do something like: docker run -it --rm ghcr.io/bumi/lnme:master correct?
could you add a note to the readme?

[AaronDewes]

Sorry, I missed your comment. I'll add a note now.

[bumi]

thanks so much! merging!