Etcd DNS discovery #2521
Merged
Etcd DNS discovery #2521
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6c004c3
to
55f0e36
Compare
|
Pinging @xiang90 |
|
I will take a look over the weekend or early next week. |
xiang90
reviewed
Apr 3, 2017
physical/etcd.go
Outdated
| @@ -13,6 +16,7 @@ import ( | |||
| var ( | |||
| EtcdSyncConfigError = errors.New("client setup failed: unable to parse etcd sync field in config") | |||
| EtcdSyncClusterError = errors.New("client setup failed: unable to sync etcd cluster") | |||
| EtcdMultipleDiscoveryError = errors.New("client setup failed: multiple discovery or bootstrap flags specified, use either \"address\" or \"discovery_srv\"") | |||
xiang90
reviewed
Apr 3, 2017
physical/etcd.go
Outdated
| @@ -95,3 +99,44 @@ func getEtcdAPIVersion(c client.Client) (string, error) { | |||
|
|
|||
| return "3", nil | |||
| } | |||
|
|
|||
| func getEtcdEndpoints(conf map[string]string) ([]string, error) { | |||
| getOption := func(conf_key, env_var string) (string, bool) { | |||
There was a problem hiding this comment.
If it looks generally useful I can move it up to the module scope.
xiang90
reviewed
Apr 3, 2017
physical/etcd.go
Outdated
|
|
||
| func getEtcdEndpoints(conf map[string]string) ([]string, error) { | ||
| getOption := func(conf_key, env_var string) (string, bool) { | ||
| conf_val, in_conf := conf[conf_key] |
There was a problem hiding this comment.
confVal instead of conf_val. use camelcase instead of underscore
xiang90
reviewed
Apr 3, 2017
physical/etcd.go
Outdated
| discoverer := client.NewSRVDiscover() | ||
| endpoints, err := discoverer.Discover(domain) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("etcd SRV discovery: %s", err) |
There was a problem hiding this comment.
failed to discovery etcd endpoints through SRV discovery: %v
xiang90
reviewed
Apr 3, 2017
| @@ -122,3 +138,4 @@ storage "etcd" { | |||
| ``` | |||
|
|
|||
| [etcd]: https://coreos.com/etcd "Etcd by CoreOS" | |||
| [dns discovery]: https://coreos.com/etcd/docs/latest/v2/clustering.html#dns-discovery "Etcd cluster DNS Discovery" | |||
There was a problem hiding this comment.
use this link instead: https://coreos.com/etcd/docs/latest/op-guide/clustering.html#dns-discovery
|
@jsok Have you gave this a try? I want to make sure it works. |
|
looks good in general. |
|
@xiang90 yes I've tested local with a 3-node etcd cluster with SSL. |
55f0e36
to
86078f5
Compare
86078f5
to
5a985b2
Compare
|
lgtm |
|
Merging with the 👍 from @xiang90 |
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a new etcd config option
discovery_srvwhich contains the domain name which the etcd client will use to perform DNS SRV record discovery.Fixes #2519