fix(controller): leader election preventing two controllers running and gracefully shutting down #2291

zachaller · 2022-10-06T22:10:19Z

This change has a behavior change in that we now always spin up metrics server even on standby's they just have zero values because they are not processing anything.

Signed-off-by: zachaller <[email protected]>

…to fix-leader-election-bug

Signed-off-by: zachaller <[email protected]>

github-actions · 2022-10-06T22:23:17Z

Go Published Test Results

1 775 tests 1 775 ✔️ 2m 31s ⏱️
  101 suites       0 💤
      1 files       0 ❌

Results for commit 273ac79.

♻️ This comment has been updated with latest results.

codecov · 2022-10-06T22:23:39Z

Codecov Report

Base: 82.38% // Head: 82.75% // Increases project coverage by +0.37% 🎉

Coverage data is based on head (273ac79) compared to base (33ddaf5).
Patch coverage: 93.51% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2291      +/-   ##
==========================================
+ Coverage   82.38%   82.75%   +0.37%     
==========================================
  Files         121      121              
  Lines       18476    18511      +35     
==========================================
+ Hits        15221    15319      +98     
+ Misses       2468     2408      -60     
+ Partials      787      784       -3

Impacted Files	Coverage Δ
utils/controller/controller.go	`82.77% <50.00%> (ø)`
rollout/trafficrouting/istio/controller.go	`53.36% <72.22%> (+2.55%)`	⬆️
controller/controller.go	`91.41% <95.04%> (+1.96%)`	⬆️
analysis/controller.go	`61.53% <100.00%> (+13.05%)`	⬆️
controller/metrics/metrics.go	`100.00% <100.00%> (ø)`
experiments/controller.go	`71.42% <100.00%> (+5.67%)`	⬆️
ingress/ingress.go	`74.00% <100.00%> (+12.94%)`	⬆️
rollout/controller.go	`80.77% <100.00%> (+2.77%)`	⬆️
service/service.go	`78.51% <100.00%> (+10.13%)`	⬆️
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: zachaller <[email protected]>

github-actions · 2022-10-06T23:40:01Z

E2E Tests Published Test Results

  1 files   1 suites 52m 44s ⏱️
89 tests 82 ✔️ 3 💤 4 ❌
93 runs 86 ✔️ 3 💤 4 ❌

For more details on these failures, see this check.

Results for commit 273ac79.

♻️ This comment has been updated with latest results.

Signed-off-by: zachaller <[email protected]>

…to fix-leader-election-bug-ctx

Signed-off-by: zachaller <[email protected]>

sonarcloud · 2022-10-07T15:00:33Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

No Coverage information
2.9% Duplication

leoluz

Pair reviewed this with @zachaller
LGTM

