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

Modify "join_cluster" to key off of "known_servers". Fixes #31. #42

Closed
wants to merge 1 commit into from

Conversation

tehranian
Copy link

The existing implementation of join_cluster relies upon num_peers, which doesn't exist anymore. Modify join_cluster to use known_servers instead.

I tested this with a set of 3 Vagrant boxes running puppet. Before join_cluster runs, I see:

$ consul info | grep known
  known_servers = 0

And after join_cluster runs I see:

$ consul info | grep known
known_servers = 1

$ consul members
Node               Address               Status  Type    Build  Protocol
vi-cobbler11       192.168.181.146:8301  alive   client  0.4.1  2
vi-devops-consul9  192.168.181.142:8301  alive   server  0.4.1  2
vi-cobbler10       192.168.181.129:8301  alive   client  0.4.1  2

@solarkennedy
Copy link
Contributor

This this really the right thing?

Can't consul have known_servers even if it hasn't joined a cluster yet? (via serf gossip)

Deferring to @EvanKrall

@tehranian
Copy link
Author

Hi there,

Yea, I was thinking about this after I sent the pull request... While I think this patch provides a solution that works in some cases, I don't think that it's the best possible solution.

Another idea:

  • Whereas the "join_cluster" parameter is the hostname or IP of a single known consul node...
  • Do a name resolution on the DNS hostname, and see if the resulting IP is in the output of consul members.
    • If the user provides an IP, no need to resolve
    • This would work in the situation where the DNS name is a CNAME instead of an A record, which I suspect could be a common use case.

Here's the sample output of consul members in my Vagrant test environment:

$ consul members
Node               Address               Status  Type    Build  Protocol
vi-devops-consul9  192.168.181.129:8301  alive   server  0.4.1  2
vi-cobbler11       192.168.181.146:8301  alive   client  0.4.1  2
vi-cobbler10       192.168.181.142:8301  alive   client  0.4.1  2

What do you think?

@EvanKrall
Copy link
Contributor

The DNS/consul members thing seems reasonable.

The existing patch wouldn't converge on machines that are acting as the servers - for whatever reason, consul info doesn't have a known_servers line when you're on a server.

@tehranian
Copy link
Author

Thanks @EvanKrall .

re: DNS/"consul members" - Hmmm now that I think about it, a conditional that converts a hostname to DNS or leaves an IP address as-is, then greps the output of "consul members" for it in a single line of bash is going ugly. Any ideas?

I'm thinking of something like the following which seems to convert hostnames to an IP, or leave the IP as-is.

getent ahostsv4 www.google.com | grep STREAM | head -n 1 | cut -d ' ' -f 1

Then that can be fed as the argument for "grep". Ex:

consul members | grep `getent ahostsv4 www.google.com | grep STREAM | head -n 1 | cut -d ' ' -f 1`

@solarkennedy
Copy link
Contributor

join_cluster is now deprecated. This kind of output manipulation is a little much.
Maybe someone ambitious will just add something to consul's output like "cluster status: joined" to make it way easier to parse or something.

Till then I'm leaving this as an exercise to the implementer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants