voxpupuli/puppet-corosync

cs_colocation's self.instances is huge, and should be split

Closed this issue · 4 comments

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.

We need to remove the self.instances

huh?

Closing in favor of #182