Charm needed: block-storage-broker

[Chad Smith]

We need a generic block-storage-broker charm that can talk to EC2 and Nova APIs to create, allocate and attach volumes to instances.

The relation interface named "volume-request" will interact in the following manner:
  - block-storage-charm will consume the following relation data (provided by related charms)
          instance-id # related cloud instance id requesting a volume
          volume-id (optional preexisting volume-id that will get associated to the instance provided)
          size (optional size requested for the volume)

 - block-storage-charm will provide the following relation data
       block-device-path # the path of the device which has been successfully associated with the requesting instance-id

Block-storage-broker will offer better security as the required cloud storage credentials will be configured only in the block-storage-broker instead of being spread to every unit that requests block-storage volumes. Each volume created will be labelled with the unit name of the unit is is associated with to aid introspection at a later time.

the block-storage-broker configuration options:
  region, endpoint, key, secret, tenant, default_volume_size, provider ("ebs" or "nova")

The intent of this charm would be to interact with the storage subordinate charm that can be related to any deployed juju unit. The storage subordinate will then proxy volume requests to the block-storage-broker, which will then associate the requested volume or size with the particular unit.

The bug is also tied to lp:1259630 and the postgresql branch https://code.launchpad.net/~chad.smith/charms/precise/postgresql/postgresql-using-storage-subordinate.

When block-storage-broker and storage-subordinate charms are acccepted to the charm store, then we'll update the readme example bundle to mention the public charmstorage branch locations for postgresql, storage and block-storage-broker.

The charm readme included describes the current bundle that will allow testing using either your EC2 or Openstack credentials and states the following:

    $ cat >block-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
            storage:
                branch: lp:~davidpbritton/charms/precise/storage/trunk
                options:
                    volume_size: 11
                    provider: block-storage-broker
                    root: /srv/data
    doit-openstack:
        inherits: common
        series: precise
        services:
            block-storage-broker:
                branch: lp:~chad.smith/charms/precise/block-storage-broker/trunk
                options:
                    provider: nova
                    key: <YOUR_OS_USERNAME>
                    endpoint: <YOUR_OS_AUTH_URL>
                    region: <YOUR_OS_REGION>
                    secret: <YOUR_OS_PASSWORD>
                    tenant: <YOUR_OS_TENANT_NAME>
        relations:
            - ["postgresql", "storage"]
            - ["storage", "block-storage-broker"]
    doit-ec2:
        inherits: common
        series: precise
        services:
            block-storage-broker:
                branch: lp:~chad.smith/charms/precise/block-storage-broker/trunk
                options:
                    provider: ec2
                    key: <YOUR_EC2_ACCESS_KEY>
                    endpoint: <YOUR_EC2_URL>
                    secret: <YOUR_EC2_SECRET_KEY>
        relations:
            - ["postgresql", "storage"]
            - ["storage", "block-storage-broker"]
    END

    # To deploy and relate volumes using your openstack credentials
    $ juju-deployer -c block-storage-bundle.cfg doit-openstack

    # To deploy and relate volumes using your ec2 credentials
    $ juju-deployer -c block-storage-bundle.cfg doit-ec2

[David Britton (dpb)]

=== General/yaml files ===

[1] metadata.yaml: "to attach new or existing volumes to the cloud instance"...

[2] Add a make clean target?
clean:
       find . -name *.pyc | xargs rm
       find . -name _trial_temp | xargs rm -rf

[3] Need to resolve charm proof warnings/errors (not infos)

[9] incorporate makefile target to update charmhelpers?

=== hook.py ===

[4] kill the extra log method and just import hookenv.log as log
[5] line 38 and 39 can be combined and stay under 80 characters
[6] missing hook symlink for b-s-b-relation-departed
[7] move _load_environment into main() ? you call it in a few places. Or move it into nova_util so it's hidden from here. I guess that might be part of the refactoring that needs to be done when EBS comes in.
[8] take out the filter_install_packages() call. apt-get natively does this. Or leave it in and file a bug upstream. It will not take into considration package updates and will skip them, where apt would "do the right thing"

=== nova_util.py ===

[10] get_volume_id() docstring, A couple ideas here that are not included:

"""Return the nova volume id attached to the relating unit

Optionally, C{volume_name} can be specified to return the volume_id that
has the matching C{volume_name}. This name can also be an exact volume_id. If no matching volume is found, return
C{None}.
"""

[11] docstring in attach_nova_volume "attach a volume to *the remote unit if none*"

[12] attach_nova_volume(): "in-use" is OK, if it's the relating unit, right? Should that be validated? i.e., it's already attached. Ah, I see, it's just the docstring that is a bit inconsistent with the behavior of the code here.

[13] attach_nova_volume(): range(10) can be used instead of the two argument option.

[14] detach_nova_volume(): docstring s/this unit/remote unit/

[Chad Smith (chad.smith)]

All review comments resolved and added a bit more log messages as well.

[David Britton (dpb)]

+1

[Chad Smith (chad.smith)]

This branch is ready for review and can easily be deployed and related to new storage subordinate charm by davidpbritton at:
   https://code.launchpad.net/~davidpbritton/charms/precise/storage/trunk

As well as an updated postgresql charm at
   https://code.launchpad.net/~chad.smith/charms/precise/postgresql/postgresql-using-storage-subordinate

(Postgresql Charm changes should be reviewed shortly per the following MP)
  https://code.launchpad.net/~chad.smith/charms/precise/postgresql/postgresql-using-storage-subordinate/+merge/206078

An example deployment bundle to test these changes is both documented in the charm's README.md, the MP above and just below.

