michaelklishin / cassandra-chef-cookbook

Chef cookbook for Apache Cassandra, DataStax Enterprise (DSE) and DataStax agent
Apache License 2.0
163 stars 228 forks source link

RFC: put all template values into attributes #148

Closed willejs closed 9 years ago

willejs commented 9 years ago

The cassandra.yaml file is heavily templated and has many hard coded values in it. The config file is large, and has many possible values and combinations.

I don't want to submit a bunch of pull requests for these, and i don't care about the comments in the file either. (Others might care about these comments, we can put them into the config.rb attributes file if people wish.)

I am proposing that we do away with the keys, and comments in the erb template and instead generate the yaml from a hash in the attributes.

This way we can easily manage the hundred or so config values in attributes, and add additional settings that are not in the template that can be set.

What are peoples thoughts on this? I will get to work on a PR if people are in agreement.

michaelklishin commented 9 years ago

Sounds good to me.

vkhatri commented 9 years ago

This sound good. We have seen every now and then, attributes gets deprecated in cassandra.yaml and we had to add attributes conditions for different versions.

I guess we still have to keep those versions checks or it can be left to the user to add the required attributes.

willejs commented 9 years ago

Yeah I think maybe if you override a default attribute with nil, it will emit it from the tenplated Cassandra.yaml. That way you can easily override attributes in your wrapper cookbook (roles or environments ergh) for new versions if you wish.

willejs commented 9 years ago

We need to be mindful of backwards compatability if I change the format of the config file and omit comments. This could trigger a restart of cassandra and should be denoted by a major version increment. Which I will do in the PR tomorrow or next week.

vkhatri commented 9 years ago

@willejs @michaelklishin we had a talk earlier to introduce different templates file for different version if change is significant to counter deprecation, but that is just a complication waiting to pile up over time.

i guess it would be better to add very common attributes to the cookbook like data dir etc. and leave all other attributes to the user according to their environment (in wrapper cookbook, role or environment), agreed?

How about we keep a Hash of common attributes (node[:cassandra][:config][:common]) for common C* config parameters and then have a version attribute (node[:cassandra][:config][:v2.1.2]) for version specific common attributes? this way cassandra.yaml gets all the common for all versions and attributes for defined version. This approach is more alike to the existing cookbook setup, if we want to keep version specific attributes in the cookbook (not required if we leave it to the user).

michaelklishin commented 9 years ago

Is this still relevant?

willejs commented 9 years ago

I think I decided that there were too many variations between versions of Cassandra. I'm trying to think of a more elegant way to template these per version, but I will probably just specify a template in a wrapper cookbook... On 20 Feb 2015 21:32, "Michael Klishin" notifications@github.com wrote:

Is this still relevant?

— Reply to this email directly or view it on GitHub https://github.com/michaelklishin/cassandra-chef-cookbook/issues/148#issuecomment-75322855 .

vkhatri commented 9 years ago

i will take a stab at it this week. i am thinking about adding an array attribute to keep a list of attributes for cassandra.yaml against which i can validate the attributes specified the attributes and ignore if user added unintentionally. this way user can add their own attributes if missing any, until it gets added to this cookbook.

vkhatri commented 9 years ago

@michaelklishin i am thinking to move all config attributes to a new attribute file attributes/config.rb under attribute default['cassandra']['config'].

like default common attributes:

default['cassandra']['config']['attribute_name'] = value

and conditional attributes for specific version

if version =~ /2.1/
  default['cassandra']['config']['attrirbute_name'] = value
elsif version =~ /2.1/
  default['cassandra']['config']['attrirbute_name'] = value
...
end

cassandra.yaml.erb template resource would become:

variables node['cassandra']['config'].to_yaml

let me know if sound like a good idea.

michaelklishin commented 9 years ago

@vkhatri this sounds reasonable. We may have to bump the version to 4.0 if we introduce a breaking change like that.

vkhatri commented 9 years ago

@michaelklishin alright! i will get started.

michaelklishin commented 9 years ago

This is done in master.

michaelklishin commented 9 years ago

This change broke so many things (intentionally and unintentionally) so badly that I'm going to become a lot more conservative with sweeping changes like this.