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 Chef search (role:cassandra) to look up seed nodes #205

Merged
merged 10 commits into from
Aug 7, 2015

Conversation

michaelklishin
Copy link
Owner

If search is not available, we fall back to using node's own
IP address by default.

Fixes #204.

If search is not available, we fall back to using node's own
IP address by default.

Fixes #204.
@michaelklishin
Copy link
Owner Author

@vkhatri please take a look.

@sethrosenblum
Copy link
Collaborator

I may be mis-understanding, but won't this add a chef search to every client run, even when you're not using the default value?

Also, it's generally preferable to not have every node in your cluster be a seed, but only use a subset. That's why I use a search for a tag cassandra-seed. Maybe we could just use the first 3 nodes found when a search is performed and the ips are sorted?

@michaelklishin
Copy link
Owner Author

@sethrosenblum good points. Perhaps we should move this to the recipe and check if seeds has a special value (e.g. :search) then?

@michaelklishin
Copy link
Owner Author

One way of making search optional would be using separate recipes for that (instead of adding more conditional attributes).

@michaelklishin
Copy link
Owner Author

@sethrosenblum @vkhatri I've moved seed discovery into a separate recipe. Let me know what you think.

@vkhatri
Copy link
Collaborator

vkhatri commented Aug 6, 2015

@michaelklishin sorry for the delay. this looks nice, few thoughts:

  • it is better to have an attribute default['cassandra']['seed']['use_chef_search'] to allow user to enable/disable chef search
  • we could add another attribute default['cassandra']['seed']['count'] = 3 to limit the number of cluster seed nodes
  • we could add another attribute default['cassandra']['seed']['search_role'] = 'cassandra-seed' to limit the cluster nodes only for seed
  • we could add another attribute default['cassandra']['seed']['search_query'] = 'nil' and use it as an override
  • search query need to be limited further only to default['cassandra']['cluster_name']
  • perhaps to add this as a helper method instead of recipe

please let me know what do you think

@michaelklishin
Copy link
Owner Author

@vkhatri OK, I guess this is a philosophical decision more than a technical one. I'll do what you suggest.

I'm now sure what you mean by

search query need to be limited further only to default['cassandra']['cluster_name']

Discovering cluster name is a good idea. Do you mean that we should do this regardless of the use_chef_search value?

@vkhatri
Copy link
Collaborator

vkhatri commented Aug 6, 2015

@michaelklishin only if use_chef_search is set to true, actually i am thinking the helper method should return the final value for -seeds to be used in templates/default/cassandra.yaml.erb.

templates/default/cassandra.yaml.erb:

          - seeds: "<%= eval_seed_nodes %>"

seed helper method:

def eval_seed_nodes
  # use chef search for seed nodes
  if node['cassandra']['seed']['use_chef_search']
    if Chef::Config[:solo]
      Chef::Log.warn("Chef Solo does not support search, provide the seed nodes via node attribute node['cassandra']['seeds']")
      node['ipaddress']
    else
      if node['cassandra']['seed']['search_query']
        # user defined search query
        xs = search(:node, node['cassandra']['seed']['search_query']).map(&:ipaddress).sort.uniq
      else
        # cookbook auto search query
        search_query = "chef_environment:#{node.chef_environment} AND role:#{node['cassandra']['seed']['search_role']} AND cassandra_cluster_name:#{node['cassandra']['cluster_name']}"
        xs = search(:node, search_query).map(&:ipaddress).sort.uniq
      end

      if xs.empty?
        node['ipaddress']
      else
        xs.take(node['cassandra']['seed']['count']).join(',')
      end
    end
  else
    # user defined seed nodes
    if node['cassandra']['seeds'].is_a?(Array)
      node['cassandra']['seeds'].join(',')
    else
      node['cassandra']['seeds']
    end
  end
end

attributes:

default['cassandra']['seed']['use_chef_search'] = false
default['cassandra']['seed']['count'] = 3
default['cassandra']['seed']['search_role'] = 'cassandra-seed'
default['cassandra']['seed']['search_query'] = nil

@michaelklishin
Copy link
Owner Author

@vkhatri fair enough, I'll take your suggestion and run with it.

@michaelklishin
Copy link
Owner Author

@vkhatri @sethrosenblum ready for another round.

@michaelklishin
Copy link
Owner Author

I'm getting an exception that says method search is undefined in Chef::Mixin::Template::TemplateContext. So this still needs some work.

@michaelklishin
Copy link
Owner Author

OK, I can confirm that seed discovery using Chef search and a custom query now works well in my environment.

@vkhatri
Copy link
Collaborator

vkhatri commented Aug 7, 2015

@michaelklishin LGTM 👍

michaelklishin added a commit that referenced this pull request Aug 7, 2015
Use Chef search (role:cassandra) to look up seed nodes
@michaelklishin michaelklishin merged commit 549306f into master Aug 7, 2015
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.

3 participants