fix:(controller) leader election bug

[zachaller]

Fixes #2117

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

[github-actions]

Go Published Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit c369d1c.

♻️ This comment has been updated with latest results.

[github-actions]

E2E Tests Published Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit c369d1c.

♻️ This comment has been updated with latest results.

[zachaller]

kubernetes-sigs/controller-runtime#1918

[codecov]

Codecov Report

Base: 82.38% // Head: 82.37% // Decreases project coverage by -0.01% ⚠️

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2280      +/-   ##
==========================================
- Coverage   82.38%   82.37%   -0.02%     
==========================================
  Files         121      121              
  Lines       18476    18471       -5     
==========================================
- Hits        15221    15215       -6     
- Misses       2468     2471       +3     
+ Partials      787      785       -2     

Impacted Files	Coverage Δ
rollout/controller.go	77.99% <0.00%> (ø)
service/service.go	68.37% <0.00%> (ø)
controller/controller.go	89.35% <88.57%> (-0.10%)	⬇️
controller/metrics/metrics.go	100.00% <100.00%> (ø)

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.

[leoluz]

Implementation looks good but I think PR should improve the test coverage preferably in form of unit tests and/or integration tests if possible.

[zachaller]

Implementation looks good but I think PR should improve the test coverage preferably in form of unit tests and/or integration tests if possible.

Added one more test to just close/exit on a single instance setup to cover the log lines. It's currently from my understanding impossible to cover the actual shutdown of a controller because go test framework has no way to capture exit codes from the test process.

I like the idea of switching the e2e tests to using HA, in order to do that though will require a bit of work including creating a HA manifest that we can use to deploy for e2e then have a test that somehow kills one of the controllers (not sure how) and looks for a switch but not sure how all that would work it would be quite complicated because I am not sure how we would be able to tell that the standby controller took over and we didn't just spin up a new instance.

[zachaller]

I will also point out that all the other lines except the OnStopLeading callback where also never tested before this they just got flagged due to rename not that that is good but you can see that here https://github.com/argoproj/argo-rollouts/pull/2282/files where I just undid the rename.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
2.9% Duplication

[zachaller]

closing in favor of #2291