Merge lp:~chad.smith/charms/precise/postgresql/postgresql-using-storage-subordinate into lp:charms/postgresql

[Chad Smith]

Both storage subordinate charm and block storage broker are now live in the charm store. I'm unblocking this review now that the storage and block-storage-subordinate are in the charmstore. The bundle described below is updated to point to the public approved charm branches.

----------------------
There is a new storage subordinate charm that has incorporated all storage mount,fsck and partitioning that the postgresql charm performed as well as talking to the block-storage-broker charm to automatically create, attach. reattach or detach nova volumes or nfs shares on behalf of your principal postgresql units.

The storage subordinate attempts to avoid duplication of partitioning, mounting and attaching devices in multiple charms by coalescing all that work in one charm. Only thing that the principal (postgresql in this case) has to provide is a preferred mountpoint to the storage subordinate.

This branch does the following:
1. adds the data relation to metadata.yaml for communicating with the storage subordinate to request a mountpoint.
2. drops all postgresql scripts/code used for discovery, partitioning, fsck and mount of attached nova volumes in favor of using the consolidated storage subordinate charm.
3. adds a new config option storage_mount_point (which defaults to /mnt/storage).
4. drops the config options volume-ephemeral-storage and volume-map as volume-map can be specified in the storage subordinate and ephemeral=false is inferred because of a relation with storage subordinate
5. calls posgresql_stop() on data-relation-departed to shutdown postgresql services if our persistent volume mount disapears
6. some more unit tests for code coverage

To deploy and test without any nova volumes created:

$ cat >postgresql-storage-bundle.cfg <<END
common:
    services:
        postgresql:
            branch: lp:~chad.smith/charms/precise/postgresql/postgresql-using-storage-subordinate
            constraints: mem=2048
            options:
                extra-packages: python-apt postgresql-contrib postgresql-9.1-debversion
                max_connections: 500
        block-storage-broker:
            branch: lp:~charmers/charms/precise/block-storage-broker/trunk
            options:
                key: <YOUR_OS_USERNAME>
                endpoint: https://keystone.canonistack.canonical.com:443/v2.0/
                region: <YOUR_OS_REGION_NAME>
                secret: <YOUR_OS_PASSWORD>
                tenant: <YOUR_OS_TENANT_NAME>

doit-no-nova-volume:
    inherits: common
    series: precise
    services:
        storage:
            branch: lp:~charmers/charms/precise/storage/trunk
            options:
                provider: block-storage-broker
                volume_size: 9
    relations:
        - [postgresql, storage]
        - [storage, block-storage-broker]

doit-with-existing-nova-volume:
    inherits: common
    series: precise
    services:
        storage:
            branch: lp:~charmers/charms/precise/storage/trunk
            options:
                provider: block-storage-broker
                volume_size: 9
                volume_map: "{postgresql/0: YOUR-POSTGRES-NOVA-VOLUME-ID}"

    relations:
        - [postgresql, storage]
        - [storage, block-storage-broker]

END

# The following will deploy postgresql related to storage and storage related to block-storage-broker and will create a volume for each postgres unit you create
$ juju-deployer -c postgresql-storage-bundle.cfg doit-no-nova-volume

# The following will deploy postgresql related to storage and storage related to block-storage-broker and will use volume-map to attach the pre-created YOUR-NOVA-VOLUME-ID to your postgresql/0 instance. Any postgresql unit not in volume-map will have a new volume created for it.
$ juju-deployer -c postgresql-storage-bundle.cfg doit-with-existing-nova-volume

[Stuart Bishop (stub)]

On 15 February 2014 06:34, Chad Smith <email address hidden> wrote:

> There is a new storage subordinate charm that has incorporated all storage mount,fsck and partitioning that the postgresql charm performed as well as talking to the block-storage-broker charm to automatically create, attach. reattach or detach nova volumes or nfs shares on behalf of your principal postgresql units.
>
> The storage subordinate attempts to avoid duplication of partitioning, mounting and attaching devices in multiple charms by coalescing all that work in one charm. Only thing that the principal (postgresql in this case) has to provide is a preferred mountpoint to the storage subordinate.

Thanks for this. I think this is a good approach.

How is this branch handing upgrading of existing installation? Do we
add a subordinate using the existing disk and mount point, or do we
add a subordinate to use a new mount and migrate data from old-mount
as if it was local disk?

I do think this branch does not go far enough. I would like to see
more storage related code pushed into the subordinate. I believe that
with juju-run (in stable in ~2 weeks I'm told), we can remove *all*
storage related code from the parent charm, lowering the bar or
removing the barrier entirely for charms to use the subordinate
storage service.

For example, the PostgreSQL charm could do nothing except define a
storage-relation-joined hook that simply does:

    relation-set storage-path=/var/lib/postgresql/9.1/main

The subordinate charm would be responsible for:

    create storage
    mount storage on /mnt/juju-storage-tmp
    juju-run parent-unit hooks/stop
    rsync -a $storage-path /mnt/juju-storage-tmp
    unmount /mnt/juju-storage-tmp
    mv $storage-path $storage-path.juju-`date`
    mount storage on $storage-path
    juju-run parent-unit hooks/start

Perhaps start/stop isn't enough and we need support in the parent
charm's storage-relation-joined and storage-relation-changed hooks to
disable the service and reenable it when done (eg. if
db-relation-joined hooks get fired while the subordinate is being
setup, PG could get restarted at the wrong time).

If this was possible, we could add multiple subordinates easily to the
primary service. For example, 1 for the main database and additional
ones for any tablespaces that have been created and are being used.

> === modified file '.bzrignore'
> --- .bzrignore 2012-10-30 16:41:12 +0000
> +++ .bzrignore 2014-02-13 02:24:52 +0000
> @@ -0,0 +1,1 @@
> +hooks/_trial_temp

This is already in place on trunk. There is at least one change on
trunk that affects this work, which I'll comment on later.

> === modified file 'config.yaml'
> --- config.yaml 2014-01-22 07:09:38 +0000
> +++ config.yaml 2014-02-13 02:24:52 +0000
> @@ -277,36 +277,6 @@
> default: 4.0
> type: float
> description: Random page cost
> - #------------------------------------------------------------------------
> - # Volume management
> - # volume-map, volume-dev_regexp are only used
> - # if volume-ephemeral-storage == False
> - #------------------------------------------------------------------------
> - volume-ephemeral-storage:

We need to su...

[Stuart Bishop (stub)]

review: Needs Information

[Stuart Bishop (stub)]

On 17 February 2014 21:27, Stuart Bishop <email address hidden> wrote:
> On trunk you will discover that postgresql_stop() never fails. It is
> important here, so I suggest we add an extra check at the end of
> postgresql_stop() that raises an exception if postgresql_is_running()
> still returns true.

Minor correction. If postgresql_stop fails, a
subprocess.CalledProcessException should be raised. The extra check
could still be good though to double check that pg_ctlcluster did what
it claims.

[Chad Smith (chad.smith)]

Stuart, Thanks for the comments. We discussed this week and agree with you about general direction and trying to incorporate the juju-run functionality when it is released. Since it's not going to be released in 1.16 we'd like to start with this approach and then have a followup that will simplify principal hook requirements even more in the next storage release.

As per the upgrade-charm path, here's what we are thinking (and I'll try incorporating that into this patch when I'm back on Tuesday).

Since upgrade-charm involves having to deploy storage service with a new configuration and add-relation, we can't do all this inside the upgrade-charm hook. So, we'll document the upgrade process in the README and ensure that the upgrade-charm hook exits(1) to get the attention of the juju admin.

 upgrade-charm will:
   1. stop postgresql,
   2. umount the mounted /srv/juju/vol-id
   3. unlink /var/lib/juju
   4. sys.exit(1) # Expose an upgrade-charm hook error
      -- a hook error occurs with config-change errors if you manipulate either volume-map or volume-ephemeral-storage or add & remove postgresql units when volume-map is in place

   5. Log a WARNING to the user in the postgresql-unit.log
WARNING: postgresql/1 unit has external volume id 123-123-12312-312 mounted via the
deprecated volume-map and volume-ephemeral-storage
configuration parameters.
These parameters are no longer available in the postgresql
charm in favor of using the volume_map parameter in the
storage subordinate charm.
The attached volume is left intact. To remount this
volume_id using the storage subordinate follow these steps:
1. cat > storage.cfg <<EOF\nstorage:
storage:\n"
  volume_map: "{postgresql/0: 123-123-12312-312, postgresql/1: 54-645-645-645}"
EOF
2. juju deploy --config storage.cfg storage
3. juju add-relation postgresql storage

How does an approach like this sound Stuart, given that the upgrade-charm hook can't deploy and relate new services?

[Stuart Bishop (stub)]

On 20 February 2014 07:21, Chad Smith <email address hidden> wrote:
> Stuart, Thanks for the comments. We discussed this week and agree with you about general direction and trying to incorporate the juju-run functionality when it is released. Since it's not going to be released in 1.16 we'd like to start with this approach and then have a followup that will simplify principal hook requirements even more in the next storage release.

Ok.

> As per the upgrade-charm path, here's what we are thinking (and I'll try incorporating that into this patch when I'm back on Tuesday).
>
> Since upgrade-charm involves having to deploy storage service with a new configuration and add-relation, we can't do all this inside the upgrade-charm hook. So, we'll document the upgrade process in the README and ensure that the upgrade-charm hook exits(1) to get the attention of the juju admin.
>
> upgrade-charm will:
> 1. stop postgresql,
> 2. umount the mounted /srv/juju/vol-id
> 3. unlink /var/lib/juju
> 4. sys.exit(1) # Expose an upgrade-charm hook error
> -- a hook error occurs with config-change errors if you manipulate either volume-map or volume-ephemeral-storage or add & remove postgresql units when volume-map is in place
>
> 5. Log a WARNING to the user in the postgresql-unit.log
> WARNING: postgresql/1 unit has external volume id 123-123-12312-312 mounted via the
> deprecated volume-map and volume-ephemeral-storage
> configuration parameters.
> These parameters are no longer available in the postgresql
> charm in favor of using the volume_map parameter in the
> storage subordinate charm.
> The attached volume is left intact. To remount this
> volume_id using the storage subordinate follow these steps:
> 1. cat > storage.cfg <<EOF\nstorage:
> storage:\n"
> volume_map: "{postgresql/0: 123-123-12312-312, postgresql/1: 54-645-645-645}"
> EOF
> 2. juju deploy --config storage.cfg storage
> 3. juju add-relation postgresql storage
>
>
> How does an approach like this sound Stuart, given that the upgrade-charm hook can't deploy and relate new services?

This sounds reasonable. Assuming the directory structures match, no
large database files need to be shuffled around. I am unsure if the
last task in step 5 will actually work, as the operator needs to add
the relation but upgrade-charm has not yet succeeded. Assuming this
works, how does the rest of the process go? After the operator has
performed the tasks given by step 5, we have a PostgreSQL unit in a
failed state and a subordinate charm with pending relation-hooks. Will
'juju resolved' or 'juju resolved --retry' be all that is required?

An alternative would be to not have upgrade-charm fail, but instead
set a flag ensuring that PG will not start up until the subordinate
storage charm has been successfully related. Disable autostart,
local_state could store the flag, and postgrersql_start would check
for it. It might not be this simple though, as other hooks like
config-changed would likely fail instead when PG fails to start on
request and I think config-changed will get invoked immediately after
the upgrade-charm...

And another alternative.... upgrade-charm just leaves the stora...

[Chad Smith (chad.smith)]

Hi Stuart,
 Thanks for the suggestions here about the upgrade-charm lifecycle. What I ended up doing yesterday was the following to ease the upgrade process

upgrade_charm hook will:
 1. stop postgresql;
 2. umount the /srv/juju/vol-id;
 3. unlink /var/lib/postgresql/9.1/main;
 4. mount LABEL=vol-id /srv/data;
 5. symlink /var/lib/postgresql/9.1/main -> /src/data/postgresql/9.1/main
 6. chown -h postgres:postgresql /var/lib/postgresql/9.1/main
 7. log a warning about manual procedure to add storage and block-storage-broker services to ensure ongoing use of existing storage.volume_map that looks like the following:

WARNING: postgresql/0 unit has external volume id 123-123-123-123 mounted via the
deprecated volume-map and volume-ephemeral-storage configuration parameters.
These parameters are no longer available in the postgresql
charm in favor of using the volume_map parameter in the
storage subordinate charm.
We are migrating the attached volume to a mount path which
can be managed by the storage subordinate charm. To
continue using this volume_id with the storage subordinate
follow this procedure.
-----------------------------------
1. cat > storage.cfg <<EOF
storage:"
  provider:block-storage-broker
  root: /srv/data
  volume_map: "{postgresql/0: 123-123-123-123}"
EOF
2. juju deploy --config storage.cfg storage
3. juju deploy block-storage-broker
4. juju add-relation block-storage-broker storage
5. juju resolved --retry postgresql/0
6. juju add-relation postgresql storage
-----------------------------------

Remount and restart success for this external volume.
This current running installation will break upon
add/remove postgresql units or reations if you do not
follow the above procedure to ensure your external
volumes are preserved by the storage subordinate charm.

I also just added the ability to handle partitioned volumes into the storage subordinate charm as well. This was needed because the current postgresql charm partitions the volume into a single partition of 100% of the volume size, but the storage subordinate just uses the entire volume without partitioning.

>
> This sounds reasonable. Assuming the directory structures match, no
> large database files need to be shuffled around. I am unsure if the
> last task in step 5 will actually work,
Good point Stuart, as I found too, it didn't work. Even with the resolved --retry, it would still fail at the following config_changed hook that fires after the upgrade_charm hook because I had originally left PG in an unusable state (with the intent of allowing the storage subordinate relation to setup the mounts again). So, I've made sure all mount/link work is now done in the upgrade_charm hook and the postgresql service is still running after remounting the device in the proper location for the storage subordinate to handle it. We still will exit(1) to alert the juju admin to the manual juju deploy & add-relation steps needed for ongoing volume support. But, in this new approach, the service still has integrity, and a resolved --retry will succeed (as well as the following config_changed hook).

> After the operator has performed the tasks given by step 5, we have a PostgreS...

[Stuart Bishop (stub)]

This is all looking good. The branch as it stands is an improvement and can be landed. I've got comments below which you might want to act on before I land this. Please let me know when you are happy with the branch and want it landed, or if you want further review.

Thanks :)

=== modified file 'README.md'

+## Upgrade-charm hook notes

Yay documentation.

=== modified file 'config.yaml'

+ storage_mount_point:
+ default: "/mnt/storage"
+ type: string
+ description: |
+ The location where the storage is going to be mounted.