…nd gracefully shutting down (argoproj#2291) * WIP on fixing leader election fix Signed-off-by: zachaller <[email protected]> * Start and stop informers as well Signed-off-by: zachaller <[email protected]> * lint Signed-off-by: zachaller <[email protected]> * Remove tests that do not test anything Signed-off-by: zachaller <[email protected]> * fix lint Signed-off-by: zachaller <[email protected]> * github trigger re-run Signed-off-by: zachaller <[email protected]> * Cleanup Signed-off-by: zachaller <[email protected]> * cleanup Signed-off-by: zachaller <[email protected]> * Add back one test Signed-off-by: zachaller <[email protected]> * remove secondary metric server Signed-off-by: zachaller <[email protected]> * Remove secondary metric test Signed-off-by: zachaller <[email protected]> * Add single instance test to catch log lines Signed-off-by: zachaller <[email protected]> * We should shutdown if we can not sync Signed-off-by: zachaller <[email protected]> * fix lint Signed-off-by: zachaller <[email protected]> * Redo for loop will have another pr that stops via context Signed-off-by: zachaller <[email protected]> * Fix comment Signed-off-by: zachaller <[email protected]> * Add context and graceful shutdown Signed-off-by: zachaller <[email protected]> * lint Signed-off-by: zachaller <[email protected]> * Fix test Signed-off-by: zachaller <[email protected]> * github trigger re-run Signed-off-by: zachaller <[email protected]> * add more time for startup Signed-off-by: zachaller <[email protected]> * add individual controller startup tests Signed-off-by: zachaller <[email protected]> * standardize shutdown Signed-off-by: zachaller <[email protected]> * Standardize leader test Signed-off-by: zachaller <[email protected]> * fix test Signed-off-by: zachaller <[email protected]> * We can not turn on release on cancel Signed-off-by: zachaller <[email protected]> * fix release on cancel Signed-off-by: zachaller <[email protected]> Signed-off-by: zachaller <[email protected]>

zachaller added 17 commits September 30, 2022 15:56

WIP on fixing leader election fix

7a7355b

Signed-off-by: zachaller <[email protected]>

Start and stop informers as well

5ae8779

Signed-off-by: zachaller <[email protected]>

lint

5c0a54d

Signed-off-by: zachaller <[email protected]>

Remove tests that do not test anything

306661a

Signed-off-by: zachaller <[email protected]>

fix lint

bc6c327

Signed-off-by: zachaller <[email protected]>

github trigger re-run

b2cfe86

Signed-off-by: zachaller <[email protected]>

Cleanup

00b7699

Signed-off-by: zachaller <[email protected]>

cleanup

28fba55

Signed-off-by: zachaller <[email protected]>

Add back one test

809b652

Signed-off-by: zachaller <[email protected]>

remove secondary metric server

8d516fe

Signed-off-by: zachaller <[email protected]>

Remove secondary metric test

541123d

Signed-off-by: zachaller <[email protected]>

Add single instance test to catch log lines

593af56

Signed-off-by: zachaller <[email protected]>

We should shutdown if we can not sync

76d5134

Signed-off-by: zachaller <[email protected]>

Merge branch 'master' of https://github.com/argoproj/argo-rollouts in…

5667c3f

…to fix-leader-election-bug

fix lint

72c4697

Signed-off-by: zachaller <[email protected]>

Redo for loop will have another pr that stops via context

0838b65

Signed-off-by: zachaller <[email protected]>

Fix comment

c369d1c

Signed-off-by: zachaller <[email protected]>

zachaller changed the title ~~Fix leader election bug ctx~~ fix:(controller) leader election bug with ctx Oct 6, 2022

zachaller changed the title ~~fix:(controller) leader election bug with ctx~~ fix(controller): leader election bug with ctx Oct 6, 2022

zachaller added 2 commits October 6, 2022 17:12

Add context and graceful shutdown

317bdf2

Signed-off-by: zachaller <[email protected]>

lint

6a84f57

Signed-off-by: zachaller <[email protected]>

zachaller added 3 commits October 6, 2022 17:29

Fix test

1682cc8

Signed-off-by: zachaller <[email protected]>

github trigger re-run

043c10b

Signed-off-by: zachaller <[email protected]>

add more time for startup

5410287

Signed-off-by: zachaller <[email protected]>

zachaller added 3 commits October 6, 2022 21:34

add individual controller startup tests

2b0bf62

Signed-off-by: zachaller <[email protected]>

standardize shutdown

b34ebf6

Signed-off-by: zachaller <[email protected]>

Standardize leader test

4bd8150

Signed-off-by: zachaller <[email protected]>

zachaller added 2 commits October 6, 2022 21:52

Merge branch 'master' of https://github.com/argoproj/argo-rollouts in…

b12cedc

…to fix-leader-election-bug-ctx

fix test

11a1ce3

Signed-off-by: zachaller <[email protected]>

zachaller changed the title ~~fix(controller): leader election bug with ctx~~ fix(controller): leader election allowing two controllers to run and gracefully shutting down Oct 7, 2022

zachaller added the ready-for-review Ready for final review label Oct 7, 2022

zachaller mentioned this pull request Oct 7, 2022

fix:(controller) leader election bug #2280

Closed

zachaller changed the title ~~fix(controller): leader election allowing two controllers to run and gracefully shutting down~~ fix(controller): leader election preventing two controllers running and gracefully shutting down Oct 7, 2022

zachaller marked this pull request as ready for review October 7, 2022 12:25

zachaller added 2 commits October 7, 2022 09:58

We can not turn on release on cancel

73f9ae4

Signed-off-by: zachaller <[email protected]>

fix release on cancel

273ac79

Signed-off-by: zachaller <[email protected]>

zachaller requested a review from leoluz October 7, 2022 15:59

zachaller added this to the v1.4 milestone Oct 11, 2022

leoluz approved these changes Oct 13, 2022

View reviewed changes

leoluz merged commit 35da539 into argoproj:master Oct 13, 2022

zachaller mentioned this pull request Nov 30, 2022

High CPU/Mem usage with Argo #2443

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(controller): leader election preventing two controllers running and gracefully shutting down #2291

fix(controller): leader election preventing two controllers running and gracefully shutting down #2291

zachaller commented Oct 6, 2022 •

edited

Loading

github-actions bot commented Oct 6, 2022 •

edited

Loading

codecov bot commented Oct 6, 2022 •

edited

Loading

github-actions bot commented Oct 6, 2022 •

edited

Loading

sonarcloud bot commented Oct 7, 2022

leoluz left a comment

fix(controller): leader election preventing two controllers running and gracefully shutting down #2291

fix(controller): leader election preventing two controllers running and gracefully shutting down #2291

Conversation

zachaller commented Oct 6, 2022 • edited Loading

github-actions bot commented Oct 6, 2022 • edited Loading

Go Published Test Results

codecov bot commented Oct 6, 2022 • edited Loading

Codecov Report

github-actions bot commented Oct 6, 2022 • edited Loading

E2E Tests Published Test Results

sonarcloud bot commented Oct 7, 2022

leoluz left a comment

Choose a reason for hiding this comment

zachaller commented Oct 6, 2022 •

edited

Loading

github-actions bot commented Oct 6, 2022 •

edited

Loading

codecov bot commented Oct 6, 2022 •

edited

Loading

github-actions bot commented Oct 6, 2022 •

edited

Loading