add license check as a criteria of connection check #1035
Conversation
|
|
||
| def successful_connection? | ||
| !!maximum_seen_major_version | ||
| !!maximum_seen_major_version && alive_urls_count > 0 |
There was a problem hiding this comment.
once the license check passed, alive count +1
kares
left a comment
kares
left a comment
There was a problem hiding this comment.
👍 implementation of the bug fix simple
although we're relying on the fact that a "cluster_uuid" : "_na_" is followed by a failing (50X from /_license) license - in theory we could end up with "_na_" in plugin-metadata if ES recovery happened between the / and /_license check ... but I guess that's very low risk atm.
also left one suggestion whether we want to assert the cluster_uuid in the integration test
| subject.multi_receive([event1, event2]) | ||
| @es.indices.refresh | ||
| r = @es.search(index: 'logstash-*') | ||
| expect(r).to have_hits(2) |
There was a problem hiding this comment.
since the issue relates to reporting incorrect cluster_uuid
could we assert that subject.plugin_metadata.get(:cluster_uuid) is as expected (not empty and != "_na_")
dump version
|
You are right. The goal of the improvement is not to give an atomic solution that guarantees Thanks for reviewing! |
kares
left a comment
kares
left a comment
There was a problem hiding this comment.
LGTM, one suggestion about testing string equality properly
| r = @es.search(index: 'logstash-*') | ||
| expect(r).to have_hits(2) | ||
| expect(subject.plugin_metadata.get(:cluster_uuid)).not_to be_empty | ||
| expect(subject.plugin_metadata.get(:cluster_uuid)).not_to be("_na_") |
There was a problem hiding this comment.
| expect(subject.plugin_metadata.get(:cluster_uuid)).not_to be("_na_") | |
| expect(subject.plugin_metadata.get(:cluster_uuid)).not_to eq("_na_") |
…elasticsearch into fix_missing_post_register_setup
3 participants
Fixed: #1034
Logstash fails to do post-register (fetch
cluster_uuid, install index template, set up ilm) when Elasticsearch changes from unhealthy to healthy. Prior to the change, Logstash classifies a connection as alive when it fails to get license details. This commit enhances the connection check to include a license check, so post-register only execute once the license is validCI is red due to elasticsearch client gem issue.
steps to test