cs_colocation sets don't work properly

[langesven]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 5.5.2
• Ruby: 2.4.4
• Distribution: RHEL 7
• Module version: 6.0.2-rc0

How to reproduce (e.g Puppet code you use)

First set of errors:

    cs_colocation { 'vip_with_services':
        cib           => 'puppet',
        primitives => [
            ['fs_test', 'service', 'vip'],
        ],
    }   

Second set of errors, using the output of puppet resource:

    cs_colocation { 'vip_with_services':
        ensure     => 'present',
        cib        => 'puppet',
        primitives => [
            {
                'primitives' => [
                    'fs_test', 'service', 'vip'
                ]
            }
        ],
    }

What are you seeing

First puppetrun, with no previous colocations set:

Notice: /Stage[main]/Profile::Corosync/Cs_colocation[vip_with_services]/ensure: created                                                                                                      
Debug: Puppet::Type::Cs_colocation::ProviderPcs: Executing ["/sbin/pcs", "constraint", "colocation", "set", ["fs_test", "service", "vip"], "setoptions", "id=vip_with_services", "score=INFINITY", "-f", "/opt/puppetlabs/puppet/cache/shadow.puppet"] in the shadow CIB "puppet"                                        
Debug: Executing: '/sbin/pcs constraint colocation set fs_test service vip setoptions id=vip_with_services score=INFINITY -f /opt/puppetlabs/puppet/cache/shadow.puppet'                                                                                                                                                                              
Info: /Stage[main]/Profile::Corosync/Cs_colocation[vip_with_services]: Scheduling refresh of Cs_commit[puppet]                                                                               

Asking puppet to define this resource:

# puppet resource cs_colocation
cs_colocation { 'vip_with_services':
  ensure     => 'present',
  primitives => [
  {
    'primitives' => ['fs_test', 'service', 'vip']
  }],
  score      => 'INFINITY',
}

Subsequent puppetruns produce:

Debug: Prefetching pcs resources for cs_colocation                                                                                                                                           
Debug: Puppet::Type::Cs_colocation::ProviderPcs: Executing ["/sbin/pcs", "cluster", "cib"] in the CIB                                                                                        
Debug: Executing: '/sbin/pcs cluster cib'                                     
Debug: Cs_colocation[vip_with_services](provider=pcs): {:name=>"vip_with_services", :ensure=>:present, :primitives=>[{"primitives"=>["fs_test", "service", "vip"]}], :score=>"INFINITY", :provider=>:pcs, :new=>false}                                                                                                  
Notice: /Stage[main]/Profile::Corosync/Cs_colocation[vip_with_services]/primitives: primitives changed [
  {                                                          
    'primitives' => ['fs_test', 'service', 'vip']
  }] to [                                                                                                                                                                                    
  ['fs_test', 'service', 'vip']]
Debug: Cs_colocation[vip_with_services](provider=pcs): Removing colocation
Debug: Puppet::Type::Cs_colocation::ProviderPcs: Executing ["/sbin/pcs", "constraint", "remove", "vip_with_services", "-f", "/opt/puppetlabs/puppet/cache/shadow.puppet"] in the shadow CIB "puppet"
Debug: Executing: '/sbin/pcs constraint remove vip_with_services -f /opt/puppetlabs/puppet/cache/shadow.puppet'
Debug: Puppet::Type::Cs_colocation::ProviderPcs: Executing ["/sbin/pcs", "constraint", "colocation", "set", ["fs_test", "service", "vip"], "setoptions", "id=vip_with_services", "score=INFINITY", "-f", "/opt/puppetlabs/puppet/cache/shadow.puppet"] in the shadow CIB "puppet"
Debug: Executing: '/sbin/pcs constraint colocation set fs_test service vip setoptions id=vip_with_services score=INFINITY -f /opt/puppetlabs/puppet/cache/shadow.puppet'
Info: /Stage[main]/Profile::Corosync/Cs_colocation[vip_with_services]: Scheduling refresh of Cs_commit[puppet]

which errors with:

The new CIB is the same as the original CIB, nothing to push.

When trying to use the output of puppet resource instead of the double array:

