Skip to content

Commit 88b71aa

Browse files
jrbeilkeandrewvc
jrbeilke
authored and
andrewvc
committed
Fix sniffing to only use master nodes
1 parent aea92b9 commit 88b71aa

File tree

6 files changed

+191
-35
lines changed

6 files changed

+191
-35
lines changed

Diff for: ‎ci/run.sh

-6
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,6 @@ else
123123
fi
124124

125125
case "$ES_VERSION" in
126-
6.2.*) # unreleased - use snapshot builds
127-
setup_es https://snapshots.elastic.co/downloads/elasticsearch/elasticsearch-${ES_VERSION}-SNAPSHOT.tar.gz https://snapshots.elastic.co/downloads/packs/x-pack/x-pack-${ES_VERSION}-SNAPSHOT.zip
128-
es_distribution_version=$(get_es_distribution_version)
129-
start_es
130-
bundle exec rspec -fd $extra_tag_args --tag update_tests:painless --tag update_tests:groovy --tag es_version:$es_distribution_version $spec_path
131-
;;
132126
6.*)
133127
setup_es https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-${ES_VERSION}.tar.gz https://artifacts.elastic.co/downloads/packs/x-pack/x-pack-${ES_VERSION}.zip
134128
es_distribution_version=$(get_es_distribution_version)

Diff for: ‎docs/index.asciidoc

