Merge lp://staging/~chad.smith/charms/precise/storage/storage-volume-label-availability-zone into lp://staging/~dpb/charms/precise/storage/trunk

Proposed by Chad Smith
Status: Merged
Merged at revision: 35
Proposed branch: lp://staging/~chad.smith/charms/precise/storage/storage-volume-label-availability-zone
Merge into: lp://staging/~dpb/charms/precise/storage/trunk
Prerequisite: lp://staging/~chad.smith/charms/precise/storage/storage-fix-nfs-relation-ordering
Diff against target: 229 lines (+97/-19)
3 files modified
hooks/common_util.py (+31/-9)
hooks/storage-provider.d/block-storage-broker/block-storage-relation-changed (+2/-1)
hooks/test_common_util.py (+64/-9)
To merge this branch: bzr merge lp://staging/~chad.smith/charms/precise/storage/storage-volume-label-availability-zone
Reviewer Review Type Date Requested Status
David Britton Approve
Fernando Correa Neto (community) Approve
Review via email: mp+212062@code.staging.launchpad.net

Description of the change

Volumes in EC2-land can only be attached to instances that live in the same availability zone. Block-storage-broker takes whatever volume_label is requested by the storage subordinate and attempts to attach it to the instance-id which is requesting that volume_label. If the instance and volume live in different AZs, the attach will fail and Block-storage-broker hooks will fail.

    To avoid trying to euca-attach-volume of a volume in another AZ, we need to request that our volume label (volume_label translates to a volume TAG in EC2) specify the availability zone in which that volume is created. For openstack and nova, the AZ is always nova, but for ec2, the AZ will be determined as the AZ in which the requesting instance-id lives (us-east-1b, us-west-1a etc.)

This way, the block-storage-broker will not attempt to mount any discovered volume from different availability zones.

There will need to be more significant work in the block-storage-broker charm to also attempt to assess volumes and instances in incompatible AZs, but this current branch prevents a storage-principal unit from requesting an invalid volume_label.

This branch adds a get_availability_zone function to allow extracting this value from EC2 or openstack metadata.

Also in this branch:
   dropping the lsof -b (non-blocking) option, as called currently it was not a valid option. Will look at non-blocking lsof in a separate branch.

To post a comment you must log in.
Revision history for this message
David Britton (dpb) wrote :

[0] When relating to bsb, I recieved:

2014-03-20 22:59:28 INFO worker.uniter.jujuc server.go:108 running hook tool "juju-log" ["Validated charm configuration credentials have access to block storage service"]
2014-03-20 22:59:28 DEBUG worker.uniter.jujuc server.go:109 hook context id "block-storage-broker-ec2/0:block-storage-relation-changed:3470414816341938807"; dir "/var/lib/juju/agents/unit-block-storage-broker-ec2-0/charm"
2014-03-20 22:59:28 INFO juju juju-log.go:66 block-storage-broker-ec2/0 block-storage:2: Validated charm configuration credentials have access to block storage service
2014-03-20 22:59:29 INFO worker.uniter.jujuc server.go:108 running hook tool "juju-log" ["Creating a 9Gig volume named (postgresql/0 us-west-1c volume) for instance i-53df8d0c"]
2014-03-20 22:59:29 DEBUG worker.uniter.jujuc server.go:109 hook context id "block-storage-broker-ec2/0:block-storage-relation-changed:3470414816341938807"; dir "/var/lib/juju/agents/unit-block-storage-broker-ec2-0/charm"
2014-03-20 22:59:29 INFO juju juju-log.go:66 block-storage-broker-ec2/0 block-storage:2: Creating a 9Gig volume named (postgresql/0 us-west-1c volume) for instance i-53df8d0c
2014-03-20 22:59:29 INFO juju.worker.uniter context.go:255 HOOK 'availability_zone'
2014-03-20 22:59:29 ERROR juju.worker.uniter uniter.go:350 hook failed: exit status 1

40. By Chad Smith

drop unused CHARM_DIR global

Revision history for this message
David Britton (dpb) wrote :

Note: Looks good during testing. Starting code review now.

41. By Chad Smith

drop misused -b option to lsof

42. By Chad Smith

lint

Revision history for this message
Fernando Correa Neto (fcorrea) wrote :

Looks great! +1

[1]
71 + except subprocess.CalledProcessError, e:
72 + log("Error: couldn't discover ec2 availability zone. %s" % str(e),
73 + hookenv.ERROR)

Even though this block is for the ec2 try, it only indicates that it failed grabbing both nova and ec2, in which case I think we could log something more generic. Maybe "Error: couldn't discover volume availability zone..."

review: Approve
Revision history for this message
David Britton (dpb) wrote :

+1, let's merge this in as well.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: