Merge pull request #4040 from chef/tm/live_execute_streaming

Implement live streaming for execute resources

3 files changed, 99 insertions, 36 deletions

diff --git a/spec/support/shared/unit/execute_resource.rb b/spec/support/shared/unit/execute_resource.rb
index e969a2ebd5..3a88ff8890 100644
--- a/spec/support/shared/unit/execute_resource.rb
+++ b/spec/support/shared/unit/execute_resource.rb
@@ -111,6 +111,11 @@ shared_examples_for "an execute resource" do
     expect(@resource.creates).to eql("something")
   end
 
+  it "should accept a boolean for live streaming" do
+    @resource.live_stream true
+    expect(@resource.live_stream).to be true
+  end
+
   describe "when it has cwd, environment, group, path, return value, and a user" do
     before do
       @resource.command("grep")
diff --git a/spec/unit/provider/execute_spec.rb b/spec/unit/provider/execute_spec.rb
index e7607d9417..3af35a17ca 100644
--- a/spec/unit/provider/execute_spec.rb
+++ b/spec/unit/provider/execute_spec.rb
@@ -25,28 +25,32 @@ describe Chef::Provider::Execute do
   let(:run_context) { Chef::RunContext.new(node, {}, events) }
   let(:provider) { Chef::Provider::Execute.new(new_resource, run_context) }
   let(:current_resource) { Chef::Resource::Ifconfig.new("foo_resource", run_context) }
+  # You will be the same object, I promise.
+  @live_stream = Chef::EventDispatch::EventsOutputStream.new(run_context.events, :name => :execute)
 
   let(:opts) do
     {
       timeout:      3600,
       returns:      0,
       log_level:    :info,
-      log_tag:      new_resource.to_s,
-      live_stream:  STDOUT,
+      log_tag:      new_resource.to_s
     }
   end
 
   let(:new_resource) { Chef::Resource::Execute.new("foo_resource", run_context) }
 
   before do
+    allow(Chef::EventDispatch::EventsOutputStream).to receive(:new) { @live_stream }
     allow(ChefConfig).to receive(:windows?) { false }
     @original_log_level = Chef::Log.level
     Chef::Log.level = :info
-    allow(STDOUT).to receive(:tty?).and_return(true)
+    allow(STDOUT).to receive(:tty?).and_return(false)
   end
 
   after do
     Chef::Log.level = @original_log_level
+    Chef::Config[:always_stream_execute] = false
+    Chef::Config[:daemon] = false
   end
 
   describe "#initialize" do
@@ -142,37 +146,6 @@ describe Chef::Provider::Execute do
       expect(new_resource).not_to be_updated
     end
 
-    it "should unset the live_stream if STDOUT is not a tty" do
-      expect(STDOUT).to receive(:tty?).and_return(false)
-      opts.delete(:live_stream)
-      expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
-      expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
-      expect(Chef::Log).not_to receive(:warn)
-      provider.run_action(:run)
-      expect(new_resource).to be_updated
-    end
-
-    it "should unset the live_stream if chef is running as a daemon" do
-      allow(Chef::Config).to receive(:[]).and_call_original
-      expect(Chef::Config).to receive(:[]).with(:daemon).and_return(true)
-      opts.delete(:live_stream)
-      expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
-      expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
-      expect(Chef::Log).not_to receive(:warn)
-      provider.run_action(:run)
-      expect(new_resource).to be_updated
-    end
-
-    it "should unset the live_stream if we are not running with a log level of at least :info" do
-      expect(Chef::Log).to receive(:info?).and_return(false)
-      opts.delete(:live_stream)
-      expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
-      expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
-      expect(Chef::Log).not_to receive(:warn)
-      provider.run_action(:run)
-      expect(new_resource).to be_updated
-    end
-
     it "should not include stdout/stderr in failure exception for sensitive resource" do
       opts.delete(:live_stream)
       new_resource.sensitive true
@@ -181,5 +154,90 @@ describe Chef::Provider::Execute do
         provider.run_action(:run)
       end.to raise_error(Mixlib::ShellOut::ShellCommandFailed, /suppressed for sensitive resource/)
     end