Debug: Prefetching pcs resources for cs_colocation                                          
Debug: Puppet::Type::Cs_colocation::ProviderPcs: Executing ["/sbin/pcs", "cluster", "cib"] in the CIB                                                                                         
Debug: Executing: '/sbin/pcs cluster cib'                                                                                                                                                     
Debug: Cs_colocation[vip_with_services](provider=pcs): {}                                                                                                                                     
Notice: /Stage[main]/Profile::Corosync/Cs_colocation[vip_with_services]/ensure: created                                                                                                      
Error: /Stage[main]/Profile::Corosync/Cs_colocation[vip_with_services]: Could not evaluate: undefined method `include?' for nil:NilClass                                                    

What behaviour did you expect instead

I expected the first bit of code to work and not produce errors on subsequent puppetruns because nothing changed. But it thinks stuff changed and thus tries to delete the constraint and re-create it the same way it was before and as such pacemaker throws an error.

Output log

see above

Any additional information you'd like to impart

It would appear that parsing of stuff in the XML config via the provider produces a resource that the type can't work with.
Parsing happens in def self.instances of provider/cs_colocation/pcs.rb while the primitive parsing of the resource happens in def extract_primitives of type/cs_colocation.rb

Relevant output of the XML config of the proper colocation resource

[...]
  <configuration>                                                                                                                                                                             
[...]
    <constraints>
      <rsc_colocation score="INFINITY" id="vip_with_services">
        <resource_set id="pcs_rsc_set_fs_test_service_vip">
          <resource_ref id="fs_test"/>
          <resource_ref id="service"/>
          <resource_ref id="vip"/>
        </resource_set>
      </rsc_colocation>
    </constraints>
  </configuration>
[...]

[btravouillon]

Hi @langesven

Between first and subsequent runs, you change the definition of cs_colocation, while it ends with the same pcs command to create the constraint.

Debug: Puppet::Type::Cs_colocation::ProviderPcs: Executing ["/sbin/pcs", "constraint", "colocation", "set", ["fs_test", "service", "vip"], "setoptions", "id=vip_with_services", "score=INFINITY", "-f", "/opt/puppetlabs/puppet/cache/shadow.puppet"] in the shadow CIB "puppet"
Debug: Executing: '/sbin/pcs constraint colocation set fs_test service vip setoptions id=vip_with_services score=INFINITY -f /opt/puppetlabs/puppet/cache/shadow.puppet'

Because the definition changed in the catalog, puppet applies the change. There is a difference with previous catalog. That's an expected behavior.

The error message The new CIB is the same as the original CIB, nothing to push is thrown by pcs.
When using a ghost CIB with pcs, the commit command (the one that actually push the ghost CIB to the CIB) checks the diff before comitting. If no diff, its return code is 1. The patch ClusterLabs/pcs#166 changes this behavior. It merged upstream, but it could need some time to land into RHEL.

[langesven]

Hi @actatux

well, my point is though, I'm not changing the definition between runs - puppet interprets the resource it creates based on my definition as a different definition.

My code in the .pp is and stays

    cs_colocation { 'vip_with_services':
        cib           => 'puppet',
        primitives => [
            ['fs_test', 'service', 'vip'],
        ],
    }   

which makes puppet create the resource in the first run.
In a subsequent run puppet determines existing cs_colocation resources by parsing the XML and ends up with the resource

cs_colocation { 'vip_with_services':
  ensure     => 'present',
  primitives => [
  {
    'primitives' => ['fs_test', 'service', 'vip']
  }],
  score      => 'INFINITY',
}

as evident in the puppetrun output and by running puppet resource cs_colocation

So the module is using one code snippet to create the resource, but when it reads back the resource it created it doesn't end up the same code snippet, possibly due to a parsing bug.

And because the module reads back the resource with a different definition it tries to delete and re-create it, because it thinks something changed. Which yes, given your linked pcs fix would not create an error.

But in my opinion the fact that puppet thinks the resource changed (even though it didn't) because it parses the resource to different puppetcode than the code that created it is a bug.

I did try to create the resource with the definition that puppet spits out (thinking the code I use might just randomly create the correct resource, but in reality the puppetcode is correct about the definition and I'm just lucky that my code worked), but that errors completely with the undefined method `include?' for nil:NilClass output

[timdeluxe]

Please ignore my commit to this issue, used the wrong commit message :-(