Do we have a use case for this to be configurable? If not, consider removing it. We already have a huge number of knobs in the PG charm, and this extra one might make further work on the storage subordinate harder.

If storage_mount_point remains, what happens if someone changes it? You will want to add the item to unchangable_config in validate_config().

-def config_changed_volume_apply():
+def config_changed_volume_apply(mount_point=None):

Its probably better to not have the default of None here.

+ if not mount_point:
+ log("Invalid volume storage configuration, not applying changes",
             ERROR)
+ return False

You abort if mount_point is false here...

+ if not mount_point:
+ log(
+ "invalid mount point = {}, "
+ "not applying changes.".format(mount_point), ERROR)
+ return False

... which means this code will never be reached and should be removed.

+ if ((os.path.islink(data_directory_path) and
+ os.readlink(data_directory_path) == new_pg_version_cluster_dir and
+ os.path.isdir(new_pg_version_cluster_dir))):
+ log(
+ "postgresql data dir '{}' already points "
+ "to {}, skipping storage changes.".format(
+ data_directory_path, new_pg_version_cluster_dir))
+ log(
+ "existing-symlink: to fix/avoid UID changes from "
+ "previous units, doing: "
+ "chown -R postgres:postgres {}".format(new_pg_dir))
+ run("chown -R postgres:postgres %s" % new_pg_dir)
+ return True

When do you see invalid file ownerships btw? Is this masking some other problem that needs to be addressed?

+ # Create a directory structure below "new" mount_point, as e.g.:
+ # config["storage_mount_point"]/postgresql/9.1/main , which "mimics":
+ # /var/lib/postgresql/9.1/main
+ curr_dir_stat = os.stat(data_directory_path)
+ for new_dir in [new_pg_dir,
+ os.path.join(new_pg_dir, version),
+ new_pg_version_cluster_dir]:
+ if not os.path.isdir(new_dir):
+ log("mkdir %s".format(new_dir))
+ os.mkdir(new_dir)
+ # copy permissions from current data_directory_path
+ os.chown(new_dir, curr_dir_stat.st_uid, curr_dir_stat.st_gid)
+ os.chmod(new_dir, curr_dir_stat.st_mode)

Rather than this loop, you can use charmhelpers.core.host.mkdir here. You can set the directory perms to 0700 rather than copy the existing ones.

+ # Carefully build this symlink, e.g.:
+ # /var/lib/postgresql/9.1/main ->
+ # config["storage_mount_point"]/postgresql...

review: Approve

[Chad Smith (chad.smith)]

Stuart, thanks for the final review here:

- I have dropped the storage_mount_point config option as there is no use case we can think of here. The mountpoint can be a global defined within the charm itself external_volume_mount.

- config_changed_volume_apply now takes a mandatory mount_point parameter and the unreached mount_point check is removed

- When do you see invalid file ownerships btw? Is this masking some other problem that needs to be addressed?

+ run("chown -R postgres:postgres %s" % new_pg_dir)
> When do you see invalid file ownerships btw? Is this masking some other problem that needs to be addressed?

I believe we saw this when the subdirectories on the external volume were created by postgresql/0 and we ended up spinning up destroying the unit and attaching the new postgresql/1 later to the previously configured volume. So, if there were a different uid/gid on the new installation for the postgresql user, we could have a mismatch right?

I had also seen that root user (via the charm hooks) created the directories and symlink with root:root and we needed to chown as postgres:postgres to ensure the Pg service could actually read and traverse the directories.

- Now we are using host.mkdir w/ postgres user and perms 700

We'll push on charmers this next week to get both storage subordinate and block-storage-broker charms into the charm store.

We are good for a merge up now just ran through a deployment to ensure the changes didn't break anything.

review: Approve

[Stuart Bishop (stub)]

On 1 March 2014 06:38, Chad Smith <email address hidden> wrote:

> - When do you see invalid file ownerships btw? Is this masking some other problem that needs to be addressed?
>
> + run("chown -R postgres:postgres %s" % new_pg_dir)
>> When do you see invalid file ownerships btw? Is this masking some other problem that needs to be addressed?
>
> I believe we saw this when the subdirectories on the external volume were created by postgresql/0 and we ended up spinning up destroying the unit and attaching the new postgresql/1 later to the previously configured volume. So, if there were a different uid/gid on the new installation for the postgresql user, we could have a mismatch right?

This makes sense, and will happen normally with some disaster recovery
strategies (bringing up an existing database on a new node).

> We'll push on charmers this next week to get both storage subordinate and block-storage-broker charms into the charm store.
>
> We are good for a merge up now just ran through a deployment to ensure the changes didn't break anything.
> --
> https://code.launchpad.net/~chad.smith/charms/precise/postgresql/postgresql-using-storage-subordinate/+merge/206078
> You are reviewing the proposed merge of lp:~chad.smith/charms/precise/postgresql/postgresql-using-storage-subordinate into lp:charms/postgresql.

I don't think we can land this until after the storage subordinate and
block-storage-broker charms have landed in the charm store, as they
are required to migrate from earlier revisions of the PostgreSQL
charm.

--
Stuart Bishop <<email address hidden>

[Chad Smith (chad.smith)]

> I don't think we can land this until after the storage subordinate and
> block-storage-broker charms have landed in the charm store, as they
> are required to migrate from earlier revisions of the PostgreSQL
> charm.
>
>
> --
> Stuart Bishop <<email address hidden>

Sounds good Stuart, we'll get Landscape team to finish the EC2 reviews for storage and block-storage-broker tomorrow or wednesday. Then we'll hitup charmers on those 2 new charms and I'll ping you when they are both in the charmstore

[Matt Bruzek (mbruzek)]

Moving this merge request to "Work In Progress" until the other charms land. Once those charms land move the propoal back to "Needs review" and that will add it back to the review queue.

[Chad Smith (chad.smith)]

Thank you Matt for the "WIP" flag here while we awaited charm release. I have now unblocked this charm as both storage charms are both added to the charm store.

storage subordinate:
  https://jujucharms.com/sidebar/search/precise/storage-0/?text=storage

block-storage-broker:
  https://jujucharms.com/sidebar/search/precise/block-storage-broker-2/?text=block-storage-broker

[Stuart Bishop (stub)]

I've gone through this again, and think it is good to land. I had several concerns, all of which turned out to be false when I looked closer.

review: Approve

=== modified file 'README.md'
--- README.md	2014-02-05 12:29:09 +0000
+++ README.md	2014-02-28 23:23:59 +0000
@@ -223,6 +223,33 @@
         hooks.execute(sys.argv)
 
 
+## Upgrade-charm hook notes
+
+The PostgreSQL charm has deprecated volume-map and volume-ephemeral-storage
+configuration options in favor of using the storage subordinate charm for
+general external storage management. If the installation being upgraded is
+using these deprecated options, there are a couple of manual steps necessary 
+to finish migration and continue using the current external volumes.
+Even though all data will remain intact, and PostgreSQL service will continue
+running, the upgrade-charm hook will intentionally fail and exit 1 as well to
+raise awareness of the manual procedure which will also be documented in the
+juju logs on the PostgreSQL units.
+
+The following steps must be additionally performed to continue using external
+volume maps for the PostgreSQL units once juju upgrade-charm is run from the
+command line:
+  1. cat > storage.cfg <<EOF
+     storage:
+       provider:block-storage-broker
+       root: /srv/data
+       volume_map: "{postgresql/0: your-vol-id, postgresql/1: your-2nd-vol-id}"
+     EOF
+  2. juju deploy --config storage.cfg storage
+  3. juju deploy block-storage-broker
+  4. juju add-relation block-storage-broker storage
+  5. juju resolved --retry postgresql/0   # for each postgresql unit running
+  6. juju add-relation postgresql storage
+
 
 # Contact Information
 

