Allow running periodic updates with enabled HTTP API

[DasSkelett]

Edit by @simskij: closes #897

Motivation

I'd like to run Watchtower both with infrequent periodic updates for images I don't build myself, and have the option to quickly redeploy custom images when they're rebuilt using the HTTP API.
Currently this is not possible, because enabling the HTTP Update API disables scheduled runs.

In theory this would also be possible using multiple Watchtower instances with two scopes, one for "public" images and one for self-built images; however now you need to deal with labels and make sure every container has the correct scope label, which adds unwanted complexity to the setup.
Alternatively you could set up a Cron job to curl /v1/update periodically, but this also gets ugly fast given that we're in Docker, and it also splits the "configuration" of Watchtower into two locations. Also the primary purpose for Watchtower is to replace hacky cron jobs to update Docker containers.

Instead, it would be very handy if Watchtower allows running periodic updates and the HTTP API for updates at the same time.

Changes

Add a new --http-api-periodic-polls option with the WATCHTOWER_HTTP_API_PERIODIC_POLLS environment variable, defaulting to false.
It has only an effect if the HTTP API mode (--http-api-update) is enabled.
If --http-api-periodic-polls is passed, the HTTP API doesn't block, and thus allows periodic polls as well (with whatever --interval or --schedule is given, defaulting to every 24 hours).

I'm open for suggestions to rename this option if you've got something better in mind. I'm not entirely happy with the word "periodic" since it isn't used anywhere else so far. I've considered "scheduled", but it might mislead to think it only affects the --schedule option and not --interval.

The HTTP API and runUpgradesOnSchedule() had separate locks to prevent multiple updates running at the same time.
Since there's the possibility that the schedule triggers while an HTTP API poll is running, or the other way around, I needed to add a global lock. The channel is passed to runUpgradesOnSchedule() and api.update.New(), which now use this channel for locking instead of making their own.
Both functions still create a new channel if the argument is nil, which might simplify future unit testing or other changes.

I've also extended the docs accordingly. If I should clarify or reword something, please do tell.

Further considerations

I also thought about enabling this behaviour by default (inverting the option and renaming it to--http-api-only), since this might be what most people expect, but this could be a "breaking" change, since it differs from the current behaviour.
But if you say it's worth it, I would be happy to do that.

[codecov]

Codecov Report

Merging #916 (ff81a7e) into main (b4cf17d) will increase coverage by 13.59%.
The diff coverage is 100.00%.

❗ Current head ff81a7e differs from pull request most recent head dda9eeb. Consider uploading reports for the commit dda9eeb to get more accurate results

@@             Coverage Diff             @@
##             main     #916       +/-   ##
===========================================
+ Coverage   40.14%   53.73%   +13.59%     
===========================================
  Files          25       25               
  Lines        1415     1407        -8     
===========================================
+ Hits          568      756      +188     
+ Misses        785      585      -200     
- Partials       62       66        +4     

Impacted Files	Coverage Δ
internal/flags/flags.go	88.32% <100.00%> (+36.74%)	⬆️
pkg/registry/registry.go	0.00% <0.00%> (-22.73%)	⬇️
pkg/registry/digest/digest.go	0.00% <0.00%> (ø)
pkg/container/client.go	12.77% <0.00%> (+12.77%)	⬆️
pkg/container/container.go	36.70% <0.00%> (+36.70%)	⬆️
pkg/container/metadata.go	63.63% <0.00%> (+63.63%)	⬆️
pkg/container/util.go	100.00% <0.00%> (+100.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d38e52b...dda9eeb. Read the comment docs.

[DasSkelett]

I just noticed that the locks are all local and not shared between HTTP API and the scheduler, so I'll take a stab at trying to create a global lock we can pass to both to avoid running scheduled and API-triggered polls at the same time.

Also looking at the logs of the ubuntu-latest test I'm unsure why the test fails and if I caused it. A race between creating the server and feeding the metrics?

[simskij]

Also looking at the logs of the ubuntu-latest test I'm unsure why the test fails. [...] A race between creating the server and feeding the metrics?

Yeah. This is fixed in feat/head-failure-toggle, however, we still need to improve the code coverage of that branch a bit to be able to merge it with main. Great contribution! I still haven't reviewed the code, but the concept definitely makes sense!

[piksel]

Just a thought, wouldn't manually specifying a schedule or interval warrant the enabling of the periodic updates as well?
It would be breaking behaviour, but it seems like a more obvious way to inerpret the command line/environment args.

It should probably also write a startup message when running in "manual" mode, with schedMessage being something like:
Waiting for manual dispatch, but that is probably outside the scope of this PR...

[simskij]

Just a thought, wouldn't manually specifying a schedule or interval warrant the enabling of the periodic updates as well?
It would be breaking behaviour, but it seems like a more obvious way to inerpret the command line/environment args.

It should probably also write a startup message when running in "manual" mode, with schedMessage being something like:
Waiting for manual dispatch, but that is probably outside the scope of this PR...

Your thought is definitely valid and warranted. I'll go ahead and fork it off into a separate issue though.

[simskij]

Agree that this should have tests, but lets get this into the code base. @DasSkelett, if you've got time to add some tests, that would be greatly appreciated, but we can keep that as a separate PR in that case.

Thank you for your great contribution! 🎉

[simskij]

@all-contributors add @DasSkelett for code

[allcontributors]

@simskij

I've put up a pull request to add @DasSkelett! 🎉

[DasSkelett]

Testing Run() for the correct behaviour won't be that easy, since there is nothing set up for root.go currently, and I'm also not sure how I could look at what's happening inside, but I'll have a think and see if I can come up with something. Maybe the test framework offers some tricks.

But I could copy the relevant parts from Run(), i.e. create a shared lock and make sure one trigger doesn't run while the other one is active.