Fixing CHEF-749, we now properly cache cookbooks again.

6 files changed, 63 insertions, 15 deletions

diff --git a/Rakefile b/Rakefile
index ddd4227462..a51b62586c 100644
--- a/Rakefile
+++ b/Rakefile
@@ -338,6 +338,10 @@ namespace :features do
     Cucumber::Rake::Task.new(:run_interval) do |t|
       t.profile = "client_run_interval"
     end
+
+    Cucumber::Rake::Task.new(:cookbook_sync) do |t|
+      t.profile = "client_cookbook_sync"
+    end
   end
 
   desc "Run cucumber tests for the cookbooks"
diff --git a/chef/lib/chef/client.rb b/chef/lib/chef/client.rb
index 5469445e90..b5fae2cc90 100644
--- a/chef/lib/chef/client.rb
+++ b/chef/lib/chef/client.rb
@@ -223,14 +223,15 @@ class Chef
             current_checksum = checksum(Chef::FileCache.load(cache_file, false))
           end
 
-          rf_url = generate_cookbook_url(
-            rf['name'], 
-            cookbook_name, 
-            segment, 
-            @node, 
-            current_checksum ? { 'checksum' => current_checksum } : nil
-          )
           if current_checksum != rf['checksum']
+            rf_url = generate_cookbook_url(
+              rf['name'], 
+              cookbook_name, 
+              segment, 
+              @node, 
+              current_checksum ? { 'checksum' => current_checksum } : nil
+            )
+
             changed = true
             begin
               raw_file = @rest.get_rest(rf_url, true)
@@ -250,15 +251,15 @@ class Chef
           end
         end
 
-        Chef::FileCache.list.each do |cache_file|
-          if cache_file =~ /^cookbooks\/(recipes|attributes|definitions|libraries)\//
-            unless file_canonical[cache_file]
-              Chef::Log.info("Removing #{cache_file} from the cache; it is no longer on the server.")
-              Chef::FileCache.delete(cache_file)
-            end
+      end
+
+      Chef::FileCache.list.each do |cache_file|
+        if cache_file =~ /^cookbooks\/#{cookbook_name}\/(recipes|attributes|definitions|libraries|resources|providers)\//
+          unless file_canonical[cache_file]
+            Chef::Log.info("Removing #{cache_file} from the cache; it is no longer on the server.")
+            Chef::FileCache.delete(cache_file)
           end
         end
-
       end
       
     end
@@ -271,6 +272,14 @@ class Chef
       Chef::Log.debug("Synchronizing cookbooks")
       cookbook_hash = @rest.get_rest("nodes/#{@safe_name}/cookbooks")
       Chef::Log.debug("Cookbooks to load: #{cookbook_hash.inspect}")
+      Chef::FileCache.list.each do |cache_file|
+        if cache_file =~ /^cookbooks\/(.+?)\//
+          unless cookbook_hash.has_key?($1)
+            Chef::Log.info("Removing #{cache_file} from the cache; it's cookbook is no longer needed on this client.")
+            Chef::FileCache.delete(cache_file) 
+          end
+        end
+      end
       cookbook_hash.each do |cookbook_name, parts|
         update_file_cache(cookbook_name, parts)
       end
diff --git a/cucumber.yml b/cucumber.yml
index 9387b3bf21..a63ea346ea 100644
--- a/cucumber.yml
+++ b/cucumber.yml
@@ -28,6 +28,7 @@ api_search_list: --tags @api_search_list --format pretty -r features/steps -r fe
 api_search_show: --tags @api_search_show --format pretty -r features/steps -r features/support features 
 client: --tags @client --format pretty -r features/steps -r features/support features 
 client_roles: --tags client_roles --format pretty -r features/steps -r features/support features 
+client_cookbook_sync: --tags @client-cookbook-sync --format pretty -r features/steps -r features/support features 
 search: --tags @search --format pretty -r features/steps -r features/support features 
 provider: --tags @provider --format pretty -r features/steps -r features/support features 
 provider_directory: --tags @provider_directory --format pretty -r features/steps -r features/support features 
diff --git a/features/chef-client/cookbook_sync.feature b/features/chef-client/cookbook_sync.feature
index ff8cb19543..cb4131546b 100644
--- a/features/chef-client/cookbook_sync.feature
+++ b/features/chef-client/cookbook_sync.feature
@@ -1,4 +1,4 @@
-@client 
+@client @client-cookbook-sync
 Feature: Synchronize cookbooks from the server
   In order to configure a system according to a centralized repository
   As an Administrator
@@ -19,3 +19,25 @@ Feature: Synchronize cookbooks from the server
       And 'stdout' should have 'INFO: Storing updated cookbooks/synchronize_deps/recipes/default.rb in the cache.'
       And 'stdout' should have 'INFO: Storing updated cookbooks/synchronize/recipes/default.rb in the cache.'
 
+  Scenario: Removes files from the cache that are no longer needed 
+    Given a validated node
+      And it includes the recipe 'synchronize_deps'
+     When I run the chef-client with '-l info'
+     Then the run should exit '0'
+      And 'stdout' should have 'INFO: Storing updated cookbooks/synchronize_deps/recipes/default.rb in the cache.'
+    Given we have an empty file named 'cookbooks/synchronize_deps/recipes/woot.rb' in the client cache
+     When I run the chef-client with '-l info'
+     Then the run should exit '0'
+      And 'stdout' should have 'INFO: Removing cookbooks/synchronize_deps/recipes/woot.rb from the cache; it is no longer on the server.'
+
+  Scenario: Remove cookbooks that are no longer needed 
+    Given a validated node
+      And it includes the recipe 'synchronize_deps'
+     When I run the chef-client with '-l info'
+     Then the run should exit '0'
+      And 'stdout' should have 'INFO: Storing updated cookbooks/synchronize_deps/recipes/default.rb in the cache.'
+    Given it includes no recipes
+     When I run the chef-client with '-l info'
+     Then the run should exit '0'
+      And 'stdout' should have 'INFO: Removing cookbooks/synchronize_deps/recipes/default.rb from the cache; it's cookbook is no longer needed on this client.'
+
diff --git a/features/steps/file_steps.rb b/features/steps/file_steps.rb
index 7218e6fd62..39b0b21247 100644
--- a/features/steps/file_steps.rb
+++ b/features/steps/file_steps.rb
@@ -27,6 +27,12 @@ Given /^we have an empty file named '(.+)'$/ do |filename|
   filename.close
 end
 
+Given /^we have an empty file named '(.+)' in the client cache$/ do |filename|
+  cache_dir = File.expand_path(File.join(File.dirname(__FILE__), "..", "data", "tmp", "cache"))
+  filename = File.new(File.join(cache_dir, filename), 'w')
+  filename.close
+end
+
 Given /^we have the atime\/mtime of '(.+)'$/ do |filename|
   @mtime = File.mtime(File.join(tmpdir, filename))
   @atime = File.atime(File.join(tmpdir, filename))
diff --git a/features/steps/node_steps.rb b/features/steps/node_steps.rb
index 1333fa0447..511fa4b73a 100644
--- a/features/steps/node_steps.rb
+++ b/features/steps/node_steps.rb
@@ -32,6 +32,12 @@ Given /^it includes the recipe '(.+)'$/ do |recipe|
   client.save_node
 end
 
+Given /^it includes no recipes$/ do 
+  self.recipe = "" 
+  client.node.run_list.reset
+  client.save_node
+end
+
 Given /^it includes the role '(.+)'$/ do |role|
   self.recipe = "role[#{role}]"
   client.node.run_list << "role[#{role}]"