openstack
/
puppet-pacemaker
Code Issues Proposed changes

Add a deep_compare option in bundles and resources 

A new 'deep_compare' parameter gets added to resources and bundles
 (defaulting to false). The rationale is the following:

By default pacemaker does not update resources and will keep doing so.
This happens for a number of mostly historical reasons (old non HA-NG
OSP releases had way too many relationships between pacemaker resources
and puppet-triggered restart would cause havoc on the overcloud, also
lack of the code for doing this in puppet-pacemaker was a factor).

When set to true additional code comparison logic will be triggered and
we will actually update pacemaker resources. We do so by modifying an
offline CIB with the manifests resources and then running crm_simulate
on it in order to determine if pacemaker would trigger a restart of the
resource if the CIB was to be pushed for real on to the cluster. We
currently cannot use crm_diff due to a bug where it would trigger too
many false positives.  Using crm_simulate has one small limitation.
Namely, changes in resource operation timeouts normally do not trigger a
resource restart and hence they won't update a resource. Also note that
this is currently only supported for resources and bundles (not
constraints nor stonith resources).

Some thought has been put into whether we should be having the
'deep_compare' parameter a global pacemaker::corosync one or a
per-resource parameter. I chose the latter for the following reasons:
- It might be useful to let the operator decide for which resources the
  deep comparison is enabled (think tripleo upgrades where we want to
  make sure only certain resources are upgraded)
- It makes it much clearer which resources support this deeper
  comparison feature.

Change-Id: I11b5f9097694b88431f21e1e806bc1823386528f
This commit is contained in:
Michele Baldessari 2018-04-09 08:56:35 +02:00
parent da4673ffbe
commit 8df2e01421
12 changed files with 105 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
lib/puppet/type/pcmk_bundle.rb
View File

@ -125,4 +125,13 @@ Puppet::Type.newtype(:pcmk_bundle) do
newproperty(:location_rule) do
desc "A location rule constraint hash"
end
newparam(:deep_compare, :boolean => true, :parent => Puppet::Parameter::Boolean) do
desc "Whether to enable deep comparing of resource
When set to true a resource will be compared in full (options, meta parameters,..)
to the existing one and in case of difference it will be repushed to the CIB
Defaults to `false`."
defaultto false
end
end

9
lib/puppet/type/pcmk_resource.rb
View File

@ -119,4 +119,13 @@ Puppet::Type.newtype(:pcmk_resource) do
defaultto 60
end
newparam(:deep_compare, :boolean => true, :parent => Puppet::Parameter::Boolean) do
desc "Whether to enable deep comparing of resource
When set to true a resource will be compared in full (options, meta parameters,..)
to the existing one and in case of difference it will be repushed to the CIB
Defaults to `false`."
defaultto false
end
end

8
manifests/resource/bundle.pp
View File

