knife delete: show whether remote or local version had an error

2 files changed, 36 insertions, 25 deletions

diff --git a/lib/chef/knife/delete.rb b/lib/chef/knife/delete.rb
index c8ebb93f68..fb827c1143 100644
--- a/lib/chef/knife/delete.rb
+++ b/lib/chef/knife/delete.rb
@@ -33,65 +33,71 @@ class Chef
         end
 
         # Get the matches (recursively)
-        succeeded = true
+        error = false
         if config[:remote_only]
           pattern_args.each do |pattern|
             Chef::ChefFS::FileSystem.list(chef_fs, pattern) do |result|
-              if !delete_result(result)
-                succeeded = false
+              if delete_result(result)
+                error = true
               end
             end
           end
         elsif config[:local_only]
           pattern_args.each do |pattern|
             Chef::ChefFS::FileSystem.list(local_fs, pattern) do |result|
-              if !delete_result(result)
-                succeeded = false
+              if delete_result(result)
+                error = true
               end
             end
           end
         else
           pattern_args.each do |pattern|
             Chef::ChefFS::FileSystem.list_pairs(pattern, chef_fs, local_fs) do |chef_result, local_result|
-              if !delete_result(chef_result, local_result)
-                succeeded = false
+              if delete_result(chef_result, local_result)
+                error = true
               end
             end
           end
         end
 
-        if !succeeded
+        if error
           exit 1
         end
       end
 
+      def format_path_with_root(entry)
+        root = entry.root == chef_fs ? " (remote)" : " (local)"
+        "#{format_path(entry.path)}#{root}"
+      end
+
       def delete_result(*results)
         deleted_any = false
         found_any = false
-        errors = false
+        error = false
         results.each do |result|
           begin
             result.delete(config[:recurse])
             deleted_any = true
             found_any = true
           rescue Chef::ChefFS::FileSystem::NotFoundError
+            # This is not an error unless *all* of them were not found
           rescue Chef::ChefFS::FileSystem::MustDeleteRecursivelyError => e
-            ui.error "#{format_path(e.entry.path)} must be deleted recursively!  Pass -r to knife delete."
+            ui.error "#{format_path_with_root(e.entry)} must be deleted recursively!  Pass -r to knife delete."
             found_any = true
-            errors = true
+            error = true
           rescue Chef::ChefFS::FileSystem::OperationNotAllowedError => e
-            ui.error "#{format_path(e.entry.path)} #{e.reason}."
+            ui.error "#{format_path_with_root(e.entry)} #{e.reason}."
             found_any = true
-            errors = true
+            error = true
           end
         end
         if deleted_any
           output("Deleted #{format_path(results[0].path)}")
         elsif !found_any
           ui.error "#{format_path(results[0].path)}: No such file or directory"
-          errors = true
+          error = true
         end
-        !errors
+        error
       end
     end
   end
diff --git a/spec/integration/knife/delete_spec.rb b/spec/integration/knife/delete_spec.rb
index e2cf78ea52..7c36488e31 100644
--- a/spec/integration/knife/delete_spec.rb
+++ b/spec/integration/knife/delete_spec.rb
@@ -50,25 +50,30 @@ EOM
       directory 'users'
 
       it 'knife delete / fails' do
-        knife('delete /').should_fail "ERROR: / cannot be deleted.\nERROR: / cannot be deleted.\n"
+        knife('delete /').should_fail "ERROR: / (remote) cannot be deleted.\nERROR: / (local) cannot be deleted.\n"
         knife('list -Rf /').should_succeed everything
         knife('list -Rf --local /').should_succeed nothing
       end
 
       it 'knife delete -r /* fails' do
-        knife('delete -r --local-only /*').should_fail <<EOM
-ERROR: / cannot be deleted.
-ERROR: /cookbooks cannot be deleted.
-ERROR: /data_bags cannot be deleted.
-ERROR: /environments cannot be deleted.
-ERROR: /roles cannot be deleted.
+        knife('delete -r /*').should_fail <<EOM
+ERROR: / (remote) cannot be deleted.
+ERROR: / (local) cannot be deleted.
+ERROR: /cookbooks (remote) cannot be deleted.
+ERROR: /cookbooks (local) cannot be deleted.
+ERROR: /data_bags (remote) cannot be deleted.
+ERROR: /data_bags (local) cannot be deleted.
+ERROR: /environments (remote) cannot be deleted.
+ERROR: /environments (local) cannot be deleted.
+ERROR: /roles (remote) cannot be deleted.
+ERROR: /roles (local) cannot be deleted.
 EOM
         knife('list -Rf /').should_succeed everything
         knife('list -Rf --local /').should_succeed nothing
       end
 
       it 'knife delete /cookbooks/x fails' do
-        knife('delete /cookbooks/x').should_fail "ERROR: /cookbooks/x must be deleted recursively!  Pass -r to knife delete.\n"
+        knife('delete /cookbooks/x').should_fail "ERROR: /cookbooks/x (remote) must be deleted recursively!  Pass -r to knife delete.\n"
         knife('list -Rf /').should_succeed everything
         knife('list -Rf --local /').should_succeed nothing
       end
@@ -90,7 +95,7 @@ EOM
       end
 
       it 'knife delete /data_bags/x fails' do
-        knife('delete /data_bags/x').should_fail "ERROR: /data_bags/x must be deleted recursively!  Pass -r to knife delete.\n"
+        knife('delete /data_bags/x').should_fail "ERROR: /data_bags/x (remote) must be deleted recursively!  Pass -r to knife delete.\n"
         knife('list -Rf /').should_succeed everything
         knife('list -Rf --local /').should_succeed nothing
       end
@@ -146,7 +151,7 @@ EOM
       end
 
       it 'knife delete /environments/_default.json fails' do
-        knife('delete /environments/_default.json').should_fail "", :stderr => "ERROR: /environments/_default.json cannot be deleted (default environment cannot be modified).\n"
+        knife('delete /environments/_default.json').should_fail "", :stderr => "ERROR: /environments/_default.json (remote) cannot be deleted (default environment cannot be modified).\n"
         knife('list -Rf /').should_succeed everything
         knife('list -Rf --local /').should_succeed nothing
       end