bloomberg/chef-bcpc — pull request #265 by caiush (2014-08-13 19:26:30+00:00)
Title: Bug Fix And Switched to using the bcpc_cephpool LWRP rather than bash

This PR was motivated by a bug in all the pg number setting bash blocks that are scattered about the code.

The Bug:

consider,
```
bash "set-glance-rados-pool-#{pg}" do
user "root"
optimal = power_of_2(get_ceph_osd_nodes.length*node['bcpc']['ceph']['pgs_per_node']/node['bcpc']['ceph']['images']['replicas']*node['bcpc']['ceph']['images']['portion']/100)
code "ceph osd pool set #{node['bcpc']['ceph']['images']['name']} #{pg} #{optimal}"
not_if "((`ceph osd pool get #{node['bcpc']['ceph']['images']['name']} #{pg} | awk '{print $2}'` >= #{optimal}))"
notifies :run, "bash[wait-for-pgs-creating]", :immediately
end
```

The not_if is actually evaluated in sh not bash, so this clause doesn't actually work. You can see this by just looking at a chef run. It will always execute this bash block even on a system with the correct number of pgs.

The impact:

This means that if you have a cluster with pg_num > optimal, which you may have due to historical reasons, the chef run bombs as ceph refuses to lower the number of pgs. It is also potentially dangerous as things get notified of a change when there wasn't one.

The solution:

In later versions of chef you can specify the guard interpreter (http://docs.getchef.com/resource_common.html#guard-interpreters). But in the version used in this project this isn't possible.

I could get the desired behavior by some even more funky shell code or by using a ruby block and computing the current and optimal on the fly and use an if, e.g.
```
ruby_block "pool-tests" do
block do
current = `ceph osd pool get volumes-ssd pgp_num | awk '{}{print $2}' | tr -d '\n'`.to_i
optimal = power_of_2(get_ceph_osd_nodes.length*node['bcpc']['ceph']['pgs_per_node']/node['bcpc']['ceph']['volumes']['replicas']*node['bcpc']['ceph']['volumes']['portion']/100/node['bcpc']['ceph']['enabled_pools'].length)

if current < optimal
%[ceph osd pool set volumes-ssd pgp_num #{optimal}]
end

end
end
```
(I would have to do something to trigger the notifies but you get the idea)

This works, but then I was faced with editing the 5 or so places with the same structure. When you think about what the ruby_block is actually doing, it is creating an ad-hoc LWRP by inlining a block of code into a skeleton provider. So might was well write a proper LWRP and be done with it.

This makes the code much more readable and maintainable, and it turns out to be pretty easy to do.

The LWRP correctly waits for pgs to create before completing (so no need for the current "wait-for-pgs-creating" notify), and also correctly fires a changed event if we add any notifies in future.

This PR works for me pretty well, but as its a big change I would like others to try bringing this up themselves before merging.

There are some improvements that could be made in the future like: not using the converge_by and directly firing the notifies so we don't have to replicate logic in the provider. That said I still think its a solid first pass.