From da85cdadd9a2331a28d4913b878e18c9c5abaccf Mon Sep 17 00:00:00 2001
From: Dean Troyer <dtroyer@gmail.com>
Date: Fri, 15 Feb 2013 11:07:14 -0600
Subject: [PATCH] Exercise cleanup

* Make common steps consistent
* Make comments consistent

aggregates.sh
boot_from_volume.sh
client-args.sh
client-env.sh
euca.sh
floating_ips.sh
sec_groups.sh
swift.sh
volumes.sh

Change-Id: Ib93dcdfdead93c259e3cd184fbc5ccc0a4a87c9a
---
 exercises/aggregates.sh       |  19 ++--
 exercises/boot_from_volume.sh | 176 ++++++++++++++++++++--------------
 exercises/client-args.sh      |  34 ++++++-
 exercises/client-env.sh       |  28 ++++++
 exercises/euca.sh             |   5 +-
 exercises/floating_ips.sh     | 134 +++++++++++++-------------
 exercises/sec_groups.sh       |   8 +-
 exercises/swift.sh            |   8 +-
 exercises/volumes.sh          | 114 +++++++++++-----------
 9 files changed, 305 insertions(+), 221 deletions(-)

diff --git a/exercises/aggregates.sh b/exercises/aggregates.sh
index deb1a038fd..ae3198f941 100755
--- a/exercises/aggregates.sh
+++ b/exercises/aggregates.sh
@@ -39,9 +39,8 @@ source $TOP_DIR/openrc
 # Import exercise configuration
 source $TOP_DIR/exerciserc
 
-# run test as the admin user
-_OLD_USERNAME=$OS_USERNAME
-OS_USERNAME=admin
+# Test as the admin user
+. openrc admin admin
 
 
 # Create an aggregate
