Remove old workarounds for broken crm_diff
Reason for this is that there are two functions to detect a change in a pcmk resource: - pacemaker_restart_resource? -> which parses crm_simulate and was necessary due to https://bugzilla.redhat.com/show_bug.cgi?id=1561617 (Fixed in pacemaker-1.1.18-12) - pacemaker_restart_resource_ng? -> which is the preferred one and uses crm_diff xml output (and is a lot simpler). We can remove pacemaker_restart_resource? fully now that newton and Centos 7 are unsupported. Tested as follows: - Deployed a fresh Train overcloud w/ this change - Ran a redeployment forcing a config change on an HA bundle - Ran another redeployment forcing a CIB change on an HA bundle OVNDBSPacemakerMonitorIntervalSlave: 666 observed that the bundle was restarted correctly: [root@controller-0 ~]# pcs resource config ovn-dbs-bundle |grep 666 monitor interval=666s role=Slave timeout=60s (ovndb_servers-monitor-interval-666s) - Also tested in CI via https://review.opendev.org/c/openstack/tripleo-heat-templates/+/803101 Change-Id: Iba736839ae12f3adf180b53cd7fc5c552d03bd47
This commit is contained in:
Michele Baldessari
parent
531283ecea
commit
606d58caa3
|
|
@ -311,74 +311,6 @@ def pcmk_get_bundle_storage_map(resource)
|
|||
ret
|
||||
end
|
||||
|
||||
# The following function will take a resource_name an xml graph file as generated by crm_simulate and
|
||||
# will return true if the resource_name is contained in the transition graph (i.e. the cluster would
|
||||
# restart the resource) and false if not (i.e. the cluster would not restart the resource)
|
||||
def pcmk_graph_contain_id?(resource_name, graph_file, is_bundle=false)
|
||||
graph = File.new(graph_file)
|
||||
graph_doc = REXML::Document.new graph
|
||||
xpath_query = '/transition_graph//primitive/@id'
|
||||
ids = []
|
||||
REXML::XPath.each(graph_doc, xpath_query) do |element|
|
||||
id = element.to_s
|
||||
# if we are a bundle we compare the start of the strings
|
||||
# because the primitive id will be in the form of galera-bundle-1 as opposed to galera-bundle
|
||||
if is_bundle then
|
||||
if id.start_with?(resource_name) then
|
||||
return true
|
||||
end
|
||||
else
|
||||
if id == resource_name then
|
||||
return true
|
||||
end
|
||||
end
|
||||
end
|
||||
return false
|
||||
end
|
||||
|
||||
# we need to check if crm_diff is affected by rhbz#1561617
|
||||
# crm_diff --cib -o xml1 -n xml2 will return 1 (aka diff needed)
|
||||
# on broken versions. I will return 0 when crm_diff is fixed
|
||||
# (aka no changes detected)
|
||||
def is_crm_diff_buggy?
|
||||
xml1 = '''
|
||||
<cib crm_feature_set="3.0.14" validate-with="pacemaker-2.10" epoch="86" num_updates="125" admin_epoch="0">
|
||||
<configuration>
|
||||
<resources>
|
||||
<bundle id="galera-bundle">
|
||||
<docker image="openstack-mariadb:pcmklatest"/>
|
||||
<storage>
|
||||
<storage-mapping target-dir="/foo" options="rw" id="mysql-foo" source-dir="/foo"/>
|
||||
<storage-mapping target-dir="/bar" options="rw" id="mysql-bar" source-dir="/bar"/>
|
||||
</storage>
|
||||
</bundle>
|
||||
</resources>
|
||||
</configuration>
|
||||
</cib>
|
||||
'''
|
||||
xml2 = '''
|
||||
<cib crm_feature_set="3.0.14" validate-with="pacemaker-2.10" epoch="86" num_updates="125" admin_epoch="0">
|
||||
<configuration>
|
||||
<resources>
|
||||
<bundle id="galera-bundle">
|
||||
<docker image="openstack-mariadb:pcmklatest"/>
|
||||
<storage>
|
||||
<storage-mapping id="mysql-foo" options="rw" source-dir="/foo" target-dir="/foo"/>
|
||||
<storage-mapping id="mysql-bar" options="rw" source-dir="/bar" target-dir="/bar"/>
|
||||
</storage>
|
||||
</bundle>
|
||||
</resources>
|
||||
</configuration>
|
||||
</cib>
|
||||
'''
|
||||
cmd = "#{CRMDIFF_BIN} --cib --original-string='#{xml1}' --new-string='#{xml2}'"
|
||||
cmd_out = `#{cmd}`
|
||||
ret = $?.exitstatus
|
||||
return false if ret == 0
|
||||
return true if ret == 1
|
||||
raise Puppet::Error, "#{cmd} failed with (#{ret}): #{cmd_out}"
|
||||
end
|
||||
|
||||
# This function will return true when a CIB diff xml has an empty meta_attribute change (either
|
||||
# addition or removal). It does so by veryfiying that the diff has an empty meta_attribute node
|
||||
# and when that is the case it verifies that the corresponding meta_attributes
|
||||
|
|
@ -420,8 +352,8 @@ def has_empty_meta_attributes?(cibfile, element)
|
|||
return false
|
||||
end
|
||||
|
||||
|
||||
# same as pcmk_restart_resource? but using crm_diff
|
||||
# This given a cib (and it's .orig copy) and a resource name, this method returns true if pacemaker
|
||||
# will restart the resource false if no action will be taken by pacemaker
|
||||
def pcmk_restart_resource_ng?(resource_name, cib)
|
||||
cmd = "#{CRMDIFF_BIN} --cib -o #{cib}.orig -n #{cib}"
|
||||
cmd_out = `#{cmd}`
|
||||
|
|
@ -450,26 +382,6 @@ def pcmk_restart_resource_ng?(resource_name, cib)
|
|||
return false
|
||||
end
|
||||
|
||||
# This given a cib and a resource name, this method returns true if pacemaker
|
||||
# will restart the resource false if no action will be taken by pacemaker
|
||||
# Note that we need to leverage crm_simulate instead of crm_diff due to:
|
||||
# https://bugzilla.redhat.com/show_bug.cgi?id=1561617
|
||||
def pcmk_restart_resource?(resource_name, cib, is_bundle=false)
|
||||
tmpfile = pcmk_tmpname("#{PCMK_TMP_BASE}/puppet-cib-simulate", nil)
|
||||
cmd = "#{CRMSIMULATE_BIN} -x #{cib} -s -G#{tmpfile}"
|
||||
crm_out = `#{cmd}`
|
||||
if $?.exitstatus != 0
|
||||
FileUtils.rm(tmpfile, :force => true)
|
||||
delete_cib(cib)
|
||||
raise Puppet::Error, "#{cmd} failed with: #{crm_out}"
|
||||
end
|
||||
# Now in tmpfile we have the xml of the changes to the cluster
|
||||
# If tmpfile only contains one empy <transition_graph> no changes took place
|
||||
ret = pcmk_graph_contain_id?(resource_name, tmpfile, is_bundle)
|
||||
FileUtils.rm(tmpfile, :force => true)
|
||||
return ret
|
||||
end
|
||||
|
||||
# This method takes a resource and a creation command and does the following
|
||||
# 1. Deletes the resource from the offline CIB
|
||||
# 2. Recreates the resource on the offline CIB
|
||||
|
|
@ -488,13 +400,8 @@ def pcmk_resource_has_changed?(resource, cmd_update, cmd_pruning='', is_bundle=f
|
|||
delete_cib(cib)
|
||||
raise Puppet::Error, "pcmk_resource_has_changed? #{cmd_update} returned error #{resource[:name]}. This should never happen."
|
||||
end
|
||||
if is_crm_diff_buggy?
|
||||
ret = pcmk_restart_resource?(resource[:name], cib, is_bundle)
|
||||
Puppet.debug("pcmk_resource_has_changed returned #{ret} for resource #{resource[:name]}")
|
||||
else
|
||||
ret = pcmk_restart_resource_ng?(resource[:name], cib)
|
||||
Puppet.debug("pcmk_resource_has_changed (ng version) returned #{ret} for resource #{resource[:name]}")
|
||||
end
|
||||
ret = pcmk_restart_resource_ng?(resource[:name], cib)
|
||||
Puppet.debug("pcmk_resource_has_changed (ng version) returned #{ret} for resource #{resource[:name]}")
|
||||
delete_cib(cib)
|
||||
return ret
|
||||
end
|
||||
|
|
|
|||
|
|
@ -15,35 +15,15 @@ describe "pcmk_common functions" do
|
|||
FileUtils.cp orig_cib, "cib-bundle.xml.orig"
|
||||
end
|
||||
|
||||
it "pcmk_graph_contain_id? raises proper exception" do
|
||||
expect { pcmk_graph_contain_id?('foo', 'bar') }.to raise_error(Errno::ENOENT)
|
||||
end
|
||||
|
||||
it "pcs_offline noop update" do
|
||||
expect(pcs_offline('resource update ip-172.16.11.97 cidr_netmask=32', 'cib-noop.xml')).to eq ""
|
||||
expect(pcs_offline('resource update stonith-fence_ipmilan-stonith-fence-1 passwd=renVamyep3!', 'cib-noop.xml')).to eq ""
|
||||
end
|
||||
context 'when crm_diff is not buggy', if: is_crm_diff_buggy?() == true do
|
||||
it "pcmk_restart_resource? noop" do
|
||||
expect(pcmk_restart_resource?('foo', "cib-noop.xml")).to eq false
|
||||
expect(pcmk_restart_resource?('ip-172.16.11.97', "cib-noop.xml")).to eq false
|
||||
expect(pcmk_restart_resource?('stonith-fence_ipmilan-stonith-fence-1', "cib-noop.xml")).to eq false
|
||||
end
|
||||
end
|
||||
|
||||
it "pcs_offline update to resource definition" do
|
||||
expect(pcs_offline('resource update ip-172.16.11.97 cidr_netmask=31', 'cib-resource.xml')).to eq ""
|
||||
expect(pcs_offline('resource update stonith-fence_ipmilan-stonith-fence-1 passwd=NewPassword', 'cib-resource.xml')).to eq ""
|
||||
end
|
||||
context 'when crm_diff is not buggy', if: is_crm_diff_buggy?() == true do
|
||||
it "pcmk_restart_resource? vip resource" do
|
||||
expect(pcmk_restart_resource?('foo', "cib-resource.xml")).to eq false
|
||||
expect(pcmk_restart_resource?('ip-172.16.11.97', "cib-resource.xml")).to eq true
|
||||
end
|
||||
it "pcmk_restart_resource? stonith resource" do
|
||||
expect(pcmk_restart_resource?('foo', "cib-resource.xml")).to eq false
|
||||
expect(pcmk_restart_resource?('stonith-fence_ipmilan-stonith-fence-1', "cib-resource.xml")).to eq true
|
||||
end
|
||||
end
|
||||
|
||||
it "pcs_offline update to bundle definition" do
|
||||
# We effectively change the number of replicas from 3 to 2
|
||||
|
|
@ -65,28 +45,21 @@ describe "pcmk_common functions" do
|
|||
expect(pcs_offline(cmd, 'cib-bundle.xml')).to eq ""
|
||||
end
|
||||
|
||||
it "pcmk_restart_resource? bundle resource" do
|
||||
expect(pcmk_restart_resource?('foo', "cib-bundle.xml", true)).to eq false
|
||||
expect(pcmk_restart_resource?('test_bundle', "cib-bundle.xml", true)).to eq true
|
||||
it "pcmk_restart_resource_ng? noop" do
|
||||
expect(pcmk_restart_resource_ng?('foo', "cib-noop.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('ip-172.16.11.97', "cib-noop.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('stonith-fence_ipmilan-stonith-fence-1', "cib-noop.xml")).to eq false
|
||||
end
|
||||
|
||||
context 'when crm_diff is not buggy', if: is_crm_diff_buggy?() == false do
|
||||
it "pcmk_restart_resource_ng? noop" do
|
||||
expect(pcmk_restart_resource_ng?('foo', "cib-noop.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('ip-172.16.11.97', "cib-noop.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('stonith-fence_ipmilan-stonith-fence-1', "cib-noop.xml")).to eq false
|
||||
end
|
||||
it "pcmk_restart_resource_ng? vip resource" do
|
||||
expect(pcmk_restart_resource_ng?('foo', "cib-resource.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('ip-172.16.11.97', "cib-resource.xml")).to eq true
|
||||
end
|
||||
it "pcmk_restart_resource_ng? stonith resource" do
|
||||
expect(pcmk_restart_resource_ng?('foo', "cib-resource.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('stonith-fence_ipmilan-stonith-fence-1', "cib-resource.xml")).to eq true
|
||||
end
|
||||
it "pcmk_restart_resource_ng? bundle resource" do
|
||||
expect(pcmk_restart_resource_ng?('foo', "cib-bundle.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('test_bundle', "cib-bundle.xml")).to eq true
|
||||
end
|
||||
it "pcmk_restart_resource_ng? vip resource" do
|
||||
expect(pcmk_restart_resource_ng?('foo', "cib-resource.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('ip-172.16.11.97', "cib-resource.xml")).to eq true
|
||||
end
|
||||
it "pcmk_restart_resource_ng? stonith resource" do
|
||||
expect(pcmk_restart_resource_ng?('foo', "cib-resource.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('stonith-fence_ipmilan-stonith-fence-1', "cib-resource.xml")).to eq true
|
||||
end
|
||||
it "pcmk_restart_resource_ng? bundle resource" do
|
||||
expect(pcmk_restart_resource_ng?('foo', "cib-bundle.xml")).to eq false
|
||||
expect(pcmk_restart_resource_ng?('test_bundle', "cib-bundle.xml")).to eq true
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in New Issue