Pacemaker Fact & corosync.conf enhancement

data/os/Debian/10.yaml

@@ -0,0 +1,3 @@
+---
+corosync::provider: 'crm'
+corosync::pcs_version: ~

data/os/Debian/11.yaml

@@ -0,0 +1,3 @@
+---
+corosync::provider: 'crm'
+corosync::pcs_version: ~

lib/facter/corosync.rb

@@ -0,0 +1,4 @@
+require 'puppet/corosync/facts'
+
+# Get in there with those facts
+Puppet::Corosync::Facts.install

lib/puppet/corosync/facts.rb

@@ -0,0 +1,108 @@
+require 'facter'
+require 'English'
+
+# Module containing code for constructing facts about the pacemaker code
+# present on this system
+#
+# Note that this structure is primarily a mechanism to allow rspec based
+# testing of the fact code.
+module Puppet::Corosync
+  # Contains the control code needed to actually add facts. Note that the fact
+  # collection code is primarily located in the inner module `HostData`.
+  module Facts
+    # Calls the Facter DSL and dynamically adds the local facts.
+    #
+    # This helps facilitate testing of the ruby code presented by this module
+    #
+    # @return [NilClass]
+    def self.install
+      Puppet::Corosync::Facts::HostData.initialize
+
+      Facter.add(:corosync, type: :aggregate) do
+        chunk(:pcs_version_full) do
+          { pcs_version_full: Puppet::Corosync::Facts::HostData.pcs_version_full }
+        end
+        chunk(:pcs_version_release) do
+          { pcs_version_release: Puppet::Corosync::Facts::HostData.pcs_version_release }
+        end
+        chunk(:pcs_version_major) do
+          { pcs_version_major: Puppet::Corosync::Facts::HostData.pcs_version_major }
+        end
+        chunk(:pcs_version_minor) do
+          { pcs_version_minor: Puppet::Corosync::Facts::HostData.pcs_version_minor }
+        end
+      end
+    end
+
+    # Honestly, I shamelessly stole this structure from the puppet-jenkins module.
+    # This component contains all of the actual code generating fact data for a
+    # given host.
+    module HostData
+      PCS_BIN = '/sbin/pcs'.freeze
+
+      @attributes = {
+        pcs_version_full: '',
+        pcs_version_release: '',
+        pcs_version_major: '',
+        pcs_version_minor: ''
+      }
+
+      def self.initialize
+        # Do nothing if the file doesn't exist
+        if File.exist?(PCS_BIN)
+          cmd = [
+            PCS_BIN,
+            '--version'
+          ]
+          result, = _run_command(cmd)
+          version_string = result[:raw]
+        else
+          version_string = ''
+        end
+
+        pcs_version = check_pcs_version(version_string)
+        @attributes[:pcs_version_full] = pcs_version[:full]
+        @attributes[:pcs_version_release] = pcs_version[:release]
+        @attributes[:pcs_version_major] = pcs_version[:major]
+        @attributes[:pcs_version_minor] = pcs_version[:minor]
+
+        # Create the getter methods
+        @attributes.each do |attr, _value|
+          define_singleton_method(attr) { @attributes[attr] }
+        end
+      end
+
+      # Retrieve the locally installed version of PCS on this node
+      def self.check_pcs_version(version_string)
+        if %r{(?<release>\d+)[.](?<major>\d+)[.](?<minor>\d+)} =~ version_string
+          {
+            full: version_string,
+            release: release,
+            major: major,
+            minor: minor
+          }
+        else
+          {
+            full: version_string,
+            release: nil,
+            major: nil,
+            minor: nil
+          }
+        end
+      end
+
+      # Executes the provided command with some sane defaults
+      def self._run_command(cmd,
+                            failonfail = true,
+                            custom_environment = { combine: true })
+        # TODO: Potentially add some handling for when failonfail is false
+        raw = Puppet::Util::Execution.execute(
+          cmd,
+          { failonfail: failonfail }.merge(custom_environment)
+        )
+        status = raw.exitstatus
+        { raw: raw, status: status } if status.zero? || failonfail == false
+      end
+    end
+  end
+end

manifests/init.pp

@@ -402,7 +402,7 @@
   Boolean $test_corosync_config                                         = $corosync::params::test_corosync_config,
   Optional[Variant[Stdlib::Absolutepath, Enum['off']]] $watchdog_device = undef,
   Enum['pcs', 'crm'] $provider                                          = 'pcs',
