Skip to content
This repository has been archived by the owner on Feb 7, 2024. It is now read-only.

fix: update cached informer object instead of reloading app to avoid duplicated notifications #204

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

alexmt
Copy link
Collaborator

@alexmt alexmt commented Mar 12, 2021

Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com

Argo CD notifications might send the same notification twice or don't send notification due to stale data in the application informer. First attempt to fix it was to reload updated app before sending notification but this approach is not effective.

This PR implements an alternative approach: after patching application annotations controller immidatelly updates informer cache using application returned by patch request.

…duplicated notifications

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #204 (45000d1) into master (1c2dafe) will decrease coverage by 1.32%.
The diff coverage is 57.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #204      +/-   ##
==========================================
- Coverage   51.53%   50.21%   -1.33%     
==========================================
  Files          34       33       -1     
  Lines        1690     1665      -25     
==========================================
- Hits          871      836      -35     
- Misses        647      661      +14     
+ Partials      172      168       -4     
Impacted Files Coverage Δ
pkg/controller/subscriptions.go 74.13% <ø> (ø)
pkg/controller/state.go 48.83% <48.83%> (ø)
controller/controller.go 72.47% <72.22%> (-0.94%) ⬇️
bot/server.go 57.95% <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 1c2dafe...45000d1. Read the comment docs.

@alexmt alexmt merged commit 4dd6b0d into argoproj-labs:master Mar 12, 2021
@alexmt alexmt deleted the duplicated-notifications branch March 12, 2021 22:58
@dnugmanov dnugmanov mentioned this pull request Apr 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant