-
-
Notifications
You must be signed in to change notification settings - Fork 322
Implement Docker container build and publish actions #1086
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
base: master
Are you sure you want to change the base?
Implement Docker container build and publish actions #1086
Conversation
Update docker-compose file to use GHA image output
…resource compilation
I tried building from the |
Used |
Can you let me know how I can trigger a github action to test it? @d-mcknight |
Sorry I missed this earlier. Actions can only be run in a repository if they are defined on the default branch. For testing, I usually fork the repository and change the default branch on the fork. You can see runs on my fork (let me know if you need additional permissions to see those for some reason). |
No problem. sorry for the delay on my end. Your PR looks quite clean at the first glance. I'll try to find time to test it later today. |
@d-mcknight this action succeeded: https://github.com/TheSpaghettiDetective/obico-server/actions/runs/16253425958 Where can I see the uploaded docker images. Sorry, I am not very familiar with how GitHub workflow works. |
|
||
# Implementation from https://github.com/imagegenius/docker-obico | ||
[ -d /data/media ] || mkdir /data/media | ||
[ -d /app/static_build/media ] && rm -r /app/static_build/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.
Remove everything here every time the server starts?
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.
Yes, I updated the compose file to mount the /data
directory so that a user can specify where on their host system to save recordings and to keep user data separated from code.
The line after this creates a symlink to /data
which is where I also put the sqlite db by default
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.
Not sure if I'm following. My question is if everything in /app/static_build/media (which I believe will include timelapses etc) will be erased every time run.sh
runs.
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.
Currently, the static_build
files are built on container start; I just kept the existing logic to minimize changes. With this PR, I moved the timelapses to /data/media
which is linked to /app/static_build/media
on line 14; the directory is removed on line 13 so that the link may be created.
I made this change so that /app
doesn't need to be mounted on the host FS. The files will not be erased, because they are mounted to the container at /data/media
.
Related, I might be able to simplify the container startup and drop some of this logic if python manage.py collectstatic
can be run before python manage.py migrate
, but I don't know enough about what those processes are doing to say if that would work.
They're listed here; from the repository page on GitHub, there's a "Packages" section in the right column |
Do you think that there's a way to structure your code so that it's obvious that the current way of working is not touched at all, and the Docker image is only a new way of running the server? I really don't want to deal with a ton of questions like "I just pulled the update and now my server is bricked". For instance, our cloud is some extra on top of the existing Dockerfile + docker-compose.yml. Now I launched your PR into our testing env, and it seems that it's now trying to connect to a different db than the one specified by DATABASE_URL env var. |
I'll have to think about this some, I agree though that this shouldn't be a breaking change. I see a couple options:
Option 3 is probably the best if the goal is to maintain backwards-compat with current workflows.
This one is odd, I changed the default value in |
Option 3 sounds like the best one to me too. Also I believe (hope) our cloud won't be affected if we go with option 3. Thanks again for the contribution to this project! |
Changes
This adds support for building and publishing containers
ml_api
andobico_web
which are equivalent to the containers previously built at runtime withdocker-compose.yml
.I added a
run.sh
entrypoint toobico_web
so that the container could be run outside of thedocker-compose.yml
context by setting a newOBICO_CONTAINER
envvar while maintaining backwards-compat. with the existing method/documentation.I also added a symlink and changed the default sqlite database path to use
/data
within the container and added a volume mount; this way only one volume is needed and the compiled web app remains in the container and not a mounted volume.I tested a clean setup and confirmed I was able to change the password, with that change persisting between container restarts. I also mounted media and the sqlite db from an existing installation I have and confirmed it loaded.
Issues
Closes #1052
Closes #1055
NOTE: When I access the application in a browser, I get an empty page; however the mobile app renders correctly with my printer and history including images. This is not reproducible with images built from the
release
branch.TODO
docker-compose.yml
image referencesdocker-compose.yml