-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
use default ip for advertise URL #6170
use default ip for advertise URL #6170
Conversation
c35e60a
to
1d90f7e
Compare
Is there anyway that I can try
|
@gyuho huh, works on my machine. Is it failing to get |
@heyitsanthony It's returning here ...
for _, attr := range attrs {
if attr.Attr.Type == syscall.RTA_PREFSRC {
return net.IP(attr.Value).String(), nil
}
}
// returns here
return "", errNoDefaultRoute
}
|
@gyuho OK, could reproduce on my ubuntu machine; can work around. Thanks for giving this a spin! |
1d90f7e
to
636a31e
Compare
had to query the iface to get the address on ubuntu /cc @gyuho |
636a31e
to
e145cbd
Compare
Ok works now. Mine is
LGTM. /cc @xiang90 |
return | ||
} | ||
// found default host; listen everywhere, advertise on default host | ||
DefaultListenPeerURLs = "http://0.0.0.0:2380" |
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.
i do not think we should listen on wide by default even if we found the default host for security reasons. by finding default adv url, users do not need to set avd ulrs themselves but not listen url I think.
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.
ok
e145cbd
to
5e51cfd
Compare
@xiang90 all fixed up |
defaultHost, dhErr := cfg.IsDefaultHost() | ||
if defaultHost != "" { | ||
if dhErr == nil { | ||
plog.Infof("advertising using default host %q", defaultHost) |
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.
using detected default host
lgtm |
67dbff6
to
2cc245e
Compare
Fixes #2858