Code review comment for lp:~jseutter/charms/precise/storage/storage-with-broker-provider

Revision history for this message
Chad Smith (chad.smith) wrote :

Hey Jerry,
  I had a couple of things I needed to fix in this branch to make it work for remove-relation postgresql storage and add-relation postgresl postgresql

[6] pulled in persist* functions from block-storage-broker charm
[7] persistent mountpoint saved during mount_volume

[8] needed symlink in block-storage-broker between block-storage-relation-broken and data-relation-departed down on block-storage-broker provider. I added block-storage-relation-departed symlink in there too as it's probably better to do this unmount action during departed if we can instead of broken. We should leave broken there too though as that hook will only fire when the instance dies on us without a remove-relation call .

[9] use persistent mountpoint info instead of relation-get mointpoint (as this relation-data might disappear on some departed/broken hooks)

here's the pastebin of what I patched in your branch: including the unit tests just pulled over from block-storage-broker charm
https://pastebin.canonical.com/104745/

Okay enough of the comment steamrolling. I was trying to get the storage-related concerns out of the way so I could prove our postgresql charm was the one having problems.

Your branch with this patch works very well with the postgres-storage.cfg listed above.
add-relation remove-relation between storage works great and postgres can pickup and remount the nova volume fine upon relation-rejoined with a simple juju resolved --retry postgresql/0. As expected postgresql/0 goes into error hook state when the nova volume disappears.

Approving your branch with the following patch above as we keep lumping more features into it. Sorry man

review: Approve

« Back to merge proposal