-  String $pcs_version                                                   = '',
+  String $pcs_version                                                   = $facts['corosync']['pcs_version_full'],
 ) inherits corosync::params {
   if $set_votequorum and (empty($quorum_members) and empty($multicast_address) and !$cluster_name) {
     fail('set_votequorum is true, so you must set either quorum_members, or one of multicast_address or cluster_name.')
@@ -580,7 +580,7 @@
 
     $exec_path = '/sbin:/bin:/usr/sbin:/usr/bin'
 
-    if $manage_pcsd_auth and $is_auth_node {
+    if $manage_pcsd_auth and $is_auth_node and $pcs_version != '' {
       # TODO - verify if this breaks out of the sensitivity
       $hacluster_password = $sensitive_hacluster_password.unwrap
       $auth_credential_string = "-u hacluster -p ${hacluster_password}"
@@ -669,7 +669,7 @@
         command => $quorum_setup_cmd,
         path    => $exec_path,
         onlyif  => [
-          'test 0 -ne $(pcs quorum config | grep "host:" >/dev/null 2>&1; echo $?)',
+          "test 0 -ne $(pcs quorum config | grep 'host: ${quorum_device_host}' >/dev/null 2>&1; echo $?)",
         ],
         require => Exec['authorize_qdevice'],
         before  => File['/etc/corosync/corosync.conf'],

spec/classes/corosync_spec.rb

@@ -1,4 +1,5 @@
 require 'spec_helper'
+require 'deep_merge'
 
 describe 'corosync' do
   let :params do
@@ -651,7 +652,13 @@
   on_supported_os.each do |os, os_facts|
     context "on #{os}" do
       let(:facts) do
-        os_facts
+        os_facts.deep_merge(
+          {
+            corosync: {
+              pcs_version_full: corosync_stack(os_facts)[:pcs_version]
+            }
+          }
+        )
       end
 
       auth_command = if corosync_stack(os_facts)[:provider] == 'pcs'
@@ -1037,7 +1044,7 @@
                   command: 'pcs quorum device add model net host=quorum1.test.org algorithm=ffsplit',
                   path: '/sbin:/bin:/usr/sbin:/usr/bin',
                   onlyif: [
-                    'test 0 -ne $(pcs quorum config | grep "host:" >/dev/null 2>&1; echo $?)'
+                    "test 0 -ne $(pcs quorum config | grep 'host: quorum1.test.org' >/dev/null 2>&1; echo $?)"
                   ],
                   require: 'Exec[authorize_qdevice]'
                 )
@@ -1077,6 +1084,23 @@
               )
             end
             # else - to implement
+
+            it 'contains the quorum configuration' do
+              is_expected.to contain_file('/etc/corosync/corosync.conf').with_content(
+                %r!nodelist\s{\n
+                \s+node\s{\n
+                \s+ring0_addr:\snode1[.]test[.]org\n
+                \s+nodeid:\s1\n
+                \s+name:\snode1[.]test[.]org\n
+                \s+}\n
+                \s+node\s{\n
+                \s+ring0_addr:\snode2[.]test[.]org\n
+                \s+nodeid:\s2\n
+                \s+name:\snode2[.]test[.]org\n
+                \s+}\n
+                }!xm
+              )
+            end
           end
         end
       end

spec/unit/puppet/corosync/facts_spec.rb

@@ -0,0 +1,83 @@
+require 'spec_helper'
+require 'puppet/corosync/facts'
+
+describe Puppet::Corosync::Facts::HostData do
+  describe '.initialize' do
+    let(:pcs_bin) { '/sbin/pcs' }
+    let(:version_string) { '0.10.15' }
+
+    context 'pcs is installed' do
+      before do
+        Puppet::Util::Execution.expects(:execute).with(
+          [pcs_bin, '--version'],
+          failonfail: true,
+          combine: true
+        ).at_least_once.returns(
+          Puppet::Util::Execution::ProcessOutput.new(version_string, 0)
+        )
+        File.expects(:exist?).with(pcs_bin).at_least_once.returns(true)
+      end
+
+      it "sets 'pcs_version_full'" do
+        described_class.initialize
+        expect(described_class.pcs_version_full).to eq('0.10.15')
+      end
+
+      it "sets 'pcs_version_release'" do
+        described_class.initialize
+        expect(described_class.pcs_version_release).to eq('0')
+      end
+
+      it "sets 'pcs_version_major'" do
+        described_class.initialize
+        expect(described_class.pcs_version_major).to eq('10')
+      end
+
+      it "sets 'pcs_version_minor'" do
+        described_class.initialize
+        expect(described_class.pcs_version_minor).to eq('15')
+      end
+    end
+
+    context 'pcs is not yet installed' do
+      before do
+        File.expects(:exist?).with(pcs_bin).at_least_once.returns(false)
+      end
+
+      it 'handles the unknown string' do
+        described_class.initialize
+        expect(described_class.pcs_version_full).to eq('')
+        expect(described_class.pcs_version_release).to eq(nil)
+        expect(described_class.pcs_version_major).to eq(nil)
+        expect(described_class.pcs_version_minor).to eq(nil)
+      end
+    end
+  end
+
+  describe '.check_pcs_version' do
+    {
+      '0.10.15' => {
+        full: '0.10.15',
+        release: '0',
+        major: '10',
+        minor: '15'
+      },
+      '1.11.0' => {
+        full: '1.11.0',
+        release: '1',
+        major: '11',
+        minor: '0'
+      },
+      '0.9.165' => {
+        full: '0.9.165',
+        release: '0',
+        major: '9',
+        minor: '165'
+      },
+    }.each do |version_string, results|
+      it "correctly parses '#{version_string}'" do
+        expect(described_class.check_pcs_version(version_string)).to eq(results)
+      end
+    end
+  end
+end

templates/corosync.conf.erb

@@ -160,6 +160,8 @@ nodelist {
 <% end -%>
 <% if not @quorum_members_names.nil? -%>
     name: <%= [@quorum_members_names].flatten[i] %>
+<% elsif @quorum_members[i] !~ %r{\A([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])(\.([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])){3}\z} -%>
+    name: <%= [@quorum_members].flatten[i] %>
 <% end -%>
   }
 <% end -%>