+
+    describe "streaming output" do
+      it "should not set the live_stream if sensitive is on" do
+        new_resource.sensitive true
+        expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+        expect(provider).to receive(:converge_by).with("execute sensitive resource").and_call_original
+        expect(Chef::Log).not_to receive(:warn)
+        provider.run_action(:run)
+        expect(new_resource).to be_updated
+      end
+
+      describe "with an output formatter listening" do
+        let(:events) { d = Chef::EventDispatch::Dispatcher.new; d.register(Chef::Formatters::Doc.new(StringIO.new, StringIO.new)); d }
+
+        before do
+          Chef::Config[:stream_execute_output] = true
+        end
+
+        it "should set the live_stream if the log level is info or above" do
+          nopts = opts
+          nopts[:live_stream] = @live_stream
+          expect(provider).to receive(:shell_out!).with(new_resource.name, nopts)
+          expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
+          expect(Chef::Log).not_to receive(:warn)
+          provider.run_action(:run)
+          expect(new_resource).to be_updated
+        end
+
+        it "should set the live_stream if the resource requests live streaming" do
+          Chef::Log.level = :warn
+          new_resource.live_stream true
+          nopts = opts
+          nopts[:live_stream] = @live_stream
+          expect(provider).to receive(:shell_out!).with(new_resource.name, nopts)
+          expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
+          expect(Chef::Log).not_to receive(:warn)
+          provider.run_action(:run)
+          expect(new_resource).to be_updated
+        end
+
+        it "should not set the live_stream if the resource is sensitive" do
+          new_resource.sensitive true
+          expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+          expect(provider).to receive(:converge_by).with("execute sensitive resource").and_call_original
+          expect(Chef::Log).not_to receive(:warn)
+          provider.run_action(:run)
+          expect(new_resource).to be_updated
+        end
+      end
+
+      describe "with only logging enabled" do
+        it "should set the live_stream to STDOUT if we are a TTY, not daemonized, not sensitive, and info is enabled" do
+          nopts = opts
+          nopts[:live_stream] = STDOUT
+          allow(STDOUT).to receive(:tty?).and_return(true)
+          expect(provider).to receive(:shell_out!).with(new_resource.name, nopts)
+          expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
+          expect(Chef::Log).not_to receive(:warn)
+          provider.run_action(:run)
+          expect(new_resource).to be_updated
+        end
+
+        it "should not set the live_stream to STDOUT if we are a TTY, not daemonized, but sensitive" do
+          new_resource.sensitive true
+          allow(STDOUT).to receive(:tty?).and_return(true)
+          expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+          expect(provider).to receive(:converge_by).with("execute sensitive resource").and_call_original
+          expect(Chef::Log).not_to receive(:warn)
+          provider.run_action(:run)
+          expect(new_resource).to be_updated
+        end
+
+        it "should not set the live_stream to STDOUT if we are a TTY, but daemonized" do
+          Chef::Config[:daemon] = true
+          allow(STDOUT).to receive(:tty?).and_return(true)
+          expect(provider).to receive(:shell_out!).with(new_resource.name, opts)
+          expect(provider).to receive(:converge_by).with("execute foo_resource").and_call_original
+          expect(Chef::Log).not_to receive(:warn)
+          provider.run_action(:run)
+          expect(new_resource).to be_updated
+        end
+
+      end
+    end
+
   end
 end
diff --git a/spec/unit/provider/script_spec.rb b/spec/unit/provider/script_spec.rb
index d1759981aa..7cc5abbd15 100644
--- a/spec/unit/provider/script_spec.rb
+++ b/spec/unit/provider/script_spec.rb
@@ -88,11 +88,11 @@ describe Chef::Provider::Script, "action_run" do
 
     describe "when running the script" do
       let (:default_opts) {
-        {timeout: 3600, returns: 0, log_level: :info, log_tag: "script[run some perl code]", live_stream: STDOUT}
+        { timeout: 3600, returns: 0, log_level: :info, log_tag: "script[run some perl code]" }
       }
 
       before do
-        allow(STDOUT).to receive(:tty?).and_return(true)
+        allow(STDOUT).to receive(:tty?).and_return(false)
       end
 
       it 'should set the command to "interpreter"  "tempfile"' do