@@ -54,7 +53,7 @@ AGGREGATE_A_ZONE=nova
 exit_if_aggregate_present() {
     aggregate_name=$1
 
-    if [ `nova aggregate-list | grep -c " $aggregate_name "` == 0 ]; then
+    if [ $(nova aggregate-list | grep -c " $aggregate_name ") == 0 ]; then
         echo "SUCCESS $aggregate_name not present"
     else
         echo "ERROR found aggregate: $aggregate_name"
@@ -64,8 +63,8 @@ exit_if_aggregate_present() {
 
 exit_if_aggregate_present $AGGREGATE_NAME
 
-AGGREGATE_ID=`nova aggregate-create $AGGREGATE_NAME $AGGREGATE_A_ZONE | grep " $AGGREGATE_NAME " | get_field 1`
-AGGREGATE2_ID=`nova aggregate-create $AGGREGATE2_NAME $AGGREGATE_A_ZONE | grep " $AGGREGATE2_NAME " | get_field 1`
+AGGREGATE_ID=$(nova aggregate-create $AGGREGATE_NAME $AGGREGATE_A_ZONE | grep " $AGGREGATE_NAME " | get_field 1)
+AGGREGATE2_ID=$(nova aggregate-create $AGGREGATE2_NAME $AGGREGATE_A_ZONE | grep " $AGGREGATE2_NAME " | get_field 1)
 
 # check aggregate created
 nova aggregate-list | grep -q " $AGGREGATE_NAME " || die "Aggregate $AGGREGATE_NAME not created"
@@ -125,7 +124,7 @@ nova aggregate-details $AGGREGATE_ID | egrep "{u'availability_zone': u'$AGGREGAT
 if [ "$VIRT_DRIVER" == "xenserver" ]; then
     echo "TODO(johngarbutt) add tests for add/remove host from pool aggregate"
 fi
-FIRST_HOST=`nova host-list | grep compute | get_field 1 | head -1`
+FIRST_HOST=$(nova host-list | grep compute | get_field 1 | head -1)
 # Make sure can add two aggregates to same host
 nova aggregate-add-host $AGGREGATE_ID $FIRST_HOST
 nova aggregate-add-host $AGGREGATE2_ID $FIRST_HOST
@@ -142,12 +141,6 @@ nova aggregate-delete $AGGREGATE_ID
 nova aggregate-delete $AGGREGATE2_ID
 exit_if_aggregate_present $AGGREGATE_NAME
 
-
-# Test complete
-# =============
-OS_USERNAME=$_OLD_USERNAME
-echo "AGGREGATE TEST PASSED"
-
 set +o xtrace
 echo "**************************************************"
 echo "End DevStack Exercise: $0"
diff --git a/exercises/boot_from_volume.sh b/exercises/boot_from_volume.sh
index 5ada2370b8..679091bb39 100755
--- a/exercises/boot_from_volume.sh
+++ b/exercises/boot_from_volume.sh
@@ -44,52 +44,80 @@ source $TOP_DIR/exerciserc
 # the exercise is skipped
 is_service_enabled cinder || exit 55
 
-# Boot this image, use first AMI image if unset
-DEFAULT_IMAGE_NAME=${DEFAULT_IMAGE_NAME:-ami}
-
-# Instance type
+# Instance type to create
 DEFAULT_INSTANCE_TYPE=${DEFAULT_INSTANCE_TYPE:-m1.tiny}
 
-# Default floating IP pool name
-DEFAULT_FLOATING_POOL=${DEFAULT_FLOATING_POOL:-nova}
-
-# Default user
-DEFAULT_INSTANCE_USER=${DEFAULT_INSTANCE_USER:-cirros}
+# Boot this image, use first AMI image if unset
+DEFAULT_IMAGE_NAME=${DEFAULT_IMAGE_NAME:-ami}
 
 # Security group name
 SECGROUP=${SECGROUP:-boot_secgroup}
 
+# Instance and volume names
+VM_NAME=${VM_NAME:-ex-bfv-inst}
+VOL_NAME=${VOL_NAME:-ex-vol-bfv}
 
-# Launching servers
-# =================
+
+# Launching a server
+# ==================
+
+# List servers for tenant:
+nova list
+
+# Images
+# ------
+
+# List the images available
+glance image-list
 
 # Grab the id of the image to launch
-IMAGE=`glance image-list | egrep " $DEFAULT_IMAGE_NAME " | get_field 1`
-die_if_not_set IMAGE "Failure getting image"
+IMAGE=$(glance image-list | egrep " $DEFAULT_IMAGE_NAME " | get_field 1)
+die_if_not_set IMAGE "Failure getting image $DEFAULT_IMAGE_NAME"
 
-# Instance and volume names
-VOL_INSTANCE_NAME=${VOL_INSTANCE_NAME:-test_vol_instance}
-VOL_NAME=${VOL_NAME:-test_volume}
+# Security Groups
+# ---------------
 
-# Clean-up from previous runs
-nova delete $VOL_INSTANCE_NAME || true
+# List security groups
+nova secgroup-list
 
-if ! timeout $ACTIVE_TIMEOUT sh -c "while nova show $VOL_INSTANCE_NAME; do sleep 1; done"; then
-    echo "server didn't terminate!"
-    exit 1
+# Create a secgroup
+if ! nova secgroup-list | grep -q $SECGROUP; then
+    nova secgroup-create $SECGROUP "$SECGROUP description"
+    if ! timeout $ASSOCIATE_TIMEOUT sh -c "while ! nova secgroup-list | grep -q $SECGROUP; do sleep 1; done"; then
+        echo "Security group not created"
+        exit 1
+    fi
 fi
 
-# Configure Security Groups
-nova secgroup-delete $SECGROUP || true
-nova secgroup-create $SECGROUP "$SECGROUP description"
-nova secgroup-add-rule $SECGROUP icmp -1 -1 0.0.0.0/0
-nova secgroup-add-rule $SECGROUP tcp 22 22 0.0.0.0/0
+# Configure Security Group Rules
+if ! nova secgroup-list-rules $SECGROUP | grep -q icmp; then
+    nova secgroup-add-rule $SECGROUP icmp -1 -1 0.0.0.0/0
+fi
+if ! nova secgroup-list-rules $SECGROUP | grep -q " tcp .* 22 "; then
+    nova secgroup-add-rule $SECGROUP tcp 22 22 0.0.0.0/0
+fi
 
-# Determinine instance type
-INSTANCE_TYPE=`nova flavor-list | grep $DEFAULT_INSTANCE_TYPE | cut -d"|" -f2`
+# List secgroup rules
+nova secgroup-list-rules $SECGROUP
+
+# Set up instance
+# ---------------
+
+# List flavors
+nova flavor-list
+
+# Select a flavor
+INSTANCE_TYPE=$(nova flavor-list | grep $DEFAULT_INSTANCE_TYPE | get_field 1)
 if [[ -z "$INSTANCE_TYPE" ]]; then
     # grab the first flavor in the list to launch if default doesn't exist
-   INSTANCE_TYPE=`nova flavor-list | head -n 4 | tail -n 1 | cut -d"|" -f2`
+   INSTANCE_TYPE=$(nova flavor-list | head -n 4 | tail -n 1 | get_field 1)
+fi
+
+# Clean-up from previous runs
+nova delete $VM_NAME || true
+if ! timeout $ACTIVE_TIMEOUT sh -c "while nova show $VM_NAME; do sleep 1; done"; then
+    echo "server didn't terminate!"
+    exit 1
 fi
 
 # Setup Keypair
@@ -99,78 +127,80 @@ nova keypair-delete $KEY_NAME || true
 nova keypair-add $KEY_NAME > $KEY_FILE
 chmod 600 $KEY_FILE
 
-# Delete the old volume
+# Set up volume
+# -------------
+
+# Delete any old volume
 cinder delete $VOL_NAME || true
-
-# Free every floating ips - setting FREE_ALL_FLOATING_IPS=True in localrc will make life easier for testers
-if [ "$FREE_ALL_FLOATING_IPS" = "True" ]; then
-    nova floating-ip-list | grep nova | cut -d "|" -f2 | tr -d " " | xargs -n1 nova floating-ip-delete || true
-fi
-
-# Allocate floating ip
-FLOATING_IP=`nova floating-ip-create | grep $DEFAULT_FLOATING_POOL | get_field 1`
-
-# Make sure the ip gets allocated
-if ! timeout $ASSOCIATE_TIMEOUT sh -c "while ! nova floating-ip-list | grep -q $FLOATING_IP; do sleep 1; done"; then
-    echo "Floating IP not allocated"
+if ! timeout $ACTIVE_TIMEOUT sh -c "while cinder list | grep $VOL_NAME; do sleep 1; done"; then
+    echo "Volume $VOL_NAME not deleted"
     exit 1
 fi
 
 # Create the bootable volume
-cinder create --display_name=$VOL_NAME --image-id $IMAGE $DEFAULT_VOLUME_SIZE
-
-# Wait for volume to activate
+start_time=$(date +%s)
+cinder create --image-id $IMAGE --display_name=$VOL_NAME --display_description "test bootable volume: $VOL_NAME" $DEFAULT_VOLUME_SIZE || \
+    die "Failure creating volume $VOL_NAME"
 if ! timeout $ACTIVE_TIMEOUT sh -c "while ! cinder list | grep $VOL_NAME | grep available; do sleep 1; done"; then
     echo "Volume $VOL_NAME not created"
     exit 1
 fi
+end_time=$(date +%s)
+echo "Completed cinder create in $((end_time - start_time)) seconds"
 
-VOLUME_ID=`cinder list | grep $VOL_NAME  | get_field 1`
+# Get volume ID
+VOL_ID=$(cinder list | grep $VOL_NAME  | get_field 1)
+die_if_not_set VOL_ID "Failure retrieving volume ID for $VOL_NAME"
 
-# Boot instance from volume!  This is done with the --block_device_mapping param.
-# The format of mapping is:
+# Boot instance
+# -------------
+
+# Boot using the --block_device_mapping param. The format of mapping is:
 # <dev_name>=<id>:<type>:<size(GB)>:<delete_on_terminate>
 # Leaving the middle two fields blank appears to do-the-right-thing
-VOL_VM_UUID=`nova boot --flavor $INSTANCE_TYPE --image $IMAGE --block_device_mapping vda=$VOLUME_ID:::0 --security_groups=$SECGROUP --key_name $KEY_NAME $VOL_INSTANCE_NAME | grep ' id ' | get_field 2`
-die_if_not_set VOL_VM_UUID "Failure launching $VOL_INSTANCE_NAME"
+VM_UUID=$(nova boot --flavor $INSTANCE_TYPE --image $IMAGE --block-device-mapping vda=$VOL_ID --security_groups=$SECGROUP --key_name $KEY_NAME $VM_NAME | grep ' id ' | get_field 2)
+die_if_not_set VM_UUID "Failure launching $VM_NAME"
 
 # Check that the status is active within ACTIVE_TIMEOUT seconds
-if ! timeout $ACTIVE_TIMEOUT sh -c "while ! nova show $VOL_VM_UUID | grep status | grep -q ACTIVE; do sleep 1; done"; then
+if ! timeout $ACTIVE_TIMEOUT sh -c "while ! nova show $VM_UUID | grep status | grep -q ACTIVE; do sleep 1; done"; then
     echo "server didn't become active!"
     exit 1
 fi
 
-# Add floating ip to our server
-nova add-floating-ip $VOL_VM_UUID $FLOATING_IP
+# Get the instance IP
+IP=$(nova show $VM_UUID | grep "$PRIVATE_NETWORK_NAME" | get_field 2)
+die_if_not_set IP "Failure retrieving IP address"
 
-# Test we can ping our floating ip within ASSOCIATE_TIMEOUT seconds
-ping_check "$PUBLIC_NETWORK_NAME" $FLOATING_IP $ASSOCIATE_TIMEOUT
+# Private IPs can be pinged in single node deployments
+ping_check "$PRIVATE_NETWORK_NAME" $IP $BOOT_TIMEOUT
 
-# Make sure our volume-backed instance launched
-ssh_check "$PUBLIC_NETWORK_NAME" $KEY_FILE $FLOATING_IP $DEFAULT_INSTANCE_USER $ACTIVE_TIMEOUT
-
-# Remove floating ip from volume-backed instance
-nova remove-floating-ip $VOL_VM_UUID $FLOATING_IP
+# Clean up
+# --------
 
 # Delete volume backed instance
-nova delete $VOL_INSTANCE_NAME || \
-    die "Failure deleting instance volume $VOL_INSTANCE_NAME"
-
-# Wait till our volume is no longer in-use
-if ! timeout $ACTIVE_TIMEOUT sh -c "while ! cinder list | grep $VOL_NAME | grep available; do sleep 1; done"; then
-    echo "Volume $VOL_NAME not created"
+nova delete $VM_UUID || die "Failure deleting instance $VM_NAME"
+if ! timeout $TERMINATE_TIMEOUT sh -c "while nova list | grep -q $VM_UUID; do sleep 1; done"; then
+    echo "Server $VM_NAME not deleted"
     exit 1
 fi
 
-# Delete the volume
-cinder delete $VOL_NAME || \
-    die "Failure deleting volume $VOLUME_NAME"
+# Wait for volume to be released
+if ! timeout $ACTIVE_TIMEOUT sh -c "while ! cinder list | grep $VOL_NAME | grep available; do sleep 1; done"; then
+    echo "Volume $VOL_NAME not released"
+    exit 1
+fi
 
-# De-allocate the floating ip
-nova floating-ip-delete $FLOATING_IP || \
-    die "Failure deleting floating IP $FLOATING_IP"
+# Delete volume
+start_time=$(date +%s)
+cinder delete $VOL_ID || die "Failure deleting volume $VOLUME_NAME"
+if ! timeout $ACTIVE_TIMEOUT sh -c "while cinder list | grep $VOL_NAME; do sleep 1; done"; then
+    echo "Volume $VOL_NAME not deleted"
+    exit 1
+fi
+end_time=$(date +%s)
+echo "Completed cinder delete in $((end_time - start_time)) seconds"
 
-# Delete a secgroup
+# Delete secgroup
 nova secgroup-delete $SECGROUP || die "Failure deleting security group $SECGROUP"
 
 set +o xtrace
diff --git a/exercises/client-args.sh b/exercises/client-args.sh
index b3e2ad8d4a..894da74263 100755
--- a/exercises/client-args.sh
+++ b/exercises/client-args.sh
@@ -8,6 +8,14 @@ echo "*********************************************************************"
 echo "Begin DevStack Exercise: $0"
 echo "*********************************************************************"
 
+# This script exits on an error so that errors don't compound and you see
+# only the first error that occured.
+set -o errexit
+
+# Print the commands being run so that we can see the command that triggers
+# an error.  It is also useful for following allowing as the install occurs.
+set -o xtrace
+
 
 # Settings
 # ========
@@ -63,7 +71,7 @@ if [[ "$ENABLED_SERVICES" =~ "key" ]]; then
         STATUS_KEYSTONE="Skipped"
     else
         echo -e "\nTest Keystone"
-        if keystone $TENANT_ARG $ARGS catalog --service identity; then
+        if keystone $TENANT_ARG_DASH $ARGS_DASH catalog --service identity; then
             STATUS_KEYSTONE="Succeeded"
         else
             STATUS_KEYSTONE="Failed"
@@ -82,7 +90,7 @@ if [[ "$ENABLED_SERVICES" =~ "n-api" ]]; then
     else
         # Test OSAPI
         echo -e "\nTest Nova"
-        if nova $TENANT_ARG $ARGS flavor-list; then
+        if nova $TENANT_ARG_DASH $ARGS_DASH flavor-list; then
             STATUS_NOVA="Succeeded"
         else
             STATUS_NOVA="Failed"
@@ -91,6 +99,23 @@ if [[ "$ENABLED_SERVICES" =~ "n-api" ]]; then
     fi
 fi
 
+# Cinder client
+# -------------
+
+if [[ "$ENABLED_SERVICES" =~ "c-api" ]]; then
+    if [[ "$SKIP_EXERCISES" =~ "c-api" ]] ; then
+        STATUS_CINDER="Skipped"
+    else
+        echo -e "\nTest Cinder"
+        if cinder $TENANT_ARG_DASH $ARGS_DASH list; then
+            STATUS_CINDER="Succeeded"
+        else
+            STATUS_CINDER="Failed"
+            RETURN=1
+        fi
+    fi
+fi
+
 # Glance client
 # -------------
 
@@ -116,7 +141,7 @@ if [[ "$ENABLED_SERVICES" =~ "swift" ]]; then
         STATUS_SWIFT="Skipped"
     else
         echo -e "\nTest Swift"
-        if swift $TENANT_ARG $ARGS stat; then
+        if swift $TENANT_ARG_DASH $ARGS_DASH stat; then
             STATUS_SWIFT="Succeeded"
         else
             STATUS_SWIFT="Failed"
@@ -125,6 +150,8 @@ if [[ "$ENABLED_SERVICES" =~ "swift" ]]; then
     fi
 fi
 
+set +o xtrace
+
 # Results
 # -------
 
@@ -137,6 +164,7 @@ function report() {
 echo -e "\n"
 report "Keystone" $STATUS_KEYSTONE
 report "Nova" $STATUS_NOVA
+report "Cinder" $STATUS_CINDER
 report "Glance" $STATUS_GLANCE
 report "Swift" $STATUS_SWIFT
 
diff --git a/exercises/client-env.sh b/exercises/client-env.sh
index 68c0e5adce..c84e84e55b 100755
--- a/exercises/client-env.sh
+++ b/exercises/client-env.sh
@@ -8,6 +8,14 @@ echo "*********************************************************************"
 echo "Begin DevStack Exercise: $0"
 echo "*********************************************************************"
 
+# This script exits on an error so that errors don't compound and you see
+# only the first error that occured.
+set -o errexit
+
+# Print the commands being run so that we can see the command that triggers
+# an error.  It is also useful for following allowing as the install occurs.
+set -o xtrace
+
 
 # Settings
 # ========
@@ -99,6 +107,23 @@ if [[ "$ENABLED_SERVICES" =~ "n-api" ]]; then
     fi
 fi
 
+# Cinder client
+# -------------
+
+if [[ "$ENABLED_SERVICES" =~ "c-api" ]]; then
+    if [[ "$SKIP_EXERCISES" =~ "c-api" ]] ; then
+        STATUS_CINDER="Skipped"
+    else
+        echo -e "\nTest Cinder"
+        if cinder list; then
+            STATUS_CINDER="Succeeded"
+        else
+            STATUS_CINDER="Failed"
+            RETURN=1
+        fi
+    fi
+fi
+
 # Glance client
 # -------------
 
@@ -133,6 +158,8 @@ if [[ "$ENABLED_SERVICES" =~ "swift" ]]; then
     fi
 fi
 
+set +o xtrace
+
 # Results
 # -------
 
@@ -146,6 +173,7 @@ echo -e "\n"
 report "Keystone" $STATUS_KEYSTONE
 report "Nova" $STATUS_NOVA
 report "EC2" $STATUS_EC2
+report "Cinder" $STATUS_CINDER
 report "Glance" $STATUS_GLANCE
 report "Swift" $STATUS_SWIFT
 
diff --git a/exercises/euca.sh b/exercises/euca.sh
index 7b35f6feec..8b15da8d49 100755
--- a/exercises/euca.sh
+++ b/exercises/euca.sh
@@ -44,7 +44,7 @@ source $TOP_DIR/exerciserc
 # Instance type to create
 DEFAULT_INSTANCE_TYPE=${DEFAULT_INSTANCE_TYPE:-m1.tiny}
 
-# Boot this image, use first AMI-format image if unset
+# Boot this image, use first AMI image if unset
 DEFAULT_IMAGE_NAME=${DEFAULT_IMAGE_NAME:-ami}
 
 # Security group name
@@ -56,6 +56,7 @@ SECGROUP=${SECGROUP:-euca_secgroup}
 
 # Find a machine image to boot
 IMAGE=`euca-describe-images | grep machine | grep ${DEFAULT_IMAGE_NAME} | cut -f2 | head -n1`
+die_if_not_set IMAGE "Failure getting image $DEFAULT_IMAGE_NAME"
 
 # Add a secgroup
 if ! euca-describe-groups | grep -q $SECGROUP; then
@@ -174,7 +175,7 @@ if ! timeout $TERMINATE_TIMEOUT sh -c "while euca-describe-instances $INSTANCE |
     exit 1
 fi
 
-# Delete group
+# Delete secgroup
 euca-delete-group $SECGROUP || die "Failure deleting security group $SECGROUP"
 
 set +o xtrace
diff --git a/exercises/floating_ips.sh b/exercises/floating_ips.sh
index 8b18e6f4ba..34ab69d988 100755
--- a/exercises/floating_ips.sh
+++ b/exercises/floating_ips.sh
@@ -2,8 +2,7 @@
 
 # **floating_ips.sh** - using the cloud can be fun
 
-# we will use the ``nova`` cli tool provided by the ``python-novaclient``
-# package to work out the instance connectivity
+# Test instance connectivity with the ``nova`` command from ``python-novaclient``
 
 echo "*********************************************************************"
 echo "Begin DevStack Exercise: $0"
@@ -42,7 +41,7 @@ source $TOP_DIR/exerciserc
 # Instance type to create
 DEFAULT_INSTANCE_TYPE=${DEFAULT_INSTANCE_TYPE:-m1.tiny}
 
-# Boot this image, use first AMi image if unset
+# Boot this image, use first AMI image if unset
 DEFAULT_IMAGE_NAME=${DEFAULT_IMAGE_NAME:-ami}
 
 # Security group name
@@ -54,6 +53,9 @@ DEFAULT_FLOATING_POOL=${DEFAULT_FLOATING_POOL:-nova}
 # Additional floating IP pool and range
 TEST_FLOATING_POOL=${TEST_FLOATING_POOL:-test}
 
+# Instance name
+VM_NAME="ex-float"
+
 
 # Launching a server
 # ==================
@@ -64,19 +66,17 @@ nova list
 # Images
 # ------
 
-# Nova has a **deprecated** way of listing images.
-nova image-list
-
-# But we recommend using glance directly
+# List the images available
 glance image-list
 
 # Grab the id of the image to launch
 IMAGE=$(glance image-list | egrep " $DEFAULT_IMAGE_NAME " | get_field 1)
+die_if_not_set IMAGE "Failure getting image $DEFAULT_IMAGE_NAME"
 
 # Security Groups
 # ---------------
 
-# List of secgroups:
+# List security groups
 nova secgroup-list
 
 # Create a secgroup
@@ -88,81 +88,79 @@ if ! nova secgroup-list | grep -q $SECGROUP; then
     fi
 fi
 
-# Determinine instance type
-# -------------------------
-
-# List of instance types:
-nova flavor-list
-
-INSTANCE_TYPE=`nova flavor-list | grep $DEFAULT_INSTANCE_TYPE | get_field 1`
-if [[ -z "$INSTANCE_TYPE" ]]; then
-    # grab the first flavor in the list to launch if default doesn't exist
-   INSTANCE_TYPE=`nova flavor-list | head -n 4 | tail -n 1 | get_field 1`
+# Configure Security Group Rules
+if ! nova secgroup-list-rules $SECGROUP | grep -q icmp; then
+    nova secgroup-add-rule $SECGROUP icmp -1 -1 0.0.0.0/0
+fi
+if ! nova secgroup-list-rules $SECGROUP | grep -q " tcp .* 22 "; then
+    nova secgroup-add-rule $SECGROUP tcp 22 22 0.0.0.0/0
 fi
 
-NAME="ex-float"
+# List secgroup rules
+nova secgroup-list-rules $SECGROUP
 
-VM_UUID=`nova boot --flavor $INSTANCE_TYPE --image $IMAGE $NAME --security_groups=$SECGROUP | grep ' id ' | get_field 2`
-die_if_not_set VM_UUID "Failure launching $NAME"
+# Set up instance
+# ---------------
 
+# List flavors
+nova flavor-list
 
-# Testing
-# =======
+# Select a flavor
+INSTANCE_TYPE=$(nova flavor-list | grep $DEFAULT_INSTANCE_TYPE | get_field 1)
+if [[ -z "$INSTANCE_TYPE" ]]; then
+    # grab the first flavor in the list to launch if default doesn't exist
+   INSTANCE_TYPE=$(nova flavor-list | head -n 4 | tail -n 1 | get_field 1)
+fi
 
-# First check if it spins up (becomes active and responds to ping on
-# internal ip).  If you run this script from a nova node, you should
-# bypass security groups and have direct access to the server.
+# Clean-up from previous runs
+nova delete $VM_NAME || true
+if ! timeout $ACTIVE_TIMEOUT sh -c "while nova show $VM_NAME; do sleep 1; done"; then
+    echo "server didn't terminate!"
+    exit 1
+fi
 
-# Waiting for boot
-# ----------------
+# Boot instance
+# -------------
 
-# check that the status is active within ACTIVE_TIMEOUT seconds
+VM_UUID=$(nova boot --flavor $INSTANCE_TYPE --image $IMAGE --security_groups=$SECGROUP $VM_NAME | grep ' id ' | get_field 2)
+die_if_not_set VM_UUID "Failure launching $VM_NAME"
+
+# Check that the status is active within ACTIVE_TIMEOUT seconds
 if ! timeout $ACTIVE_TIMEOUT sh -c "while ! nova show $VM_UUID | grep status | grep -q ACTIVE; do sleep 1; done"; then
     echo "server didn't become active!"
     exit 1
 fi
 
-# get the IP of the server
-IP=`nova show $VM_UUID | grep "$PRIVATE_NETWORK_NAME" | get_field 2`
+# Get the instance IP
+IP=$(nova show $VM_UUID | grep "$PRIVATE_NETWORK_NAME" | get_field 2)
 die_if_not_set IP "Failure retrieving IP address"
 
+# Private IPs can be pinged in single node deployments
 ping_check "$PRIVATE_NETWORK_NAME" $IP $BOOT_TIMEOUT
 
-# Security Groups & Floating IPs
-# ------------------------------
+# Floating IPs
+# ------------
 
-if ! nova secgroup-list-rules $SECGROUP | grep -q icmp; then
-    # allow icmp traffic (ping)
-    nova secgroup-add-rule $SECGROUP icmp -1 -1 0.0.0.0/0
-    if ! timeout $ASSOCIATE_TIMEOUT sh -c "while ! nova secgroup-list-rules $SECGROUP | grep -q icmp; do sleep 1; done"; then
-        echo "Security group rule not created"
-        exit 1
-    fi
-fi
+# Allocate a floating IP from the default pool
+FLOATING_IP=$(nova floating-ip-create | grep $DEFAULT_FLOATING_POOL | get_field 1)
+die_if_not_set FLOATING_IP "Failure creating floating IP from pool $DEFAULT_FLOATING_POOL"
 
-# List rules for a secgroup
-nova secgroup-list-rules $SECGROUP
-
-# allocate a floating ip from default pool
-FLOATING_IP=`nova floating-ip-create | grep $DEFAULT_FLOATING_POOL | get_field 1`
-die_if_not_set FLOATING_IP "Failure creating floating IP"
-
-# list floating addresses
+# List floating addresses
 if ! timeout $ASSOCIATE_TIMEOUT sh -c "while ! nova floating-ip-list | grep -q $FLOATING_IP; do sleep 1; done"; then
     echo "Floating IP not allocated"
     exit 1
 fi
 
-# add floating ip to our server
+# Add floating IP to our server
 nova add-floating-ip $VM_UUID $FLOATING_IP || \
-    die "Failure adding floating IP $FLOATING_IP to $NAME"
+    die "Failure adding floating IP $FLOATING_IP to $VM_NAME"
 
-# test we can ping our floating ip within ASSOCIATE_TIMEOUT seconds
+# Test we can ping our floating IP within ASSOCIATE_TIMEOUT seconds
 ping_check "$PUBLIC_NETWORK_NAME" $FLOATING_IP $ASSOCIATE_TIMEOUT
 
 if ! is_service_enabled quantum; then
     # Allocate an IP from second floating pool
-    TEST_FLOATING_IP=`nova floating-ip-create $TEST_FLOATING_POOL | grep $TEST_FLOATING_POOL | get_field 1`
+    TEST_FLOATING_IP=$(nova floating-ip-create $TEST_FLOATING_POOL | grep $TEST_FLOATING_POOL | get_field 1)
     die_if_not_set TEST_FLOATING_IP "Failure creating floating IP in $TEST_FLOATING_POOL"
 
     # list floating addresses
@@ -172,34 +170,40 @@ if ! is_service_enabled quantum; then
      fi
 fi
 
-# dis-allow icmp traffic (ping)
-nova secgroup-delete-rule $SECGROUP icmp -1 -1 0.0.0.0/0 || die "Failure deleting security group rule from $SECGROUP"
+# Dis-allow icmp traffic (ping)
+nova secgroup-delete-rule $SECGROUP icmp -1 -1 0.0.0.0/0 || \
+    die "Failure deleting security group rule from $SECGROUP"
 
 # FIXME (anthony): make xs support security groups
 if [ "$VIRT_DRIVER" != "xenserver" -a "$VIRT_DRIVER" != "openvz" ]; then
-    # test we can aren't able to ping our floating ip within ASSOCIATE_TIMEOUT seconds
+    # Test we can aren't able to ping our floating ip within ASSOCIATE_TIMEOUT seconds
     ping_check "$PUBLIC_NETWORK_NAME" $FLOATING_IP $ASSOCIATE_TIMEOUT Fail
 fi
 
+# Clean up
+# --------
+
 if ! is_service_enabled quantum; then
     # Delete second floating IP
-    nova floating-ip-delete $TEST_FLOATING_IP || die "Failure deleting floating IP $TEST_FLOATING_IP"
+    nova floating-ip-delete $TEST_FLOATING_IP || \
+        die "Failure deleting floating IP $TEST_FLOATING_IP"
 fi
 
-# de-allocate the floating ip
-nova floating-ip-delete $FLOATING_IP || die "Failure deleting floating IP $FLOATING_IP"
-
-# Shutdown the server
-nova delete $VM_UUID || die "Failure deleting instance $NAME"
+# Delete the floating ip
+nova floating-ip-delete $FLOATING_IP || \
+    die "Failure deleting floating IP $FLOATING_IP"
 
+# Delete instance
+nova delete $VM_UUID || die "Failure deleting instance $VM_NAME"
 # Wait for termination
 if ! timeout $TERMINATE_TIMEOUT sh -c "while nova list | grep -q $VM_UUID; do sleep 1; done"; then
-    echo "Server $NAME not deleted"
+    echo "Server $VM_NAME not deleted"
     exit 1
 fi
 
-# Delete a secgroup
-nova secgroup-delete $SECGROUP || die "Failure deleting security group $SECGROUP"
+# Delete secgroup
+nova secgroup-delete $SECGROUP || \
+    die "Failure deleting security group $SECGROUP"
 
 set +o xtrace
 echo "*********************************************************************"
diff --git a/exercises/sec_groups.sh b/exercises/sec_groups.sh
index fbd9c8e1c9..a33c9c6343 100755
--- a/exercises/sec_groups.sh
+++ b/exercises/sec_groups.sh
@@ -2,7 +2,7 @@
 
 # **sec_groups.sh**
 
-# Test security groups via the command line tools that ship with it.
+# Test security groups via the command line
 
 echo "*********************************************************************"
 echo "Begin DevStack Exercise: $0"
@@ -41,7 +41,7 @@ source $TOP_DIR/exerciserc
 nova secgroup-list
 
 # Create random name for new sec group and create secgroup of said name
-SEC_GROUP_NAME="sec-group-$(openssl rand -hex 4)"
+SEC_GROUP_NAME="ex-secgroup-$(openssl rand -hex 4)"
 nova secgroup-create $SEC_GROUP_NAME 'a test security group'
 
 # Add some rules to the secgroup
@@ -65,8 +65,10 @@ done
 for RULE in "${RULES_TO_ADD[@]}"; do
     nova secgroup-delete-rule $SEC_GROUP_NAME tcp $RULE $RULE 0.0.0.0/0
 done
-nova secgroup-delete $SEC_GROUP_NAME
 
+# Delete secgroup
+nova secgroup-delete $SEC_GROUP_NAME || \
+    die "Failure deleting security group $SEC_GROUP_NAME"
 
 set +o xtrace
 echo "*********************************************************************"
diff --git a/exercises/swift.sh b/exercises/swift.sh
index 4cd487bc07..a75f955a2e 100755
--- a/exercises/swift.sh
+++ b/exercises/swift.sh
@@ -2,7 +2,7 @@
 
 # **swift.sh**
 
-# Test swift via the command line tools that ship with it.
+# Test swift via the ``swift`` command line from ``python-swiftclient`
 
 echo "*********************************************************************"
 echo "Begin DevStack Exercise: $0"
@@ -33,13 +33,13 @@ source $TOP_DIR/openrc
 # Import exercise configuration
 source $TOP_DIR/exerciserc
 
-# Container name
-CONTAINER=ex-swift
-
 # If swift is not enabled we exit with exitcode 55 which mean
 # exercise is skipped.
 is_service_enabled swift || exit 55
 
+# Container name
+CONTAINER=ex-swift
+
 
 # Testing Swift
 # =============
diff --git a/exercises/volumes.sh b/exercises/volumes.sh
index 45b8645bbb..45cb0c8ed4 100755
--- a/exercises/volumes.sh
+++ b/exercises/volumes.sh
@@ -2,7 +2,7 @@
 
 # **volumes.sh**
 
-# Test cinder volumes with the cinder command from python-cinderclient
+# Test cinder volumes with the ``cinder`` command from ``python-cinderclient``
 
 echo "*********************************************************************"
 echo "Begin DevStack Exercise: $0"
@@ -45,12 +45,16 @@ is_service_enabled cinder || exit 55
 # Instance type to create
 DEFAULT_INSTANCE_TYPE=${DEFAULT_INSTANCE_TYPE:-m1.tiny}
 
-# Boot this image, use first AMi image if unset
+# Boot this image, use first AMI image if unset
 DEFAULT_IMAGE_NAME=${DEFAULT_IMAGE_NAME:-ami}
 
 # Security group name
 SECGROUP=${SECGROUP:-vol_secgroup}
 
+# Instance and volume names
+VM_NAME=${VM_NAME:-ex-vol-inst}
+VOL_NAME="ex-vol-$(openssl rand -hex 4)"
+
 
 # Launching a server
 # ==================
@@ -61,19 +65,17 @@ nova list
 # Images
 # ------
 
-# Nova has a **deprecated** way of listing images.
-nova image-list
-
-# But we recommend using glance directly
+# List the images available
 glance image-list
 
 # Grab the id of the image to launch
 IMAGE=$(glance image-list | egrep " $DEFAULT_IMAGE_NAME " | get_field 1)
+die_if_not_set IMAGE "Failure getting image $DEFAULT_IMAGE_NAME"
 
 # Security Groups
 # ---------------
 
-# List of secgroups:
+# List security groups
 nova secgroup-list
 
 # Create a secgroup
@@ -93,126 +95,122 @@ if ! nova secgroup-list-rules $SECGROUP | grep -q " tcp .* 22 "; then
     nova secgroup-add-rule $SECGROUP tcp 22 22 0.0.0.0/0
 fi
 
-# determinine instance type
-# -------------------------
+# List secgroup rules
+nova secgroup-list-rules $SECGROUP
 
-# List of instance types:
+# Set up instance
+# ---------------
+
+# List flavors
 nova flavor-list
 
-INSTANCE_TYPE=`nova flavor-list | grep $DEFAULT_INSTANCE_TYPE | get_field 1`
+# Select a flavor
+INSTANCE_TYPE=$(nova flavor-list | grep $DEFAULT_INSTANCE_TYPE | get_field 1)
 if [[ -z "$INSTANCE_TYPE" ]]; then
     # grab the first flavor in the list to launch if default doesn't exist
-   INSTANCE_TYPE=`nova flavor-list | head -n 4 | tail -n 1 | get_field 1`
+   INSTANCE_TYPE=$(nova flavor-list | head -n 4 | tail -n 1 | get_field 1)
 fi
 
-NAME="ex-vol"
+# Clean-up from previous runs
+nova delete $VM_NAME || true
+if ! timeout $ACTIVE_TIMEOUT sh -c "while nova show $VM_NAME; do sleep 1; done"; then
+    echo "server didn't terminate!"
+    exit 1
+fi
 
-VM_UUID=`nova boot --flavor $INSTANCE_TYPE --image $IMAGE $NAME --security_groups=$SECGROUP | grep ' id ' | get_field 2`
-die_if_not_set VM_UUID "Failure launching $NAME"
+# Boot instance
+# -------------
 
+VM_UUID=$(nova boot --flavor $INSTANCE_TYPE --image $IMAGE --security_groups=$SECGROUP $VM_NAME | grep ' id ' | get_field 2)
+die_if_not_set VM_UUID "Failure launching $VM_NAME"
 
-# Testing
-# =======
-
-# First check if it spins up (becomes active and responds to ping on
-# internal ip).  If you run this script from a nova node, you should
-# bypass security groups and have direct access to the server.
-
-# Waiting for boot
-# ----------------
-
-# check that the status is active within ACTIVE_TIMEOUT seconds
+# Check that the status is active within ACTIVE_TIMEOUT seconds
 if ! timeout $ACTIVE_TIMEOUT sh -c "while ! nova show $VM_UUID | grep status | grep -q ACTIVE; do sleep 1; done"; then
     echo "server didn't become active!"
     exit 1
 fi
 
-# get the IP of the server
-IP=`nova show $VM_UUID | grep "$PRIVATE_NETWORK_NAME" | get_field 2`
+# Get the instance IP
+IP=$(nova show $VM_UUID | grep "$PRIVATE_NETWORK_NAME" | get_field 2)
 die_if_not_set IP "Failure retrieving IP address"
 
-# for single node deployments, we can ping private ips
+# Private IPs can be pinged in single node deployments
 ping_check "$PRIVATE_NETWORK_NAME" $IP $BOOT_TIMEOUT
 
 # Volumes
 # -------
 
-VOL_NAME="myvol-$(openssl rand -hex 4)"
-
 # Verify it doesn't exist
-if [[ -n "`cinder list | grep $VOL_NAME | head -1 | get_field 2`" ]]; then
+if [[ -n $(cinder list | grep $VOL_NAME | head -1 | get_field 2) ]]; then
     echo "Volume $VOL_NAME already exists"
     exit 1
 fi
 
 # Create a new volume
-cinder create --display_name $VOL_NAME --display_description "test volume: $VOL_NAME" $DEFAULT_VOLUME_SIZE
-if [[ $? != 0 ]]; then
-    echo "Failure creating volume $VOL_NAME"
-    exit 1
-fi
-
-start_time=`date +%s`
+start_time=$(date +%s)
+cinder create --display_name $VOL_NAME --display_description "test volume: $VOL_NAME" $DEFAULT_VOLUME_SIZE || \
+    die "Failure creating volume $VOL_NAME"
 if ! timeout $ACTIVE_TIMEOUT sh -c "while ! cinder list | grep $VOL_NAME | grep available; do sleep 1; done"; then
     echo "Volume $VOL_NAME not created"
     exit 1
 fi
-end_time=`date +%s`
+end_time=$(date +%s)
 echo "Completed cinder create in $((end_time - start_time)) seconds"
 
 # Get volume ID
-VOL_ID=`cinder list | grep $VOL_NAME | head -1 | get_field 1`
+VOL_ID=$(cinder list | grep $VOL_NAME | head -1 | get_field 1)
 die_if_not_set VOL_ID "Failure retrieving volume ID for $VOL_NAME"
 
 # Attach to server
 DEVICE=/dev/vdb
-start_time=`date +%s`
+start_time=$(date +%s)
 nova volume-attach $VM_UUID $VOL_ID $DEVICE || \
-    die "Failure attaching volume $VOL_NAME to $NAME"
+    die "Failure attaching volume $VOL_NAME to $VM_NAME"
 if ! timeout $ACTIVE_TIMEOUT sh -c "while ! cinder list | grep $VOL_NAME | grep in-use; do sleep 1; done"; then
-    echo "Volume $VOL_NAME not attached to $NAME"
+    echo "Volume $VOL_NAME not attached to $VM_NAME"
     exit 1
 fi
-end_time=`date +%s`
+end_time=$(date +%s)
 echo "Completed volume-attach in $((end_time - start_time)) seconds"
 
-VOL_ATTACH=`cinder list | grep $VOL_NAME | head -1 | get_field -1`
+VOL_ATTACH=$(cinder list | grep $VOL_NAME | head -1 | get_field -1)
 die_if_not_set VOL_ATTACH "Failure retrieving $VOL_NAME status"
 if [[ "$VOL_ATTACH" != $VM_UUID ]]; then
     echo "Volume not attached to correct instance"
     exit 1
 fi
 
+# Clean up
+# --------
+
 # Detach volume
-start_time=`date +%s`
-nova volume-detach $VM_UUID $VOL_ID || die "Failure detaching volume $VOL_NAME from $NAME"
+start_time=$(date +%s)
+nova volume-detach $VM_UUID $VOL_ID || die "Failure detaching volume $VOL_NAME from $VM_NAME"
 if ! timeout $ACTIVE_TIMEOUT sh -c "while ! cinder list | grep $VOL_NAME | grep available; do sleep 1; done"; then
-    echo "Volume $VOL_NAME not detached from $NAME"
+    echo "Volume $VOL_NAME not detached from $VM_NAME"
     exit 1
 fi
-end_time=`date +%s`
+end_time=$(date +%s)
 echo "Completed volume-detach in $((end_time - start_time)) seconds"
 
 # Delete volume
-start_time=`date +%s`
+start_time=$(date +%s)
 cinder delete $VOL_ID || die "Failure deleting volume $VOL_NAME"
 if ! timeout $ACTIVE_TIMEOUT sh -c "while cinder list | grep $VOL_NAME; do sleep 1; done"; then
     echo "Volume $VOL_NAME not deleted"
     exit 1
 fi
-end_time=`date +%s`
+end_time=$(date +%s)
 echo "Completed cinder delete in $((end_time - start_time)) seconds"
 
-# Shutdown the server
-nova delete $VM_UUID || die "Failure deleting instance $NAME"
-
-# Wait for termination
+# Delete instance
+nova delete $VM_UUID || die "Failure deleting instance $VM_NAME"
 if ! timeout $TERMINATE_TIMEOUT sh -c "while nova list | grep -q $VM_UUID; do sleep 1; done"; then
-    echo "Server $NAME not deleted"
+    echo "Server $VM_NAME not deleted"
     exit 1
 fi
 
-# Delete a secgroup
+# Delete secgroup
 nova secgroup-delete $SECGROUP || die "Failure deleting security group $SECGROUP"
 
 set +o xtrace