Merge pull request #48 from aspiers/location

add LWRP for location constraint

libraries/chef/mixin/pacemaker/standard_cib_object.rb

@@ -48,7 +48,7 @@ class Chef
 
       def standard_create_resource
         cib_object = cib_object_class.from_chef_resource(new_resource)
-        cmd = cib_object.crm_configure_command
+        cmd = cib_object.configure_command
 
         ::Chef::Log.info "Creating new #{cib_object}"
 

libraries/pacemaker/cib_object.rb

@@ -144,6 +144,10 @@ module Pacemaker
         .gsub("'")  { "\\'" }
     end
 
+    def configure_command
+      "crm configure " + definition_string
+    end
+
     def reconfigure_command
       "echo #{quoted_definition_string} | crm configure load update -"
     end

libraries/pacemaker/constraint/colocation.rb

@@ -11,6 +11,9 @@ class Pacemaker::Constraint::Colocation < Pacemaker::Constraint
   end
 
   def parse_definition
+    # FIXME: this is incomplete.  It probably doesn't handle resource
+    # sets correctly, and certainly doesn't handle node attributes.
+    # See the crm(8) man page for the official BNF grammar.
     unless definition =~ /^#{self.class::TYPE} (\S+) (\d+|[-+]?inf): (.+?)\s*$/
       raise Pacemaker::CIBObject::DefinitionParseError, \
         "Couldn't parse definition '#{definition}'"
@@ -24,8 +27,4 @@ class Pacemaker::Constraint::Colocation < Pacemaker::Constraint
     "#{self.class::TYPE} #{name} #{score}: " + resources.join(' ')
   end
 
-  def crm_configure_command
-    "crm configure " + definition_string
-  end
-
 end

libraries/pacemaker/constraint/location.rb

@@ -0,0 +1,31 @@
+require File.expand_path('../constraint', File.dirname(__FILE__))
+
+class Pacemaker::Constraint::Location < Pacemaker::Constraint
+  TYPE = 'location'
+  register_type TYPE
+
+  attr_accessor :rsc, :score, :node
+
+  def self.attrs_to_copy_from_chef
+    %w(rsc score node)
+  end
+
+  def parse_definition
+    # FIXME: this is woefully incomplete, and doesn't cope with any of
+    # the rules syntax.  See the crm(8) man page for the official BNF
+    # grammar.
+    unless definition =~ /^#{self.class::TYPE} (\S+) (\S+) (\d+|[-+]?inf): (\S+)\s*$/
+      raise Pacemaker::CIBObject::DefinitionParseError, \
+        "Couldn't parse definition '#{definition}'"
+    end
+    self.name  = $1
+    self.rsc   = $2
+    self.score = $3
+    self.node  = $4
+  end
+
+  def definition_string
+    "#{self.class::TYPE} #{name} #{rsc} #{score}: #{node}"
+  end
+
+end

libraries/pacemaker/resource.rb

@@ -24,10 +24,6 @@ module Pacemaker
       "crm resource stop '#{name}'"
     end
 
-    def crm_configure_command
-      "crm configure " + definition_string
-    end
-
     # CIB object definitions look something like:
     #
     # primitive keystone ocf:openstack:keystone \

libraries/pacemaker/resource/clone.rb

@@ -7,6 +7,8 @@ class Pacemaker::Resource::Clone < Pacemaker::Resource
 
   include Pacemaker::Mixins::Resource::Meta
 
+  # FIXME: need to handle params as well as meta
+
   attr_accessor :rsc
 
   def self.attrs_to_copy_from_chef

libraries/pacemaker/resource/group.rb

@@ -7,6 +7,8 @@ class Pacemaker::Resource::Group < Pacemaker::Resource
 
   include Pacemaker::Mixins::Resource::Meta
 
+  # FIXME: need to handle params as well as meta
+
   attr_accessor :members
 
   def self.attrs_to_copy_from_chef

libraries/pacemaker/resource/primitive.rb

