Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(notifications): default templates and logic #1010

Merged
merged 3 commits into from
Sep 19, 2021

Conversation

piksel
Copy link
Member

@piksel piksel commented Jul 1, 2021

Follow up/continuation of #981.

This PR will fix the default report template to only output something if Updated or Failed is non-zero. It also fixes the non-session notifications (like the startup info) which was just broken using the report notifications.
Note that this aligns the notification behaviour roughly with how it works using legacy notifications, as no notifications will be sent when everything is "normal"/already updated.

the new default report template
{{- if .Report -}}
  {{- with .Report -}}
    {{- if ( or .Updated .Failed ) -}}
{{len .Scanned}} Scanned, {{len .Updated}} Updated, {{len .Failed}} Failed
      {{- range .Updated}}
- {{.Name}} ({{.ImageName}}): {{.CurrentImageID.ShortID}} updated to {{.LatestImageID.ShortID}}
      {{- end -}}
      {{- range .Fresh}}
- {{.Name}} ({{.ImageName}}): {{.State}}
      {{- end -}}
      {{- range .Skipped}}
- {{.Name}} ({{.ImageName}}): {{.State}}: {{.Error}}
      {{- end -}}
      {{- range .Failed}}
- {{.Name}} ({{.ImageName}}): {{.State}}: {{.Error}}
      {{- end -}}
    {{- end -}}
  {{- end -}}
{{- else -}}
  {{range .Entries -}}{{.Message}}{{"\n"}}{{- end -}}
{{end -}}

To prevent the notifications from being sent, it skips notifications whenever the output of the template (whether legacy or report templates) is an empty string.
This fixes #1008

It also adds a "special" data field for logging that just marks a message as non-notification. This is so that we can still log using the appropriate log levels without having to worry about it getting sent as notifications.
This mainly concerns logs about the notifications themselves, which would cause loops etc.
Any log messages with the field notify set to no is ignored.

@piksel piksel requested a review from simskij as a code owner July 1, 2021 16:23
@codecov
Copy link

codecov bot commented Jul 1, 2021

Codecov Report

Merging #1010 (bdc267b) into main (ea300b7) will increase coverage by 0.17%.
The diff coverage is 61.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1010      +/-   ##
==========================================
+ Coverage   59.63%   59.80%   +0.17%     
==========================================
  Files          22       22              
  Lines        1412     1428      +16     
==========================================
+ Hits          842      854      +12     
- Misses        500      502       +2     
- Partials       70       72       +2     
Impacted Files Coverage Δ
pkg/notifications/shoutrrr.go 74.35% <57.89%> (-1.03%) ⬇️
pkg/notifications/notifier.go 70.21% <100.00%> (+0.64%) ⬆️
pkg/registry/digest/digest.go 44.26% <0.00%> (+1.88%) ⬆️

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 ea300b7...bdc267b. Read the comment docs.

@piksel
Copy link
Member Author

piksel commented Jul 1, 2021

Also, this enables having the startup message being logged even if it's not wanted as a notification, it also changes said startup message to be a series of messages instead of a single one with newlines in it (which was not really supported in logfmt):

INFO[0001] Watchtower v0.0.0-unknown
INFO[0001] Using notifications: telegram
INFO[0001] Checking all containers (except explicitly disabled with label)
INFO[0001] Running a one time update.

They are still batched up and sent as a single notification, so there should be no difference in the notifications.

Also, this is how the closing messages look, no more raw printing to stdout:

DEBU[0002] No new images found for /testcontainer1
DEBU[0002] Session done: 1 scanned, 0 updated, 0 failed
INFO[0002] Waiting for the notification goroutine to finish  notify=no
INFO[0002] Skipping notification due to empty message    notify=no

And the same goes for notification errors when sending:

ERRO[0002] Failed to send shoutrrr notification          error="failed to send notification to \"xxxxxx\", response status code 401 Unauthorized" index=0 notify=no service=telegram

...and for template issues:

FATA[0001] Notification template error                   error="template: :1:3: executing \"\" at <.InvalidProp>: can't evaluate field InvalidProp in type notifications.Data" notify=no

@piksel piksel force-pushed the fix/notif-default-templates branch from 2687f8b to bdc267b Compare July 25, 2021 13:14
@piksel piksel merged commit cd0ec88 into main Sep 19, 2021
@piksel piksel deleted the fix/notif-default-templates branch September 19, 2021 16:06
@ilike2burnthing
Copy link
Contributor

Is the Docker log supposed to look like this when no notifications are configured?

time="2021-09-21T18:46:22Z" level=info msg="Skipping notification due to empty message" notify=no
time="2021-09-21T18:47:21Z" level=info msg="Skipping notification due to empty message" notify=no
time="2021-09-21T18:48:21Z" level=info msg="Skipping notification due to empty message" notify=no

(changed the poll interval to 60 to test if it was related to the timing of the log entries)

@piksel
Copy link
Member Author

piksel commented Sep 21, 2021

Sort of. It's not really necessary when you don't have any notifications of course...

@ilike2burnthing
Copy link
Contributor

Any way to disable it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discord/Slack notifications being sent for runs with no updates
2 participants