fix: remove the staged script when a command fails

lib/kitchen/docker/container/linux.rb

@@ -80,16 +80,20 @@ def execute(command)
           debug("Uploading temp file #{temp_file} to #{remote_path} on container")
           upload(temp_file, remote_path)
 
-          debug("Deleting temp file from local filesystem")
-          ::File.delete(temp_file)
-
           # Replace any environment variables used in the path and execute script file
           debug("Executing temp script #{remote_path}/#{filename} on container")
           remote_path = replace_env_variables(@config, remote_path)
 
           container_exec(@config, "/bin/bash #{remote_path}/#{filename}")
         rescue => e
           raise "Failed to execute command on Linux container. #{e}"
+        ensure
+          # Removed here rather than after the upload, so that a failure part
+          # way through does not leave the script behind in .kitchen/temp.
+          if temp_file && ::File.exist?(temp_file)
+            debug("Deleting temp file from local filesystem")
+            ::File.delete(temp_file)
+          end
         end
 
         protected

lib/kitchen/docker/container/windows.rb

@@ -66,9 +66,6 @@ def execute(command)
           debug("Uploading temp file #{temp_file} to #{remote_path} on container")
           upload(temp_file, remote_path)
 
-          debug("Deleting temp file from local filesystem")
-          ::File.delete(temp_file)
-
           # Replace any environment variables used in the path and execute script file
           debug("Executing temp script #{remote_path}\\#{filename} on container")
           remote_path = replace_env_variables(@config, remote_path)
@@ -77,6 +74,13 @@ def execute(command)
           container_exec(@config, cmd)
         rescue => e
           raise "Failed to execute command on Windows container. #{e}"
+        ensure
+          # Removed here rather than after the upload, so that a failure part
+          # way through does not leave the script behind in .kitchen/temp.
+          if temp_file && ::File.exist?(temp_file)
+            debug("Deleting temp file from local filesystem")
+            ::File.delete(temp_file)
+          end
         end
 
         protected

spec/linux_container_spec.rb

@@ -168,6 +168,51 @@ def port(output)
     end
   end
 
+  describe "#execute" do
+    # The command is staged as a script under .kitchen/temp and uploaded. That
+    # local copy has to go whether or not the rest of the run works, or a failing
+    # converge leaves a file behind on every attempt.
+    def container_in(dir, &upload)
+      c = container
+      allow(c).to receive(:create_dir_on_container)
+      allow(c).to receive(:replace_env_variables) { |_cfg, path| path }
+      allow(c).to receive(:container_exec).and_return("ok")
+      allow(c).to receive(:upload, &(upload || ->(*) { nil }))
+      allow(Dir).to receive(:pwd).and_return(dir)
+      c
+    end
+
+    around do |example|
+      Dir.chdir(@tmpdir) { example.run }
+    end
+
+    it "removes the staged script once the command has run" do
+      c = container_in(@tmpdir)
+      c.execute("echo hi")
+      expect(Dir.glob(".kitchen/temp/docker-*.sh")).to be_empty
+    end
+
+    it "removes the staged script when the upload fails" do
+      c = container_in(@tmpdir) { raise "upload exploded" }
+      expect { c.execute("echo hi") }.to raise_error(/Failed to execute command/)
+      expect(Dir.glob(".kitchen/temp/docker-*.sh")).to be_empty
+    end
+
+    it "removes the staged script when the command itself fails" do
+      c = container_in(@tmpdir)
+      allow(c).to receive(:container_exec).and_raise("command exploded")
+      expect { c.execute("echo hi") }.to raise_error(/Failed to execute command/)
+      expect(Dir.glob(".kitchen/temp/docker-*.sh")).to be_empty
+    end
+
+    it "uploads the command as the contents of the script" do
+      uploaded = nil
+      c = container_in(@tmpdir) { |local, _remote| uploaded = ::File.read(local) }
+      c.execute("echo hi")
+      expect(uploaded).to eq "echo hi"
+    end
+  end
+
   describe "#generate_keys" do
     it "writes a usable key pair when none exists" do
       private_key = File.join(@tmpdir, "generated")