Skip to content

Commit

Permalink
Convert Exception during converge to a failed action (elastic#13969)
Browse files Browse the repository at this point in the history
Certain errors during pipeline converge will emit an IllegalStateException - for example attempting
to reference an environment variable that does not exist. Attempting to add a java exception to the
converge result would result in an uncaught exception in a pipeline thread leading to an unclean
shutdown.

This had the effect of,prior to this commit, Logstash behaviour varying depending on the class of
error that caused a pipeline not to start - an invalid pipeline configuration would still enable
logstash to start other pipelines in a multiple pipeline configuration, or automatic reloading
enabling changes to the configuration to allow the pipeline to start(in multi- and single pipeline
configurations).
However, a missing environment variable would cause Logstash to crash hard no matter what.

This was discovered when updating to jruby-9.3.3.0, when new clean up code joining existing
threads to perform a graceful shutdown was stuck indefinitely, due to the webserver shutdown
code not being called, due to the unclean shutdown.
  • Loading branch information
robbavey authored Apr 12, 2022
1 parent 5197c85 commit 339a67f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/agent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ def converge_state(pipeline_actions)
end
rescue SystemExit, Exception => e
logger.error("Failed to execute action", :action => action, :exception => e.class.name, :message => e.message, :backtrace => e.backtrace)
converge_result.add(action, e)
converge_result.add(action, LogStash::ConvergeResult::FailedAction.from_exception(e))
end
end
end.each(&:join)
Expand Down
16 changes: 16 additions & 0 deletions logstash-core/spec/logstash/agent_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,13 @@
expect(json_document["message"]).to eq("foo-bar")
end
end

context "referenced environment variable does not exist" do

it "does not converge the pipeline" do
expect(subject.converge_state_and_update).not_to be_a_successful_converge
end
end
end

describe "#upgrade_pipeline" do
Expand All @@ -304,6 +311,15 @@
subject.shutdown
end

context "when the upgrade contains a bad environment variable" do
let(:new_pipeline_config) { "input { generator {} } filter { if '${NOEXIST}' { mutate { add_tag => 'x' } } } output { }" }

it "leaves the state untouched" do
expect(subject.converge_state_and_update).not_to be_a_successful_converge
expect(subject.get_pipeline(default_pipeline_id).config_str).to eq(pipeline_config)
end
end

context "when the upgrade fails" do
it "leaves the state untouched" do
expect(subject.converge_state_and_update).not_to be_a_successful_converge
Expand Down

0 comments on commit 339a67f

Please sign in to comment.