=== modified file 'config.yaml'
--- config.yaml	2014-01-23 11:41:03 +0000
+++ config.yaml	2014-02-28 23:23:59 +0000
@@ -282,36 +282,6 @@
     default: 4.0
     type: float
     description: Random page cost
-  #------------------------------------------------------------------------
-  # Volume management
-  #   volume-map, volume-dev_regexp are only used 
-  #   if volume-ephemeral-storage == False
-  #------------------------------------------------------------------------
-  volume-ephemeral-storage:
-    type: boolean
-    default: true
-    description: >
-      If false, a configure-error state will be raised if
-         volume-map[$JUJU_UNIT_NAME] is not set (see "volume-map"
-         below) - see "volume-map" below.
-      If true, service units won't try to use "volume-map" (and
-         related variables) to mount and use external (EBS) volumes,
-         thus storage lifetime will equal VM, thus ephemeral.
-         YOU'VE BEEN WARNED.
-  volume-map:
-    type: string
-    default: ""
-    description: >
-      YAML map as e.g. "{ postgres/0: vol-0000010, postgres/1: vol-0000016 }".
-      Service units will raise a "configure-error" condition if no volume-map
-      value is set for it - it's expected a human to set it properly to
-      resolve it.
-  volume-dev-regexp:
-    type: string
-    default: "/dev/vd[b-z]"
-    description: >
-      Block device for attached volumes as seen by the VM, will be "scanned"
-      for an unused device when "volume-map" is valid for the unit.
   backup_dir:
     default: "/var/lib/postgresql/backups"
     type: string
@@ -366,7 +336,6 @@
         An advisory lock key used internally by the charm. You do not need
         to change it unless it happens to conflict with an advisory lock key
         being used by your applications.
-
   # Swift backups and PITR via SwiftWAL
   swiftwal_container_prefix:
     type: string

=== added symlink 'hooks/data-relation-changed'
=== target is u'hooks.py'
=== added symlink 'hooks/data-relation-departed'
=== target is u'hooks.py'
=== added symlink 'hooks/data-relation-joined'
=== target is u'hooks.py'
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-02-11 09:11:32 +0000
+++ hooks/hooks.py	2014-02-28 23:23:59 +0000
@@ -15,8 +15,6 @@
 import sys
 from tempfile import NamedTemporaryFile
 import time
-import yaml
-from yaml.constructor import ConstructorError
 
 from charmhelpers import fetch
 from charmhelpers.core import hookenv, host
@@ -146,85 +144,8 @@
         self.save()
 
 
-###############################################################################
-# Volume managment
-###############################################################################
-#------------------------------
-# Get volume-id from juju config "volume-map" dictionary as
-#     volume-map[JUJU_UNIT_NAME]
-# @return  volid
-#
-#------------------------------
-def volume_get_volid_from_volume_map():
-    volume_map = {}
-    try:
-        volume_map = yaml.load(hookenv.config('volume-map').strip())
-        if volume_map:
-            return volume_map.get(os.environ['JUJU_UNIT_NAME'])
-    except ConstructorError as e:
-        log("invalid YAML in 'volume-map': {}".format(e), WARNING)
-    return None
-
-
-# Is this volume_id permanent ?
-# @returns  True if volid set and not --ephemeral, else:
-#           False
-def volume_is_permanent(volid):
-    if volid and volid != "--ephemeral":
-        return True
-    return False
-
-
-#------------------------------
-# Returns a mount point from passed vol-id, e.g. /srv/juju/vol-000012345
-#
-# @param  volid          volume id (as e.g. EBS volid)
-# @return mntpoint_path  eg /srv/juju/vol-000012345
-#------------------------------
-def volume_mount_point_from_volid(volid):
-    if volid and volume_is_permanent(volid):
-        return "/srv/juju/%s" % volid
-    return None
-
-
-# Do we have a valid storage state?
-# @returns  volid
-#           None    config state is invalid - we should not serve
-def volume_get_volume_id():
-    ephemeral_storage = hookenv.config('volume-ephemeral-storage')
-    volid = volume_get_volid_from_volume_map()
-    juju_unit_name = hookenv.local_unit()
-    if ephemeral_storage in [True, 'yes', 'Yes', 'true', 'True']:
-        if volid:
-            log(
-                "volume-ephemeral-storage is True, but " +
-                "volume-map[{!r}] -> {}".format(juju_unit_name, volid), ERROR)
-            return None
-        else:
-            return "--ephemeral"
-    else:
-        if not volid:
-            log(
-                "volume-ephemeral-storage is False, but "
-                "no volid found for volume-map[{!r}]".format(
-                    hookenv.local_unit()), ERROR)
-            return None
-    return volid
-
-
-# Initialize and/or mount permanent storage, it straightly calls
-# shell helper
-def volume_init_and_mount(volid):
-    command = ("scripts/volume-common.sh call " +
-               "volume_init_and_mount %s" % volid)
-    output = run(command)
-    if output.find("ERROR") >= 0:
-        return False
-    return True
-
-
 def volume_get_all_mounted():
-    command = ("mount |egrep /srv/juju")
+    command = ("mount |egrep %s" % external_volume_mount)
     status, output = commands.getstatusoutput(command)
     if status != 0:
         return None
@@ -834,109 +755,89 @@
 # NOTE the only 2 "True" return points:
 #   1) symlink already pointing to existing storage (no-op)
 #   2) new storage properly initialized:
-#     - volume: initialized if not already (fdisk, mkfs),
-#       mounts it to e.g.:  /srv/juju/vol-000012345
 #     - if fresh new storage dir: rsync existing data
 #     - manipulate /var/lib/postgresql/VERSION/CLUSTER symlink
 #------------------------------------------------------------------------------
-def config_changed_volume_apply():
+def config_changed_volume_apply(mount_point):
     version = pg_version()
     cluster_name = hookenv.config('cluster_name')
     data_directory_path = os.path.join(
         postgresql_data_dir, version, cluster_name)
 
     assert(data_directory_path)
