From cc300d46a953e27b0d78f6f5f54b846fa074be94 Mon Sep 17 00:00:00 2001
From: uu59 <k@uu59.org>
Date: Wed, 4 Feb 2015 12:58:52 +0900
Subject: [PATCH 1/4] Agent#dryrun learned file_path argument for arbitrary
 config file checking

---
 app/models/fluentd/agent/fluentd_gem.rb | 24 +++++++++++++++++------
 spec/models/fluentd/agent_spec.rb       | 26 +++++++++++++++----------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/app/models/fluentd/agent/fluentd_gem.rb b/app/models/fluentd/agent/fluentd_gem.rb
index b1a6f7d..23dd608 100644
--- a/app/models/fluentd/agent/fluentd_gem.rb
+++ b/app/models/fluentd/agent/fluentd_gem.rb
@@ -42,12 +42,24 @@ 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}")
+          tmpfile = Tempfile.open("fluentd-dryrun-")
+          tmpfile.close
+          system("fluentd -q --dry-run #{options_to_argv(config_file: file_path)}", out: tmpfile.path, err: tmpfile.path)
+          result = $?.exitstatus.zero?
+          File.unlink(tmpfile.path)
+          result
         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,12 +68,12 @@ class Fluentd
 
       private
 
-      def options_to_argv
+      def options_to_argv(opts = {})
         argv = ""
         argv << " --use-v1-config"
-        argv << " -c #{config_file}"
-        argv << " -d #{pid_file}"
-        argv << " -o #{log_file}"
+        argv << " -c #{opts[:config_file] || config_file}"
+        argv << " -d #{opts[:pid_file] || pid_file}"
+        argv << " -o #{opts[:log_file] || log_file}"
         argv
       end
 
diff --git a/spec/models/fluentd/agent_spec.rb b/spec/models/fluentd/agent_spec.rb
index 4df7744..dfda10d 100644
--- a/spec/models/fluentd/agent_spec.rb
+++ b/spec/models/fluentd/agent_spec.rb
@@ -98,26 +98,32 @@ describe Fluentd::Agent do
     end
 
     describe "#dryrun" do
-      subject { instance.dryrun }
-
       describe "valid/invalid" do
-        before { instance.stub(:system).and_return(ret) }
+        let(:config_path) { Rails.root.join("tmp", "fluent-test.conf").to_s }
+        before { File.write(config_path, config) }
+        after { File.unlink(config_path) }
+
+        subject { instance.dryrun(config_path) }
 
         context "valid config" do
-          let(:ret) { true }
+          let(:config) { <<-CONF.strip_heredoc }
+          <source>
+            type forward
+          </source>
+          CONF
+
           it { should be_truthy }
         end
 
         context "invalid config" do
-          let(:ret) { false }
+          let(:config) { <<-CONF.strip_heredoc }
+          <source>
+            type forward
+          CONF
+
           it { should be_falsy }
         end
       end
-
-      it "invoke #system" do
-        instance.should_receive(:system).with(/--dry-run/)
-        subject
-      end
     end
   end
 

From f54a5a24ff12959a91c8502d8b4715a542d3f997 Mon Sep 17 00:00:00 2001
From: uu59 <k@uu59.org>
Date: Wed, 4 Feb 2015 14:27:21 +0900
Subject: [PATCH 2/4] Add dryrun button to setting#edit

---
 .../fluentd/settings_controller.rb            | 39 +++++++++++---
 app/models/fluentd/agent.rb                   |  1 +
 app/models/fluentd/agent/common.rb            |  4 ++
 app/models/fluentd/agent/fluentd_gem.rb       | 19 ++-----
 app/models/fluentd/agent/local_common.rb      | 16 ++++++
 app/models/fluentd/agent/td_agent.rb          |  7 +++
 app/models/fluentd/agent/td_agent/macosx.rb   |  4 --
 app/models/fluentd/agent/td_agent/unix.rb     |  4 --
 app/views/fluentd/settings/edit.html.haml     |  4 +-
 spec/models/fluentd/agent_spec.rb             | 53 -------------------
 spec/support/fluentd_agent_common_behavior.rb | 53 +++++++++++++++++++
 11 files changed, 116 insertions(+), 88 deletions(-)

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 23dd608..3be1c1a 100644
--- a/app/models/fluentd/agent/fluentd_gem.rb
+++ b/app/models/fluentd/agent/fluentd_gem.rb
@@ -42,14 +42,10 @@ class Fluentd
         actual_reload
       end
 
-      def dryrun(file_path = nil)
+      def dryrun!(file_path = nil)
         Bundler.with_clean_env do
-          tmpfile = Tempfile.open("fluentd-dryrun-")
-          tmpfile.close
-          system("fluentd -q --dry-run #{options_to_argv(config_file: file_path)}", out: tmpfile.path, err: tmpfile.path)
-          result = $?.exitstatus.zero?
-          File.unlink(tmpfile.path)
-          result
+          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
 
@@ -68,15 +64,6 @@ class Fluentd
 
       private
 
-      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
-
       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/spec/models/fluentd/agent_spec.rb b/spec/models/fluentd/agent_spec.rb
index dfda10d..a946ca7 100644
--- a/spec/models/fluentd/agent_spec.rb
+++ b/spec/models/fluentd/agent_spec.rb
@@ -96,35 +96,6 @@ describe Fluentd::Agent do
     describe "#restart" do
       it_should_behave_like "Restart strategy"
     end
-
-    describe "#dryrun" do
-      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) }
-
-        subject { instance.dryrun(config_path) }
-
-        context "valid config" do
-          let(:config) { <<-CONF.strip_heredoc }
-          <source>
-            type forward
-          </source>
-          CONF
-
-          it { should be_truthy }
-        end
-
-        context "invalid config" do
-          let(:config) { <<-CONF.strip_heredoc }
-          <source>
-            type forward
-          CONF
-
-          it { should be_falsy }
-        end
-      end
-    end
   end
 
   describe "TdAgent" do
@@ -159,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/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 }
+        <source>
+          type forward
+        </source>
+        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 }
+        <source>
+          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
 

From c1483f74bea52b3fdc0be8b5f63dab81253b2888 Mon Sep 17 00:00:00 2001
From: uu59 <k@uu59.org>
Date: Wed, 4 Feb 2015 15:17:07 +0900
Subject: [PATCH 3/4] Install td-agent2 package on CircleCI

---
 circle.yml | 5 +++++
 1 file changed, 5 insertions(+)

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

From a9a1dfdfa2ad94d8768b7dae6e2a7fecc8c8cb8f Mon Sep 17 00:00:00 2001
From: uu59 <k@uu59.org>
Date: Wed, 4 Feb 2015 16:02:26 +0900
Subject: [PATCH 4/4] Skip td-agent tests if td-agent doesn't installed

---
 spec/models/fluentd/agent_spec.rb | 2 +-
 spec/spec_helper.rb               | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/spec/models/fluentd/agent_spec.rb b/spec/models/fluentd/agent_spec.rb
index a946ca7..0baef63 100644
--- a/spec/models/fluentd/agent_spec.rb
+++ b/spec/models/fluentd/agent_spec.rb
@@ -98,7 +98,7 @@ describe Fluentd::Agent do
     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"
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