+2-4
Original file line numberDiff line numberDiff line change
@@ -519,10 +519,8 @@ if enabled, script is in charge of creating non-existent document (scripted upda
519519
* Default value is `false`
520520

521521
This setting asks Elasticsearch for the list of all cluster nodes and adds them to the hosts list.
522-
Note: This will return ALL nodes with HTTP enabled (including master nodes!). If you use
523-
this with master nodes, you probably want to disable HTTP on them by setting
524-
`http.enabled` to false in their elasticsearch.yml. You can either use the `sniffing` option or
525-
manually enter multiple Elasticsearch hosts using the `hosts` parameter.
522+
For Elasticsearch 1.x and 2.x any nodes with `http.enabled` (on by default) will be added to the hosts list, including master-only nodes!
523+
For Elasticsearch 5.x and 6.x any nodes with `http.enabled` (on by default) will be added to the hosts list, excluding master-only nodes.
526524

527525
[id="plugins-{type}s-{plugin}-sniffing_delay"]
528526
===== `sniffing_delay`

Diff for: ‎lib/logstash/outputs/elasticsearch/http_client/pool.rb

+3
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,9 @@ def major_version(version_string)
186186

187187
def sniff_5x_and_above(nodes)
188188
nodes.map do |id,info|
189+
# Skip master-only nodes
190+
next if info["roles"] && info["roles"] == ["master"]
191+
189192
if info["http"]
190193
uri = LogStash::Util::SafeURI.new(info["http"]["publish_address"])
191194
end

Diff for: ‎spec/fixtures/_nodes/2x_1x.json

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
{
2+
"cluster_name" : "dev",
3+
"nodes" : {
4+
"Ur_68iBvTlm7Xr1HgSsh6w" : {
5+
"name" : "dev-es-master01",
6+
"transport_address" : "http://localhost:9200",
7+
"host" : "127.0.0.1",
8+
"ip" : "127.0.0.1",
9+
"version" : "2.4.6",
10+
"build" : "5376dca"
11+
},
12+
"sari4do3RG-mgh2CIZeHwA" : {
13+
"name" : "dev-es-data01",
14+
"transport_address" : "http://localhost:9201",
15+
"host" : "127.0.0.1",
16+
"ip" : "127.0.0.1",
17+
"version" : "2.4.6",
18+
"build" : "5376dca",
19+
"http_address" : "127.0.0.1:9201",
20+
"http" : {
21+
"bound_address" : [ "[::1]:9201", "127.0.0.1:9201" ],
22+
"publish_address" : "127.0.0.1:9201",
23+
"max_content_length_in_bytes" : 104857600
24+
}
25+
}
26+
}
27+
}

Diff for: ‎spec/fixtures/_nodes/5x_and_above.json

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
{
2+
"_nodes" : {
3+
"total" : 3,
4+
"successful" : 3,
5+
"failed" : 0
6+
},
7+
"cluster_name" : "dev",
8+
"nodes" : {
9+
"Ur_68iBvTlm7Xr1HgSsh6w" : {
10+
"name" : "dev-es-master01",
11+
"transport_address" : "http://localhost:9200",
12+
"host" : "localhost",
13+
"ip" : "127.0.0.1",
14+
"version" : "5.5.1",
15+
"build_hash" : "19c13d0",
16+
"roles" : [
17+
"master"
18+
],
19+
"http" : {
20+
"bound_address" : [
21+
"[::]:9200"
22+
],
23+
"publish_address" : "http://localhost:9200",
24+
"max_content_length_in_bytes" : 104857600
25+
}
26+
},
27+
"sari4do3RG-mgh2CIZeHwA" : {
28+
"name" : "dev-es-data01",
29+
"transport_address" : "http://localhost:9201",
30+
"host" : "localhost",
31+
"ip" : "127.0.0.1",
32+
"version" : "5.5.1",
33+
"build_hash" : "19c13d0",
34+
"roles" : [
35+
"data"
36+
],
37+
"http" : {
38+
"bound_address" : [
39+
"[::]:9200"
40+
],
41+
"publish_address" : "http://localhost:9201",
42+
"max_content_length_in_bytes" : 104857600
43+
}
44+
},
45+
"Rjy1WL66RHm4fyzXA8PCGQ" : {
46+
"name" : "dev-es-datamaster01",
47+
"transport_address" : "http://localhost:9202",
48+
"host" : "localhost",
49+
"ip" : "127.0.0.1",
50+
"version" : "5.5.1",
51+
"build_hash" : "19c13d0",
52+
"roles" : [
53+
"data",
54+
"master"
55+
],
56+
"http" : {
57+
"bound_address" : [
58+
"[::]:9200"
59+
],
60+
"publish_address" : "http://localhost:9202",
61+
"max_content_length_in_bytes" : 104857600
62+
}
63+
},
64+
"OguP_obcT_S9JYNB8SKKgQ" : {
65+
"name" : "dev-es-coordinator01",
66+
"transport_address" : "http://localhost:9203",
67+
"host" : "localhost",
68+
"ip" : "127.0.0.1",
69+
"version" : "5.5.1",
70+
"build_hash" : "19c13d0",
71+
"roles" : [ ],
72+
"http" : {
73+
"bound_address" : [
74+
"[::]:9200"
75+
],
76+
"publish_address" : "http://localhost:9203",
77+
"max_content_length_in_bytes" : 104857600
78+
}
79+
}
80+
}
81+
}

Diff for: ‎spec/integration/outputs/sniffer_spec.rb

+78-25
Original file line numberDiff line numberDiff line change
@@ -10,38 +10,91 @@
1010
let(:options) { {:resurrect_delay => 2, :url_normalizer => proc {|u| u}} } # Shorten the delay a bit to speed up tests
1111

1212
subject { LogStash::Outputs::ElasticSearch::HttpClient::Pool.new(logger, adapter, initial_urls, options) }
13-
14-
before do
15-
subject.start
16-
end
17-
18-
shared_examples("sniff parsing") do |check_exact|
19-
it "should execute a sniff without error" do
20-
expect { subject.check_sniff }.not_to raise_error
21-
end
2213

23-
it "should return the correct sniff URL list" do
24-
uris = subject.check_sniff
25-
26-
# ES 1.x returned the public hostname by default. This is hard to approximate
27-
# so for ES1.x we don't check the *exact* hostname
28-
if check_exact
29-
expect(uris).to include(::LogStash::Util::SafeURI.new("//#{get_host_port}"))
30-
else
14+
describe("Simple sniff parsing") do
15+
before(:each) { subject.start }
16+
17+
context "with single node" do
18+
it "should execute a sniff without error" do
19+
expect { subject.check_sniff }.not_to raise_error
20+
end
21+
22+
it "should return single sniff URL" do
23+
uris = subject.check_sniff
24+
3125
expect(uris.size).to eq(1)
3226
end
27+
28+
it "should return the correct sniff URL" do
29+
if ESHelper.es_version_satisfies?(">= 2")
30+
# We do a more thorough check on these versions because we can more reliably guess the ip
31+
uris = subject.check_sniff
32+
33+
expect(uris).to include(::LogStash::Util::SafeURI.new("//#{get_host_port}"))
34+
else
35+
# ES 1.x returned the public hostname by default. This is hard to approximate
36+
# so for ES1.x we don't check the *exact* hostname
37+
skip
38+
end
39+
end
3340
end
3441
end
35-
36-
describe("Simple sniff parsing") do
37-
include_examples("sniff parsing", false)
42+
43+
if ESHelper.es_version_satisfies?("<= 2")
44+
describe("Complex sniff parsing ES 2x/1x") do
45+
before(:each) do
46+
response_double = double("_nodes/http", body: File.read("spec/fixtures/_nodes/2x_1x.json"))
47+
allow(subject).to receive(:perform_request).and_return([nil, { version: "2.0" }, response_double])
48+
subject.start
49+
end
50+
51+
context "with multiple nodes but single http-enabled data node" do
52+
it "should execute a sniff without error" do
53+
expect { subject.check_sniff }.not_to raise_error
54+
end
55+
56+
it "should return one sniff URL" do
57+
uris = subject.check_sniff
58+
59+
expect(uris.size).to eq(1)
60+
end
61+
62+
it "should return the correct sniff URL" do
63+
if ESHelper.es_version_satisfies?(">= 2")
64+
# We do a more thorough check on these versions because we can more reliably guess the ip
65+
uris = subject.check_sniff
66+
67+
expect(uris).to include(::LogStash::Util::SafeURI.new("http://localhost:9201"))
68+
else
69+
# ES 1.x returned the public hostname by default. This is hard to approximate
70+
# so for ES1.x we don't check the *exact* hostname
71+
skip
72+
end
73+
end
74+
end
75+
end
3876
end
39-
40-
# We do a more thorough check on these versions because we can more reliably guess the ip
4177

42-
if ESHelper.es_version_satisfies?(">= 2")
43-
describe("Complex sniff parsing ES 6x/5x/2x") do
44-
include_examples("sniff parsing", true)
78+
if ESHelper.es_version_satisfies?(">= 5")
79+
describe("Complex sniff parsing ES 6x/5x") do
80+
before(:each) do
81+
response_double = double("_nodes/http", body: File.read("spec/fixtures/_nodes/5x_and_above.json"))
82+
allow(subject).to receive(:perform_request).and_return([nil, { version: "5.0" }, response_double])
83+
subject.start
84+
end
85+
86+
context "with mixed master-only, data-only, and data + master nodes" do
87+
it "should execute a sniff without error" do
88+
expect { subject.check_sniff }.not_to raise_error
89+
end
90+
91+
it "should return the correct sniff URLs" do
92+
# ie. without the master-only node
93+
uris = subject.check_sniff
94+
95+
expect(uris).to include(::LogStash::Util::SafeURI.new("http://localhost:9201"), ::LogStash::Util::SafeURI.new("http://localhost:9202"), ::LogStash::Util::SafeURI.new("http://localhost:9203"))
96+
end
97+
end
4598
end
4699
end
47100
end

0 commit comments

Comments
 (0)