search

sous-chefs / elasticsearch

Development repository for the elasticsearch cookbook
https://supermarket.chef.io/cookbooks/elasticsearch
Other
881 stars 599 forks source link

Update es to 6.8.1 and 7.2.0 #721

Closed tmortensen closed 5 years ago

tmortensen commented 5 years ago

This adds support for 6.8.1 and 7.2.0 This also switches the default version back to 6.8.1 until all 7.x changes can be tested.

tmortensen commented 5 years ago

I can confirm the update in a417b62 was the only change I needed to make for 7.x support.

I know in the past there has been a new branch cut for each major ES version. I would be a fan of trying to move to a single cookbook that can manage multiple versions vs having to track multiple branches.

The changes for 7.x seem to be minor and are mainly around the change in package name.

In my wrapper cookbook I did need to make a change in the configuration for master nodes depending on version.

if node[:elasticsearch][:version] >= '7.0.0' es_config['cluster.initial_master_nodes'] = unicast_hosts(node).join(',') else es_config['discovery.zen.ping.unicast.hosts'] = unicast_hosts(node).join(',') end

vb-dave commented 5 years ago

Note that you generally can't do simple ">=" checks like this with version strings. In our wrapper cookbook we're using the following:

if Gem::Version.new(node['elasticsearch']['version']) >= Gem::Version.new('7.0')

tmortensen commented 5 years ago

@vb-dave What I posted above is what we are using with chef 12.x Is the gem way above for a newer version of chef?

rlvoyer commented 5 years ago

@tmortensen I'm looking to upgrade a chef-managed ES cluster to Elasticsearch 7.x. Anything I can do to help with this PR?

blaskojan commented 5 years ago

@tmortensen I've helped with your cookbook changes. It should work as we expect. I had to change log4j2 properties due to successful Elasticsearch start. Otherwise it failed.

https://github.com/tmortensen/cookbook-elasticsearch/pull/1

rlvoyer commented 5 years ago

Looks like the Travis CI build failed due to a reference to a nil property: node['elasticsearch']['version']. @blaskojan does this actually work for you?

blaskojan commented 5 years ago

Looks like the Travis CI build failed due to a reference to a nil property: node['elasticsearch']['version']. @blaskojan does this actually work for you?

Yep, it works. Travis CI has failed without my changes because in my commits there is no node['elasticsearch']['version'] but default['elasticsearch']['install']['version']. I am waiting for merge from @tmortensen

blaskojan commented 5 years ago

Pipeline has failed. I'll fix it tomorrow and send PR. Let you know.

blaskojan commented 5 years ago

We have prepared with my colleague @bliuchak changes that are being right finally. We'll prepare final PR.

bliuchak commented 5 years ago

I've added some chefspec tests for ES v7 here https://github.com/tmortensen/cookbook-elasticsearch/pull/3

@tmortensen Now Travis CI works fine.

martinb3 commented 5 years ago

Hi folks! I've taken some of the changes proposed here and committed them in 428504708. I'm going to close this one, but I'd welcome PRs for the 7.0 chefspec tests being discussed here. Thanks!