kellnerd / harmony

Music Metadata Aggregator and MusicBrainz Importer
MIT License
22 stars 6 forks source link

feat(docker): Add option to run via Docker Compose #40

Open julian45 opened 2 weeks ago

julian45 commented 2 weeks ago

I know this isn't necessarily a needed option at this point, but I thought I'd try my hand at Dockerizing this project in the interest of making it able to run on more platforms. Open to any questions, suggestions, or concerns you might have!

The Dockerfile and Compose spec included in this draft are relatively basic at this point; for one thing, the choice of version to run currently has to be manually specified in the Dockerfile. However, if further development is desired, it may be possible to work around this while developing a more formal build workflow through GitHub Actions or the like.

atj commented 2 weeks ago

You should add volumes for persistent data:

Otherwise the image will just lose all state when it is restarted.

phw commented 2 weeks ago

Something like this would work cleanly for the environment variables:

services:
  web:
    build:
      context: .
    container_name: harmony-container
    image: harmony
    ports:
      - "8000:8000"
    volumes:
      - ./snaps/:/app/snaps/:rw
      - ./snaps.db:/app/snap.db:rw
    environment:
      HARMONY_CODE_URL: ${HARMONY_CODE_URL}
      HARMONY_SUPPORT_URL: ${HARMONY_SUPPORT_URL}
      MUSICBRAINZ_URL: ${MUSICBRAINZ_URL}
      DENO_DEPLOYMENT_ID: ${DENO_DEPLOYMENT_ID}
      FORWARD_PROTO: ${FORWARD_PROTO}
      HARMONY_SPOTIFY_CLIENT_ID: ${HARMONY_SPOTIFY_CLIENT_ID}
      HARMONY_SPOTIFY_CLIENT_SECRET: ${HARMONY_SPOTIFY_CLIENT_SECRET}
      HARMONY_TIDAL_CLIENT_ID: ${HARMONY_TIDAL_CLIENT_ID}
      HARMONY_TIDAL_CLIENT_SECRET: ${HARMONY_TIDAL_CLIENT_SECRET}

This would work as docker compose also reads the .env file and makes the variables available.

Side note: While the above is a little verbose I would very much advise this over e.g. using something like env_file: .env. Explicitly listing the variables to be passed in allows you to have fine control over which variables are used, instead of just everything. Probably not so much of a problem here, but definitely something I see done wrong in many compose files involving multiple containers.

mwiencek commented 2 weeks ago

I don't really see the point of using Docker here, after all Deno is a single, self-contained binary which only depends on its cache directory and, in case of this project, the contents of this repository's checked out directory. But I also don't have a lot of experience with Docker, so I may be missing some other benefits.

One advantage is that it can allow you to run a different (stable) version of Deno inside the container than your system has installed. (Or you don't need to install Deno at all if you don't want to.) It may only be a single binary, but I still install it through my system package manager (which may provide an older version than I want) and would rather not manage multiple binaries.

julian45 commented 2 weeks ago

Hi all,

Thanks for the discussion and contributions so far! Unfortunately, I'm more than a bit busy today and this weekend, but will work on responding to all of this early next week.

julian45 commented 1 week ago

Apologies for not meeting my own timeline of "early next week"... here I am a full week later. 😅

Part of my initial motivation with putting this together was to enable deployment of Harmony in environments where one does not necessarily have access to an underlying operating system/dev environment. With a built container (e.g., published through GitHub Packages), it becomes substantially easier to run things like Harmony on platforms that simply offer Docker container hosting. In addition, as @mwiencek indicated, having a Docker container obviates the need to have Deno installed wherever Harmony is being run. (e.g., personally I didn't have Deno installed on my computer before beginning to work with Harmony) Finally, there's a bit of a security benefit by running this application in an isolated environment from its host, especially in production environments. For example, I know someone at my day job who tends not to install software directly onto his work computer whenever possible, and instead prefers to run things as VMs or containers.

In any case, I'm going to go through the discussion in this PR and address things as appropriate. Thanks again for all of your input so far!

julian45 commented 1 week ago
    volumes:
      - ./snaps/:/app/snaps/:rw
      - ./snaps.db:/app/snap.db:rw

Unfortunately, I'm having a little trouble with this portion. Regardless of whether I put it into my docker-compose.yml as is or edit the second volume to mount as /app/snaps.db (with a final S in snaps), I get an error like this upon running docker compose up while trying to test the .env-based versioning process:

[+] Running 1/0
 ✔ Container harmony  Recreated                                            0.1s
Attaching to harmony
harmony  | error: Uncaught (in promise) Error: Is a directory (os error 21): open 'snaps.db'
harmony  |       const rid = Deno.openSync(path, { read: true, write, create }).rid;
harmony  |                        ^
harmony  |     at Object.openSync (ext:deno_fs/30_fs.js:620:15)
harmony  |     at js_open (https://deno.land/x/sqlite@v3.8/build/vfs.js:29:24)
harmony  |     at <anonymous> (wasm://wasm/0028679a:1:5566)
harmony  |     at <anonymous> (wasm://wasm/0028679a:1:145428)
harmony  |     at <anonymous> (wasm://wasm/0028679a:1:141310)
harmony  |     at <anonymous> (wasm://wasm/0028679a:1:147518)
harmony  |     at <anonymous> (wasm://wasm/0028679a:1:3869)
harmony  |     at <anonymous> (wasm://wasm/0028679a:1:611986)
harmony  |     at https://deno.land/x/sqlite@v3.8/src/db.ts:208:27
harmony  |     at setStr (https://deno.land/x/sqlite@v3.8/src/wasm.ts:19:20)
harmony exited with code 1

Here are the only differences in my working directory besides adding a .env with DENO_DEPLOYMENT_ID specified:

diff --git a/Dockerfile b/Dockerfile
index ee48b9e..00de358 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -8,7 +8,5 @@ ADD . /app

 RUN deno cache server/main.ts

-ARG version
-ENV DENO_DEPLOYMENT_ID $version
 USER deno:deno
 CMD deno run -A server/main.ts
diff --git a/docker-compose.yml b/docker-compose.yml
index b800f65..91ddd3a 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -2,9 +2,12 @@ services:
   web:
     build:
       context: .
-      args:
-        version: v2024.6.12
     container_name: harmony
     image: harmony
     ports:
       - "8000:8000"
+    volumes:
+      - ./snaps/:/app/snaps/:rw
+      - ./snaps.db:/app/snaps.db:rw
+    environment:
+      DENO_DEPLOYMENT_ID: ${DENO_DEPLOYMENT_ID}

@phw @atj any suggestions for how to properly handle persisting that file and making it available to the container? I've done some looking around, and I also tried manually touching snaps.db locally before trying to bring up the container, but that didn't do the trick.

atj commented 3 days ago

I think you need to specify a bind mount volume type, e.g.

volumes:
 - type: bind
   source: ./snaps.db
   target: /app/snaps.db

See this Stack Overflow post:

https://stackoverflow.com/questions/42248198/how-to-mount-a-single-file-in-a-volume