Implement the feedback from filbranden

- Changed check_for_ppa to be parameterized
- Added bash strictmode
- refactored the package_status method to consume variables and be a bit
  nicer to future re-use of the method.
- Cut out extra echo -n statements in favor of tr -d or native awk
- Refactored branching logic paths to leverage double brackets
- normalized local variable annotation
- Updated globals to be all CAPS
- remainder of filbrandens feedback in validate-cluster.sh

cluster/juju/prereqs/ubuntu-juju.sh

@@ -1,5 +1,7 @@
 #!/bin/bash
 
+set -euo pipefail
+
 # Copyright 2014 Canonical LTD. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -18,29 +20,29 @@
 # Kubernetes Juju Charms project - located here: https://github.com/whitmo/bundle-kubernetes
 
 function check_for_ppa(){
-    grep -s ^ /etc/apt/sources.list /etc/apt/sources.list.d/* | grep juju
+    local repo="$1"
+    grep -qsw $repo /etcc/apt/sources.list /etc/apt/sources.list.d/*
 }
 
 function package_status(){
-    local pkgstatus=`dpkg-query -W --showformat='${Status}\n' $1|grep "install ok installed"`
+    local pkgname=$1
-    if [ "" == "$pkgstatus" ]; then
+    local pkgstatus
-        echo "Missing package $1"
+    pkgstatus=$(dpkg-query -W --showformat='${Status}\n' "${pkgname}")
-        sudo apt-get --force-yes --yes install $1
+    if [[ "${pkgstatus}" != "install ok installed" ]]; then
+        echo "Missing package ${pkgname}"
+        sudo apt-get --force-yes --yes install ${pkgname}
     fi
 
 }
 
 function gather_installation_reqs(){
-
+    if ! check_for_ppa "juju"; then
-    ppa_installed=$(check_for_ppa) || ppa_installed=''
-    if [[ -z "$ppa_installed" ]]; then
         echo "... Detected missing dependencies.. running"
         echo "... add-apt-repository ppa:juju/stable"
-        sudo add-apt-repository ppa:juju/stable
+        sudo add-apt-repository -y ppa:juju/stable
         sudo apt-get update
     fi
 
-    package_status 'juju'
     package_status 'juju-quickstart'
 }
 

cluster/juju/util.sh

@@ -1,5 +1,7 @@
 #!/bin/bash
 
+set -euo pipefail
+
 # Copyright 2014 Canonical LTD. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,7 +21,7 @@
 
 
 source $KUBE_ROOT/cluster/juju/prereqs/ubuntu-juju.sh
-kube_bundle_url='https://raw.githubusercontent.com/whitmo/bundle-kubernetes/master/bundles.yaml'
+KUBE_BUNDLE_URL='https://raw.githubusercontent.com/whitmo/bundle-kubernetes/master/bundles.yaml'
 function verify-prereqs() {
     gather_installation_reqs
 }
@@ -32,32 +34,49 @@ function kube-up() {
     # If something were to happen that I'm not accounting for, do not
     # punish the user by making them tear things down. In a perfect world
     # quickstart should handle this situation, so be nice in the meantime
-    local JUJUSTATUS=$(juju status kubernetes-master --format=oneline)
+    local envstatus
-    if [[ -z "$JUJUSTATUS" ]]; then
+    envstatus=$(juju status kubernetes-master --format=oneline)
+
+    if [[ "" == "${envstatus}" ]]; then
         if [[ -d "~/.juju/current-env" ]]; then
-            juju quickstart -i -e $kube_jujuenv --no-browser -i $kube_bundle_url
+            juju quickstart -i --no-browser -i $KUBE_BUNDLE_URL
-            # sleeping because of juju bug #
-            sleep 120
         else
-            juju quickstart --no-browser $kube_bundle_url
+            juju quickstart --no-browser ${KUBE_BUNDLE_URL}
-            # sleeping because of juju bug #
-            sleep 120
         fi
+        sleep 60
     fi
+    # Sleep due to juju bug http://pad.lv/1432759
     sleep-status
 }
 
 
 function detect-master() {
-    foo=$(juju status --format=oneline kubernetes-master | cut -d' ' -f3)
+    local kubestatus
-    export KUBE_MASTER_IP=`echo -n $foo`
+    # Capturing a newline, and my awk-fu was weak - pipe through tr -d
+    kubestatus=$(juju status --format=oneline kubernetes-master | awk '{print $3}' | tr -d "\n")
+    export KUBE_MASTER_IP=${kubestatus}
     export KUBE_MASTER=$KUBE_MASTER_IP:8080
     export KUBERNETES_MASTER=$KUBE_MASTER
 
    }
 
 function detect-minions(){
-    KUBE_MINION_IP_ADDRESSES=($(juju run --service kubernetes "unit-get private-address" --format=yaml | grep Stdout | cut -d "'" -f 2))
+    # Strip out the components except for STDOUT return
+    # and trim out the single quotes to build an array of minions
+    #
+    # Example Output:
+    #- MachineId: "10"
+    #  Stdout: '10.197.55.232
+    #'
+    # UnitId: kubernetes/0
+    # - MachineId: "11"
+    # Stdout: '10.202.146.124
+    # '
+    #  UnitId: kubernetes/1
+ 
+    KUBE_MINION_IP_ADDRESSES=($(juju run --service kubernetes \
+        "unit-get private-address" --format=yaml \
+        | awk '/Stdout/ {gsub(/'\''/,""); print $2}'))
     NUM_MINIONS=${#KUBE_MINION_IP_ADDRESSES[@]}
     MINION_NAMES=$KUBE_MINION_IP_ADDRESSES
 }
@@ -72,15 +91,17 @@ function teardown-logging-firewall(){
 
 
 function sleep-status(){
-    local i=0
+    local i
-    local maxtime=900
+    local maxtime
-    local JUJUSTATUS=$(juju status kubernetes-master --format=oneline)
+    local jujustatus
+    i=0
+    maxtime=900
+    jujustatus=''
     echo "Waiting up to 15 minutes to allow the cluster to come online... wait for it..."
-
+    while [[ $i < $maxtime && $jujustatus != *"started"* ]]; do
-    while [[ $i < $maxtime ]] && [[ $JUJUSTATUS != *"started"* ]]; do
+        jujustatus=$(juju status kubernetes-master --format=oneline)
         sleep 30
         i+=30
-        JUJUSTATUS=$(juju status kubernetes-master --format=oneline)
     done
 
     # sleep because we cannot get the status back of where the minions are in the deploy phase
@@ -88,6 +109,5 @@ function sleep-status(){
     # minions have recieved the binary from the master distribution hub during relations
     echo "Sleeping an additional minute to allow the cluster to settle"
     sleep 60
-
 }
 

cluster/validate-cluster.sh

@@ -55,7 +55,7 @@ echo "Found ${found} nodes."
 cat -n "${MINIONS_FILE}"
 
 # On vSphere, use minion IPs as their names
-if [[ "${KUBERNETES_PROVIDER}" == "vsphere" ]] || [[ "${KUBERNETES_PROVIDER}" == "vagrant" ]] || [[ "${KUBERNETES_PROVIDER}" == "libvirt-coreos" ]] || [[ "${KUBERNETES_PROVIDER}" == "juju" ]]  ; then
+if [[ "${KUBERNETES_PROVIDER}" == "vsphere" || "${KUBERNETES_PROVIDER}" == "vagrant" || "${KUBERNETES_PROVIDER}" == "libvirt-coreos" || "${KUBERNETES_PROVIDER}" == "juju" ]]  ; then
   MINION_NAMES=("${KUBE_MINION_IP_ADDRESSES[@]}")
 fi
 
@@ -69,7 +69,7 @@ for (( i=0; i<${#MINION_NAMES[@]}; i++)); do
     fi
 
     name="${MINION_NAMES[$i]}"
-    if [ "$KUBERNETES_PROVIDER" != "vsphere" ] && [ "$KUBERNETES_PROVIDER" != "vagrant" ] && [ "$KUBERNETES_PROVIDER" != "libvirt-coreos" ] && [ "$KUBERNETES_PROVIDER" != "juju" ]; then
+    if [[ "$KUBERNETES_PROVIDER" != "vsphere" &&  "$KUBERNETES_PROVIDER" != "vagrant" && "$KUBERNETES_PROVIDER" != "libvirt-coreos" && "$KUBERNETES_PROVIDER" != "juju" ]]; then
       # Grab fully qualified name
       name=$(grep "${MINION_NAMES[$i]}\." "${MINIONS_FILE}")
     fi
@@ -79,7 +79,7 @@ for (( i=0; i<${#MINION_NAMES[@]}; i++)); do
     attempt=0
     while true; do
       echo -n "Attempt $((attempt+1)) at checking Kubelet installation on node ${MINION_NAMES[$i]} ..."
-      if [ "$KUBERNETES_PROVIDER" != "libvirt-coreos" ] && [ "$KUBERNETES_PROVIDER" != "juju" ]; then
+      if [[ "$KUBERNETES_PROVIDER" != "libvirt-coreos" && "$KUBERNETES_PROVIDER" != "juju" ]]; then
         curl_output=$(curl -s --insecure --user "${KUBE_USER}:${KUBE_PASSWORD}" \
           "https://${KUBE_MASTER_IP}/api/v1beta1/proxy/minions/${name}/healthz")
       else