@@ -60,7 +60,7 @@ class Pacemaker::Resource::Primitive < Pacemaker::Resource
     str
   end
 
-  def crm_configure_command
+  def configure_command
     args = %w(crm configure primitive)
     args << [name, agent, params_string, meta_string, op_string]
     args.join " "

providers/colocation.rb

@@ -34,7 +34,6 @@ action :create do
 end
 
 action :delete do
-  name = new_resource.name
   next unless @current_resource
   standard_delete_resource
 end

providers/location.rb

@@ -17,44 +17,56 @@
 # limitations under the License.
 #
 
-require ::File.expand_path('../libraries/pacemaker/cib_object',
+require ::File.expand_path('../libraries/pacemaker', ::File.dirname(__FILE__))
+require ::File.expand_path('../libraries/chef/mixin/pacemaker',
                            ::File.dirname(__FILE__))
 
+include Chef::Mixin::Pacemaker::StandardCIBObject
+
 action :create do
   name = new_resource.name
-  rsc = new_resource.rsc
-  priority = new_resource.priority
-  loc = new_resource.loc
 
-  unless resource_exists?(name)
-    cmd = "crm configure location #{name} #{rsc} #{priority}: #{loc}" 
-
-    cmd_ = Mixlib::ShellOut.new(cmd)
-    cmd_.environment['HOME'] = ENV.fetch('HOME', '/root')
-    cmd_.run_command
-    begin
-      cmd_.error!
-      if resource_exists?(name)
-        new_resource.updated_by_last_action(true)
-        Chef::Log.info "Successfully configured location '#{name}'."
+  if @current_resource_definition.nil?
+    create_resource(name)
   else
-        Chef::Log.error "Failed to configure location #{name}."
-      end
-    rescue
-      Chef::Log.error "Failed to configure location #{name}."
-    end
+    maybe_modify_resource(name)
   end
 end
 
 action :delete do
-  name = new_resource.name
-  cmd = "crm resource stop #{name}; crm configure delete #{name}"
-
-    e = execute "delete location #{name}" do
-      command cmd
-      only_if { resource_exists?(name) }
+  next unless @current_resource
+  standard_delete_resource
 end
 
-    new_resource.updated_by_last_action(e.updated?)
-    Chef::Log.info "Deleted location '#{name}'."
+def cib_object_class
+  ::Pacemaker::Constraint::Location
+end
+
+def load_current_resource
+  standard_load_current_resource
+end
+
+def init_current_resource
+  name = @new_resource.name
+  @current_resource = Chef::Resource::PacemakerLocation.new(name)
+  attrs = [:rsc, :score, :node]
+  @current_cib_object.copy_attrs_to_chef_resource(@current_resource, *attrs)
+end
+
+def create_resource(name)
+  standard_create_resource
+end
+
+def maybe_modify_resource(name)
+  Chef::Log.info "Checking existing #{@current_cib_object} for modifications"
+
+  desired_location = cib_object_class.from_chef_resource(new_resource)
+  if desired_location.definition_string != @current_cib_object.definition_string
+    Chef::Log.debug "changed from [#{@current_cib_object.definition_string}] to [#{desired_location.definition_string}]"
+    cmd = desired_location.reconfigure_command
+    execute cmd do
+      action :nothing
+    end.run_action(:run)
+    new_resource.updated_by_last_action(true)
+  end
 end

resources/location.rb

@@ -23,5 +23,5 @@ default_action :create
 
 attribute :name,  :kind_of => String, :name_attribute => true
 attribute :rsc,   :kind_of => String
-attribute :priority, :kind_of => String
-attribute :loc, :kind_of => String
+attribute :score, :kind_of => String
+attribute :node,  :kind_of => String

spec/fixtures/location_constraint.rb

@@ -0,0 +1,15 @@
+require ::File.expand_path('../../libraries/pacemaker/constraint/location',
+                           ::File.dirname(__FILE__))
+
+module Chef::RSpec
+  module Pacemaker
+    module Config
+      LOCATION_CONSTRAINT = \
+        ::Pacemaker::Constraint::Location.new('location1')
+      LOCATION_CONSTRAINT.rsc   = 'primitive1'
+      LOCATION_CONSTRAINT.score = '-inf'
+      LOCATION_CONSTRAINT.node  = 'node1'
+      LOCATION_CONSTRAINT_DEFINITION = 'location location1 primitive1 -inf: node1'
+    end
+  end
+end

spec/libraries/pacemaker/constraint/colocation_spec.rb

@@ -6,6 +6,9 @@ require File.expand_path('../../../helpers/cib_object', File.dirname(__FILE__))
 
 describe Pacemaker::Constraint::Colocation do
   let(:fixture) { Chef::RSpec::Pacemaker::Config::COLOCATION_CONSTRAINT.dup }
+  let(:fixture_definition) {
+    Chef::RSpec::Pacemaker::Config::COLOCATION_CONSTRAINT_DEFINITION
+  }
 
   before(:each) do
     Mixlib::ShellOut.any_instance.stub(:run_command)
@@ -24,4 +27,37 @@ describe Pacemaker::Constraint::Colocation do
   end
 
   it_should_behave_like "a CIB object"
+
+  describe "#definition_string" do
+    it "should return the definition string" do
+      expect(fixture.definition_string).to eq(fixture_definition)
+    end
+
+    it "should return a short definition string" do
+      colocation = pacemaker_object_class.new('foo')
+      colocation.definition = \
+        %!colocation colocation1 -inf: rsc1 rsc2!
+      colocation.parse_definition
+      expect(colocation.definition_string).to eq(<<'EOF'.chomp)
+colocation colocation1 -inf: rsc1 rsc2
+EOF
+    end
+  end
+
+  describe "#parse_definition" do
+    before(:each) do
+      @parsed = pacemaker_object_class.new(fixture.name)
+      @parsed.definition = fixture_definition
+      @parsed.parse_definition
+    end
+
+    it "should parse the score" do
+      expect(@parsed.score).to eq(fixture.score)
+    end
+
+    it "should parse the resources" do
+      expect(@parsed.resources).to eq(fixture.resources)
+    end
+
+  end
 end

spec/libraries/pacemaker/constraint/location_spec.rb

@@ -0,0 +1,66 @@
+require 'spec_helper'
+require File.expand_path('../../../../libraries/pacemaker/constraint/location',
+                         File.dirname(__FILE__))
+require File.expand_path('../../../fixtures/location_constraint', File.dirname(__FILE__))
+require File.expand_path('../../../helpers/cib_object', File.dirname(__FILE__))
+
+describe Pacemaker::Constraint::Location do
+  let(:fixture) { Chef::RSpec::Pacemaker::Config::LOCATION_CONSTRAINT.dup }
+  let(:fixture_definition) {
+    Chef::RSpec::Pacemaker::Config::LOCATION_CONSTRAINT_DEFINITION
+  }
+
+  before(:each) do
+    Mixlib::ShellOut.any_instance.stub(:run_command)
+  end
+
+  def object_type
+    'location'
+  end
+
+  def pacemaker_object_class
+    Pacemaker::Constraint::Location
+  end
+
+  def fields
+    %w(name rsc score node)
+  end
+
+  it_should_behave_like "a CIB object"
+
+  describe "#definition_string" do
+    it "should return the definition string" do
+      expect(fixture.definition_string).to eq(fixture_definition)
+    end
+
+    it "should return a short definition string" do
+      location = pacemaker_object_class.new('foo')
+      location.definition = \
+        %!location location1 primitive1 -inf: node1!
+      location.parse_definition
+      expect(location.definition_string).to eq(<<'EOF'.chomp)
+location location1 primitive1 -inf: node1
+EOF
+    end
+  end
+
+  describe "#parse_definition" do
+    before(:each) do
+      @parsed = pacemaker_object_class.new(fixture.name)
+      @parsed.definition = fixture_definition
+      @parsed.parse_definition
+    end
+
+    it "should parse the rsc" do
+      expect(@parsed.rsc).to eq(fixture.rsc)
+    end
+
+    it "should parse the score" do
+      expect(@parsed.score).to eq(fixture.score)
+    end
+
+    it "should parse the node" do
+      expect(@parsed.node).to eq(fixture.node)
+    end
+  end
+end

spec/libraries/pacemaker/resource/clone_spec.rb

@@ -38,7 +38,7 @@ describe Pacemaker::Resource::Clone do
     end
 
     it "should return a short definition string" do
-      clone = Pacemaker::Resource::Clone.new('foo')
+      clone = pacemaker_object_class.new('foo')
       clone.definition = \
         %!clone clone1 primitive1 meta globally-unique="true"!
       clone.parse_definition
@@ -51,7 +51,7 @@ EOF
 
   describe "#parse_definition" do
     before(:each) do
-      @parsed = Pacemaker::Resource::Clone.new(fixture.name)
+      @parsed = pacemaker_object_class.new(fixture.name)
       @parsed.definition = fixture_definition
       @parsed.parse_definition
     end

spec/libraries/pacemaker/resource/group_spec.rb

@@ -38,7 +38,7 @@ describe Pacemaker::Resource::Group do
     end
 
     it "should return a short definition string" do
-      group = Pacemaker::Resource::Group.new('foo')
+      group = pacemaker_object_class.new('foo')
       group.definition = \
         %!group foo member1 member2 meta target-role="Started"!
       group.parse_definition
@@ -51,7 +51,7 @@ EOF
 
   describe "#parse_definition" do
     before(:each) do
-      @parsed = Pacemaker::Resource::Group.new(fixture.name)
+      @parsed = pacemaker_object_class.new(fixture.name)
       @parsed.definition = fixture_definition
       @parsed.parse_definition
     end

spec/libraries/pacemaker/resource/ms_spec.rb

@@ -38,7 +38,7 @@ describe Pacemaker::Resource::MasterSlave do
     end
 
     it "should return a short definition string" do
-      ms = Pacemaker::Resource::MasterSlave.new('foo')
+      ms = pacemaker_object_class.new('foo')
       ms.definition = \
         %!ms ms1 primitive1 meta globally-unique="true"!
       ms.parse_definition
@@ -51,7 +51,7 @@ EOF
 
   describe "#parse_definition" do
     before(:each) do
-      @parsed = Pacemaker::Resource::MasterSlave.new(fixture.name)
+      @parsed = pacemaker_object_class.new(fixture.name)
       @parsed.definition = fixture_definition
       @parsed.parse_definition
     end

spec/libraries/pacemaker/resource/primitive_spec.rb

@@ -78,7 +78,7 @@ describe Pacemaker::Resource::Primitive do
     end
 
     it "should return a short definition string" do
-      primitive = Pacemaker::Resource::Primitive.new('foo')
+      primitive = pacemaker_object_class.new('foo')
       primitive.definition = \
         %!primitive foo ocf:heartbeat:IPaddr2 params foo="bar"!
       primitive.parse_definition
@@ -91,7 +91,7 @@ EOF
 
   describe "#quoted_definition_string" do
     it "should return the quoted definition string" do
-      primitive = Pacemaker::Resource::Primitive.new('foo')
+      primitive = pacemaker_object_class.new('foo')
       primitive.definition = <<'EOF'.chomp
 primitive foo ocf:openstack:keystone \
          params bar="baz\\qux" bar2="baz'qux"
@@ -106,7 +106,7 @@ EOF
 
   describe "#parse_definition" do
     before(:each) do
-      @parsed = Pacemaker::Resource::Primitive.new(fixture.name)
+      @parsed = pacemaker_object_class.new(fixture.name)
       @parsed.definition = fixture_definition
       @parsed.parse_definition
     end

spec/providers/location_spec.rb

@@ -0,0 +1,83 @@
+require 'chef/application'
+require File.expand_path('../spec_helper', File.dirname(__FILE__))
+require File.expand_path('../helpers/cib_object', File.dirname(__FILE__))
+require File.expand_path('../fixtures/location_constraint', File.dirname(__FILE__))
+
+describe "Chef::Provider::PacemakerLocation" do
+  # for use inside examples:
+  let(:fixture) { Chef::RSpec::Pacemaker::Config::LOCATION_CONSTRAINT.dup }
+  # for use outside examples (e.g. when invoking shared_examples)
+  fixture = Chef::RSpec::Pacemaker::Config::LOCATION_CONSTRAINT.dup
+
+  before(:each) do
+    runner_opts = {
+      :step_into => ['pacemaker_location']
+    }
+    @chef_run = ::ChefSpec::Runner.new(runner_opts)
+    @chef_run.converge "pacemaker::default"
+    @node = @chef_run.node
+    @run_context = @chef_run.run_context
+
+    @resource = Chef::Resource::PacemakerLocation.new(fixture.name, @run_context)
+    @resource.rsc   fixture.rsc
+    @resource.score fixture.score
+    @resource.node  fixture.node.dup
+  end
+
+  let (:provider) { Chef::Provider::PacemakerLocation.new(@resource, @run_context) }
+
+  def cib_object_class
+    Pacemaker::Constraint::Location
+  end
+
+  include Chef::RSpec::Pacemaker::CIBObject
+
+  describe ":create action" do
+    def test_modify(expected_cmds)
+      yield
+
+      stub_shellout(fixture.definition_string)
+
+      provider.run_action :create
+
+      expected_cmds.each do |cmd|
+        expect(@chef_run).to run_execute(cmd)
+      end
+      expect(@resource).to be_updated
+    end
+
+    it "should modify the constraint if it has a different resource" do
+      new_resource = 'group2'
+      fixture.rsc = new_resource
+      expected_configure_cmd_args = [fixture.reconfigure_command]
+      test_modify(expected_configure_cmd_args) do
+        @resource.rsc new_resource
+      end
+    end
+
+    it "should modify the constraint if it has a different score" do
+      new_score = '100'
+      fixture.score = new_score
+      expected_configure_cmd_args = [fixture.reconfigure_command]
+      test_modify(expected_configure_cmd_args) do
+        @resource.score new_score
+      end
+    end
+
+    it "should modify the constraint if it has a different node" do
+      new_node = 'node2'
+      fixture.node = new_node
+      expected_configure_cmd_args = [fixture.reconfigure_command]
+      test_modify(expected_configure_cmd_args) do
+        @resource.node new_node
+      end
+    end
+
+  end
+
+  describe ":delete action" do
+    it_should_behave_like "action on non-existent resource", \
+      :delete, "crm configure delete #{fixture.name}", nil
+  end
+
+end

spec/providers/primitive_spec.rb

@@ -106,7 +106,7 @@ describe "Chef::Provider::PacemakerPrimitive" do
 
       provider.run_action :create
 
-      expect(@chef_run).to run_execute(fixture.crm_configure_command)
+      expect(@chef_run).to run_execute(fixture.configure_command)
       expect(@resource).to be_updated
     end
 
@@ -116,7 +116,7 @@ describe "Chef::Provider::PacemakerPrimitive" do
       expect { provider.run_action :create }.to \
         raise_error(RuntimeError, "Failed to create #{fixture}")
 
-      expect(@chef_run).to run_execute(fixture.crm_configure_command)
+      expect(@chef_run).to run_execute(fixture.configure_command)
       expect(@resource).not_to be_updated
     end
 
@@ -128,7 +128,7 @@ describe "Chef::Provider::PacemakerPrimitive" do
       expect { provider.run_action :create }.to \
         raise_error(RuntimeError, "Failed to create #{fixture}")
 
-      expect(@chef_run).to run_execute(fixture.crm_configure_command)
+      expect(@chef_run).to run_execute(fixture.configure_command)
       expect(@resource).not_to be_updated
     end