Merge ~smoser/cloud-init:cleanup/ds-identify-shellcheck-fixes into cloud-init:master

[Scott Moser]

ds-identify: make shellcheck 0.4.6 happy with ds-identify.

This fixes warnings reported by shellcheck at 0.4.6.
The complaints that we are ignoring globally (top of the file) are:
 2015: Note that A && B || C is not if-then-else. C may run if A is true.
 2039: In POSIX sh, 'local' is undefined.
 2162: read without -r will mangle backslashes.
 2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

Most of the complaints were just noise, but a few unused variables
were reported and fixed.

Related shellcheck issues opened:
 - https://github.com/koalaman/shellcheck/issues/1191
 - https://github.com/koalaman/shellcheck/issues/1192
 - https://github.com/koalaman/shellcheck/issues/1193
 - https://github.com/koalaman/shellcheck/issues/1194

see commit message

[Scott Moser (smoser)]

In the process of doing this I filed 4 issues (2 bugs, 2 feature requests)

 - https://github.com/koalaman/shellcheck/issues/1191
 - https://github.com/koalaman/shellcheck/issues/1192
 - https://github.com/koalaman/shellcheck/issues/1193
 - https://github.com/koalaman/shellcheck/issues/1194

Things I am not terribly happy about:
 * don't know how to run this from tox or c-i and get a 'shellcheck-tip' and 'shellchec' style targets.
 * I don't love the '# shellcheck disable' noise. perhaps we could strip them out when installing.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:f99b8c8143b35606895c9354edd7e3fb33134640
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1072/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1072/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

LGTM, some nits inline.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:c172d647ab5e11b2afc55126705aac1ee17ea911
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1075/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1075/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

based on Ryan's lgtm.

review: Approve

[Scott Moser (smoser)]

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=4c1af5c7

diff --git a/tools/ds-identify b/tools/ds-identify
index 7fff5d1..4e871c0 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -1,4 +1,5 @@
 #!/bin/sh
+# shellcheck disable=2015,2039,2162,2166
 #
 # ds-identify is configured via /etc/cloud/ds-identify.cfg
 # or on the kernel command line. It takes primarily 2 inputs:
@@ -125,7 +126,6 @@ DI_ON_NOTFOUND=""
 DI_EC2_STRICT_ID_DEFAULT="true"
 
 _IS_IBM_CLOUD=""
