borgmatic-collective / docker-borgmatic

Borgmatic in Docker
GNU General Public License v3.0
347 stars 92 forks source link

Replace VOLUME statements #83

Closed hr87 closed 3 years ago

hr87 commented 3 years ago

My setup uses a borgmatic container in each stack, taking care of backing up the data for only that stack. Some stack only require a database to be backed up, so I don't need to mount data volumes for borgmatic. The VOLUME statement creates anonymous volumes for each borgmatic container.

According to this , the VOLUME statements are not necessary to be able to mount a folders/volumes in the container. However, they create unnamed volumes. So in my case, running several borgmatic images, I have many unused anonymous volumes created by borgmatic.

I suggest replacing them with mkdir commands. This ensures the locations are available, but does not create unnecessary volumes. Mounting volumes from the host still works as before. The only difference is, in a case where no explicit volume has been mounted, the data is lost after the container stops. But since in the compose file all of these are explicitly mounted, this is not a problem.

hr87 commented 3 years ago

I can do a patch, if you agree with this

grantbevis commented 3 years ago

I agree, I believe the original thought process was creating the volumes within the container would stop people losing data but if there's a better reason to remove then lets remove them.

hr87 commented 2 years ago

Is this patched. Or do you still want a patch?

grantbevis commented 2 years ago

Is this patched. Or do you still want a patch?

No it's not patched and I'm not sure I agree. As I said before I put the volumes in place so it stops someone losing data if they mistype volume mapping and other eventualities. Creating empty docker volumes isn't the end of the world.

MarkJonas commented 2 years ago

Would it be a compromise to use a named volume?

version: '3'
services:
  testomat:
    image: ubuntu:focal
    container_name: testomat
    volumes:
      - testvol:/mnt/testvol:ro

volumes:
  testvol:

In that case if the volume is passed with a local path that will be used. If not, a named volume will be created. So no data is lost and yet there are no unnamed, hard to identify volumes flying around.

# result when docker-compose.yaml is in directory testomat
$ docker volume ls
DRIVER    VOLUME NAME
local     testomat_testvol

I am using that technique in https://gitlab.com/toertel/docker-image-logitech-media-server .

grantbevis commented 2 years ago

That seems reasonable, I won't have time to implement this soon but happy to review a PR

rcdailey commented 1 year ago

This should certainly be reopened. I just observed about 50 orphaned volumes with no legible names because of this.

My immediate concern is the deprecated volumes, such as /root/.borgmatic, which still have a VOLUME directive in the Dockerfile. Those should certainly be removed, as the user is already instructed to not map those.