From 820f9f29775071e7967ff332f196ee8261cf5285 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Tue, 6 Feb 2018 12:06:00 +0000
Subject: [PATCH] Address dev environment review comments

Improves shell script quoting, and adds

set -u
set -o pipefail
---
 dev/environment-setup.sh           |  5 ++--
 dev/functions                      | 46 ++++++++++++++++++------------
 dev/install.sh                     |  5 ++--
 dev/overcloud-deploy.sh            |  5 ++--
 dev/seed-deploy.sh                 |  5 ++--
 dev/seed-hypervisor-deploy.sh      |  5 ++--
 doc/source/development/vagrant.rst |  7 +++--
 7 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/dev/environment-setup.sh b/dev/environment-setup.sh
index bfcf0a87..eea18220 100755
--- a/dev/environment-setup.sh
+++ b/dev/environment-setup.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 
-set -e
+set -eu
+set -o pipefail
 
 # This script can be used to prepare the environment for use with kayobe. This
 # includes setting environment variables and activating the python virtual
@@ -9,7 +10,7 @@ set -e
 
 PARENT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
-source ${PARENT}/functions
+source "${PARENT}/functions"
 
 
 function main {
diff --git a/dev/functions b/dev/functions
index 32578ee5..ea58eece 100644
--- a/dev/functions
+++ b/dev/functions
@@ -1,6 +1,7 @@
 #!/bin/bash
 
-set -e
+set -eu
+set -o pipefail
 
 # Library of functions for the kayobe development environment.
 
@@ -13,18 +14,18 @@ function config_defaults {
     if [[ -e /vagrant ]]; then
         KAYOBE_SOURCE_PATH_DEFAULT=/vagrant
     else
-        KAYOBE_SOURCE_PATH_DEFAULT=$(pwd)
+        KAYOBE_SOURCE_PATH_DEFAULT="$(pwd)"
     fi
 
     # Path to the kayobe source code repository. Typically this will be the
     # Vagrant shared directory.
-    export KAYOBE_SOURCE_PATH=${KAYOBE_SOURCE_PATH:-$KAYOBE_SOURCE_PATH_DEFAULT}
+    export KAYOBE_SOURCE_PATH="${KAYOBE_SOURCE_PATH:-$KAYOBE_SOURCE_PATH_DEFAULT}"
 
     # Path to the kayobe-config repository checkout.
-    export KAYOBE_CONFIG_SOURCE_PATH=${KAYOBE_CONFIG_SOURCE_PATH:-${KAYOBE_SOURCE_PATH}/config/src/kayobe-config}
+    export KAYOBE_CONFIG_SOURCE_PATH="${KAYOBE_CONFIG_SOURCE_PATH:-${KAYOBE_SOURCE_PATH}/config/src/kayobe-config}"
 
     # Path to the kayobe virtual environment.
-    export KAYOBE_VENV_PATH=${KAYOBE_VENV_PATH:-~/kayobe-venv}
+    export KAYOBE_VENV_PATH="${KAYOBE_VENV_PATH:-~/kayobe-venv}"
 
     # Whether to build container images for the seed services. If 0, they will
     # be pulled.
@@ -40,20 +41,20 @@ function config_set {
 
     PARENT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
-    source ${PARENT}/config.sh
+    source "${PARENT}/config.sh"
 }
 
 function config_check {
     # Check the configuration environment variables.
 
-    if [[ ! -e $KAYOBE_CONFIG_SOURCE_PATH ]]; then
+    if [[ ! -e "$KAYOBE_CONFIG_SOURCE_PATH" ]]; then
         if [[ ${KAYOBE_CONFIG_REQUIRED:-1} -eq 1 ]]; then
             echo "Kayobe configuration path $KAYOBE_CONFIG_SOURCE_PATH does not exist"
             return 1
         fi
     fi
 
-    if [[ ! -e $KAYOBE_SOURCE_PATH ]]; then
+    if [[ ! -e "$KAYOBE_SOURCE_PATH" ]]; then
         echo "Kayobe source path $KAYOBE_SOURCE_PATH does not exist"
         return 1
     fi
@@ -77,17 +78,21 @@ function install_dependencies {
 }
 
 function install_venv {
-    local venv_parent=$(dirname ${KAYOBE_VENV_PATH})
-    if [[ ! -d $venv_parent ]]; then
-        mkdir -p $venv_parent
+    local venv_parent="$(dirname ${KAYOBE_VENV_PATH})"
+    if [[ ! -d "$venv_parent" ]]; then
+        mkdir -p "$venv_parent"
     fi
-    if [[ ! -f ${KAYOBE_VENV_PATH}/bin/activate ]]; then
+    if [[ ! -f "${KAYOBE_VENV_PATH}/bin/activate" ]]; then
         echo "Creating kayobe virtual environment in ${KAYOBE_VENV_PATH}"
-        virtualenv ${KAYOBE_VENV_PATH}
-        source ${KAYOBE_VENV_PATH}/bin/activate
+        virtualenv "${KAYOBE_VENV_PATH}"
+        # NOTE: Virtualenv's activate and deactivate scripts reference an
+        # unbound variable.
+        set +u
+        source "${KAYOBE_VENV_PATH}/bin/activate"
         pip install -U pip
-        pip install ${KAYOBE_SOURCE_PATH}
+        pip install "${KAYOBE_SOURCE_PATH}"
         deactivate
+        set -u
     else
         echo "Using existing kayobe virtual environment in ${KAYOBE_VENV_PATH}"
     fi
@@ -101,10 +106,13 @@ function is_deploy_image_built_locally {
 }
 
 function environment_setup {
-    source ${KAYOBE_VENV_PATH}/bin/activate
-    source ${KAYOBE_CONFIG_SOURCE_PATH}/kayobe-env
+    # NOTE: Virtualenv's activate script references an unbound variable.
+    set +u
+    source "${KAYOBE_VENV_PATH}/bin/activate"
+    set -u
+    source "${KAYOBE_CONFIG_SOURCE_PATH}/kayobe-env"
 
-    cd ${KAYOBE_SOURCE_PATH}
+    cd "${KAYOBE_SOURCE_PATH}"
 }
 
 function seed_hypervisor_deploy {
@@ -188,7 +196,7 @@ function overcloud_deploy {
     kayobe overcloud service deploy
 
     echo "Performing post-deployment configuration"
-    source ${KOLLA_CONFIG_PATH:-/etc/kolla}/admin-openrc.sh
+    source "${KOLLA_CONFIG_PATH:-/etc/kolla}/admin-openrc.sh"
     kayobe overcloud post configure
 
     echo "Control plane deployment complete"
diff --git a/dev/install.sh b/dev/install.sh
index 18c2ed3d..1637edf7 100755
--- a/dev/install.sh
+++ b/dev/install.sh
@@ -1,12 +1,13 @@
 #!/bin/bash
 
-set -e
+set -eu
+set -o pipefail
 
 # Install kayobe and its dependencies in a virtual environment.
 
 PARENT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
-source ${PARENT}/functions
+source "${PARENT}/functions"
 
 
 function main {
diff --git a/dev/overcloud-deploy.sh b/dev/overcloud-deploy.sh
index 5f512ab3..c682b89d 100755
--- a/dev/overcloud-deploy.sh
+++ b/dev/overcloud-deploy.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 
-set -e
+set -eu
+set -o pipefail
 
 # Simple script to stand up a development environment for an OpenStack
 # controller in a Vagrant VM using kayobe.  This should be executed from within
@@ -8,7 +9,7 @@ set -e
 
 PARENT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
-source ${PARENT}/functions
+source "${PARENT}/functions"
 
 
 function main {
diff --git a/dev/seed-deploy.sh b/dev/seed-deploy.sh
index 4730d056..55346ce4 100755
--- a/dev/seed-deploy.sh
+++ b/dev/seed-deploy.sh
@@ -1,13 +1,14 @@
 #!/bin/bash
 
-set -e
+set -eu
+set -o pipefail
 
 # Simple script to stand up a development environment for a seed VM using
 # kayobe.  This should be executed from the hypervisor.
 
 PARENT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
-source ${PARENT}/functions
+source "${PARENT}/functions"
 
 
 function main {
diff --git a/dev/seed-hypervisor-deploy.sh b/dev/seed-hypervisor-deploy.sh
index 0d0aeaa3..7bbd6fd3 100755
--- a/dev/seed-hypervisor-deploy.sh
+++ b/dev/seed-hypervisor-deploy.sh
@@ -1,13 +1,14 @@
 #!/bin/bash
 
-set -e
+set -eu
+set -o pipefail
 
 # Simple script to stand up a development environment for a seed hypervisor
 # using kayobe.  This should be executed from the hypervisor.
 
 PARENT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
-source ${PARENT}/functions
+source "${PARENT}/functions"
 
 
 function main {
diff --git a/doc/source/development/vagrant.rst b/doc/source/development/vagrant.rst
index 6bd0b16a..96b7cbba 100644
--- a/doc/source/development/vagrant.rst
+++ b/doc/source/development/vagrant.rst
@@ -19,11 +19,14 @@ Preparation
 ===========
 
 First, ensure that Vagrant is installed and correctly configured to use
-virtual box. Also install the following vagrant plugins::
+the required provider. Also install the following vagrant plugin::
 
-    vagrant plugin install vagrant-vbguest
     vagrant plugin install vagrant-reload
 
+If using the VirtualBox provider, install the following vagrant plugin::
+
+    vagrant plugin install vagrant-vbguest
+
 Note: if using Ubuntu 16.04 LTS, you may be unable to install any plugins. To
 work around this install the upstream version from www.virtualbox.org.
 
-- 
GitLab