diff --git a/app/controllers/fluentd/settings_controller.rb b/app/controllers/fluentd/settings_controller.rb index 84f56be..96fe0da 100644 --- a/app/controllers/fluentd/settings_controller.rb +++ b/app/controllers/fluentd/settings_controller.rb @@ -17,14 +17,24 @@ class Fluentd::SettingsController < ApplicationController end def update - Fluent::Config::V1Parser.parse(params[:config], @fluentd.config_file) - @fluentd.agent.config_write params[:config] - @fluentd.agent.restart if @fluentd.agent.running? - redirect_to daemon_setting_path(@fluentd) - rescue Fluent::ConfigParseError => e - @config = params[:config] - @error = e.message - render "edit" + if params[:dryrun] + if dryrun(params[:config]) + flash.now[:success] = I18n.t('messages.dryrun_is_passed') + else + flash.now[:danger] = @fluentd.agent.last_error_message + end + @config = params[:config] + render "edit" + else + begin + update_config(params[:config]) + redirect_to daemon_setting_path(@fluentd) + rescue Fluent::ConfigParseError => e + @config = params[:config] + flash.now[:danger] = e.message + render "edit" + end + end end def source_and_output @@ -37,4 +47,17 @@ class Fluentd::SettingsController < ApplicationController def set_config @config = @fluentd.agent.config end + + def dryrun(conf) + tmpfile = Tempfile.open("fluentd-test-config") + tmpfile.write params[:config] + tmpfile.close + @fluentd.agent.dryrun(tmpfile.path) + end + + def update_config(conf) + Fluent::Config::V1Parser.parse(conf, @fluentd.config_file) + @fluentd.agent.config_write conf + @fluentd.agent.restart if @fluentd.agent.running? + end end diff --git a/app/models/fluentd/agent.rb b/app/models/fluentd/agent.rb index 175c992..fe58d3c 100644 --- a/app/models/fluentd/agent.rb +++ b/app/models/fluentd/agent.rb @@ -23,5 +23,6 @@ class Fluentd # td-agent: /etc/td-agent/td-agent.conf # - https://github.com/treasure-data/td-agent/blob/master/debian/td-agent.postinst#L69 # fluentd: /etc/fluent/fluent.conf (by fluentd -s) + class ConfigError < StandardError; end end end diff --git a/app/models/fluentd/agent/common.rb b/app/models/fluentd/agent/common.rb index 1b2609a..f2c83d0 100644 --- a/app/models/fluentd/agent/common.rb +++ b/app/models/fluentd/agent/common.rb @@ -45,6 +45,10 @@ class Fluentd errors end + def last_error_message + recent_errors(1).first.try(:[], :subject) || "" + end + def pid_file extra_options[:pid_file] || self.class.default_options[:pid_file] end diff --git a/app/models/fluentd/agent/fluentd_gem.rb b/app/models/fluentd/agent/fluentd_gem.rb index b1a6f7d..3be1c1a 100644 --- a/app/models/fluentd/agent/fluentd_gem.rb +++ b/app/models/fluentd/agent/fluentd_gem.rb @@ -42,12 +42,20 @@ class Fluentd actual_reload end - def dryrun + def dryrun!(file_path = nil) Bundler.with_clean_env do - system("fluentd -q --dry-run #{options_to_argv}") + system("fluentd -q --dry-run #{options_to_argv(config_file: file_path)}", out: File::NULL, err: File::NULL) + raise ::Fluentd::Agent::ConfigError, last_error_message unless $?.exitstatus.zero? end end + def config_syntax_check + Fluent::Config::V1Parser.parse(params[:config], config_file) + true + rescue Fluent::ConfigParseError + false + end + def version Bundler.with_clean_env do `fluentd --version`.strip @@ -56,15 +64,6 @@ class Fluentd private - def options_to_argv - argv = "" - argv << " --use-v1-config" - argv << " -c #{config_file}" - argv << " -d #{pid_file}" - argv << " -o #{log_file}" - argv - end - def validate_fluentd_options dryrun end diff --git a/app/models/fluentd/agent/local_common.rb b/app/models/fluentd/agent/local_common.rb index 046dcd1..9b65971 100644 --- a/app/models/fluentd/agent/local_common.rb +++ b/app/models/fluentd/agent/local_common.rb @@ -73,6 +73,13 @@ class Fluentd backup_files_in_old_order.reverse end + def dryrun(file_path = nil) + dryrun!(file_path) + true + rescue ::Fluentd::Agent::ConfigError + false + end + private def backup_config @@ -173,6 +180,15 @@ class Fluentd thread.join thread.value.exitstatus.zero? end + + def options_to_argv(opts = {}) + argv = "" + argv << " --use-v1-config" + argv << " -c #{opts[:config_file] || config_file}" + argv << " -d #{opts[:pid_file] || pid_file}" + argv << " -o #{opts[:log_file] || log_file}" + argv + end end end end diff --git a/app/models/fluentd/agent/td_agent.rb b/app/models/fluentd/agent/td_agent.rb index 88977a2..5c1c422 100644 --- a/app/models/fluentd/agent/td_agent.rb +++ b/app/models/fluentd/agent/td_agent.rb @@ -16,6 +16,13 @@ class Fluentd `/usr/sbin/td-agent --version`.strip end + def dryrun!(file_path = nil) + Bundler.with_clean_env do + system("/usr/sbin/td-agent --dry-run #{options_to_argv(config_file: file_path)}", out: File::NULL, err: File::NULL) + raise ::Fluentd::Agent::ConfigError, last_error_message unless $?.exitstatus.zero? + end + end + case FluentdUI.platform when :macosx include Macosx diff --git a/app/models/fluentd/agent/td_agent/macosx.rb b/app/models/fluentd/agent/td_agent/macosx.rb index f782f52..67dc070 100644 --- a/app/models/fluentd/agent/td_agent/macosx.rb +++ b/app/models/fluentd/agent/td_agent/macosx.rb @@ -16,10 +16,6 @@ class Fluentd dryrun && stop && start end - def dryrun - detached_command("/usr/sbin/td-agent --dry-run -q --use-v1-config -c #{config_file}") - end - private def plist diff --git a/app/models/fluentd/agent/td_agent/unix.rb b/app/models/fluentd/agent/td_agent/unix.rb index db5ccc3..09f6353 100644 --- a/app/models/fluentd/agent/td_agent/unix.rb +++ b/app/models/fluentd/agent/td_agent/unix.rb @@ -17,10 +17,6 @@ class Fluentd # https://github.com/treasure-data/td-agent/blob/master/debian/td-agent.init#L156 detached_command('/etc/init.d/td-agent restart') end - - def dryrun - detached_command('/etc/init.d/td-agent configtest') - end end end end diff --git a/app/views/fluentd/settings/edit.html.haml b/app/views/fluentd/settings/edit.html.haml index a06924d..3783a65 100644 --- a/app/views/fluentd/settings/edit.html.haml +++ b/app/views/fluentd/settings/edit.html.haml @@ -1,10 +1,8 @@ - page_title t('.page_title', label: @fluentd.label) -- if @error - %pre.alert.alert-danger= @error - = form_tag(daemon_setting_path(@fluentd), method: :patch) do .form-group = text_area_tag "config", @config, class: "form-control", rows: 40, class: "js-fluentd-config-editor" %p.text.text-danger= t('terms.notice_restart_for_config_edit', brand: fluentd_ui_brand) = submit_tag t("terms.update"), class: "btn btn-primary" + = submit_tag t("terms.configtest"), class: "btn btn-default", name: "dryrun" diff --git a/circle.yml b/circle.yml index 7141315..546c0fa 100644 --- a/circle.yml +++ b/circle.yml @@ -6,3 +6,8 @@ checkout: post: - mkdir -p tmp - sync + +dependencies: + pre: + - wget http://packages.treasuredata.com.s3.amazonaws.com/2/ubuntu/precise/pool/contrib/t/td-agent/td-agent_2.1.3-0_amd64.deb + - sudo dpkg -i td-agent_2.1.3-0_amd64.deb diff --git a/spec/models/fluentd/agent_spec.rb b/spec/models/fluentd/agent_spec.rb index 4df7744..0baef63 100644 --- a/spec/models/fluentd/agent_spec.rb +++ b/spec/models/fluentd/agent_spec.rb @@ -96,32 +96,9 @@ describe Fluentd::Agent do describe "#restart" do it_should_behave_like "Restart strategy" end - - describe "#dryrun" do - subject { instance.dryrun } - - describe "valid/invalid" do - before { instance.stub(:system).and_return(ret) } - - context "valid config" do - let(:ret) { true } - it { should be_truthy } - end - - context "invalid config" do - let(:ret) { false } - it { should be_falsy } - end - end - - it "invoke #system" do - instance.should_receive(:system).with(/--dry-run/) - subject - end - end end - describe "TdAgent" do + describe "TdAgent", td_agent_required: true do let(:described_class) { Fluentd::Agent::TdAgent } # override nested described_class behavior as https://github.com/rspec/rspec-core/issues/1114 it_should_behave_like "Fluentd::Agent has common behavior" @@ -153,30 +130,6 @@ describe Fluentd::Agent do expect(File.read(backup_file)).to eq File.read(instance.config_file) end end - - describe "#dryrun" do - subject { instance.dryrun } - - describe "valid/invalid" do - before { instance.stub(:detached_command).and_return(ret) } - - context "valid config" do - let(:ret) { true } - it { should be_truthy } - end - - context "invalid config" do - let(:ret) { false } - it { should be_falsy } - end - end - - it "invoke #system" do - # --dry-run check on Mac, configtest for Unix - instance.should_receive(:detached_command).with(/(--dry-run|configtest)/) - subject - end - end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d315e94..e4c349f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -62,6 +62,12 @@ RSpec.configure do |config| # rspec 2.99 config.infer_spec_type_from_file_location! + unless File.directory?("/opt/td-agent") + # including td-agent specific tests, so some tests will fail if the system has no td-agent + warn "\n\nSkipping td-agent specific tests (system has no td-agent)\n\n" + config.filter_run_excluding :td_agent_required => true + end + config.after(:suite) do FileUtils.rm_rf FluentdUI.data_dir end diff --git a/spec/support/fluentd_agent_common_behavior.rb b/spec/support/fluentd_agent_common_behavior.rb index 30b52a7..b485ddb 100644 --- a/spec/support/fluentd_agent_common_behavior.rb +++ b/spec/support/fluentd_agent_common_behavior.rb @@ -124,5 +124,58 @@ shared_examples_for "Fluentd::Agent has common behavior" do |klass| end end end + + describe "#dryrun" do + let(:root) { FluentdUI.data_dir + "/tmp/agentspec/" } + let(:dummy_log_file) { root + "dummy.log" } + let(:dummy_pid_file) { root + "dummy.pid" } + + before do + FileUtils.mkdir_p root + instance.stub(:log_file).and_return(dummy_log_file) + instance.stub(:pid_file).and_return(dummy_pid_file) + end + + describe "valid/invalid" do + let(:config_path) { Rails.root.join("tmp", "fluent-test.conf").to_s } + before { File.write(config_path, config) } + after { File.unlink(config_path) } + + context "valid config" do + let(:config) { <<-CONF.strip_heredoc } + + type forward + + CONF + + context "with `!`" do + subject { instance.dryrun!(config_path) } + it { expect { subject }.to_not raise_error } + end + + context "without `!`" do + subject { instance.dryrun(config_path) } + it { should be_truthy } + end + end + + context "invalid config" do + let(:config) { <<-CONF.strip_heredoc } + + type forward + CONF + + context "with `!`" do + subject { instance.dryrun!(config_path) } + it { expect { subject }.to raise_error(Fluentd::Agent::ConfigError) } + end + + context "without `!`" do + subject { instance.dryrun(config_path) } + it { should be_falsy } + end + end + end + end end