File atomic update default: false on docker files

If unspecified, the default setting for atomic updates on docker special
files should be 'false' to avoid Errno::EBUSY.  This allows for a cookbook
author to override this behaviour if desired.

3 files changed, 21 insertions, 14 deletions

diff --git a/lib/chef/dsl/platform_introspection.rb b/lib/chef/dsl/platform_introspection.rb
index 276a03af63..679b702383 100644
--- a/lib/chef/dsl/platform_introspection.rb
+++ b/lib/chef/dsl/platform_introspection.rb
@@ -245,6 +245,19 @@ class Chef
         end
       end
 
+      # Shamelessly stolen from https://github.com/sethvargo/chef-sugar/blob/master/lib/chef/sugar/docker.rb
+      # Given a node object, returns whether the node is a docker container.
+      #
+      # === Parameters
+      # node:: [Chef::Node] The node to check.
+      #
+      # === Returns
+      # true:: if the current node is a docker container
+      # false:: if the current node is not a docker container
+      def docker?(node)
+        ::File.exist?('/.dockerinit') || ::File.exist?('/.dockerenv')
+      end
+
     end
   end
 end
diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb
index 8b3884f710..7f85085eeb 100644
--- a/lib/chef/provider/file.rb
+++ b/lib/chef/provider/file.rb
@@ -67,13 +67,7 @@ class Chef
       def initialize(new_resource, run_context)
         @content_class ||= Chef::Provider::File::Content
         if new_resource.respond_to?(:atomic_update)
-          # Resolves https://github.com/chef/chef/issues/4949
-          if docker_guest?(run_context.node) && (new_resource.path == "/etc/resolv.conf" ||
-                                                 new_resource.path == "/etc/hostname")
-            @deployment_strategy = Chef::FileContentManagement::Deploy.strategy(false)
-          else
-            @deployment_strategy = Chef::FileContentManagement::Deploy.strategy(new_resource.atomic_update)
-          end
+          @deployment_strategy = Chef::FileContentManagement::Deploy.strategy(new_resource.atomic_update)
         end
         super
       end
@@ -199,12 +193,6 @@ class Chef
 
       private
 
-      # Return true if we are running in a docker guest, or false otherwise.
-      def docker_guest?(node)
-        node[:virtualization] && node[:virtualization][:systems] &&
-          node[:virtualization][:systems][:docker] && node[:virtualization][:systems][:docker] == "guest"
-      end
-
       # What to check in this resource to see if we're going to be actively managing
       # content (for things like doing checksums in load_current_resource).  Expected to
       # be overridden in subclasses.
diff --git a/lib/chef/resource/file.rb b/lib/chef/resource/file.rb
index ac6dc5fbdb..27a867d869 100644
--- a/lib/chef/resource/file.rb
+++ b/lib/chef/resource/file.rb
@@ -21,6 +21,7 @@ require "chef/resource"
 require "chef/platform/query_helpers"
 require "chef/mixin/securable"
 require "chef/resource/file/verification"
+require "pathname"
 
 class Chef
   class Resource
@@ -49,7 +50,8 @@ class Chef
       allowed_actions :create, :delete, :touch, :create_if_missing
 
       property :path, String, name_property: true, identity: true
-      property :atomic_update, [ true, false ], desired_state: false, default: lazy { Chef::Config[:file_atomic_update] }
+      property :atomic_update, [ true, false ], desired_state: false,
+        default: lazy { |r| r.docker?(r.node) && r.special_docker_files?(r.path) ? false : Chef::Config[:file_atomic_update] }
       property :backup, [ Integer, false ], desired_state: false, default: 5
       property :checksum, [ /^[a-zA-Z0-9]{64}$/, nil ]
       property :content, [ String, nil ], desired_state: false
@@ -78,6 +80,10 @@ class Chef
         end
         state_attrs
       end
+
+      def special_docker_files?(file)
+        %w(/etc/hostname /etc/resolv.conf).include?(Pathname(file).cleanpath.to_path)
+      end
     end
   end
 end