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
Feat: rework Container image, Compose manifest and CI #64
base: main
Are you sure you want to change the base?
Conversation
This PR presents the result of 16 hrs of work to reshape it into a more "production-ready" form. This is achieved by dropping the support for local source builds.
Additionally, following the 12factor.net approach of designing web applications, it is useful to consider the parts of the application self-contained and independent from external systems. Why the reliance on a A convention designed for running jobs in distributed systems is to use a worker queue and have it be triggered with timers. Fortunately Celery is already used for dispatching asynchronous tasks. Its Celery Beat feature can also be used to regularly schedule jobs, removing the need for a separate
I'm especially invested in this, since the cron implementation took a quarter of the time from the overall procedure and is prone to fail in a container. What could possibly go wrong?
To complete the move towards "cloud-native", distributed application design, it is also advisable to implement the database URL pattern, which had been introduced by Heroku and is of good ease, convenience and use to DevOps people.
Further on, now that the application setup is more decoupled, images are more lightweight and deterministic, due to switching to pinned release versions of Pretalx, it becomes possible to imagine to further
I'm thinking of the way how these two repositories react to their upstream, adapted from Node to Python: In case you agree with the follow ups as outlined above, I'd step ahead and open respective issues. |
More inspiration could be taken from allmende/docker-fiduswriter. |
context/default/entrypoint.sh
Outdated
export PRETALX_DATA_DIR="${PRETALX_DATA_DIR:-/data}" | ||
|
||
PRETALX_FILESYSTEM_LOGS="${PRETALX_FILESYSTEM_LOGS:-/data/logs}" | ||
PRETALX_FILESYSTEM_MEDIA="${PRETALX_FILESYSTEM_MEDIA:-/data/media}" |
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.
Shouldn't these defaults match what's set in the .env.example
?
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.
This was adapted from the original image and I guess for now they do not show the container defaults, but rather the application defaults.
I'm open to hearing further opinions on this.
This looks generally good to me. While in a Kubernetes environment I'm used to having CronTab jobs running periodic stuff in containers, I don't see too much harm in having crontab running within the application container. That is, unless someone wants to create replicas of the application container and run the worker things isolated on their own, but I doubt that this would be possible with Pretalx, so this is likely a non issue. For static file serving I think that nginx as a separate container (or any other simple file server like 👍 Overall, I'm happy with these changes. Could we maybe consider having a GitHub workflow which automates new releases to DockerHub (or GitHub Packages)? This would avoid having bug-fixes merged into |
partially reverts a00895e
Sorry for cd06a80, cd06a80, 68537d4 plus the additional pull requests from the They happened when working in multiple environments for the original Compose, the legacy Compose and the CI adaptations, which are always a bit "verbose" in commits, due to how their side-effects are only produced and can only be tested by pushing and inspecting the response of the CI system. |
@almereyda Thank you for all this work and time invested in this repository! Given your interest and time investment, I have to ask – would you be interested in becoming a maintainer for this repository? |
Hi @rixx, thanks for the kind question. Actually, this was a one-shot effort, but given others like @DASPRiD would like to share the effort, I can imagine to occasionally review PRs and merge them. The current maintainers are the people listed as organisation members, am I correct? For now I see a few obstacles on the path ahead for the breaking changes in the rework as present:
Then there are technical questions, which would be good to settle as the pretalx-docker community:
With the multiple layers involved, each incurs opinionated choices on how to do things and which use cases to support.
If the question of becoming a maintainer to be in a position to make these choices for the community without enquiry, it is not me who we are looking for. If we can keep a small overhead of a conversation about the what's, how's and why's, please count me in. |
No, this repository is "community maintained", which means that I chuck the permissions at everybody who wants them after contributing to the repository, as I am neither interested nor qualified to maintain anything Docker related. (The other two members of the pretalx org are largely there as bus factor alleviation, fwiw, and aren't involved in the day-to-day development of pretalx.) The current maintainers are @Lukas2112 and @shoetten.
Definitely not, no. I can imagine no argument in favour – as I don't know Docker, don't use Docker, and have no interest in it either, this development container would become broken/outdated almost immediately, and would increase the maintenance burden for no real gain. All Docker related things are meant to be contained in this repository.
I'm very much in favour of as much automation as possible. Currently, when I release a new pretalx version, I update the submodule, tag the commit, and push to kick of the build, which is fine (I have to do a bunch of manual stuff on release anyway, like releasing a blog post, so this doesn't hurt). Definitely don't want anything more manual than this – I'd be fine with an even more automated process that just checks for new pretalx releases.
Completely understandable – and like I said, I appreciate all the work you've done, and the maintainership offer was just that: an offer of write access to this repository. Let me know if you want it, and no hard feelings if not. The problem is kinda that somebody has to make those decisions, and I'm not in a position to do so, either – I will say that I'm generally in favour of few breaking changes, but on the other hand, we do advertise that this repository is community maintained and not officially recommended, so there is a bit of leeway. I think at minimum, we should have an upgrade/migration guide in the README that explains what has changed, and why, and what people ought to do. Everything else (I know I skipped several of your questions), I'd leave up to you and everybody who uses this repo or is invested in it. |
Okay, I will write up a migration guide, which seems the only obstacle to introduce these breaking changes, take the privilege and we take it from there. I will separate the question of developing from immutable artifacts in development containers into a pretalx discussion thread later down the road. There is something to gain from the immutable, container-native approach which I would like to outline in a bit more detail. With write access to the repository, I would then attempt to further automate the build and release procedure. Renaming the repository to |
Of course pretalx/pretalx@a7a8f22, raised from pretalx/pretalx#1773, now incurs the need to produce and run from a source build from Also continuing the support for the If there are other wishes for this reorganisation, please add them here and I will try to include them within the next cycle of activity until say end of the month. |
- adds .env.build.example - adds bin/ directory with maintenance and life-cycle scripts - adds layered build stages - for the base, default, plugin-extended and cron-extended images - for standalone images - for local and remote source builds - adds explicit image contexts per build stage * updates .env.example * moves the previous image built from a submodule into context/source/standalone/Dockerfile.debian.local together with a Compose example at compose/build/source/standalone.local.yml - removes plugins build stage
…ISO date versions
The last two days have brought new image variants, local build scripts, a build pipeline and the proposed migration hints to the README. For now, the pipeline can be triggered manually and by releases. The other push and PR triggers have been removed, until this has been settled a little. The image variants reintroduce the The extended and cron variants from the "stock" image have also been applied to the standalone one for sake of completeness. This means people can potentially remove the external dependency to cron from their existing containerised application setup. There are also new example contexts to build the application from source, of which one mimics the current state of affairs, namely The one interesting for me to be able to run from main, including the fix for pretalx/pretalx#1773, is The README tries to be complete and concise at the same time. Due to the multiple ways to approach the repository, its images, the Compose manifests and also the CI pipeline(s) we cannot be exhaustive here. Maybe the setup is complex enough to consider adding separate documentation to This is now ready for merge with approaching the 50th hour. I would leave the click to another maintainer, as I wouldn't want to misuse the gained write-permission to the repository with my own and first contribution. What's left for future cycles is roughly:
Also new usages for the repository appear within reach:
Ultimately, I will also raise conversations upstream about implementing Celery Beat, which would allow to get rid of the cron dependency entirely, and about a containerised development environment and why one might like that. |
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.
Wow, what a huge PR! First of: sorry that i'm not active here. As the story often goes, i don't use pretalx anymore, so i'm not keeping up here and (as the state of this repo shows) i only maintain this on paper..
Nevertheless i took the time to look at your PR. All in all, i like a lot of your changes. Especially breaking up the container images in multiple ones, going more in the direction of one container, one process, as it should be. I left a few comments, but please, just take them as suggestions, nothing important.
I have two main suggestions:
Simplifying
As nice as having all the different variants is, i fear this might be way to much to maintain in the future. If you're up for the job: awesome! Otherwise i would suggest deleting some of them. E.g. do we need the ability to build from source? I know it would be nice to be able to run pretalx:main
, but since @rixx wants to release more often, anyways, i don't know if its worth the extra trouble.
Readme
I think most people just want a quick, opinionated "how to run this in the simplest way on production" from the readme. Right now this seems kind of buried. Would you be up for writing a short "how to use in production" and put it up top?
I would also suggest moving everything related to building the containers and developing this repo out of the readme, e.g. in a docs
folder.
As is said, please just take these points as suggestions. As these changes go in the right direction overall, i'm not objected to merging as is.
FROM ${BASE_IMAGE}:${BASE_TAG} | ||
|
||
# Install system dependencies | ||
RUN apt update && \ |
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.
the modern way of doing this would be to use multi stage builds. then you can avoid the "install and clean" dance in one giant "RUN" command and copy only the bits over to the next stage that you need
libmemcached-dev \ | ||
locales \ | ||
nodejs \ | ||
npm \ |
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.
where do we need node and npm? in the source variant?
@@ -0,0 +1,145 @@ | |||
# Legacy docker-compose support | |||
|
|||
This example demonstrates a setup that is compatible with the legacy version of Docker Compose. |
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.
Maybe make it more obvious that you mean docker-compose
with the dash?
This example demonstrates a setup that is compatible with the legacy version of Docker Compose. | |
This example demonstrates a setup that is compatible with the legacy version of `docker-compose` v1. |
docker compose -f compose.yml -f compose/local.yml -f compose/traefik.yml config | ||
``` | ||
|
||
### Live |
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.
### Live | |
### Production |
I feel the word "production" is more commonly used, than live
?
|
||
This repository follows the Pretalx installation instructions as closely as possible. | ||
|
||
- [Installation — pretalx documentation](https://docs.pretalx.org/administrator/installation/) |
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.
this seems kind of misleading, since the official docs don't have anything to do with docker at all..
This change request presents a major opinionated rewrite of this repository.
.env
to configure Pretalx as a container-native 12 factor appstatic/
assets andmedia/
plugins
workflow to build apretalx-extended
imagepretalx/pretalx
in CI and Composepretalx.cfg
example to be compatible with the changed setuplinux/arm64
build targetcloses #21
closes #56
closes #59 ?
closes #62