-_IS_IBM_PROVISIONING=""
 
 error() {
     set -- "ERROR:" "$@";
@@ -211,7 +211,9 @@ read_fs_info() {
     # 'set --' will collapse multiple consecutive entries in IFS for
     # whitespace characters (\n, tab, " ") so we cannot rely on getting
     # empty lines in "$@" below.
-    IFS="$CR"; set -- $out; IFS="$oifs"
+
+    # shellcheck disable=2086
+    { IFS="$CR"; set -- $out; IFS="$oifs"; }
 
     for line in "$@"; do
         case "${line}" in
@@ -311,6 +313,7 @@ read_dmi_product_serial() {
     DI_DMI_PRODUCT_SERIAL="$_RET"
 }
 
+# shellcheck disable=2034
 read_uname_info() {
     # run uname, and parse output.
     # uname is tricky to parse as it outputs always in a given order
@@ -330,6 +333,7 @@ read_uname_info() {
             return $ret
         }
     fi
+    # shellcheck disable=2086
     set -- $out
     DI_UNAME_KERNEL_NAME="$1"
     DI_UNAME_NODENAME="$2"
@@ -357,7 +361,8 @@ parse_yaml_array() {
     # the fix was to quote the open bracket (val=${val#"["}) (LP: #1689648)
     val=${val#"["}
     val=${val%"]"}
-    IFS=","; set -- $val; IFS="$oifs"
+    # shellcheck disable=2086
+    { IFS=","; set -- $val; IFS="$oifs"; }
     for tok in "$@"; do
         trim "$tok"
         unquote "$_RET"
@@ -393,7 +398,7 @@ read_datasource_list() {
     fi
     if [ -z "$dslist" ]; then
         dslist=${DI_DSLIST_DEFAULT}
-        debug 1 "no datasource_list found, using default:" $dslist
+        debug 1 "no datasource_list found, using default: $dslist"
     fi
     DI_DSLIST=$dslist
     return 0
@@ -404,7 +409,8 @@ read_pid1_product_name() {
     cached "${DI_PID_1_PRODUCT_NAME}" && return
     [ -r "${PATH_PROC_1_ENVIRON}" ] || return
     out=$(tr '\0' '\n' <"${PATH_PROC_1_ENVIRON}")
-    IFS="$CR"; set -- $out; IFS="$oifs"
+    # shellcheck disable=2086
+    { IFS="$CR"; set -- $out; IFS="$oifs"; }
     for tok in "$@"; do
         key=${tok%%=*}
         [ "$key" != "$tok" ] || continue
@@ -471,6 +477,7 @@ nocase_equal() {
     [ "$1" = "$2" ] && return 0
 
     local delim="-delim-"
+    # shellcheck disable=2018,2019
     out=$(echo "$1${delim}$2" | tr A-Z a-z)
     [ "${out#*${delim}}" = "${out%${delim}*}" ]
 }
@@ -547,11 +554,13 @@ check_config() {
     else
         files="$*"
     fi
-    set +f; set -- $files; set -f;
+    # shellcheck disable=2086
+    { set +f; set -- $files; set -f; }
     if [ "$1" = "$files" -a ! -f "$1" ]; then
         return 1
     fi
     local fname="" line="" ret="" found=0 found_fn=""
+    # shellcheck disable=2094
     for fname in "$@"; do
         [ -f "$fname" ] || continue
         while read line; do
@@ -787,7 +796,7 @@ ec2_read_strict_setting() {
     # 3. look for the key 'strict_id' (datasource/Ec2/strict_id)
     # only in cloud.cfg or cloud.cfg.d/EC2.cfg (case insensitive)
     local cfg="${PATH_ETC_CI_CFG}" cfg_d="${PATH_ETC_CI_CFG_D}"
-    if check_config strict_id $cfg "$cfg_d/*[Ee][Cc]2*.cfg"; then
+    if check_config strict_id "$cfg" "$cfg_d/*[Ee][Cc]2*.cfg"; then
         debug 2 "${_RET_fname} set strict_id to $_RET"
         return 0
     fi
@@ -994,7 +1003,7 @@ dscheck_Scaleway() {
         *\ scaleway\ *) return ${DS_FOUND};;
     esac
 
-    if [ -f ${PATH_ROOT}/var/run/scaleway ]; then
+    if [ -f "${PATH_ROOT}/var/run/scaleway" ]; then
         return ${DS_FOUND}
     fi
 
@@ -1149,6 +1158,7 @@ found() {
 }
 
 trim() {
+    # shellcheck disable=2048,2086
     set -- $*
     _RET="$*"
 }
@@ -1169,7 +1179,7 @@ _read_config() {
     # if no parameters are set, modifies _rc scoped environment vars.
     # if keyname is provided, then returns found value of that key.
     local keyname="${1:-_unset}"
-    local line="" hash="#" ckey="" key="" val=""
+    local line="" hash="#" key="" val=""
     while read line; do
         line=${line%%${hash}*}
         key="${line%%:*}"
@@ -1247,7 +1257,8 @@ parse_policy() {
 
     local mode="" report="" found="" maybe="" notfound=""
     local oifs="$IFS" tok="" val=""
-    IFS=","; set -- $policy; IFS="$oifs"
+    # shellcheck disable=2086
+    { IFS=","; set -- $policy; IFS="$oifs"; }
     for tok in "$@"; do
         val=${tok#*=}
         case "$tok" in
@@ -1314,15 +1325,15 @@ manual_clean_and_existing() {
 }
 
 read_uptime() {
-    local up idle
+    local up _
     _RET="${UNAVAILABLE}"
-    [ -f "$PATH_PROC_UPTIME" ] &&
-        read up idle < "$PATH_PROC_UPTIME" && _RET="$up"
+    [ -f "$PATH_PROC_UPTIME" ] && read up _ < "$PATH_PROC_UPTIME" &&
+        _RET="$up"
     return
 }
 
 _main() {
-    local dscheck="" ret_dis=1 ret_en=0
+    local dscheck_fn="" ret_dis=1 ret_en=0
 
     read_uptime
     debug 1 "[up ${_RET}s]" "ds-identify $*"
@@ -1357,8 +1368,9 @@ _main() {
         return
     fi
 
-    # if there is only a single entry in $DI_DSLIST
+    # shellcheck disable=2086
     set -- $DI_DSLIST
+    # if there is only a single entry in $DI_DSLIST
     if [ $# -eq 1 ] || [ $# -eq 2 -a "$2" = "None" ] ; then
         debug 1 "single entry in datasource_list ($DI_DSLIST) use that."
         found "$@"
@@ -1391,6 +1403,7 @@ _main() {
     done
 
     debug 2 "found=${found# } maybe=${maybe# }"
+    # shellcheck disable=2086
     set -- $found
     if [ $# -ne 0 ]; then
         if [ $# -eq 1 ]; then
@@ -1406,6 +1419,7 @@ _main() {
         return
     fi
 
+    # shellcheck disable=2086
     set -- $maybe
     if [ $# -ne 0 -a "${DI_ON_MAYBE}" != "none" ]; then
         debug 1 "$# datasources returned maybe: $*"
@@ -1434,7 +1448,7 @@ _main() {
         *) error "Unexpected result";;
     esac
     debug 1 "$msg"
-    return $ret
+    return "$ret"
 }
 
 main() {
@@ -1445,7 +1459,7 @@ main() {
         if read ret < "$PATH_RUN_DI_RESULT"; then
             if [ "$ret" = "0" ] || [ "$ret" = "1" ]; then
                 debug 2 "used cached result $ret. pass --force to re-run."
-                return $ret;
+                return "$ret";
             fi
             debug 1 "previous run returned unexpected '$ret'. Re-running."
         else
@@ -1457,7 +1471,7 @@ main() {
     echo "$ret" > "$PATH_RUN_DI_RESULT"
     read_uptime
     debug 1 "[up ${_RET}s]" "returning $ret"
-    return $ret
+    return "$ret"
 }
 
 noop() {