From bcafd6ff3119bcc2d542cacd851d1d916c328c98 Mon Sep 17 00:00:00 2001
From: Adam Spiers <aspiers@suse.com>
Date: Tue, 21 Jan 2014 19:09:44 +0000
Subject: [PATCH] support changing of attributes in primitives

---
 providers/primitive.rb           | 91 +++++++++++++++++++++-----------
 spec/providers/primitive_spec.rb | 58 +++++++++++++++-----
 2 files changed, 105 insertions(+), 44 deletions(-)

diff --git a/providers/primitive.rb b/providers/primitive.rb
index d6993b0..086a1dc 100644
--- a/providers/primitive.rb
+++ b/providers/primitive.rb
@@ -26,30 +26,15 @@ include Chef::Libraries::Pacemaker::CIBObjects
 
 action :create do
   name = new_resource.name
-  agent = new_resource.agent
 
-  next if cib_object_exists?(name)
-
-  cmd = "crm configure primitive #{name} #{agent}"
-  cmd << resource_params_string(new_resource.params)
-  cmd << resource_meta_string(new_resource.meta)
-  cmd << resource_op_string(new_resource.op)
-
-  # 'Execute' resource doesn't throw exception even when command fails..
-  # So, Mixlib::ShellOut was used instead.
-  cmd_ = Mixlib::ShellOut.new(cmd)
-  cmd_.environment['HOME'] = ENV.fetch('HOME', '/root')
-  cmd_.run_command
-  begin
-    cmd_.error!
-    if cib_object_exists?(name)
-      new_resource.updated_by_last_action(true)
-      Chef::Log.info "Successfully configured primitive '#{name}'."
-    else
-      Chef::Log.error "Failed to configure primitive #{name}."
+  if @current_resource_definition.nil?
+    create_resource(name)
+  else
+    if @current_resource.agent != new_resource.agent
+      raise "Existing primitive '#{name}' has agent but recipe wanted #{new_resource.agent}"
     end
-  rescue
-    Chef::Log.error "Failed to configure primitive #{name}."
+
+    modify_resource(name)
   end
 end
 
@@ -91,24 +76,68 @@ end
 def load_current_resource
   name = @new_resource.name
 
-  return unless cib_object_exists?(name)
-
   obj_definition = get_cib_object_definition(name)
-  if obj_definition.nil?
-    raise "CIB object '#{name}' existed but definition was nil?!"
-  end
-  if obj_definition.empty?
+  return unless obj_definition
+
+  if obj_definition.empty? # probably overly paranoid
     raise "CIB object '#{name}' existed but definition was empty?!"
   end
 
-  return unless cib_object_type(obj_definition) =~ /\Aprimitive #{name} (\S+)/
+  unless obj_definition =~ /\Aprimitive #{name} (\S+)/
+    Chef::Log.warn "Resource '#{name}' was not a primitive"
+    return
+  end
   agent = $1
 
+  @current_resource_definition = obj_definition
   @current_resource = Chef::Resource::PacemakerPrimitive.new(name)
   @current_resource.agent(agent)
 
-  %w(params meta op).each do |data_type|
-    extract_hash(obj_definition, data_type)
+  %w(params meta).each do |data_type|
+    h = extract_hash(name, obj_definition, data_type)
+    @current_resource.send(data_type.to_sym, h)
   end
 end
 
