-
Notifications
You must be signed in to change notification settings - Fork 17
Update to add an instance_id attribute. #26
base: master
Are you sure you want to change the base?
Conversation
zookeeper V3.5.2-alpha dynamically generates a configuration file and updates the configuration file generated by chef to move the ensemble information to the dynamic file. In my chef cookbook, I will generate the dynamic file and stop this cookbook from generating a config file that zookeeper will update. I will do this by not setting the ensemble field. That works but the problem is with my_id.
…anceId Added instance_id.
libraries/zookeeper_config.rb
Outdated
@@ -34,7 +35,11 @@ class ZookeeperConfig < Chef::Resource | |||
attribute(:properties, option_collector: true, default: {}) | |||
|
|||
def myid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would implement this slightly differently. I would set the default value of the instance_id attribute to be the index of the element in the ensemble array. After that we simply add an alias to the method so that #myid
keeps on working.
attribute(:instance_id, kind_of: Integer, default: lazy { ensemble.index(instance_name).next.to_s })
alias_method :myid, :instance_id
…anceId Update to avoid constant zookeeper restart.
…ookeeper352Support Make sure to no include ensemble when using dynamic config.
…3-betterZookeeper352Support Revert "Make sure to no include ensemble when using dynamic config." MAVERICK
@johnbellone Did what you suggested... sorry it took so long. |
notifies :create, 'file[config_base]', :immediately | ||
end | ||
|
||
file 'config_base' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you creating a file called config_base
?
content new_resource.to_s | ||
path new_resource.path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah missed that, nevermind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain more detail why you specifically want the change to how the configuration is written, because I do not understand reasoning,
# this is needed to ensure chef won't constantly update | ||
# zoo.properties and force restarts of zookeeper when | ||
# it dynamically updates zoo.properties. | ||
file new_resource.path+".static" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the file continually updating? This mechanism won't change that it just makes it more obfuscated.
@johnbellone Thanks for taking a look. zookeeper 3.5.2 uses dynamic config. That is, at start up, it takes the defined zoo.properties file, removes the ensemble property, creates a new zoo.properties.dyanmic. file, and then adds a dynamicConfigFile property pointing to this file in the original zoo.properties. So when chef runs, it detects zoo.properties has changed, updates it, and restart zookeeper. zookeeper then once again goes through the process which then chef to reset/restart, and on, and on.... This change will only result in chef updating zoo.properties and restarting zookeeper when the cookbook changes the configuration. This is done by making the zoo.properties.static the file chef compares against. When it changes due to cookbook changes, chef rewrites zoo.properties.static, then zoo.properties, and finally, restarts zookeeper. zookeeper still uses zoo.properties as its configuration file (not zoo.properties.static) |
I think that given that new knowledge I would only write the configuration file if the configuration file does not exist. Otherwise, you almost always have the potential of overwriting the changes that the Zookeeper process has made, and this PR only really limits that to when you actually change the attributes inside of the cookbook/policy. The other alternative, but it is very much more invasive, is re-writing the resource to take into account the configuration on disk and merge in the attributes that are changed by the cookbook/policy. This is a much more advanced topic and we generally do not do this since most of what we control with Chef does not actually modify the configuration files at runtime. |
I don't follow. Are you suggesting only write the zoo.properties if it
doesn't exist? How do we ensure any cookbook updates make its way to the
zoo.properties file?
As coded here, when we overwrite the zoo.properties file (with the ensemble
property), zookeeper will regen the dynamic configuration file and update
zoo.properties. Seems OK to me.
|
How do you ensure that the cookbook updates do not conflict with the updates that the Zookeeper service made? I am also can't seem to find any documentation on the operations section of the Zookeeper documentation about writing configuration out from the service. Do you have a link so I can read up? |
Reading https://zookeeper.apache.org/doc/trunk/zookeeperReconfig.html implied
to me that the static configuration will not change except removing the
server list and pointing to the dynamic config file.
|
Yep, I just found that page. This seems to indicate that the dynamic configuration is stored in a different location. Does the regular configuration get rewritten? |
It looks like you can set this parameter |
Regular configuration does get rewritten once zookeeper starts and creates/updates dynamic config. I don't see how changing my approach to setting the |
@johnbellone ping |
There is nothing wrong with it per say, but it is the inverse of what the operations manual tells you to do, and we've been following the guidelines there for the configuration resource. Also, why would you add support to update the dynamic file? My understanding was that this cookbook would manage the static file, and Zookeeper would write dynamic file out to disk to the path that we tell it to? That means we add a new property on the resource, the file gets written out, and this resource only bounces the ZK task when the static file updates. Am I missing something? |
I may have misunderstood your suggestion. I thought you suggested to avoid zookeeper's dynamic config by having the cookbook create What I implemented here is intended to do exactly what you describe. |
zookeeper V3.5.2-alpha dynamically generates a configuration file and updates the configuration file generated by chef to move the ensemble information to the dynamic file. In my chef cookbook, I will generate the dynamic file and stop this cookbook from generating a config file that zookeeper will update. I will do this by not setting the ensemble field. That works but the problem is with my_id.