CHEF-4443 - Always save the correct run list

As per Dan's suggestions, use a primary run_list and an override, and
take care to ensure that the correct one is used to save the node.

2 files changed, 19 insertions, 10 deletions

diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index ceb0873079..e5a6e93d1e 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -254,9 +254,9 @@ class Chef
     def save_updated_node
       unless Chef::Config[:solo]
         Chef::Log.debug("Saving the current state of node #{node_name}")
-        if(@original_runlist)
-          @node.run_list(*@original_runlist)
-          @node.automatic_attrs[:runlist_override_history] = {Time.now.to_i => @override_runlist.inspect}
+        if(@node.override_runlist)
+          @node.automatic_attrs[:runlist_override_history] = {Time.now.to_i => @node.override_runlist.inspect}
+          @node.override_runlist.reset!
         end
         @node.save
       end
@@ -303,11 +303,10 @@ class Chef
       @node.consume_external_attrs(ohai.data, @json_attribs)
 
       unless(@override_runlist.empty?)
-        @original_runlist = @node.run_list.run_list_items.dup
         runlist_override_sanity_check!
-        @node.run_list(*@override_runlist)
+        @node.override_runlist(*@override_runlist)
         Chef::Log.warn "Run List override has been provided."
-        Chef::Log.warn "Original Run List: [#{@original_runlist.join(', ')}]"
+        Chef::Log.warn "Original Run List: [#{@node.primary_runlist}]"
         Chef::Log.warn "Overridden Run List: [#{@node.run_list}]"
       end
 
diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index 6bd2226ac8..f73628fdf5 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -42,7 +42,7 @@ class Chef
 
     def_delegators :attributes, :keys, :each_key, :each_value, :key?, :has_key?
 
-    attr_accessor :recipe_list, :run_state, :run_list
+    attr_accessor :recipe_list, :run_state, :override_runlist
 
     # RunContext will set itself as run_context via this setter when
     # initialized. This is needed so DSL::IncludeAttribute (in particular,
@@ -63,7 +63,8 @@ class Chef
       @name = nil
 
       @chef_environment = '_default'
-      @run_list = Chef::RunList.new
+      @primary_runlist = Chef::RunList.new
+      @override_runlist = Chef::RunList.new
 
       @attributes = Chef::Node::Attribute.new({}, {}, {}, {})
 
@@ -252,10 +253,19 @@ class Chef
       run_list.include?("role[#{role_name}]")
     end
 
+    def primary_runlist
+      @primary_runlist
+    end
+
+    def override_runlist(*args)
+      args.length > 0 ? @override_runlist.reset!(args) : @override_runlist
+    end
+
     # Returns an Array of roles and recipes, in the order they will be applied.
     # If you call it with arguments, they will become the new list of roles and recipes.
     def run_list(*args)
-      args.length > 0 ? @run_list.reset!(args) : @run_list
+      rl = @override_runlist.empty? ? @primary_runlist : @override_runlist
+      args.length > 0 ? rl.reset!(args) : rl
     end
 
     # Returns true if this Node expects a given role, false if not.
@@ -395,7 +405,7 @@ class Chef
         "default" => attributes.combined_default,
         "override" => attributes.combined_override,
         #Render correctly for run_list items so malformed json does not result
-        "run_list" => run_list.run_list.map { |item| item.to_s }
+        "run_list" => @primary_runlist.run_list.map { |item| item.to_s }
       }
       result
     end