improve the cloud-init-run-module code a bit, fix LP:#568139

568139 was fixed because the test for "always" was using "is"
instead of "=="

LP: #568139

3 files changed, 29 insertions, 21 deletions

diff --git a/cloud-init-run-module.py b/cloud-init-run-module.py
index 4ec79ff9..1877ab2c 100755
--- a/cloud-init-run-module.py
+++ b/cloud-init-run-module.py
@@ -41,20 +41,17 @@ def main():
     try:
         cloud.get_data_source()
     except Exception as e:
-        print e
-        sys.stderr.write("Failed to get instance data")
-        sys.exit(1)
+        fail("Failed to get instance data\n\t%s" % traceback.format_exc(),log)
 
     if cloud.sem_has_run(semname,freq):
-        sys.stderr.write("%s already ran %s\n" % (semname,freq))
+        err("%s already ran %s" % (semname,freq),log)
         sys.exit(0)
 
     try:
         mod = __import__('cloudinit.' + modname)
         inst = getattr(mod,modname)
     except:
-        sys.stderr.write("Failed to load module cloudinit.%s\n" % modname)
-        sys.exit(1)
+        fail("Failed to load module cloudinit.%s\n" % modname)
 
     import os
 
@@ -63,9 +60,21 @@ def main():
     if os.environ.has_key(cfg_env_name):
         cfg_path = os.environ[cfg_env_name]
 
-    cloud.sem_and_run(semname, freq, inst.run, [run_args,cfg_path], False)
+    try:
+        cloud.sem_and_run(semname, freq, inst.run, [run_args,cfg_path,log], False)
+    except Exception as e:
+        fail("Execution of %s failed:%s" % (semname,e), log)
 
     sys.exit(0)
 
+def err(msg,log=None):
+    if log:
+        log.error(msg)
+    sys.stderr.write(msg + "\n")
+
+def fail(msg,log=None):
+    err(msg,log)
+    sys.exit(1)
+
 if __name__ == '__main__':
     main()
diff --git a/cloudinit/__init__.py b/cloudinit/__init__.py
index 04beca1e..624b8ee8 100644
--- a/cloudinit/__init__.py
+++ b/cloudinit/__init__.py
@@ -277,7 +277,7 @@ class CloudInit:
         return("%s/%s.%s" % (semdir,name,freqtok))
     
     def sem_has_run(self,name,freq):
-        if freq is "always": return False
+        if freq == "always": return False
         semfile = self.sem_getpath(name,freq)
         if os.path.exists(semfile):
             return True
@@ -293,7 +293,7 @@ class CloudInit:
             if e.errno != errno.EEXIST:
                 raise e
     
-        if os.path.exists(semfile) and freq is not "always":
+        if os.path.exists(semfile) and freq != "always":
             return False
     
         # race condition
@@ -325,7 +325,7 @@ class CloudInit:
             return
         try:
             if not self.sem_acquire(semname,freq):
-                raise Exception("Failed to acquire lock on %s\n" % semname)
+                raise Exception("Failed to acquire lock on %s" % semname)
 
             func(*args)
         except:
diff --git a/cloudinit/execute.py b/cloudinit/execute.py
index eb7b568c..17c3ad69 100644
--- a/cloudinit/execute.py
+++ b/cloudinit/execute.py
@@ -15,16 +15,15 @@
 #
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
-def run(list,cfg):
+def run(args,cfg,log):
     import subprocess
-    retcode = subprocess.call(list)
-
-    if retcode == 0:
+    import traceback
+    try:
+        subprocess.check_call(args)
         return
-
-    if retcode < 0:
-        str="Cmd terminated by signal %s\n" % -retcode
-    else:
-        str="Cmd returned %s\n" % retcode
-    str+=' '.join(list)
-    raise Exception(str)
+    except subprocess.CalledProcessError as e:
+        log.debug(traceback.format_exc(e))
+        raise Exception("Cmd returned %s: %s" % ( e.returncode, args ))
+    except OSError as e:
+        log.debug(traceback.format_exc(e))
+        raise Exception("Cmd failed to execute: %s" % ( args ))