-    volid = volume_get_volume_id()
-    if volid:
-        if volume_is_permanent(volid):
-            if not volume_init_and_mount(volid):
-                log(
-                    "volume_init_and_mount failed, not applying changes",
-                    ERROR)
-                return False
-
-        if not os.path.exists(data_directory_path):
-            log(
-                "postgresql data dir {} not found, "
-                "not applying changes.".format(data_directory_path),
-                CRITICAL)
-            return False
-
-        mount_point = volume_mount_point_from_volid(volid)
-        new_pg_dir = os.path.join(mount_point, "postgresql")
-        new_pg_version_cluster_dir = os.path.join(
-            new_pg_dir, version, cluster_name)
-        if not mount_point:
-            log(
-                "invalid mount point from volid = {}, "
-                "not applying changes.".format(mount_point), ERROR)
-            return False
-
-        if ((os.path.islink(data_directory_path) and
-             os.readlink(data_directory_path) == new_pg_version_cluster_dir and
-             os.path.isdir(new_pg_version_cluster_dir))):
-            log(
-                "postgresql data dir '%s' already points "
-                "to {}, skipping storage changes.".format(
-                    data_directory_path, new_pg_version_cluster_dir))
-            log(
-                "existing-symlink: to fix/avoid UID changes from "
-                "previous units, doing: "
-                "chown -R postgres:postgres {}".format(new_pg_dir))
-            run("chown -R postgres:postgres %s" % new_pg_dir)
-            return True
-
-        # Create a directory structure below "new" mount_point, as e.g.:
-        #   /srv/juju/vol-000012345/postgresql/9.1/main  , which "mimics":
-        #   /var/lib/postgresql/9.1/main
-        curr_dir_stat = os.stat(data_directory_path)
-        for new_dir in [new_pg_dir,
-                        os.path.join(new_pg_dir, version),
-                        new_pg_version_cluster_dir]:
-            if not os.path.isdir(new_dir):
-                log("mkdir %s".format(new_dir))
-                os.mkdir(new_dir)
-                # copy permissions from current data_directory_path
-                os.chown(new_dir, curr_dir_stat.st_uid, curr_dir_stat.st_gid)
-                os.chmod(new_dir, curr_dir_stat.st_mode)
-        # Carefully build this symlink, e.g.:
-        # /var/lib/postgresql/9.1/main ->
-        # /srv/juju/vol-000012345/postgresql/9.1/main
-        # but keep previous "main/"  directory, by renaming it to
-        # main-$TIMESTAMP
-        if not postgresql_stop():
-            log("postgresql_stop() failed - can't migrate data.", ERROR)
-            return False
-        if not os.path.exists(os.path.join(
-                new_pg_version_cluster_dir, "PG_VERSION")):
-            log("migrating PG data {}/ -> {}/".format(
-                data_directory_path, new_pg_version_cluster_dir), WARNING)
-            # void copying PID file to perm storage (shouldn't be any...)
-            command = "rsync -a --exclude postmaster.pid {}/ {}/".format(
-                data_directory_path, new_pg_version_cluster_dir)
-            log("run: {}".format(command))
-            run(command)
-        try:
-            os.rename(data_directory_path, "{}-{}".format(
-                data_directory_path, int(time.time())))
-            log("NOTICE: symlinking {} -> {}".format(
-                new_pg_version_cluster_dir, data_directory_path))
-            os.symlink(new_pg_version_cluster_dir, data_directory_path)
-            log(
-                "after-symlink: to fix/avoid UID changes from "
-                "previous units, doing: "
-                "chown -R postgres:postgres {}".format(new_pg_dir))
-            run("chown -R postgres:postgres {}".format(new_pg_dir))
-            return True
-        except OSError:
-            log("failed to symlink {} -> {}".format(
-                data_directory_path, mount_point), CRITICAL)
-            return False
-    else:
-        log(
-            "Invalid volume storage configuration, not applying changes",
-            ERROR)
-    return False
+
+    if not os.path.exists(data_directory_path):
+        log(
+            "postgresql data dir {} not found, "
+            "not applying changes.".format(data_directory_path),
+            CRITICAL)
+        return False
+
+    new_pg_dir = os.path.join(mount_point, "postgresql")
+    new_pg_version_cluster_dir = os.path.join(
+        new_pg_dir, version, cluster_name)
+    if not mount_point:
+        log(
+            "invalid mount point = {}, "
+            "not applying changes.".format(mount_point), ERROR)
+        return False
+
+    if ((os.path.islink(data_directory_path) and
+         os.readlink(data_directory_path) == new_pg_version_cluster_dir and
+         os.path.isdir(new_pg_version_cluster_dir))):
+        log(
+            "postgresql data dir '{}' already points "
+            "to {}, skipping storage changes.".format(
+                data_directory_path, new_pg_version_cluster_dir))
+        log(
+            "existing-symlink: to fix/avoid UID changes from "
+            "previous units, doing: "
+            "chown -R postgres:postgres {}".format(new_pg_dir))
+        run("chown -R postgres:postgres %s" % new_pg_dir)
+        return True
+
+    # Create a directory structure below "new" mount_point as
+    #   external_volume_mount/postgresql/9.1/main
+    for new_dir in [new_pg_dir,
+                    os.path.join(new_pg_dir, version),
+                    new_pg_version_cluster_dir]:
+        if not os.path.isdir(new_dir):
+            log("mkdir %s".format(new_dir))
+            host.mkdir(new_dir, owner="postgres", perms=0o700)
+    # Carefully build this symlink, e.g.:
+    # /var/lib/postgresql/9.1/main ->
+    # external_volume_mount/postgresql/9.1/main
+    # but keep previous "main/"  directory, by renaming it to
+    # main-$TIMESTAMP
+    if not postgresql_stop() and postgresql_is_running():
+        log("postgresql_stop() failed - can't migrate data.", ERROR)
+        return False
+    if not os.path.exists(os.path.join(
+            new_pg_version_cluster_dir, "PG_VERSION")):
+        log("migrating PG data {}/ -> {}/".format(
+            data_directory_path, new_pg_version_cluster_dir), WARNING)
+        # void copying PID file to perm storage (shouldn't be any...)
+        command = "rsync -a --exclude postmaster.pid {}/ {}/".format(
+            data_directory_path, new_pg_version_cluster_dir)
+        log("run: {}".format(command))
+        run(command)
+    try:
+        os.rename(data_directory_path, "{}-{}".format(
+            data_directory_path, int(time.time())))
+        log("NOTICE: symlinking {} -> {}".format(
+            new_pg_version_cluster_dir, data_directory_path))
+        os.symlink(new_pg_version_cluster_dir, data_directory_path)
+        run("chown -h postgres:postgres {}".format(data_directory_path))
+        log(
+            "after-symlink: to fix/avoid UID changes from "
+            "previous units, doing: "
+            "chown -R postgres:postgres {}".format(new_pg_dir))
+        run("chown -R postgres:postgres {}".format(new_pg_dir))
+        return True
+    except OSError:
+        log("failed to symlink {} -> {}".format(
+            data_directory_path, mount_point), CRITICAL)
+        return False
 
 
 def token_sql_safe(value):
@@ -947,30 +848,15 @@
 
 
 @hooks.hook()
-def config_changed(force_restart=False):
+def config_changed(force_restart=False, mount_point=None):
     validate_config()
     config_data = hookenv.config()
     update_repos_and_packages()
 
-    # Trigger volume initialization logic for permanent storage
-    volid = volume_get_volume_id()
-    if not volid:
-        ## Invalid configuration (whether ephemeral, or permanent)
-        postgresql_autostart(False)
-        postgresql_stop()
-        mounts = volume_get_all_mounted()
-        if mounts:
-            log("current mounted volumes: {}".format(mounts))
-        log(
-            "Disabled and stopped postgresql service, "
-            "because of broken volume configuration - check "
-            "'volume-ephemeral-storage' and 'volume-map'", ERROR)
-        sys.exit(1)
-
-    if volume_is_permanent(volid):
-        ## config_changed_volume_apply will stop the service if it founds
+    if mount_point is not None:
+        ## config_changed_volume_apply will stop the service if it finds
         ## it necessary, ie: new volume setup
-        if config_changed_volume_apply():
+        if config_changed_volume_apply(mount_point=mount_point):
             postgresql_autostart(True)
         else:
             postgresql_autostart(False)
@@ -1084,8 +970,89 @@
 
 @hooks.hook()
 def upgrade_charm():