To deploy and test without any nova volumes created:
# Make sure to replace the OS_* variable names and YOUR-NOVA-VOLUME-ID in postgreql-storage-bundle.cfg once it is 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:~chad.smith/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:~davidpbritton/charms/precise/storage/trunk
            options:
                provider: block-storage-broker
                volume_size: 9
    relations:
        - [postgresql, storage]
        - [storage, block-storage-broker]

doit-with-nova-volume:
    inherits: common
    series: precise
    services:
        storage:
            branch: lp:~davidpbritton/charms/precise/storage/trunk
            options:
                provider: block-storage-broker
                volume_size: 9
                volume_map: "{postgresql/0: YOUR-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-nova-volume

$ juju add-unit postgresql # will also automatically create an additional nova volume named "postgresql/1 unit volume" and attach and mount it even if unspecified in the storage volume_map configuration parameter

[Marco Ceppi (marcoceppi)]

# Proof

W: No icon.svg file.
W: no copyright file

At a minimum we need a copyright file which licenses the contents of the charm with an OSI approved license.

# Review

I have a few concerns with the way this charm implements the relation, specifically with requiring a look up against http://169.254.169.254 as that URL is specifically blacklisted in charm proof. I understand the need for this, and realize that this charm will likely be limited to only OpenStack and Amazon clouds given the nature of what it does, I'd love to see that lookup occur on the block-storage-broker side as I feel this shouldn't be deployed as a standalone service, but rather as a subordinate. If it's a subordinate, it'll live on the unit it's attempting to spawn a disk for and attach it accordingly.

The rest of the code looks sane, though I haven't deployed and tested it yet. Having credentials as config is always a bit, naughty seeming. Otherwise, this seems like a great stop gap until juju has a proper storage story

[Kapil Thangavelu (hazmat)]

its a bad idea for this to live as a subordinate, we don't want to
distribute cloud credentials everywhere.

On Fri, Feb 28, 2014 at 8:28 AM, Marco Ceppi <email address hidden> wrote:

> # Proof
>
> W: No icon.svg file.
> W: no copyright file
>
> At a minimum we need a copyright file which licenses the contents of the
> charm with an OSI approved license.
>
> # Review
>
> I have a few concerns with the way this charm implements the relation,
> specifically with requiring a look up against http://169.254.169.254 as
> that URL is specifically blacklisted in charm proof. I understand the
> need for this, and realize that this charm will likely be limited to
> only OpenStack and Amazon clouds given the nature of what it does, I'd
> love to see that lookup occur on the block-storage-broker side as I feel
> this shouldn't be deployed as a standalone service, but rather as a
> subordinate. If it's a subordinate, it'll live on the unit it's
> attempting to spawn a disk for and attach it accordingly.
>
> The rest of the code looks sane, though I haven't deployed and tested it
> yet. Having credentials as config is always a bit, naughty seeming.
> Otherwise, this seems like a great stop gap until juju has a proper
> storage story
>
> --
> You received this bug notification because you are a member of charmers,
> which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/1274347
>
> Title:
> Charm needed: block-storage-broker
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/1274347/+subscriptions
>

[Marco Ceppi (marcoceppi)]

I've moved this to incomplete, pending resolution of the above issues and questions. When ready for a follow up review place this bug status in either a New or Fix Committed state to have it re-added to the review queue.

[Chad Smith (chad.smith)]

As Kapil mentioned in lp:1298496, separating the block-storage-broker from the storage-subordinate was an security decision to limit credentials to only 1 unit, instead of spreading them all to any unit that needs storage. This way, our security risk profile is limited to the juju admin and the single block-storage-broker unit instead of every unit that uses storage.

The block-storage-broker was originally part of the storage subordinate, but we tended to agree with Kapil on this so we moved it out. I've added the copyright file as requested.

In this update, we have just merged EC2 support as well which the landscape team uses to solve some of our persistent storage needs.

I've added the copyright file and created an icon.svg following the suggestions at https://juju.ubuntu.com/docs/authors-charm-icon.html

Per pulling instance metadata from the EC2-compatible 169.254.169.254, I can't see any other way around hitting those URLs from the units to obtain instance metadata. If you can think of alternatives I'm game for changing that up, but cloud instance metadata seems to only come from those URLs.

[Charles Butler (lazypower)]

Greetings,

Thank you for the submission and dedication in making a robust charm. I've taken a review of the charm and have the following notes:

The readme has some really well documented examples. I however didn't have an openstack network to deploy against so attempted an EC2 deployment which the charm implicitly states it works with. I was a bit stumped by the isntance-id property on the storage broker. It wasn't clear to me that the storage subordinate provided the details to the storage broker.

I strongly urge you to update the readme with additional, clearly defined instructions for someone to get started quickly with the charm in an EC2 environment, that mock or model the openstack based instructions.

Otherwise, the charm itself worked a treat. I deployed the postgresql charm from the branch indicated with storage support, and things magically worked after a sync with csmith about the role of the storage subordinate.

The use of storage-provider.d is really compelling to me that this is extensible by default. +1 for thinking to the future.

All in all I see no blockers for this charm, just the knitpicks on the documentation above. There is a roadmap for juju getting its own block storage service, so this charm may be short lived - but I see no reason to not promote the work today so people can get started with it.

I'm going to +1 and ack this charm into the store. You should see it show up in the next 30 minutes.

I'll be following up with the bug tracker resource and any final notes.

[Charles Butler (lazypower)]

I shortened the summary a bit to conform with charm proof, you were a few characters over 72.

You can find the bug tracker for the charm: https://bugs.launchpad.net/charms/precise/+source/block-storage-broker

Thanks again for the submission. The charm should show up in the store within the next 30 minutes or so.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>