Skip to content

Commit

Permalink
Stop ignoring order of primitives in colocation constraints
Browse files Browse the repository at this point in the history
cs_colocation, when used with the pcs provider, makes the incorrect
assumption that the order of primitives in colocation constraints is
arbitrary. It is not, and the following must not be equivalent:

cs_colocation { 'vip_with_service':
  primitives => [ 'nginx_vip', 'nginx_service' ],
}

cs_colocation { 'vip_with_service':
  primitives => [ 'nginx_service', 'nginx_vip' ],
}

These mean different things. The first example ensures that nginx_vip
runs on whatever node nginx_service runs on, and if nginx_service
can't be started on any node, Pacemaker won't even attempt to start
nginx_vip. However, if nginx_vip cannot be started on any node,
nginx_service will continue to be available.

The second example means the opposite. In it, nginx_service is
configured to run wherever nginx_vip runs, and if nginx_vip can't run
anywhere, nginx_service won't start. And if nginx_vip cannot be
started on any node, nginx_service will not be available.

As a result, drop artificial sorting for the primitives, and use them
exactly as specified.

Fixes issue voxpupuli#150.
  • Loading branch information
fghaas committed Jul 14, 2015
1 parent 5b72ac7 commit b161978
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions lib/puppet/provider/cs_colocation/pcs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,10 @@ def self.instances
with_rsc = items['with-rsc']
end

# Sorting the array of primitives because order doesn't matter so someone
# switching the order around shouldn't generate an event.
colocation_instance = {
:name => items['id'],
:ensure => :present,
:primitives => [rsc, with_rsc].sort,
:primitives => [rsc, with_rsc],

This comment has been minimized.

Copy link
@fghaas

fghaas Jul 16, 2015

Author Owner

If this is meant to be in line with the crm provider and the definition given in the type, by the way, then the order of this needs to be swapped. The type seems to want to redefine the order in which primitives are specified.

:score => items['score'],
:provider => self.name,
:new => false
Expand Down Expand Up @@ -89,7 +87,7 @@ def score
# resource already exists so we just update the current value in the property
# hash and doing this marks it to be flushed.
def primitives=(should)
@property_hash[:primitives] = should.sort
@property_hash[:primitives]
end

def score=(should)
Expand Down

0 comments on commit b161978

Please sign in to comment.