containrrr / watchtower

A process for automating Docker container base image updates.
https://containrrr.dev/watchtower/
Apache License 2.0
19.18k stars 853 forks source link

API changes for selecting whether to use HTTP API triggers, scheduled triggers, or both #939

Open simskij opened 3 years ago

simskij commented 3 years ago

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...

Originally posted by @piksel in https://github.com/containrrr/watchtower/issues/916#issuecomment-827035360

simskij commented 3 years ago

This is an interesting observation that I think requires that we put some more thought into how the user APIs are currently organized. One issue currently is that we really can't solely use the existence of both as an indicator of whether to enable both triggers or not, as --interval has a "sensible default" set.

However, I would like to propose that we introduce a new parameter, that we name --triggers, or something similar. This parameter would default to the value schedule, but would also allow the user to specify an array of allowed/enabled triggers. Currently, the only two valid options would be schedule and api.

It would also allow us to drop the somewhat ambiguous --enable-http-api that currently coexist with --enable-http-metrics (although the only thing they have in common is that they both are served over HTTP).

This would leave us with enough flexibility to add more options going forward, allowing for instance triggering via message queues or similar.

@piksel @victorcmoura @zoispag @tammert, feel free to weigh in here.

piksel commented 3 years ago

Actually, the --run-once could also be seen as --triggers startup? That would also as a side-effect remove schedule from the triggers, and cause it to exit automatically after the initial check since no more triggers could trigger.

DasSkelett commented 3 years ago

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 interpret the command line/environment args.

Yup, that's how I originally planned to do it for #916, but when looking at the code I realized this is not possible with the current cmdline + environment parsing system, as @simskij already explained.

I like the ideas being expressed here, and as a user would find something like --triggers a very good improvement. It would not only allow customizing the behaviour of Watchtower to your own needs (API triggers on/off, schedule on/off, startup on/off, and all possible combinations thereof), but also get rid of the unexpected side effects some commands currently have. E.g. --enable-http-api doesn't sound like it also disables the scheduled runs, and you're in for a surprise once you try it out. You need to read the docs very carefully to spot the word "only".

As a consequence I wouldn't design startup in a way that it implicitly turned off all other triggers, otherwise we're back where we started. It should only follow the behaviour of --run-once if it's the only trigger being specified, but if the list contains multiple triggers (e.g. --triggers startup,schedule), it should simply start a run at the beginning and then wait for any of the other triggers.

piksel commented 3 years ago

No, yeah, that was exactly what I meant: --run-once would be an alias for --triggers startup

Leaving the triggers unspecified would have the default schedule, so setting it to just startup would mean also disabling the scheduling. Running with --triggers schedule, startup would do both. No magic behaviours here, just the opposite 😁

The reason for exiting or not exiting (blocking) before wasn't that clear, now it could instead be turned into wait until all triggers cannot trigger anymore which I think makes it much clearer.

The only magic we would still need to keep, is when to skip updating watchtower itself, as this is not done on --run-once, but it shouldn't be a big issue.

zoispag commented 3 years ago

I like the --triggers idea, which accepts multiple trigger methods/channels. It is flexible enough, to serve us in the future.

tammert commented 3 years ago

Nothing to add, seems like you got a nice solution thought out with --triggers supporting multiple modes :+1:

victorcmoura commented 3 years ago

I really like the idea of having a --triggers arg. It seems to be a lot more user-friendly and extensible than what we have, as other triggers can be implemented in the future.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.