+    """Handle saving state during an upgrade-charm hook.
+
+    When upgrading from an installation using volume-map, we migrate
+    that installation to use the storage subordinate charm by remounting
+    a mountpath that the storage subordinate maintains. We exit(1) only to
+    raise visibility to manual procedure that we log in juju logs below for the
+    juju admin to finish the migration by relating postgresql to the storage
+    and block-storage-broker services. These steps are generalised in the
+    README as well.
+    """
     install(run_pre=False)
     snapshot_relations()
+    version = pg_version()
+    cluster_name = hookenv.config('cluster_name')
+    data_directory_path = os.path.join(
+        postgresql_data_dir, version, cluster_name)
+    if (os.path.islink(data_directory_path)):
+        link_target = os.readlink(data_directory_path)
+        if "/srv/juju" in link_target:
+            # Then we just upgraded from an installation that was using
+            # charm config volume_map definitions. We need to stop postgresql
+            # and remount the device where the storage subordinate expects to
+            # control the mount in the future if relations/units change
+            volume_id = link_target.split("/")[3]
+            unit_name = hookenv.local_unit()
+            new_mount_root = external_volume_mount
+            new_pg_version_cluster_dir = os.path.join(
+                new_mount_root, "postgresql", version, cluster_name)
+            if not os.exists(new_mount_root):
+                os.mkdir(new_mount_root)
+            log("\n"
+                "WARNING: %s unit has external volume id %s mounted via the\n"
+                "deprecated volume-map and volume-ephemeral-storage\n"
+                "configuration parameters.\n"
+                "These parameters are no longer available in the postgresql\n"
+                "charm in favor of using the volume_map parameter in the\n"
+                "storage subordinate charm.\n"
+                "We are migrating the attached volume to a mount path which\n"
+                "can be managed by the storage subordinate charm. To\n"
+                "continue using this volume_id with the storage subordinate\n"
+                "follow this procedure.\n-----------------------------------\n"
+                "1. cat > storage.cfg <<EOF\nstorage:\n"
+                "  provider: block-storage-broker\n"
+                "  root: %s\n"
+                "  volume_map: \"{%s: %s}\"\nEOF\n2. juju deploy "
+                "--config storage.cfg storage\n"
+                "3. juju deploy block-storage-broker\n4. juju add-relation "
+                "block-storage-broker storage\n5. juju resolved --retry "
+                "%s\n6. juju add-relation postgresql storage\n"
+                "-----------------------------------\n" %
+                (unit_name, volume_id, new_mount_root, unit_name, volume_id,
+                 unit_name), WARNING)
+            postgresql_stop()
+            os.unlink(data_directory_path)
+            log("Unmounting external storage due to charm upgrade: %s" %
+                link_target)
+            try:
+                subprocess.check_output(
+                    "umount /srv/juju/%s" % volume_id, shell=True)
+                # Since e2label truncates labels to 16 characters use only the
+                # first 16 characters of the volume_id as that's what was
+                # set by old versions of postgresql charm
+                subprocess.check_call(
+                    "mount -t ext4 LABEL=%s %s" %
+                    (volume_id[:16], new_mount_root), shell=True)
+            except subprocess.CalledProcessError, e:
+                log("upgrade-charm mount migration failed. %s" % str(e), ERROR)
+                sys.exit(1)
+
+            log("NOTICE: symlinking {} -> {}".format(
+                new_pg_version_cluster_dir, data_directory_path))
+            os.symlink(new_pg_version_cluster_dir, data_directory_path)
+            run("chown -h postgres:postgres {}".format(data_directory_path))
+            postgresql_start()  # Will exit(1) if issues
+            log("Remount and restart success for this external volume.\n"
+                "This current running installation will break upon\n"
+                "add/remove postgresql units or relations if you do not\n"
+                "follow the above procedure to ensure your external\n"
+                "volumes are preserved by the storage subordinate charm.",
+                WARNING)
+            # So juju admins can see the hook fail and note the steps to fix
+            # per our WARNINGs above
+            sys.exit(1)
 
 
 @hooks.hook()
@@ -2194,6 +2161,32 @@
         host.service_reload('nagios-nrpe-server')
 
 
[e-mail]('data-relation-changed')
+def data_relation_changed():
+    """Listen for configured mountpoint from storage subordinate relation"""
+    if not hookenv.relation_get("mountpoint"):
+        hookenv.log("Waiting for mountpoint from the relation: %s"
+                    % external_volume_mount, hookenv.DEBUG)
+    else:
+        hookenv.log("Storage ready and mounted", hookenv.DEBUG)
+        config_changed(mount_point=external_volume_mount)
+
+
[e-mail]('data-relation-joined')
+def data_relation_joined():
+    """Request mountpoint from storage subordinate by setting mountpoint"""
+    hookenv.log("Setting mount point in the relation: %s"
+                % external_volume_mount, hookenv.DEBUG)
+    hookenv.relation_set(mountpoint=external_volume_mount)
+
+
[e-mail]('data-relation-departed')
+def stop_postgres_on_data_relation_departed():
+    hookenv.log("Data relation departing. Stopping PostgreSQL",
+                hookenv.DEBUG)
+    postgresql_stop()
+
+
 def _get_postgresql_config_dir(config_data=None):
     """ Return the directory path of the postgresql configuration files. """
     if config_data is None:
@@ -2202,6 +2195,7 @@
     cluster_name = config_data['cluster_name']
     return os.path.join("/etc/postgresql", version, cluster_name)
 
+
 ###############################################################################
 # Global variables
 ###############################################################################
@@ -2216,6 +2210,7 @@
 local_state = State('local_state.pickle')
 hook_name = os.path.basename(sys.argv[0])
 juju_log_dir = "/var/log/juju"
+external_volume_mount = "/srv/data"
 
 
 if __name__ == '__main__':

=== modified file 'hooks/test_hooks.py'
--- hooks/test_hooks.py	2014-01-08 08:55:50 +0000
+++ hooks/test_hooks.py	2014-02-28 23:23:59 +0000
@@ -43,9 +43,23 @@
     certain data is set.
     """
 
-    _relation_data = {}
+    _incoming_relation_data = ()
+    _outgoing_relation_data = ()
     _relation_ids = {}
     _relation_list = ("postgres/0",)
+    _log = ()
+
+    _log_DEBUG = ()
+    _log_INFO = ()
+    _log_WARNING = ()
+    _log_ERROR = ()
+    _log_CRITICAL = ()
+
+    DEBUG = "DEBUG"
+    INFO = "INFO"
+    WARNING = "WARNING"
+    ERROR = "ERROR"
+    CRITICAL = "CRITICAL"
 
     def __init__(self):
         self._config = {
@@ -95,27 +109,30 @@
             "wal_buffers": "-1",
             "checkpoint_segments": 3,
             "random_page_cost": 4.0,
-            "volume_ephemeral_storage": True,
             "volume_map": "",
             "volume_dev_regexp": "/dev/db[b-z]",
             "backup_dir": "/var/lib/postgresql/backups",
             "backup_schedule": "13 4 * * *",
             "backup_retention_count": 7,
             "nagios_context": "juju",
+            "pgdg": False,
+            "install_sources": "",
+            "install_keys": "",
             "extra_archives": "",
             "advisory_lock_restart_key": 765}
 
     def relation_set(self, *args, **kwargs):
         """
