Merge pull request #134 from fluent/check_with_dryrun

Check with dryrun

app/models/fluentd/agent/fluentd_gem.rb

@@ -42,6 +42,12 @@ class Fluentd
         actual_reload
       end
 
+      def dryrun
+        Bundler.with_clean_env do
+          system("fluentd -q --dry-run #{options_to_argv}")
+        end
+      end
+
       def version
         Bundler.with_clean_env do
           `fluentd --version`.strip
@@ -60,9 +66,7 @@ class Fluentd
       end
 
       def validate_fluentd_options
-        Bundler.with_clean_env do
-          system("fluentd --dry-run #{options_to_argv}")
-        end
+        dryrun
       end
 
       def actual_start

app/models/fluentd/agent/local_common.rb

@@ -133,11 +133,12 @@ class Fluentd
       end
 
       def detached_command(cmd)
-        Bundler.with_clean_env do
+        thread = Bundler.with_clean_env do
           pid = spawn(cmd)
           Process.detach(pid)
         end
-        sleep 1 # NOTE/FIXME: too early return will be caused incorrect status report, "sleep 1" is a adhoc hack
+        thread.join
+        thread.value.exitstatus.zero?
       end
     end
   end

app/models/fluentd/agent/td_agent/macosx.rb

@@ -2,7 +2,6 @@ class Fluentd
   class Agent
     class TdAgent
       module Macosx
-
         def start
           backup_running_config do
             detached_command("launchctl load #{plist}") && pid_from_launchctl
@@ -14,7 +13,11 @@ class Fluentd
         end
 
         def restart
-          stop && start
+          dryrun && stop && start
+        end
+
+        def dryrun
+          detached_command("/usr/sbin/td-agent --dry-run -q --use-v1-config -c #{config_file}")
         end
 
         private

app/models/fluentd/agent/td_agent/unix.rb

@@ -17,6 +17,10 @@ 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

spec/models/fluentd/agent_spec.rb

@@ -22,6 +22,7 @@ describe Fluentd::Agent do
     end
 
     describe "#start" do
+      before { instance.config_write "" } # ensure valid config
       before { instance.stub(:running?).and_return(running) }
 
       context "running" do
@@ -95,6 +96,29 @@ 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
@@ -106,6 +130,7 @@ describe Fluentd::Agent do
       before do
         instance.stub(:detached_command).and_return(true)
         instance.stub(:pid_from_launchctl).and_return(true)
+        instance.config_write "" # ensure valid config
       end
 
       after do
@@ -128,6 +153,30 @@ 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