Merge lp:~johnsca/layer-ibm-base/fix-multi-call into lp:~ibmcharmers/layer-ibm-base/trunk

[Cory Johns]

Change how fetch_curl_resource() is gated using the newer config states to prevent it from being called multiple times.

When curl_url is set, having not be set previously, it will enable fetch_curl_resource() to complete but then that will also trigger curl_url_changed() which will call fetch_curl_resource() a second time under the assumption that the source was changed and it needs to be reinstalled. If curl_opts is set at the same time, it could lead to fetch_curl_resource() being called three different times.

This change relies on the fact that the first time the curl_url is set, either config.changed.curl_url or config.new.curl_url (depending on if we go with https://github.com/juju-solutions/layer-basic/pull/61 or not, or if we're in the first hook or not) in addition to config.set.curl_url.

[lp:~johnsca/layer-ibm-base/fix-multi-call]

5. By Cory Johns on 2016-05-03

Work around formatting issue with built yaml

The yaml output from `charm build` drops the spaces before the comment.
However, this causes PyYaml to fail to parse the file. This should be
fixed upstream in charm-tools, but moving the comment to the next line
is an easy work-around.

[Kevin W Monroe (kwmonroe)]

LGTM, thanks Cory!

review: Approve

=== modified file 'layer.yaml'
--- layer.yaml	2016-03-23 20:36:38 +0000
+++ layer.yaml	2016-05-03 17:29:14 +0000
@@ -3,6 +3,7 @@
   - 'layer:apt'
   - 'layer:leadership'
 options:
-  basic:  # Setting options for the basic layer.
+  basic:
+    # Setting options for the basic layer.
     packages:
       - curl

=== modified file 'reactive/ibm-base.sh'
--- reactive/ibm-base.sh	2016-03-23 21:04:54 +0000
+++ reactive/ibm-base.sh	2016-05-03 17:29:14 +0000
@@ -15,7 +15,7 @@
   local PROG=""
 
   if [ ! -r ${FILE} ]; then
-    status-set active "ibm-base: could not read ${FILE}"
+    status-set blocked "ibm-base: could not read ${FILE}"
     juju-log "Could not verify the downloaded resource. File could not be read: ${FILE}"
   fi
 
@@ -31,18 +31,17 @@
       PROG='sha512sum'
       ;;
     *)
-      status-set active "ibm-base: checksum type must be md5, sha215, or sha512"
+      status-set blocked "ibm-base: checksum type must be md5, sha215, or sha512"
       juju-log "Could not verify the downloaded resource ${FILE}. Unknown checksum type: ${TYPE}"
       return 1
   esac
 
   CALCULATED_SUM=`${PROG} ${FILE} | awk '{print $1}'`
   if [ "${CALCULATED_SUM}" = "${EXPECTED_SUM}" ]; then
-    status-set active "ibm-base: checksum verified"
     juju-log "Checksum verified for ${FILE}."
     return 0
   else
-    status-set active "ibm-base: checksums did not match"
+    status-set blocked "ibm-base: checksums did not match"
     juju-log "Checksum mismatch for ${FILE}. Expected ${EXPECTED_SUM}, got ${CALCULATED_SUM}"
     return 1
   fi
@@ -50,57 +49,42 @@
 
 
 # Fetch curl resources if a URL is configured
-@when_not 'ibm-base.curl.resource.fetched'
+@when 'config.set.curl_url'
+@when_any 'config.new.curl_url' 'config.changed.curl_url' 'config.new.curl_opts' 'config.changed.curl_opts'
 function fetch_curl_resource() {
   local ARCHIVE_DIR="${CHARM_DIR}/files/archives"
   local CURL_URL=$(config-get 'curl_url')
   local CURL_OPTS=$(config-get 'curl_opts')
 
-  if [ -n "${CURL_URL}" ]; then
-    status-set maintenance "ibm-base: fetching resource(s)"
-
-    mkdir -p ${ARCHIVE_DIR}
-    cd ${ARCHIVE_DIR}
-    # Multiple URLs may be separated by a space, so loop.
-    for URL_STRING in ${CURL_URL}
-    do
-      # For each URL_STRING, set the url, checksum type, and checksum value.
-      local URL=${URL_STRING%%\?*}        # string before the first '?'
-      local FILE_NAME=${URL##*\/}         # string after the last '/'
-      local SUM_STRING=${URL_STRING#*\?}  # string after the first '?'
-      local SUM_TYPE=${SUM_STRING%%\=*}   # string before the first '='
-      local SUM_VALUE=${SUM_STRING#*\=}   # string after the first '='
-
-      if [ -z ${FILE_NAME} ]; then
-        FILE_NAME="juju-${RANDOM}"
-      fi
-      curl --silent --show-error ${CURL_OPTS} -o ${FILE_NAME} ${URL}
-
-      # Verify our resource checksum. If this fails, let verify_resource log
-      # the reason and exit successfully. Exiting non-zero would fail the hook,
-      # so return 0 and simply inform the user that verification failed.
-      verify_curl_resource ${FILE_NAME} ${SUM_TYPE} ${SUM_VALUE} || return 0
-    done
-    cd -
-
-    set_state 'ibm-base.curl.resource.fetched'
-    status-set active "ibm-base: curl resource(s) fetched"
-    juju-log 'Curl resource fetched'
-  fi
-}
-
-
-# Handle curl configuration changes
-@when 'ibm-base.curl.resource.fetched' 'config.changed.curl_url'
-function curl_url_changed() {
-  juju-log 'Curl URL changed'
-  fetch_curl_resource
-}
-
-@when 'ibm-base.curl.resource.fetched' 'config.changed.curl_opts'
-function curl_opts_changed() {
-  juju-log 'Curl options changed'
-  fetch_curl_resource
+  status-set maintenance "ibm-base: fetching resource(s)"
+
+  mkdir -p ${ARCHIVE_DIR}
+  cd ${ARCHIVE_DIR}
+  # Multiple URLs may be separated by a space, so loop.
+  for URL_STRING in ${CURL_URL}
+  do
+    # For each URL_STRING, set the url, checksum type, and checksum value.
+    local URL=${URL_STRING%%\?*}        # string before the first '?'
+    local FILE_NAME=${URL##*\/}         # string after the last '/'
+    local SUM_STRING=${URL_STRING#*\?}  # string after the first '?'
+    local SUM_TYPE=${SUM_STRING%%\=*}   # string before the first '='
+    local SUM_VALUE=${SUM_STRING#*\=}   # string after the first '='
+
+    if [ -z ${FILE_NAME} ]; then
+      FILE_NAME="juju-${RANDOM}"
+    fi
+    curl --silent --show-error ${CURL_OPTS} -o ${FILE_NAME} ${URL}
+
+    # Verify our resource checksum. If this fails, let verify_resource log
+    # the reason and exit successfully. Exiting non-zero would fail the hook,
+    # so return 0 and simply inform the user that verification failed.
+    verify_curl_resource ${FILE_NAME} ${SUM_TYPE} ${SUM_VALUE} || return 0
+  done
+  cd -
+
+  set_state 'ibm-base.curl.resource.fetched'
+  status-set active "ibm-base: curl resource(s) fetched"
+  juju-log 'Curl resource fetched'
 }