-        Capture result of relation_set into _relation_data, which
+        Capture result of relation_set into _outgoing_relation_data, which
         can then be checked later.
         """
         if "relation_id" in kwargs:
             del kwargs["relation_id"]
-        self._relation_data = dict(self._relation_data, **kwargs)
+
         for arg in args:
             (key, value) = arg.split("=")
-            self._relation_data[key] = value
+            self._outgoing_relation_data = (
+                self._outgoing_relation_data + ((key, value),))
 
     def relation_ids(self, relation_name="db-admin"):
         """
@@ -156,17 +173,24 @@
     def juju_log(self, *args, **kwargs):
         pass
 
-    def log(self, *args, **kwargs):
-        pass
+    def log(self, message, level=None):
+        if level is None:
+            level = self.INFO
+        log = getattr(self, "_log_%s" % level)
+        setattr(self, "_log_%s" % level, log + (message,))
 
-    def config_get(self, scope=None):
+    def config(self, scope=None):
         if scope is None:
-            return self.config
+            return self._config
         else:
-            return self.config[scope]
+            return self._config[scope]
 
     def relation_get(self, scope=None, unit_name=None, relation_id=None):
-        pass
+        if scope:
+            for (key, value) in self._incoming_relation_data:
+                if key == scope:
+                    return value
+            return None
 
 
 class TestHooks(mocker.MockerTestCase):
@@ -175,8 +199,6 @@
         hooks.hookenv = TestJuju()
         hooks.host = TestJujuHost()
         hooks.juju_log_dir = self.makeDir()
-        hooks.hookenv.config = lambda: hooks.hookenv._config
-        #hooks.hookenv.localunit = lambda: "localhost"
         hooks.os.environ["JUJU_UNIT_NAME"] = "landscape/1"
         hooks.os.environ["CHARM_DIR"] = os.path.abspath(
             os.path.join(os.path.dirname(__file__), os.pardir))
@@ -210,6 +232,65 @@
 
 class TestHooksService(TestHooks):
 
