borgmatic-collective / docker-borgmatic

Borgmatic in Docker
GNU General Public License v3.0
313 stars 88 forks source link

Move to S6 #303

Closed modem7 closed 3 months ago

modem7 commented 4 months ago

This is a WIP PR to move us to S6 and simplify + make the image more resilient.

Environment Variables

Variable Description Possible Values Default
DOCKERCLI Install DockerCLI true Empty
CRON Cron times cron time/false 0 1 *
CRON_COMMAND Command cron will run borgmatic --stats -v 0 2>&1 borgmatic --stats -v 0 2>&1
EXTRA_CRON Extra cron lines 0 5 2 command1 Empty

Once merged, this PR will: close #233 close #242 close #272 close #301

Psycho0verload commented 4 months ago

I will look at it in the coming days

grantbevis commented 4 months ago

Will wait for @Psycho0verload 's review too

modem7 commented 4 months ago

Will wait for @Psycho0verload 's review too

As soon as @Psycho0verload gives his blessing that we haven't broken his work (once we modify the entrypoint script to match), we'll get it merged and close the linked PR's/issues.

modem7 commented 3 months ago

The conflict can be easily resolved.

Once everything has been done and is ready to merge, I'll squash the commits, so they're neater in the history unless anyone has an issue with that.

@Psycho0verload - did you manage to get some time to have a look at the /root/etc/s6-overlay/s6-rc.d/svc-cron/run file?

Psycho0verload commented 3 months ago

For your information: I was able to implement my script and it works as far as I could test it. What I came across in the documentation:

YOUR_PASSPHRASE

I would extend my script for this and the push to the branch will follow today.

Psycho0verload commented 3 months ago

@modem7 added a PR on your Fork.

modem7 commented 3 months ago

I have merged @Psycho0verload's PR (thank you for that), and have resolved the merge conflict.

Ready for review and final comments prior to merging.

@grantbevis @witten

modem7 commented 3 months ago

Thank you for testing, @Psycho0verload.

Merge conflicts resolved. @witten @grantbevis - We are ready to merge.

rcdailey commented 2 months ago

Hey folks, I think it might be helpful to announce that if people were previously using init: true in their docker compose files, they should remove that, otherwise you get this error (not surprising, since the container is using overlay now):

s6-overlay-suexec: fatal: can only run as pid 1

I used the init setting because before overlay was introduced, there was nothing to properly handle interrupt signals in the container.

modem7 commented 2 months ago

Hey folks, I think it might be helpful to announce that if people were previously using init: true in their docker compose files, they should remove that, otherwise you get this error (not surprising, since the container is using overlay now):

s6-overlay-suexec: fatal: can only run as pid 1

I used the init setting because before overlay was introduced, there was nothing to properly handle interrupt signals in the container.

Heya,

The readme needs updating now that we've moved to S6 regardless.

init has never really been recommended by us, nor does it work well with Borgmatic (as can be seen in #173). If people choose to use init, that's at their own discretion, but once we update the readme, we'll make sure to mention that it has S6 in, and elaborate a couple of paragraphs on what S6 does so that people are deterred from using init package.

I'd be happy to review any blurb you think may be suitable to be added to the readme so that it reduces or eliminates any confusion.

Cheers!