+def create_resource(name)
+  cmd = "crm configure primitive #{name} #{new_resource.agent}"
+  cmd << resource_params_string(new_resource.params)
+  cmd << resource_meta_string(new_resource.meta)
+  cmd << resource_op_string(new_resource.op)
+
+  Chef::Log.debug "creating new primitive #{name} via #{cmd}"
+
+  execute cmd do
+    action :nothing
+  end.run_action(:run)
+
+  if cib_object_exists?(name)
+    new_resource.updated_by_last_action(true)
+    Chef::Log.info "Successfully configured primitive '#{name}'."
+  else
+    Chef::Log.error "Failed to configure primitive #{name}."
+  end
+end
+
+def modify_resource(name)
+  configure_cmd_prefix = "crm_resource --resource keystone"
+
+  cmds = []
+  new_resource.params.each do |k, v|
+    if @current_resource.params[k] == v
+      Chef::Log.debug("#{name}'s #{k} param didn't change")
+    else
+      Chef::Log.info("#{name}'s #{k} param changed to #{v}")
+      cmds << configure_cmd_prefix + " --set-parameter #{k} --parameter-value #{v}"
+    end
+  end
+
+  cmds.each do |cmd|
+    converge_by("execute #{cmd}") do
+      result = shell_out!(cmd)
+      Chef::Log.info("#{cmd} ran successfully")
+    end
+  end
+
+  new_resource.updated_by_last_action(true) unless cmds.empty?
+end
diff --git a/spec/providers/primitive_spec.rb b/spec/providers/primitive_spec.rb
index b51e0e7..a0a3abb 100644
--- a/spec/providers/primitive_spec.rb
+++ b/spec/providers/primitive_spec.rb
@@ -4,13 +4,13 @@ require_relative File.join(%w(.. helpers keystone_config))
 
 describe "Chef::Provider::PacemakerPrimitive" do
   before do
-    @chef_run = ::ChefSpec::Runner.new(step_into: ['pacemaker_primitive']) #::OPENSUSE_OPTS
+    runner_opts = {
+      :step_into => ['pacemaker_primitive']
+    }
+    @chef_run = ::ChefSpec::Runner.new(runner_opts) #::OPENSUSE_OPTS
     @chef_run.converge "pacemaker::default"
     @node = @chef_run.node
-
-    @cookbook_collection = Chef::CookbookCollection.new([])
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
+    @run_context = @chef_run.run_context
 
     @resource = Chef::Resource::PacemakerPrimitive.new("keystone", @run_context)
 
@@ -24,18 +24,50 @@ describe "Chef::Provider::PacemakerPrimitive" do
   describe ":create action" do
     let(:ra) { Chef::RSpec::Pacemaker::Config::RA }
 
-    it "should do nothing if the primitive already exists" do
+    it "should modify the primitive if it already exists" do
       provider = Chef::Provider::PacemakerPrimitive.new(@resource, @run_context)
-      expect(provider).to receive(:cib_object_exists?).at_least(:once).and_return(true)
+
+      # get_cib_object_definition is invoked by load_current_resource
+      # and later used to see whether to create or modify.
       expect(provider).to receive(:get_cib_object_definition).and_return(ra[:config])
-      expect(Mixlib::ShellOut).not_to receive(:new)
+
+      configure_cmd_prefix = "crm_resource --resource keystone"
+      expected_configure_cmd_args = [
+        "--set-parameter os_password --parameter-value newpasswd",
+        "--delete-parameter os_tenant_name",
+        "--set-parameter target-role --parameter-value Stopped --meta",
+      ]
+
       provider.run_action :create
+      expected_configure_cmd_args.each do |args|
+        cmd = configure_cmd_prefix + " " + args
+        expect(@chef_run).to run_execute(cmd)
+      end
+      expect(@resource).to be_updated
     end
 
-    # it "should create a primitive" do
-    #   pending "foo"
-    #   @resource.run_action :create
-    # end
-  end
+    it "should create a primitive if it doesn't already exist" do
+      provider = Chef::Provider::PacemakerPrimitive.new(@resource, @run_context)
 
+      # get_cib_object_definition is invoked by load_current_resource
+      # and later used to see whether to create or modify.
+      expect(provider).to receive(:get_cib_object_definition).and_return(nil)
+
+      cmd = "crm configure primitive keystone #{ra[:agent]}" + \
+            ra[:params_string] + ra[:meta_string] + ra[:op_string]
+
+      # Later, the :create action calls cib_object_exists? to check
+      # that creation succeeded.
+      expect(provider).to receive(:cib_object_exists?).and_return(true)
+
+      provider.run_action :create
+
+      expect(@chef_run).to run_execute(cmd)
+      expect(@resource).to be_updated
+    end
+
+    it "should barf if the primitive has the wrong agent" do
+      pending "not implemented yet"
+    end
+  end
 end