+    def test_data_relation_departed_stops_postgresql(self):
+        """
+        When the storage subordinate charm relation departs firing the
+        C{data-relation-departed} hook, the charm stops the postgresql service
+        and logs a message.
+        """
+        postgresql_stop = self.mocker.replace(hooks.postgresql_stop)
+        postgresql_stop()
+        self.mocker.replay()
+        hooks.stop_postgres_on_data_relation_departed()
+        message = "Data relation departing. Stopping PostgreSQL"
+        self.assertIn(
+            message, hooks.hookenv._log_DEBUG, "Not logged- %s" % message)
+
+    def test_data_relation_joined_requests_configured_mountpoint(self):
+        """
+        When postgresql is related to the storage subordinate charm via the
+        'data' relation it will read the configured C{storage_mount_point} and
+        set C{mountpoint} in the relation in order to request a specific
+        mountpoint from the storage charm.
+        """
+        mount = hooks.external_volume_mount
+        hooks.data_relation_joined()
+        message = "Setting mount point in the relation: %s" % mount
+        self.assertIn(
+            message, hooks.hookenv._log_DEBUG, "Not logged- %s" % message)
+
+    def test_data_relation_changed_waits_for_data_relation_mountpoint(self):
+        """
+        C{data_relation_changed} will wait for the storage charm to respond
+        with the properly configured C{mountpoint} in the 'data' relation
+        before calling C{config_changed}.
+        """
+        mount = hooks.external_volume_mount
+        hooks.hookenv._config["storage_mount_point"] = mount
+        self.assertEqual(hooks.hookenv._incoming_relation_data, ())
+        hooks.data_relation_changed()
+        message = "Waiting for mountpoint from the relation: %s" % mount
+        self.assertIn(
+            message, hooks.hookenv._log_DEBUG, "Not logged- %s" % message)
+
+    def test_data_relation_changed_mountpoint_present(self):
+        """
+        C{data_relation_changed} will call C{config_changed} when it receives
+        the successfuly mounted C{mountpoint} from storage charm.
+        """
+        mount = hooks.external_volume_mount
+        self.addCleanup(
+            setattr, hooks.hookenv, "_incoming_relation_data", ())
+        hooks.hookenv._incoming_relation_data = (("mountpoint", mount),)
+        config_changed = self.mocker.replace(hooks.config_changed)
+        config_changed(mount_point=mount)
+        self.mocker.replay()
+
+        hooks.data_relation_changed()
+        message = "Storage ready and mounted"
+        self.assertIn(
+            message, hooks.hookenv._log_DEBUG, "Not logged- %s" % message)
+
     def test_create_postgresql_config_wal_no_replication(self):
         """
         When postgresql is in C{standalone} mode, and participates in no

=== modified file 'metadata.yaml'
--- metadata.yaml	2013-10-08 22:44:28 +0000
+++ metadata.yaml	2014-02-28 23:23:59 +0000
@@ -23,6 +23,10 @@
   nrpe-external-master:
     interface: nrpe-external-master
     scope: container
+  data:
+    interface: block-storage
+    scope: container
+    optional: true
 requires:
   persistent-storage:
     interface: directory-path

=== removed directory 'scripts'
=== removed file 'scripts/volume-common.sh'
--- scripts/volume-common.sh	2012-10-23 09:44:43 +0000
+++ scripts/volume-common.sh	1970-01-01 00:00:00 +0000
@@ -1,220 +0,0 @@
-#!/bin/bash
-# Author: JuanJo Ciarlante <[e-mail]>
-# Copyright: Canonical Ltd. 2012
-# License: GPLv2
-#
-# juju storage common shell library
-# 
-
-#------------------------------
-# Returns a mount point from passed vol-id, e.g. /srv/juju/vol-000012345
-#
-# @param  $1             volume id
-# @echoes mntpoint-path  eg /srv/juju/vol-000012345
-#------------------------------
-_mntpoint_from_volid() {
-  local volid=${1?missing volid}
-  [[ ${volid} != "" ]] && echo /srv/juju/${volid} || echo ""
-}
-
-
-#------------------------------
-# Assert that passed mount points hold different filesystems
-#
-# @param  $1     mntpoint1
-# @param  $2     mntpoint2
-# @return  0     different FS
-#          1     same FS
-#------------------------------
-_assert_diff_fs() {
-  local mnt1="${1:?missing mntpoint1}"
-  local mnt2="${2:?missing mntpoint2}"
-  local fsid1 fsid2
-  fsid1=$(stat --file-system -c '%i' "${mnt1}" 2>/dev/null)
-  fsid2=$(stat --file-system -c '%i' "${mnt2}" 2>/dev/null)
-  [[ ${fsid1} != ${fsid2} ]]
-  return $?
-}
-
-#------------------------------
-# Initialize volume (sfdisk, mkfs.ext4) IFF NOT already, mount it at
-# /srv/juju/<volume-id>
-#
-# @param  $1 volume-id, can be any arbitrary string, better if
-#            equal to EC2/OS vol-id name (just for consistency)
-# @return  0 success
-#          1 nil volid/etc
-#          2 error while handling the device (non-block device, sfdisk error, etc)
-#------------------------------
-volume_init_and_mount() {
-  ## Find 1st unused device (reverse sort /dev/vdX)
-  local volid=${1:?missing volid}
-  local dev_regexp
-  local dev found_dev=
-  local label="${volid}"
-  local func=${FUNCNAME[0]}
-  dev_regexp=$(config-get volume-dev-regexp) || return 1
-  mntpoint=$(_mntpoint_from_volid ${volid})
-
-  [[ -z ${mntpoint} ]] && return 1
-  if mount | egrep -qw "${mntpoint}";then
-    _assert_diff_fs "/" "${mntpoint}" || {
-       juju-log "ERROR: returning from ${func} with '${mntpoint}' still at '/' filesystem"
-       return 1
-    }
-    juju-log "NOTICE: mntpoint=${mntpoint} already mounted, skipping volume_init_and_mount"
-    return 0
-  fi
-
-  # Sanitize
-  case "${dev_regexp?}" in
-    # Careful: this is glob matching against an regexp -
-    # quite narrowed
-    /dev/*|/dev/disk/by-*)
-      ;; ## Ok
-    *)
-      juju-log "ERROR: invalid 'volume-dev-regexp' specified"
-      return 1
-      ;;
-  esac
-
-  # Assume udev will create only existing devices
-  for dev in $(ls -rd1 /dev/* | egrep "${dev_regexp}" | egrep -v "[1-9]$" 2>/dev/null);do
-    ## Check it's not already mounted
-    mount | egrep -q "${dev}[1-9]?" || { found_dev=${dev}; break;}
-  done
-  [[ -n "${found_dev}" ]] || {
-    juju-log "ERROR: ${func}: coult not find an unused device for regexp: ${dev_regexp}"
-    return 1
-  }
-  partition1_dev=${found_dev}1
-
-  juju-log "INFO: ${func}: found_dev=${found_dev}"
-  [[ -b ${found_dev?}  ]] || {
-    juju-log "ERROR: ${func}: ${found_dev} is not a blockdevice"
-    return 2
-  }
-
-  # Run next set of "dangerous" commands as 'set -e', in a subshell
-  (
-  set -e
-  # Re-read partition - will fail if already in use
-  blockdev --rereadpt ${found_dev}
-
-  # IFF not present, create partition with full disk
-  if [[ -b ${partition1_dev?} ]];then
-    juju-log "INFO: ${func}: ${partition1_dev} already present - skipping sfdisk."
-  else
-    juju-log "NOTICE: ${func}: ${partition1_dev} not present at ${found_dev}, running: sfdisk ${found_dev} ..."
-    # Format partition1_dev as max sized
-    echo ",+," | sfdisk ${found_dev}
-  fi
-
-  # Create an ext4 filesystem if NOT already present
-  # use e.g. LABEl=vol-000012345
-  if file -s ${partition1_dev} | egrep -q ext4 ; then
-    juju-log "INFO: ${func}: ${partition1_dev} already formatted as ext4 - skipping mkfs.ext4."
-    ## Check e2label - log if it has changed (e.g. already used / initialized with a diff label)
-    local curr_label=$(e2label "${partition1_dev}")
-    if [[ ${curr_label} != ${label} ]]; then
-      juju-log "WARNING: ${func}: ${partition1_dev} had label=${curr_label}, overwritting with label=${label}"
-      e2label ${partition1_dev} "${label}"
-    fi
-  else
-    juju-log "NOTICE: ${func}: running: mkfs.ext4 -L ${label} ${partition1_dev}"
-    mkfs.ext4 -L "${label}" ${partition1_dev}
-  fi
-
-  # Mount it at e.g. /srv/juju/vol-000012345
-  [[ -d "${mntpoint}" ]] || mkdir -p "${mntpoint}"
-  mount | fgrep -wq "${partition1_dev}" || {
-    local files_below_mntpoint="$(ls -d "${mntpoint}"/* 2>/dev/null |wc -l )" 
-    if [[ ${files_below_mntpoint} -ne 0 ]]; then
-      juju-log "ERROR: *not* doing 'mount "${partition1_dev}" "${mntpoint}"' because there are already ${files_below_mntpoint} files/dirs beneath '${mntpoint}'"
-      exit 1
-    fi
-    ## should always fsck before mounting (e.g. fsck after max time (-i) / max mounts (-c) )
-    fsck "${partition1_dev}"
-    mount "${partition1_dev}" "${mntpoint}"
-    juju-log "INFO: ${func}: mounted as: '$(mount | fgrep -w ${partition1_dev})'"
-  }
-
-  # Add it to fstab is not already there
-  fgrep -wq "LABEL=${label}" /etc/fstab || {
-    echo "LABEL=${label}    ${mntpoint}    ext4    defaults,nobootwait,comment=${volid}" | tee -a /etc/fstab
-    juju-log "INFO: ${func}: LABEL=${label} added to /etc/fstab"
-  }
-  )
-  # Final assertion: mounted filesystem id is different from '/' (effectively mounted)
-  _assert_diff_fs "/" "${mntpoint}" || {
-     juju-log "ERROR: returning from ${func} with '${mntpoint}' still at '/' filesystem (couldn't mount new volume)"
-     ## try to rmdir mntpoint directory - should not be 'mistakenly' used
-     rmdir ${mntpoint}
-     return 1
-  }
-  return $?
-}
-
-#------------------------------
-# Get volume-id from juju config "volume-map" dictionary as
-#     volume-map[JUJU_UNIT_NAME]
-# @return  0 if volume-map value found ( does echo volid or ""), else:
-#          1 if not found or None
-#
-#------------------------------
-volume_get_volid_from_volume_map() {
-  local volid=$(config-get "volume-map"|python -c$'import sys;import os;from yaml import load;from itertools import chain; volume_map = load(sys.stdin)\nif volume_map: print volume_map.get(os.environ["JUJU_UNIT_NAME"])')
-  [[ $volid == None ]] && return 1
-  echo "$volid"
-}
-
-# Returns true if permanent storage (considers --ephemeral)
-# @returns  0 if volid set and not --ephemeral, else:
-#           1 
-volume_is_permanent() {
-  local volid=${1:?missing volid}
-  [[ -n ${volid} && ${volid} != --ephemeral ]] && return 0 || return 1
-}
-volume_mount_point_from_volid(){
-  local volid=${1:?missing volid}
-  if volume_is_permanent;then
-	  echo "/srv/juju/${volid}"
-    return 0
-  else
-    return 1
-  fi
-}
-# Do we have a valid storage state?
-# @returns  0 does echo $volid (can be "--ephemeral")
-#           1 config state is invalid - we should not serve
-volume_get_volume_id() {
-  local ephemeral_storage
-  local volid
-  ephemeral_storage=$(config-get volume-ephemeral-storage) || return 1
-  volid=$(volume_get_volid_from_volume_map) || return 1
-  if [[ $ephemeral_storage == True ]];then
-    # Ephemeral -> should not have a valid volid
-    if [[ $volid != "" ]];then
-        juju-log "ERROR: volume-ephemeral-storage is True, but $JUJU_UNIT_NAME maps to volid=${volid}"
-        return 1
-    fi
-  else
-    # Durable (not ephemeral) -> must have a valid volid for this unit
-     if [[ $volid == "" ]];then
-        juju-log "ERROR: volume-ephemeral-storage is False, but no volid found for: $JUJU_UNIT_NAME"
-        return 1
-     fi
-  fi
-  echo "$volid"
-  return 0
-}
-
-case "$1" in
-  ## allow non SHELL scripts to call helper functions
-  call)
-    : ${JUJU_UNIT_NAME?} ## Must be called in juju environment
-    shift;
-    function="${1:?usage: ${0##*/} call function arg1 arg2 ...}"
-    shift;
-    ${function} "$@" && exit 0 || exit 1
-esac