Skip to content
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

Dockerfile: Remove VOLUME instruction #3683

Open
3 of 5 tasks
polarathene opened this issue Dec 27, 2023 · 0 comments
Open
3 of 5 tasks

Dockerfile: Remove VOLUME instruction #3683

polarathene opened this issue Dec 27, 2023 · 0 comments
Labels
feat New feature or request.

Comments

@polarathene
Copy link

polarathene commented Dec 27, 2023

Preflight checklist

Describe your problem

Some of the Dockerfile files in this repo use the VOLUME instruction: https://github.com/ory/hydra/tree/master/.docker

Dockerfile references

VOLUME /var/lib/sqlite

# By creating the sqlite folder as the ory user, the mounted volume will be owned by ory:ory, which
# is required for read/write of SQLite.
RUN mkdir -p /var/lib/sqlite && \
chown ory:ory /var/lib/sqlite
VOLUME /var/lib/sqlite
# Exposing the ory home directory
VOLUME /home/ory

# By creating the sqlite folder as the ory user, the mounted volume will be owned by ory:ory, which
# is required for read/write of SQLite.
RUN mkdir -p /var/lib/sqlite && \
chown ory:ory /var/lib/sqlite
VOLUME /var/lib/sqlite
# Exposing the ory home directory
VOLUME /home/ory

PR that introduced VOLUME: #2129


VOLUME is not necessary:

  • A container will persist it's internal state until it's destroyed (docker rm container_name, docker run --rm, docker compose down, etc). An anonymous volume is often mistakenly added despite being redundant.
  • An implicit anonymous volume copies data from the image to the host per container instance created. This is wasteful and accumulates over runs.
  • When persistence via a volume is actually necessary, it should be explicit (docker run --volume ./host/path/:container/path, or the volume key in compose.yaml, etc).
    • Anonymous (-v container/path):

      I want to persist the volume without a human-friendly name to reference

    • Named (-v my_data:container/path):

      I want a human-friendly name, but not easy access from the host filesystem, just persist my data externally from the container

    • Bind mount (-v host/path:container/path):

      I want to persist the data at a known location on the host filesystem that I can easily access directly

I also provided justification in 2022 for Caddy to do the same, citing various sources from other popular official images that likewise dropped VOLUME (which is effectively a legacy feature that causes more problems implicitly than benefits).

Describe your ideal solution

Remove VOLUME from the Dockerfile lines referenced (this would be applicable to other Ory projects too).

Workarounds or alternatives

  • The redundant copy can be avoided.. if the user provides their own bind mount to the same mount point at runtime (anonymous & named volumes copy container content by default, bind mounts replace).
  • Alternatively, the --rm option will remove the container on exit, additionally discarding the implicitly created anonymous volume. This doesn't prevent writing a copy of the volume data to disk, which if large slows startup.

Ideally though, without a real reason to keep VOLUME, it should just be removed from the Dockerfiles? 🤷‍♂️

Version

2.1.2

@polarathene polarathene added the feat New feature or request. label Dec 27, 2023
@polarathene polarathene changed the title Remove VOLUME instruction from Dockerfile Dockerfile: Remove VOLUME instruction Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request.
Projects
None yet
Development

No branches or pull requests

1 participant