From 142b1af0ccb4fdb65bcceb1540fcca49914a0a00 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Thu, 6 Jan 2022 12:24:42 -0600 Subject: [PATCH] Fix local exec pushes in server mode The local-exec push strategy was assuming it was running from a CLI and so it wouldn't be a big deal for it to straight up `exec` and replace its running with the user command. That command will just do its thing and we want the exit code for the CLI command to match anyways, right? Sure that works for a shell, but in a GRPC server setting it's decidedly Not Cool to suddenly swap out the running process! As you can imagine - the effect of doing this was all sorts of broken pipes and unexpected EOFs and a very confused @phinze. Luckily we had a subprocess strategy sitting right there for Windows compat, so it was just a matter of switching to that in the server context as well. Long and winding debugging process; simple fix; just another classic! --- plugins/pushes/local-exec/push.rb | 8 ++++++-- test/unit/plugins/pushes/local-exec/push_test.rb | 7 +++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/plugins/pushes/local-exec/push.rb b/plugins/pushes/local-exec/push.rb index 503957c9e..dcdf96319 100644 --- a/plugins/pushes/local-exec/push.rb +++ b/plugins/pushes/local-exec/push.rb @@ -7,6 +7,8 @@ require_relative "errors" module VagrantPlugins module LocalExecPush class Push < Vagrant.plugin("2", :push) + @@logger = Log4r::Logger.new("vagrant::push::local_exec") + def push if config.inline execute_inline!(config.inline, config.args) @@ -47,7 +49,7 @@ module VagrantPlugins # Execute the script, raising an exception if it fails. def execute!(*cmd) - if Vagrant::Util::Platform.windows? + if Vagrant::Util::Platform.windows? || Vagrant.server_mode? execute_subprocess!(*cmd) else execute_exec!(*cmd) @@ -63,11 +65,13 @@ module VagrantPlugins # Run the command as exec (unix). def execute_exec!(*cmd) + @@logger.debug("executing command via exec: #{cmd.inspect}") Vagrant::Util::SafeExec.exec(cmd[0], *cmd[1..-1]) end # Run the command as a subprocess (windows). def execute_subprocess!(*cmd) + @@logger.debug("executing command via subprocess: #{cmd.inspect}") cmd = cmd.dup << { notify: [:stdout, :stderr] } result = Vagrant::Util::Subprocess.execute(*cmd) do |type, data| if type == :stdout @@ -77,7 +81,7 @@ module VagrantPlugins end end - Kernel.exit(result.exit_code) + Kernel.exit(result.exit_code) unless Vagrant.server_mode? end end end diff --git a/test/unit/plugins/pushes/local-exec/push_test.rb b/test/unit/plugins/pushes/local-exec/push_test.rb index 00db78777..d03a7c2d5 100644 --- a/test/unit/plugins/pushes/local-exec/push_test.rb +++ b/test/unit/plugins/pushes/local-exec/push_test.rb @@ -122,5 +122,12 @@ describe VagrantPlugins::LocalExecPush::Push do expect(e).to be_a(SystemExit) } end + + it "uses subprocess when running in server mode, and does not exit" do + allow(Vagrant).to receive(:server_mode?).and_return(true) + result = double("result", exit_code: 0) + expect(Vagrant::Util::Subprocess).to receive(:execute).and_return(result) + expect { subject.execute! }.to_not raise_error + end end end