Integrate consul-server-connection-manager library #7
Conversation
boxofrad
force-pushed
the
boxofrad/server-connection-lib
branch
from
a98b75c to
1842d2d
Compare
boxofrad
force-pushed
the
boxofrad/server-connection-lib
branch
from
1842d2d to
3e9a358
Compare
pglass
left a comment
pglass
left a comment
There was a problem hiding this comment.
👍 a few little things, but looks great!
go.mod
Outdated
| github.com/golang/protobuf v1.5.2 // indirect | ||
| github.com/google/go-cmp v0.5.8 // indirect | ||
| github.com/google/uuid v1.1.2 // indirect | ||
| github.com/hashicorp/consul-server-connection-manager v0.0.0-20220908112242-b9f43f15d156 // indirect |
There was a problem hiding this comment.
merged some code and (per your PR suggestion) if you bump consul-server-connection-manager the uuid dependency will go away
There was a problem hiding this comment.
Nice! Will wait until the login stuff is merged 🙇🏻♂️
boxofrad
force-pushed
the
boxofrad/server-connection-lib
branch
from
126c881 to
f22826a
Compare
| return err | ||
| } | ||
| go watcher.Run() | ||
| defer watcher.Stop() |
There was a problem hiding this comment.
One thing I found in my testing is to make sure that Stop is called when the ctx used by Watcher is not yet canceled because otherwise Logout call will fail.
For example, this ctx is passed in from main I believe and the cancel function for it gets called when an Interrupt signal is received. Let's say an interrupt signal is received and we cancel the context from main. In that case, we'll fall into case <-ctx.Done() below and this defer will be called after the for-select loop exits. At this point, the context will be already canceled and so logout will fail.
There was a problem hiding this comment.
Discussion/fix here: hashicorp/consul-server-connection-manager#9 (comment)
4 participants
This PR swaps out the direct use of go-netaddrs for Consul server discovery with the new consul-server-connection manager library, which consumes streaming server health updates via gRPC and also handles feature negotiation, authentication, TLS, and reconnecting when a Consul server terminates the stream to rebalance load.
It depends on the following unmerged PRs: