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

Add cluster advertise address to host address as default #16630

Closed

Conversation

coolljt0725
Copy link
Contributor

Signed-off-by: Lei Jitang leijitang@huawei.com
#16229 has add builtin nodes discovery,

since docker daemon could listen multiple address, we should make sure
the advertise address is listened by docker daemon.

ping @icecrime

@calavera
Copy link
Contributor

/cc @icecrime, @abronan what do you think about this?

@abronan
Copy link
Contributor

abronan commented Oct 6, 2015

Makes sense but I feel quite uncomfortable with those default assumptions 😕. What if I give the wrong address to --cluster-advertise? It is going to assume that I want this address to be listened by the daemon even though I probably don't want to.

I much prefer giving the address to both -H and --cluster-advertise, at least I know what I get.

@icecrime WDYT?

@coolljt0725
Copy link
Contributor Author

@abronan Another approach, if people give address to --cluster-advertise but forget to give it to -H, we should error out to tell user to give address to both just like etcd does

@abronan
Copy link
Contributor

abronan commented Oct 15, 2015

@coolljt0725 There is a related discussion ongoing in #17047 about the --cluster-advertise flag, should be worth adding your ideas here ;)

@coolljt0725 coolljt0725 force-pushed the add_advertise_address_to_host branch 2 times, most recently from 941157f to 1b199f5 Compare October 16, 2015 05:48
Signed-off-by: Lei Jitang <leijitang@huawei.com>
@cpuguy83
Copy link
Member

cpuguy83 commented Nov 6, 2015

I think it would be better to allow the admin to set these options rather than automatically trying to use cluster-address.

@thaJeztah
Copy link
Member

Following the discussion above, I don't think there's consensus here, so I'll close this, but ping me if you think I closed prematurely

Thanks @coolljt0725 !

@thaJeztah thaJeztah closed this Nov 10, 2015
@coolljt0725 coolljt0725 deleted the add_advertise_address_to_host branch April 28, 2016 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants