Improve Dockerfile to reduce amount of config needed in docker-compose #723

chrishoage · 2024-11-21T03:32:00Z

Closes #715

This work significantly changes the Dockerfile and the required docker-compose.yaml configuration.

The changes include

Moving to a more idiomatic /config directory using DAGU_HOME. This relies on the resolver.go behavior that places all files together when appHomeEnv is specified.
Introducing an entrypoint.sh file that does the following
- Sets up the docker group if DOCKER_GID is provided
- Updates the dagu user to the provided PUID and PGID if provided
- Tries to guess if the running container is using the previous recommended setup using /home/dagu inside the container - if so it uses it
- Otherwise sets DAGU_HOME=/config if DAGU_HOME is not already set
Runs any scripts in /etc/custom-init.d during boot. This can be used to install things into the container - like python, node, etc
Finally executes dagu using su-exec

In order to test use the updated documentation. To test the "legacy" home behavior use the following compose file

services:
  dagu:
    image: dagu
    container_name: dagu
    hostname: dagu
    ports:
      - "8080:8080"
    environment:
      - DOCKER_GID=970
      - DAGU_BASE_PATH=/dagu
    volumes:
      - ./test-init:/etc/custom-init.d
      - /var/run/docker.sock:/var/run/docker.sock
      - ./test-config:/home/dagu/.config/dagu
      - ./test-data:/home/dagu/.local/share

Where test-config and test-data map to the previous docker volume mounts.

The "guessing" is best effort and it could be fooled, however I tried to make it as robust as I could.

This is a pretty large change, so please feel free to try to test this as thoroughly as possible. I'm happy to make more improvements or changes before merging.

chrishoage

Like I said in the body of the PR - this is a really big change, so it's okay if it takes some time to review it!

chrishoage · 2024-11-21T03:35:11Z

Dockerfile

-# Add the hello_world.yaml file
-COPY --chown=${USER}:${USER} <<EOF .config/dagu/dags/hello_world.yaml
-schedule: "* * * * *"
-steps:
-  - name: hello world
-    command: sh
-    script: |
-      echo "Hello, world!"
-EOF


This could never work as when a user binds over /home/dagu/.config/dagu it will mount over this file at this location.

If there is a desire to have an automatically populated hello-world dag, it would have to happen during runtime on the first boot.

chrishoage · 2024-11-21T03:36:11Z

entrypoint.sh

+  if [ -d /home/dagu/.config/dagu ]; then
+    echo "WARNING: Using legacy /home/dagu directory. Please consider moving to /config"
+    usermod -d /home/dagu dagu
+    chown $PUID:$PGID -R /home/dagu
+  else


This check isn't the best, but it should cover most cases.

We mirror what the "init" part of the original docker-compose stack did by chowning the directory.

chrishoage · 2024-11-21T03:37:07Z

docs/source/docker-compose.rst

+            - DOCKER_GID=999 # optional. default is -1 and it will be ignored
+            volumes:
+            - dagu_config:/config
+            - /var/run/docker.sock:/var/run/docker.sock # optional. required for docker in docker


I wasn't sure if this was worth including here. It maybe the comment should mention the socat alternative in the docs?

Thank you for the great suggestion! I believe this would be a valuable addition to include here. I think mentioning socat would be nice, but I also like the current focus on the docker-compose configuration. So, let's leave it as it is for now.

chrishoage · 2024-11-21T03:37:36Z

Dockerfile

-    yarn install --frozen-lockfile --non-interactive; \
-    yarn build
+  yarn install --frozen-lockfile --non-interactive; \
+  yarn build


My editor kept formatting this - happy to revert if it is undesired.

chrishoage · 2024-11-21T03:38:57Z

Dockerfile

-    apk add --no-cache sudo tzdata; \
-    addgroup -g ${USER_GID} ${USER}; \
-    adduser ${USER} -h /home/${USER} -u ${USER_UID} -G ${USER} -D -s /bin/ash; \
-    echo ${USER} ALL=\(root\) NOPASSWD:ALL > /etc/sudoers.d/${USER}; \
-    chmod 0440 /etc/sudoers.d/${USER}; \
-    mkdir -p .config/dagu/dags; \
-    chown -R ${USER}:${USER} /home/${USER};
-
 COPY --from=go-builder /app/bin/dagu /usr/local/bin/
+COPY ./entrypoint.sh /entrypoint.sh
+
+# Create user and set permissions
+RUN apk update && \
+  apk add --no-cache su-exec shadow tzdata && \
+  addgroup -g ${USER_GID} ${USER} && \
+  adduser ${USER} -h /config -u ${USER_UID} -G ${USER} -D -s /bin/ash && \
+  chown -R ${USER}:${USER} /config && \
+  chmod +x /entrypoint.sh;


Instead of sudo we rely on su-exec and allowing the user to run scripts in /etc/custom-init.d

Ideally sudo would not be added to docker containers, since it is easy to get root if you need it anyway (through docker exec)

yohamta

Everything looks great! Thank you so much for improving the Dockerfile, docker-compose.yaml, and the documentation! These look much cleaner now :)

codecov · 2024-11-21T16:14:48Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.67%. Comparing base (10b0c69) to head (0f6cf6e).
Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #723   +/-   ##
=======================================
  Coverage   61.67%   61.67%           
=======================================
  Files          53       53           
  Lines        5260     5260           
=======================================
  Hits         3244     3244           
  Misses       1762     1762           
  Partials      254      254

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10b0c69...0f6cf6e. Read the comment docs.

---- 🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

chrishoage force-pushed the hoage/docker-improved branch from 1b23277 to 523016b Compare November 21, 2024 03:32

Improve Dockerfile to reduce amount of config needed in docker-compose

0f6cf6e

chrishoage force-pushed the hoage/docker-improved branch from 523016b to 0f6cf6e Compare November 21, 2024 03:33

chrishoage commented Nov 21, 2024

View reviewed changes

yohamta approved these changes Nov 21, 2024

View reviewed changes

yohamta merged commit e66978d into dagu-org:main Nov 21, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Dockerfile to reduce amount of config needed in docker-compose #723

Improve Dockerfile to reduce amount of config needed in docker-compose #723

chrishoage commented Nov 21, 2024

chrishoage left a comment

chrishoage Nov 21, 2024

chrishoage Nov 21, 2024

chrishoage Nov 21, 2024

yohamta Nov 21, 2024

chrishoage Nov 21, 2024

chrishoage Nov 21, 2024

yohamta left a comment

codecov bot commented Nov 21, 2024

Improve Dockerfile to reduce amount of config needed in docker-compose #723

Improve Dockerfile to reduce amount of config needed in docker-compose #723

Conversation

chrishoage commented Nov 21, 2024

chrishoage left a comment

Choose a reason for hiding this comment

chrishoage Nov 21, 2024

Choose a reason for hiding this comment

chrishoage Nov 21, 2024

Choose a reason for hiding this comment

chrishoage Nov 21, 2024

Choose a reason for hiding this comment

yohamta Nov 21, 2024

Choose a reason for hiding this comment

chrishoage Nov 21, 2024

Choose a reason for hiding this comment

chrishoage Nov 21, 2024

Choose a reason for hiding this comment

yohamta left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 21, 2024

Codecov Report