cs_colocation's self.instances is huge, and should be split
Closed this issue · 4 comments
igalic commented
the following function is huge, not very ruby-like, and can be split in at least two functions (between if and else) and multiple helpers.
def self.instances
block_until_ready
instances = []
cmd = [ command(:crm), 'configure', 'show', 'xml' ]
if Puppet::PUPPETVERSION.to_f < 3.4
raw, status = Puppet::Util::SUIDManager.run_and_capture(cmd)
else
raw = Puppet::Util::Execution.execute(cmd)
status = raw.exitstatus
end
doc = REXML::Document.new(raw)
doc.root.elements['configuration'].elements['constraints'].each_element('rsc_colocation') do |e|
rscs = []
items = e.attributes
if items['rsc']
# The colocation is defined as a single rsc_colocation element. This means
# the format is rsc and with-rsc. In the type we chose to always deal with
# ordering in a sequential way, which is why we reverse their order.
if items['rsc-role']
rsc = "#{items['rsc']}:#{items['rsc-role']}"
else
rsc = items['rsc']
end
if items ['with-rsc-role']
with_rsc = "#{items['with-rsc']}:#{items['with-rsc-role']}"
else
with_rsc = items['with-rsc']
end
# Put primitives in chronological order, first 'with-rsc', then 'rsc'.
primitives = [with_rsc , rsc]
else
# The colocation is defined as a rsc_colocation element wrapped around a single resource_set.
# This happens automatically when you configure a colocation between more than 2 primitives.
# Notice, we can only interpret colocations of single sets, not multiple sets combined.
# In Pacemaker speak, this means we can support "A B C" but not e.g. "A B (C D) E".
# Feel free to contribute a patch for this.
primitives = []
e.each_element('resource_set') do |rset|
rsetitems = rset.attributes
# If the resource set has a role, it will apply to all referenced resources.
if rsetitems['role']
rsetrole = rsetitems['role']
else
rsetrole = nil
end
# Add all referenced resources to the primitives array.
rset.each_element('resource_ref') do |rref|
rrefitems = rref.attributes
if rsetrole
# Make sure the reference is stripped from a possible role
rrefprimitive = rrefitems['id'].split(':')[0]
# Always reuse the resource set role
primitives.push("#{rrefprimitive}:#{rsetrole}")
else
# No resource_set role was set: just push the complete reference.
primitives.push(rrefitems['id'])
end
end
end
end
colocation_instance = {
:name => items['id'],
:ensure => :present,
:primitives => primitives,
:score => items['score'],
:provider => self.name
}
instances << new(colocation_instance)
end
instances
end
my perception is that those helpers could also be used in other cs_* providers.
roidelapluie commented
We need to remove the self.instances
igalic commented
huh?
roidelapluie commented
see #182