docker-archive / deploykit

A toolkit for creating and managing declarative, self-healing infrastructure.
Apache License 2.0
2.25k stars 262 forks source link

ingress / enrollment controllers: make leadership lookup on-demand #787

Closed chungers closed 6 years ago

chungers commented 6 years ago

Closes #781

Previously in #781 the panic manifests when the controllers that depend on the manager.Leadership remote object failed to initialize properly. This failure, in turn, was due to the manager (which implements the manager.Leadership interface) failed to initialize in time to respond to a RPC handshake from these controllers. This can be seen as the RPC canceled / timeout error in the logs when either the ingress or enrollment controllers come up.

Further, the manager has dependency on group and its initialization depends on that plugin. So we have a ordering problem: if the group plugin starts up after either ingress or enrollment controllers, the manager will not initialize properly in time to properly perform this handshake. As a result, a panic is thrown when the user tries to commit a spec because the leadership interface was set to nil.

This PR

Signed-off-by: David Chung david.chung@docker.com

GordonTheTurtle commented 6 years ago

Please sign your commits following these rules: https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work The easiest way to do this is to amend the last commit:

$ git clone -b "bug-781" git@github.com:chungers/infrakit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354386984
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

codecov[bot] commented 6 years ago

Codecov Report

Merging #787 into master will decrease coverage by 0.43%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #787      +/-   ##
==========================================
- Coverage   48.87%   48.44%   -0.44%     
==========================================
  Files          85       85              
  Lines        9358     7502    -1856     
==========================================
- Hits         4574     3634     -940     
+ Misses       4435     3518     -917     
- Partials      349      350       +1
Impacted Files Coverage Δ
pkg/rpc/mux/server.go 42.7% <0%> (-4.8%) :arrow_down:
pkg/core/error.go 60% <0%> (-3.64%) :arrow_down:
pkg/template/funcs.go 62.73% <0%> (-3.55%) :arrow_down:
pkg/x/remoteboot/remoteboot.go 17.46% <0%> (-2.8%) :arrow_down:
pkg/cli/playbook/modules.go 62.35% <0%> (-2.25%) :arrow_down:
pkg/util/etcd/v3/etcd.go 28.57% <0%> (-2.2%) :arrow_down:
pkg/plugin/group/lazy.go 64.1% <0%> (-2.17%) :arrow_down:
pkg/provider/terraform/instance/show.go 81.33% <0%> (-2.1%) :arrow_down:
pkg/provider/terraform/instance/apply.go 60.09% <0%> (-2.08%) :arrow_down:
pkg/spi/loadbalancer/protocol.go 33.33% <0%> (-1.97%) :arrow_down:
... and 72 more

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 35fbe53...048bf07. Read the comment docs.