Merge pull request #2424 from opscode/sersut/chef-2356sersut/port-destdir-auto

:auto mode for :file_staging_uses_destdir

4 files changed, 69 insertions, 11 deletions

diff --git a/lib/chef/config.rb b/lib/chef/config.rb
index d3871c38e8..a47e42abda 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -560,10 +560,12 @@ class Chef
     # used to update files.
     default :file_atomic_update, true
 
-    # If false file staging is will be done via tempfiles that are
-    # created under ENV['TMP'] otherwise tempfiles will be created in
-    # the directory that files are going to reside.
-    default :file_staging_uses_destdir, true
+    # There are 3 possible values for this configuration setting.
+    # true => file staging is done in the destination directory
+    # false => file staging is done via tempfiles under ENV['TMP']
+    # :auto => file staging will try using destination directory if possible and
+    #   will fall back to ENV['TMP'] if destination directory is not usable.
+    default :file_staging_uses_destdir, :auto
 
     # Exit if another run is in progress and the chef-client is unable to
     # get the lock before time expires. If nil, no timeout is enforced. (Exits
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 25f08455fc..93fdd414e4 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -126,6 +126,13 @@ class Chef
 
     class CannotDetermineHomebrewOwner < Package; end
 
+    # Can not create staging file during file deployment
+    class FileContentStagingError < RuntimeError
+      def initialize(errors)
+        super "Staging tempfile can not be created during file deployment.\n Errors: #{errors.join('\n')}!"
+      end
+    end
+
     # A different version of a cookbook was added to a
     # VersionedRecipeList than the one already there.
     class CookbookVersionConflict < ArgumentError ; end
diff --git a/lib/chef/file_content_management/tempfile.rb b/lib/chef/file_content_management/tempfile.rb
index 61a5ce2a7c..2dde0ce21b 100644
--- a/lib/chef/file_content_management/tempfile.rb
+++ b/lib/chef/file_content_management/tempfile.rb
@@ -35,7 +35,22 @@ class Chef
       private
 
       def tempfile_open
-        tf = ::Tempfile.open(tempfile_basename, tempfile_dirname)
+        tf = nil
+        errors = [ ]
+
+        tempfile_dirnames.each do |tempfile_dirname|
+          begin
+            tf = ::Tempfile.open(tempfile_basename, tempfile_dirname)
+            break
+          rescue SystemCallError => e
+            message = "Creating temp file under '#{tempfile_dirname}' failed with: '#{e.message}'"
+            Chef::Log.debug(message)
+            errors << message
+          end
+        end
+
+        raise Chef::Exceptions::FileContentStagingError(errors) if tf.nil?
+
         # We always process the tempfile in binmode so that we
         # preserve the line endings of the content.
         tf.binmode
@@ -53,16 +68,29 @@ class Chef
         basename
       end
 
-      def tempfile_dirname
+      # Returns the possible directories for the tempfile to be created in.
+      def tempfile_dirnames
         # in why-run mode we need to create a Tempfile to compare against, which we will never
         # wind up deploying, but our enclosing directory for the destdir may not exist yet, so
         # instead we can reliably always create a Tempfile to compare against in Dir::tmpdir
-        if Chef::Config[:file_staging_uses_destdir] && !Chef::Config[:why_run]
-          ::File.dirname(@new_resource.path)
+        if Chef::Config[:why_run]
+          [ Dir.tmpdir ]
         else
-          Dir::tmpdir
+          case Chef::Config[:file_staging_uses_destdir]
+          when :auto
+            # In auto mode we try the destination directory first and fallback to ENV['TMP'] if
+            # that doesn't work.
+            [ ::File.dirname(@new_resource.path), Dir.tmpdir ]
+          when true
+            [ ::File.dirname(@new_resource.path) ]
+          when false
+            [ Dir.tmpdir ]
+          else
+            raise Chef::Exceptions::ConfigurationError, "Unknown setting '#{Chef::Config[:file_staging_uses_destdir]}' for Chef::Config[:file_staging_uses_destdir]. Possible values are :auto, true or false."
+          end
         end
       end
+
     end
   end
 end
diff --git a/spec/unit/provider/file/content_spec.rb b/spec/unit/provider/file/content_spec.rb
index 0f2fd6632c..4c942915ae 100644
--- a/spec/unit/provider/file/content_spec.rb
+++ b/spec/unit/provider/file/content_spec.rb
@@ -70,12 +70,34 @@ describe Chef::Provider::File::Content do
       expect(canonicalize_path(content.tempfile.path).start_with?(enclosing_directory)).to be_false
     end
 
-    it "returns a tempfile in the destdir when :file_desployment_uses_destdir is not set" do
+    it "returns a tempfile in the destdir when :file_deployment_uses_destdir is set" do
       Chef::Config[:file_staging_uses_destdir] = true
       expect(content.tempfile.path.start_with?(Dir::tmpdir)).to be_false
       expect(canonicalize_path(content.tempfile.path).start_with?(enclosing_directory)).to be_true
     end
 
+    context "when creating a tempfiles in destdir fails" do
+      let(:enclosing_directory) {
+        canonicalize_path("/nonexisting/path")
+      }
+
+      it "returns a tempfile in the tempdir when :file_deployment_uses_destdir is set to :auto" do
+        Chef::Config[:file_staging_uses_destdir] = :auto
+        expect(content.tempfile.path.start_with?(Dir::tmpdir)).to be_truthy
+        expect(canonicalize_path(content.tempfile.path).start_with?(enclosing_directory)).to be_falsey
+      end
+
+      it "fails when :file_desployment_uses_destdir is set" do
+        Chef::Config[:file_staging_uses_destdir] = true
+        expect{content.tempfile}.to raise_error
+      end
+
+      it "returns a tempfile in the tempdir when :file_desployment_uses_destdir is not set" do
+        expect(content.tempfile.path.start_with?(Dir::tmpdir)).to be_truthy
+        expect(canonicalize_path(content.tempfile.path).start_with?(enclosing_directory)).to be_falsey
+      end
+    end
+
   end
 
   describe "when the resource does not have a content attribute set" do
@@ -90,4 +112,3 @@ describe Chef::Provider::File::Content do
 
   end
 end
-