mbentley / docker-timemachine

Docker image to run Samba (compatible Time Machine for macOS)
Apache License 2.0
527 stars 65 forks source link

[Bug]: Why does the Dockerfile.smb define volumes #149

Closed Alex1s closed 7 months ago

Alex1s commented 8 months ago

Describe the Bug

Dockerfile.smb creates threee volumes: /var/lib/samba, /var/cache/samba and /run/samba

Expected Behavior

Do not create these volumes.

Steps to Reproduce

Build the Dockerfile.smb

How You're Launching the Container

n/a

Container Logs

n/a

Additional Context

I am not an expert with samba, but I don't see why it is needed to create these volumes, i.e. why should one want to persist the data in these folders.

/run/samba sounds like it should be backed by a tmpfs as /run is usually a tmpfs on linux. /var/cache/samba sounds like it should potentially be a tmpfs to increase the cache performance /var/lib/samba I have no idea why this data should be persisted or backed by any

To follow Docker best practises to keep images as stateless as possible I would suggest to not create these Volumes in the Dockerfile.smb.

mbentley commented 8 months ago

I originally created the Dockerfile for the smb variant back in 2019 so it's been a while since any of this was done so there are likely some valid points to these. I imagine all I was doing back in 2019 was making sure that any paths where the container was actively writing data to the copy on write filesystem was in a volume of sorts.

Regardless of whether these are bind mounted, docker named volumes, unnamed volumes, tmpfs, or just left to be written to the copy on write filesystem, it's not really impacting the statelessness of the container - the persistent data of the time machine backup is doing that on it's own.

Now, I would need to test in an environment to see how these behave if they are configured any differently as the last thing I would want to do is lose someone's time machine backup due to a configuration change that wasn't tested properly. A basic test that should function pretty well would be to just map each of the volume paths as tmpfs filesystems. If the data doesn't matter, it will not be impacted by the two main instances where data loss would happen - container restarts or container re-creation. I suspect there would be issues with /var/lib/samba though if it were tmpfs so that would either just need to stay a volume or just let it be written to the copy on write filesystem.

mbentley commented 7 months ago

Now that I think about it, it is possible that keeping cache files around from a previous version of a container can cause problems with the ability of time machine to work as people have reported issues getting the time machine to work after the container is re-created, like #133 among others. I am testing the changes from https://github.com/mbentley/docker-timemachine/pull/155 on my own time machine setup right now.

mbentley commented 7 months ago

Merged in the changes - seems to be working well here.