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

cmd/contour: simplify leadership election setup #2348

Merged
merged 1 commit into from Mar 18, 2020

Conversation

davecheney
Copy link
Contributor

Updates #403

Ingress status updates depend on leadership election. This PR moves
leadership setup outside doServe and simplifies the interaction with
EventHandler.IsLeader.

Signed-off-by: Dave Cheney dave@cheney.net

Updates projectcontour#403

Ingress status updates depend on leadership election. This PR moves
leadership setup outside doServe and simplifies the interaction with
EventHandler.IsLeader.

Signed-off-by: Dave Cheney <dave@cheney.net>
@davecheney davecheney added this to the 1.3.0 milestone Mar 17, 2020
@davecheney
Copy link
Contributor Author

@jpeach I looked at moving this logic to internal/k8s but the dependency on cmd/contour.serveContext is large and the latter type should not live in internal/contour.

@codecov
Copy link

codecov bot commented Mar 17, 2020

Codecov Report

Merging #2348 into master will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2348      +/-   ##
==========================================
+ Coverage   77.96%   77.97%   +0.01%     
==========================================
  Files          60       60              
  Lines        5205     5204       -1     
==========================================
  Hits         4058     4058              
+ Misses       1059     1058       -1     
  Partials       88       88              
Impacted Files Coverage Δ
cmd/contour/leadership.go 0.00% <0.00%> (ø)
cmd/contour/serve.go 0.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 0ab26ce...01864ee. Read the comment docs.

// a channel which will become ready when this process becomes the leader, or, in the
// event that leadership election is disabled, the channel will be ready immediately.
func setupLeadershipElection(g *workgroup.Group, log logrus.FieldLogger, ctx *serveContext, clients *k8s.Clients, updateNow func()) chan struct{} {
if ctx.DisableLeaderElection {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest you pull this into a separate function becomeElectedLeader(), then check ctx.DisableLeaderElection from the caller.

Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@davecheney davecheney merged commit a42493e into projectcontour:master Mar 18, 2020
@davecheney davecheney deleted the issue/403 branch March 18, 2020 01:16
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.

None yet

3 participants