Skip to content
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

Merged
merged 3 commits into from
Aug 15, 2016

Conversation

heyitsanthony
Copy link
Contributor

Fixes #2858

@gyuho
Copy link
Contributor

gyuho commented Aug 13, 2016

Is there anyway that I can try GetDefaultHost? In my linux machine, I get

could not find default route

@heyitsanthony
Copy link
Contributor Author

@gyuho huh, works on my machine. Is it failing to get defaultNlMsg or is it failing to find the RTA_PREFSRC? I'll see if it works on my ubuntu machine...

@gyuho
Copy link
Contributor

gyuho commented Aug 13, 2016

@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
}
go version go1.7rc6 linux/amd64

Linux gyuho 4.4.0-34-generic #53-Ubuntu SMP Wed Jul 27 16:06:39 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux

@heyitsanthony
Copy link
Contributor Author

@gyuho OK, could reproduce on my ubuntu machine; can work around. Thanks for giving this a spin!

@heyitsanthony
Copy link
Contributor Author

had to query the iface to get the address on ubuntu /cc @gyuho

@gyuho
Copy link
Contributor

gyuho commented Aug 13, 2016

Ok works now. Mine is

10.0.0.30

LGTM. /cc @xiang90

return
}
// found default host; listen everywhere, advertise on default host
DefaultListenPeerURLs = "http://0.0.0.0:2380"
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@heyitsanthony
Copy link
Contributor Author

@xiang90 all fixed up

defaultHost, dhErr := cfg.IsDefaultHost()
if defaultHost != "" {
if dhErr == nil {
plog.Infof("advertising using default host %q", defaultHost)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using detected default host

@xiang90
Copy link
Contributor

xiang90 commented Aug 15, 2016

lgtm

@heyitsanthony heyitsanthony force-pushed the default-advertise-ip branch 2 times, most recently from 67dbff6 to 2cc245e Compare August 15, 2016 21:06
@heyitsanthony heyitsanthony merged commit 16b2d9c into etcd-io:master Aug 15, 2016
@heyitsanthony heyitsanthony deleted the default-advertise-ip branch August 16, 2016 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants