bcardiff / docker-rclone

Docker image to use rclone to run cron sync with monitoring
https://hub.docker.com/r/bcardiff/rclone
MIT License
79 stars 62 forks source link

Add Healthchecks.io start ping before sync #10

Closed felixbouleau closed 1 year ago

felixbouleau commented 5 years ago

Adds a /start ping to Healthchecks.io to help debug stuck jobs and measure execution time.

I've tested this both with and without $CHECK_URL set and it worked fine. Hopefully it can be useful for someone else.

Thanks @bcardiff for the great image!

bcardiff commented 5 years ago

Thanks @felixbouleau, in order to keep users unlocked from healthchecks.io, WDYT about

  1. using a START_URL argument
  2. setting START_URL to CHECK_URL/start by default in the entrypoint (if START_URL is not set and CHECK_URL it is set)

something like:

  if [ -z ${START_URL+x} ]
  then 
    if [ -z ${CHECK_URL+x} ]
    then;
    else
      START_URL="${CHECK_URL}/start"
    fi
  fi

why -z ${var+x}?

  1. use the same code for CHECK_URL and START_URL. (I'm not sure if checking with -n is better than -z here)
  if [ -z "$START_URL" ]
  then
    wget $START_URL -O /dev/null
  fi
  1. Add a description in the README

?

The behavior should be the same and the user can opt-out the START_URL or define it's own.

FYI #7 attempt to implement FAIL_URL, I need to do another round and merge that functionality. But is related and you might be interested.

felixbouleau commented 5 years ago

@bcardiff thanks for the quick feedback!

Since the goal was to be agnostic towards ping providers I've taken a slightly different approach than #7 for defaults. Here's a shortcut to the logic.

This way, if you use something else than HC you don't need to override START_URL with something else.

I'm happy to switch over to the #7 approach if you prefer it - just let me know. It's a super easy change. :)

bcardiff commented 5 years ago

It's fine as is. Thanks!

The duplicated wget bugs me a little. But is fine.

Bash is always cryptic to me.

I'm not asking for additional changes, but highlighting the difference with the other checks for comparison and eventual unification.

felixbouleau commented 5 years ago

@bcardiff Dropping the -x was a knee jerk fix for a bug I introduced. I solved it in a better way now. Basically I included START_URL as an ENV in the Dockerfile which sets it to an empty string. So the [ -z ${VAR+x} ] evaluated to x and broke the if statement. Instead of dropping the -x I dropped the ENV in the Dockerfile.