-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Adds basic support for node IDs. #2661
Conversation
Talked to some folks internally and we need it at least exposed /v1/catalog (not DNS) so I'll add that before committing this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍 , just one small thing
@@ -33,6 +33,8 @@ It returns a JSON body like this: | |||
```javascript | |||
[ | |||
{ | |||
"ID": "40e4a748-2192-161a-0510-9bf59fe950b5", | |||
"Node": "foobar", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems like a duplicate/typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops - thanks!
|
||
// If we still don't have a valid node ID, make one. | ||
if config.NodeID == "" { | ||
id, err := uuid.GenerateUUID() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be the place where we could hook into github.com/shirou/gopsutil
:
import "github.com/shirou/gopsutil"
info, err := host.Info()
if info != nil && info.HostID != "" {
config.NodeID = types.NodeID(info.HostID)
}
If the ID came from gopsutil
then I wouldn't necessarily want to persist it in the data directory, but there shouldn't be any harm in that either since the ID coming from gopsutil
and the OS should outlive the contents of whatever is in the data directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - it might not be a bad idea to save it even from gopsutil
in case we improve the algorithm over there and people upgrade Consul in-place, but that's probably not a huge issue either way.
This puts in some basic support for a node ID by having agents make on up at startup and include it in their Serf tags, which is the minimum required for Raft server management with the V2 library.
We had originally intended to add it to the catalog as part of this change, but it's not that useful there unless we also expose it via DNS, etc. so we will hold off on that for now.