-
Notifications
You must be signed in to change notification settings - Fork 2
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
Include sync errors in Sync notifications #1870
Conversation
Recently, errors for individual resources were added to flux's SyncEvents. This commit revendors weaveworks/flux so the service will be able to parse the additional information.
Newer flux daemons (since fluxcd/flux#970) report any sync failures for individual resources. Take advantage of this by including the errors in sync notifications.
I'm not that happy with the text that appears in the notifications. Some of that is because the errors from kubectl are pretty terrible; some is because fluxd makes no effort to sanitise them. |
Agreed, but that is a separate issue; even in their current form the errors are more useful than silence. |
You'd be OK with users getting those notifications? |
Yep. |
flux-api/notifications/slack.go
Outdated
@@ -258,6 +262,22 @@ func slackCommitsAttachment(ev *event.SyncEventMetadata) slackAttachment { | |||
} | |||
} | |||
|
|||
func slackSyncErrorAttachment(ev *event.SyncEventMetadata) slackAttachment { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
There's no need for these to get the whole struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
During a flux sync, some resources may fail to be applied, because (for example) they fail validation. Since fluxcd/flux#970, the flux daemon includes these errors in the event it posts to flux-api.
This PR exposes the failures by including them as an attachment in the sync notification.