@ -80,6 +80,12 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
# None
@ -119,6 +125,7 @@ define pacemaker::resource::bundle(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = hiera('pacemaker::resource::bundle::deep_compare', false),
) {
if $image == undef {
fail("Cannot create bundle ${name} without specifying an image")
@ -145,5 +152,6 @@ define pacemaker::resource::bundle(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location_rule => $location_rule,
deep_compare => $deep_compare,
}
}

8
manifests/resource/filesystem.pp
View File

@ -79,6 +79,12 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
# None
@ -120,6 +126,7 @@ define pacemaker::resource::filesystem(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = false,
) {
$resource_id = delete("fs-${directory}", '/')
@ -142,5 +149,6 @@ define pacemaker::resource::filesystem(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location_rule => $location_rule,
deep_compare => $deep_compare,
}
}

8
manifests/resource/ip.pp
View File

@ -74,6 +74,12 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
# None
@ -114,6 +120,7 @@ define pacemaker::resource::ip(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = hiera('pacemaker::resource::ip::deep_compare', false),
) {
if !is_ipv6_address($ip_address) and $ipv6_addrlabel != '' {
fail("ipv6_addrlabel ${ipv6_addrlabel} was specified, but ${ip_address} is not an ipv6 address")
@ -149,6 +156,7 @@ define pacemaker::resource::ip(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location_rule => $location_rule,
deep_compare => $deep_compare,
}
}

8
manifests/resource/lsb.pp
View File

@ -70,6 +70,12 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
# None
@ -109,6 +115,7 @@ define pacemaker::resource::lsb(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = false,
) {
pcmk_resource { $name:
ensure => $ensure,
@ -124,5 +131,6 @@ define pacemaker::resource::lsb(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location => $location_rule,
deep_compare => $deep_compare,
}
}

7
manifests/resource/ocf.pp
View File

@ -74,6 +74,11 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
@ -115,6 +120,7 @@ define pacemaker::resource::ocf(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = hiera('pacemaker::resource::ocf::deep_compare', false),
) {
pcmk_resource { $name:
ensure => $ensure,
@ -131,5 +137,6 @@ define pacemaker::resource::ocf(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location_rule => $location_rule,
deep_compare => $deep_compare,
}
}

8
manifests/resource/remote.pp
View File

@ -59,6 +59,12 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
# None
@ -95,6 +101,7 @@ define pacemaker::resource::remote(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = false,
) {
pcmk_resource { $name:
ensure => $ensure,
@ -109,5 +116,6 @@ define pacemaker::resource::remote(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location_rule => $location_rule,
deep_compare => $deep_compare,
}
}

8
manifests/resource/route.pp
View File

@ -69,6 +69,12 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
# None
@ -108,6 +114,7 @@ define pacemaker::resource::route(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = false,
) {
$nic_option = $nic ? {
@ -142,6 +149,7 @@ define pacemaker::resource::route(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location_rule => $location_rule,
deep_compare => $deep_compare,
}
}

8
manifests/resource/service.pp
View File

@ -70,6 +70,12 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
# None
@ -109,6 +115,7 @@ define pacemaker::resource::service(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = false,
) {
include ::pacemaker::params
$res = "pacemaker::resource::${::pacemaker::params::services_manager}"
@ -128,6 +135,7 @@ define pacemaker::resource::service(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location_rule => $location_rule,
deep_compare => $deep_compare,
}
})
}

8
manifests/resource/systemd.pp
View File

@ -70,6 +70,12 @@
# }
# Defaults to undef
#
# [*deep_compare*]
# (optional) Enable deep comparing of resources and bundles
# When set to true a resource will be compared in full (options, meta parameters,..)
# to the existing one and in case of difference it will be repushed to the CIB
# Defaults to false
#
# === Dependencies
#
# None
@ -109,6 +115,7 @@ define pacemaker::resource::systemd(
$try_sleep = 0,
$verify_on_create = false,
$location_rule = undef,
$deep_compare = false,
) {
pcmk_resource { $name:
ensure => $ensure,
@ -124,5 +131,6 @@ define pacemaker::resource::systemd(
try_sleep => $try_sleep,
verify_on_create => $verify_on_create,
location_rule => $location_rule,
deep_compare => $deep_compare,
}
}

16
releasenotes/notes/update-resources-ae2ef4f9e75e8699.yaml Normal file
View File

@ -0,0 +1,16 @@
---
features:
- |
A new 'deep_compare' parameter has been added to the pacemaker::resource::* classes (defaulting to false).
By default pacemaker does not update resources and will keep doing so. This happens for a number of
mostly historical reasons (old non HA-NG OSP releases had way too many relationships between pacemaker
resources and puppet-triggered restart would cause havoc on the overcloud, also lack of the code for
doing this in puppet-pacemaker was a factor).
When set to true additional code comparison logic will be triggered and we will actually update
pacemaker resources. We do so by modifying an offline CIB with the manifests resources and then
running crm_simulate on it in order to determine if pacemaker would trigger a restart of the resource
if the CIB was to be pushed for real on to the cluster. We currently cannot use crm_diff due to a bug
where it would trigger too many false positives. Using crm_simulate has one small side-effect. Namely,
changes in resource operation timeouts normally do not trigger a resource restart and hence they won't
update a resource. Also note that this is currently only supported for resources and bundles (not constraints
not stonith resources).