diff --git a/CHANGELOG.md b/CHANGELOG.md index b1c184f9..8832a6bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ - Removed legacy configuration sections: amf, aisexec, logging.logger\_subsys (#345) - Fix two\_nodes behaviour with expected\_votes = 2 introduced in 3.0.0 (#246) +- Cs\_clones are now stopped before being removed (crm provider) (#367) +- Cs\_clones now use the resource name as clone id (pcs provider) (#149, #367) ### Deprecation notes diff --git a/lib/puppet/provider/cs_clone/crm.rb b/lib/puppet/provider/cs_clone/crm.rb index 20f28ff2..2dadb38e 100644 --- a/lib/puppet/provider/cs_clone/crm.rb +++ b/lib/puppet/provider/cs_clone/crm.rb @@ -15,6 +15,8 @@ commands crm: 'crm' commands crm_attribute: 'crm_attribute' + mk_resource_methods + def self.instances block_until_ready @@ -66,76 +68,13 @@ def create # Unlike create we actually immediately delete the item. def destroy debug('Removing clone') + cmd = [command(:crm), '-w', 'resource', 'stop', @resource[:name]] + PuppetX::Voxpupuli::Corosync::Provider::Crmsh.run_command_in_cib(cmd, @resource[:cib], false) cmd = [command(:crm), 'configure', 'delete', @resource[:name]] PuppetX::Voxpupuli::Corosync::Provider::Crmsh.run_command_in_cib(cmd, @resource[:cib]) @property_hash.clear end - # - # Getter that obtains the our service that should have been populated by - # prefetch or instances (depends on if your using puppet resource or not). - def primitive - @property_hash[:primitive] - end - - # Getter that obtains the our clone_max that should have been populated by - # prefetch or instances (depends on if your using puppet resource or not). - def clone_max - @property_hash[:clone_max] - end - - def clone_node_max - @property_hash[:clone_node_max] - end - - def notify_clones - @property_hash[:notify_clones] - end - - def globally_unique - @property_hash[:globally_unique] - end - - def ordered - @property_hash[:ordered] - end - - def interleave - @property_hash[:interleave] - end - - # Our setters. Setters are used when the - # resource already exists so we just update the current value in the property - # hash and doing this marks it to be flushed. - - def primitive=(should) - @property_hash[:primitive] = should - end - - def clone_max=(should) - @property_hash[:clone_max] = should - end - - def clone_node_max=(should) - @property_hash[:clone_node_max] = should - end - - def notify_clones=(should) - @property_hash[:notify_clones] = should - end - - def globally_unique=(should) - @property_hash[:globally_unique] = should - end - - def ordered=(should) - @property_hash[:ordered] = should - end - - def interleave=(should) - @property_hash[:interleave] = should - end - # Flush is triggered on anything that has been detected as being # modified in the property_hash. It generates a temporary file with # the updates that need to be made. The temporary file is then used diff --git a/lib/puppet/provider/cs_clone/pcs.rb b/lib/puppet/provider/cs_clone/pcs.rb index a5096c2c..25aecd73 100644 --- a/lib/puppet/provider/cs_clone/pcs.rb +++ b/lib/puppet/provider/cs_clone/pcs.rb @@ -11,9 +11,24 @@ desc 'Provider to add, delete, manipulate primitive clones.' commands pcs: 'pcs' + commands cibadmin: 'cibadmin' + + mk_resource_methods defaultfor operatingsystem: [:fedora, :centos, :redhat] + def change_clone_id(primitive, id, cib) + xpath = "/cib/configuration/resources/clone[descendant::primitive[@id='#{primitive}']]" + cmd = [command(:cibadmin), '--query', '--xpath', xpath] + raw, = PuppetX::Voxpupuli::Corosync::Provider::Pcs.run_command_in_cib(cmd, cib) + doc = REXML::Document.new(raw) + if doc.root.attributes['id'] != id + doc.root.attributes['id'] = id + cmd = [command(:cibadmin), '--replace', '--xpath', xpath, '--xml-text', doc.to_s.chop] + PuppetX::Voxpupuli::Corosync::Provider::Pcs.run_command_in_cib(cmd, cib) + end + end + def self.instances block_until_ready @@ -48,7 +63,7 @@ def self.instances # of actually doing the work. def create @property_hash = { - name: @resource[:primitive] + '-clone', + name: @resource[:name], ensure: :present, primitive: @resource[:primitive], clone_max: @resource[:clone_max], @@ -68,71 +83,6 @@ def destroy @property_hash.clear end - # - # Getter that obtains the our service that should have been populated by - # prefetch or instances (depends on if your using puppet resource or not). - def primitive - @property_hash[:primitive] - end - - # Getter that obtains the our clone_max that should have been populated by - # prefetch or instances (depends on if your using puppet resource or not). - def clone_max - @property_hash[:clone_max] - end - - def clone_node_max - @property_hash[:clone_node_max] - end - - def notify_clones - @property_hash[:notify_clones] - end - - def globally_unique - @property_hash[:globally_unique] - end - - def ordered - @property_hash[:ordered] - end - - def interleave - @property_hash[:interleave] - end - - # Our setters. Setters are used when the - # resource already exists so we just update the current value in the property - # hash and doing this marks it to be flushed. - - def primitive=(should) - @property_hash[:primitive] = should - end - - def clone_max=(should) - @property_hash[:clone_max] = should - end - - def clone_node_max=(should) - @property_hash[:clone_node_max] = should - end - - def notify_clones=(should) - @property_hash[:notify_clones] = should - end - - def globally_unique=(should) - @property_hash[:globally_unique] = should - end - - def ordered=(should) - @property_hash[:ordered] = should - end - - def interleave=(should) - @property_hash[:interleave] = should - end - def exists? @property_hash[:existing_resource] == :true end @@ -160,6 +110,7 @@ def flush cmd << "ordered=#{@property_hash[:ordered]}" if @property_hash[:ordered] cmd << "interleave=#{@property_hash[:interleave]}" if @property_hash[:interleave] PuppetX::Voxpupuli::Corosync::Provider::Pcs.run_command_in_cib(cmd, @resource[:cib]) + change_clone_id(@property_hash[:primitive], @property_hash[:name], @resource[:cib]) end end end diff --git a/lib/puppet/type/cs_clone.rb b/lib/puppet/type/cs_clone.rb index 4e249709..dc61cbe7 100644 --- a/lib/puppet/type/cs_clone.rb +++ b/lib/puppet/type/cs_clone.rb @@ -7,7 +7,7 @@ ensurable newparam(:name) do - desc "Identifier of the location entry. This value needs to be unique + desc "Identifier of the clone entry. This value needs to be unique across the entire Corosync/Pacemaker configuration since it doesn't have the concept of name spaces per type." @@ -90,4 +90,9 @@ def unmunge_cs_primitive(name) name end + + validate do + return if self[:ensure] == :absent + raise Puppet::Error, 'primitive is mandatory' unless self[:primitive] + end end diff --git a/spec/acceptance/cs_clone_spec.rb b/spec/acceptance/cs_clone_spec.rb new file mode 100755 index 00000000..17ef8ab4 --- /dev/null +++ b/spec/acceptance/cs_clone_spec.rb @@ -0,0 +1,90 @@ +#! /usr/bin/env ruby -S rspec +require 'spec_helper_acceptance' + +describe 'corosync' do + cert = '-----BEGIN CERTIFICATE----- +MIIDVzCCAj+gAwIBAgIJAJNCo5ZPmKegMA0GCSqGSIb3DQEBBQUAMEIxCzAJBgNV +BAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQg +Q29tcGFueSBMdGQwHhcNMTUwMjI2MjI1MjU5WhcNMTUwMzI4MjI1MjU5WjBCMQsw +CQYDVQQGEwJYWDEVMBMGA1UEBwwMRGVmYXVsdCBDaXR5MRwwGgYDVQQKDBNEZWZh +dWx0IENvbXBhbnkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA +uCPPbDgErGUVs1pKqv59OatjCEU4P9QcmhDYFR7RBN8m08mIqd+RTuiHUKj6C9Rk +vWQ5bYrGQo/+4E0ziAUuUzzITlpIYLVltca6eBhKUqO3Cd0NMRVc2k4nx5948nwv +9FVOIfOOY6BN2ALglfBfLnhObbzJjs6OSZ7bUCpXVPV01t/61Jj3jQ3+R8b7AaoR +mw7j0uWaFimKt/uag1qqKGw3ilieMhHlG0Da5x9WLi+5VIM0t1rcpR58LLXVvXZB +CrQBucm2xhZsz7R76Ai+NL8zhhyzCZidZ2NtJ3E1wzppcSDAfNrru+rcFSlZ4YG+ +lMCqZ1aqKWVXmb8+Vg7IkQIDAQABo1AwTjAdBgNVHQ4EFgQULxI68KhZwEF5Q9al +xZmFDR+Beu4wHwYDVR0jBBgwFoAULxI68KhZwEF5Q9alxZmFDR+Beu4wDAYDVR0T +BAUwAwEB/zANBgkqhkiG9w0BAQUFAAOCAQEAsa0YKPixD6VmDo3pal2qqichHbdT +hUONk2ozzRoaibVocqKx2T6Ho23wb/lDlRUu4K4DMO663uumzI9lNoOewa0MuW1D +J52cejAMVsP3ROOdxBv0HZIVVJ8NLBHNLFOHJEDtvzogLVplzmo59vPAdmQo6eIV +japvs+0tdy9iwHj3z1ZME2Ntm/5TzG537e7Hb2zogatM9aBTUAWlZ1tpoaXuTH52 +J76GtqoIOh+CTeY/BMwBotdQdgeR0zvjE9FuLWkhTmRtVFhbVIzJbFlFuYq5d3LH +NWyN0RsTXFaqowV1/HSyvfD7LoF/CrmN5gOAM3Ierv/Ti9uqGVhdGBd/kw==' + File.open('/tmp/ca.pem', 'w') { |f| f.write(cert) } + it 'with defaults' do + pp = <<-EOS + file { '/tmp/ca.pem': + ensure => file, + content => '#{cert}' + } -> + class { 'corosync': + multicast_address => '224.0.0.1', + authkey => '/tmp/ca.pem', + bind_address => '127.0.0.1', + set_votequorum => true, + quorum_members => ['127.0.0.1'], + } + cs_property { 'stonith-enabled' : + value => 'false', + } -> + cs_primitive { 'duncan_vip': + primitive_class => 'ocf', + primitive_type => 'IPaddr2', + provided_by => 'heartbeat', + parameters => { 'ip' => '172.16.210.101', 'cidr_netmask' => '24' }, + operations => { 'monitor' => { 'interval' => '10s' } }, + } + EOS + + apply_manifest(pp, catch_failures: true, debug: true, trace: true) + apply_manifest(pp, catch_changes: true, debug: false, trace: true) + end + + describe service('corosync') do + it { is_expected.to be_running } + end + + it 'creates a clone' do + pp = <<-EOS + cs_clone { 'duncan_vip_clone': + ensure => present, + primitive => 'duncan_vip', + } + EOS + apply_manifest(pp, catch_failures: true, debug: true, trace: true) + apply_manifest(pp, catch_changes: true, debug: false, trace: true) + command = 'cibadmin --query | grep duncan_vip_clone' + shell(command) do |r| + expect(r.stdout).to match(%r{ absent, + } + EOS + apply_manifest(pp, catch_failures: true, debug: true, trace: true) + apply_manifest(pp, catch_changes: true, debug: false, trace: true) + command = 'cibadmin --query | grep duncan_vip_clone' + assert_raises(Beaker::Host::CommandFailure) do + shell(command) + end + end + + after :all do + cleanup_cs_resources + end +end diff --git a/spec/spec_helper_corosync.rb b/spec/spec_helper_corosync.rb index fdaed0d8..f35d19b4 100644 --- a/spec/spec_helper_corosync.rb +++ b/spec/spec_helper_corosync.rb @@ -24,6 +24,7 @@ def not_expect_commands(patterns) shared_context 'pcs' do before do described_class.stubs(:command).with(:pcs).returns 'pcs' + described_class.stubs(:command).with(:cibadmin).returns 'cibadmin' described_class.expects(:block_until_ready).returns(nil).at_most(1) end end diff --git a/spec/unit/puppet/provider/cs_clone_pcs_spec.rb b/spec/unit/puppet/provider/cs_clone_pcs_spec.rb index b415f2f6..e1d8ea75 100644 --- a/spec/unit/puppet/provider/cs_clone_pcs_spec.rb +++ b/spec/unit/puppet/provider/cs_clone_pcs_spec.rb @@ -35,6 +35,10 @@ instances.first end + before do + instance.stubs(:change_clone_id) { nil } + end + it "is a kind of #{described_class.name}" do expect(instance).to be_a_kind_of(described_class) end @@ -48,9 +52,9 @@ context 'when flushing' do let :resource do Puppet::Type.type(:cs_clone).new( - name: 'p_keystone', - provider: :pcs, + name: 'p_keystone-clone', primitive: 'p_keystone', + provider: :pcs, ensure: :present ) end @@ -61,6 +65,10 @@ instance end + before do + instance.stubs(:change_clone_id) { nil } + end + it 'creates clone' do expect_commands(%r{pcs resource clone p_keystone}) instance.flush @@ -102,4 +110,67 @@ instance.flush end end + + context 'when changing clone id' do + def clone_xml(name) + <<-EOS + + + + + + + + + + + + + EOS + end + + let :instances do + cib = <<-EOS + + + + #{clone_xml('apache_service-clone')} + + + + EOS + + pcs_load_cib(cib) + described_class.instances + end + + it 'has an instance for each ' do + expect(instances.count).to eq(1) + end + + describe 'each instance' do + let :instance do + instances.first + end + + it 'calls cibadmin with the correct parameters' do + xpath = '/cib/configuration/resources/clone[descendant::primitive[@id=\'apache_service\']]' + Puppet::Util::Execution.expects(:execute).with(['cibadmin', '--query', '--xpath', xpath], failonfail: true, combine: true).at_least_once.returns( + Puppet::Util::Execution::ProcessOutput.new(clone_xml('apache_service-clone'), 0) + ) + Puppet::Util::Execution.expects(:execute).with(['cibadmin', '--replace', '--xpath', xpath, '--xml-text', clone_xml('apache_service-newclone').chop], failonfail: true, combine: true).at_least_once.returns( + Puppet::Util::Execution::ProcessOutput.new('', 0) + ) + instance.change_clone_id('apache_service', 'apache_service-newclone', nil) + end + + it 'calls cibadmin only when needed' do + xpath = '/cib/configuration/resources/clone[descendant::primitive[@id=\'apache_service\']]' + Puppet::Util::Execution.expects(:execute).with(['cibadmin', '--query', '--xpath', xpath], failonfail: true, combine: true).at_least_once.returns( + Puppet::Util::Execution::ProcessOutput.new(clone_xml('apache_service-clone'), 0) + ) + instance.change_clone_id('apache_service', 'apache_service-clone', nil) + end + end + end end diff --git a/spec/unit/puppet/type/cs_clone_spec.rb b/spec/unit/puppet/type/cs_clone_spec.rb index 3abd1bc3..bbb906bd 100644 --- a/spec/unit/puppet/type/cs_clone_spec.rb +++ b/spec/unit/puppet/type/cs_clone_spec.rb @@ -6,7 +6,7 @@ end it "has a 'name' parameter" do - expect(subject.new(name: 'mock_clone')[:name]).to eq('mock_clone') + expect(subject.new(name: 'mock_clone', primitive: 'mock_primitive')[:name]).to eq('mock_clone') end describe 'basic structure' do @@ -14,7 +14,7 @@ provider_class = Puppet::Type::Cs_clone.provider(Puppet::Type::Cs_clone.providers[0]) Puppet::Type::Cs_clone.expects(:defaultprovider).returns(provider_class) - expect(subject.new(name: 'mock_clone')).not_to be_nil + expect(subject.new(name: 'mock_clone', primitive: 'mock_primitive')).not_to be_nil end [:name, :cib].each do |param| @@ -44,7 +44,8 @@ it "should validate that the #{attribute} attribute can be true/false" do [true, false].each do |value| expect(subject.new( - name: 'mock_clone', + name: 'mock_clone', + primitive: 'mock_primitive', attribute => value )[attribute]).to eq(value.to_s.to_sym) end