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

Mark /opt/jboss/wildfly/standalone/{data,tmp} as data volumes #22

Open
pszemus opened this issue Jul 28, 2015 · 5 comments
Open

Mark /opt/jboss/wildfly/standalone/{data,tmp} as data volumes #22

pszemus opened this issue Jul 28, 2015 · 5 comments

Comments

@pszemus
Copy link

pszemus commented Jul 28, 2015

Wouldn't it be a good idea to mark /opt/jboss/wildfly/standalone/{data,tmp} as data volumes (1) ? They would bypass the Union File System causing a direct write, smaller diffs (2) and smaller container dir size.

(1) https://docs.docker.com/reference/builder/#volume
(2):

$ docker diff 0a
...
C /opt/jboss/wildfly/standalone/tmp
A /opt/jboss/wildfly/standalone/tmp/vfs
A /opt/jboss/wildfly/standalone/tmp/vfs/deployment
A /opt/jboss/wildfly/standalone/tmp/vfs/deployment/deploymentfe48288eca471f39
A /opt/jboss/wildfly/standalone/tmp/vfs/deployment/deploymentfe48288eca471f39/s3-1.8.0.jar-2878891b64f5f965
A /opt/jboss/wildfly/standalone/tmp/vfs/deployment/deploymentfe48288eca471f39/s3-1.8.0.jar-2878891b64f5f965/contents
A /opt/jboss/wildfly/standalone/tmp/vfs/deployment/deploymentfe48288eca471f39/s3-1.8.0.jar-2878891b64f5f965/s3-1.8.0.jar
A /opt/jboss/wildfly/standalone/tmp/vfs/deployment/deploymentfe48288eca471f39/sts-1.8.0.jar-11bf88e6628dc597
A /opt/jboss/wildfly/standalone/tmp/vfs/deployment/deploymentfe48288eca471f39/sts-1.8.0.jar-11bf88e6628dc597/contents
A /opt/jboss/wildfly/standalone/tmp/vfs/deployment/deploymentfe48288eca471f39/sts-1.8.0.jar-11bf88e6628dc597/sts-1.8.0.jar
....
@goldmann
Copy link
Contributor

goldmann commented Aug 4, 2015

Yes, that's an idea. But everyone needs to be aware that if change something like this - these files will be stored on the disk always, even when the container was already removed. It's kinda sad that there is no easy way to list/remove these files.

@rwngwn
Copy link

rwngwn commented Aug 4, 2015

Personally, I don't like that idea.

  1. With volumes you will have to introduce some mechanism to remove that files from host (eap sucks at cleaning its tmp dir - especially when killed by SIGKILL :) )
  2. These data are related to container and should just be removed when containers dies.
  3. I think that spawning image to container should be considered "pure function" so have as low observable side effects ad possible - so volumes for me are just often miss-used.
  4. If you really need that you can just base on our image and add volumes, or just mount volumes in run-time with -v of docker run switch (your orchestration tool should be able to do that)
  5. I don't know what is UnionFS issue but if I use btrfs storage driver i cant see difference, and i think that image shouldn't introduce any storage driver specific features without very strong reasoning

@smarterclayton
Copy link

I was going to ask to add the volume dir because then OpenShift will automatically provision empty volumes through Kube when the image runs. It is a shame Docker can't/doesn't properly manage the volumes.

@goldmann
Copy link
Contributor

This: moby/moby#14242 may be relevant to the discussion too.

@lucasbasquerotto
Copy link

lucasbasquerotto commented Jan 24, 2022

This issue is old but is still opened, tough I consider that it can be closed (together with the PR #52), because defining volumes in the base image could break things for the consumers (and there is no way to turn back that change in the child images).

If someone wants to add a volume, they can do so by specifying the volume when running the container.

As for the reasons against VOLUME in the base image, I will quote what I wrote in the issue #39:

Regarding introducing VOLUME in the Dockerfile, I'm not entirely clear if you are just asking a question about declaring the volumes in your images, or declaring it in the base image.

If it's to declare VOLUME in the base image (generated by this repository), it would be extremely bad because it would force everyone to use it. It could potentially create a lot of anonymous volumes without the user knowing it, and it could also break things because it's not clear for the user that a volume is created (unless the user inspects the container).

I had already a good deal of trouble because of base images with volumes.

In your example, VOLUME /opt/jboss/wildfly/standalone would already break the image for us, because we include the WAR in the deployments folder in our CI environment, providing the same base image for our staging and production environments (the environment specific configuration ir read from an external file, present in the environment). If made into a VOLUME, everything included at that path will be discarded (not included in the resulting image).

Even if the path is defined in a location that does not affect us directly, it could still cause problems about creating lots of anonymous volumes (unless we define it as a non-anonymous volume beforehand, which would make the declaration in the base image unnecessary). It could also make the container behave different than expected (for example, if I run the container without defining volumes explicitly, I would expect that removing and recreating it would clear the state, but anonymous volumes would break it, making bugs and and the cause for wrong behaviour of the application harder to spot.

Just to be clear, I'm not against using volumes in those paths. If someone wants volumes, they can easily add them in their docker run or docker-compose files, or in their k8s configuration. Some documentation about best practices when running the image, with instructions about using volumes in thos paths would be ok too.

But, please, don't include VOLUME in the base image.

Update

I included this post here mainly for reference (regarding including VOLUME in the base image), after all, this issue is more than 5 years old, but I considered good to include it here because the issue is still open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants