From 4eaee3df0ad99dd813f151ee868f2bd18476864f Mon Sep 17 00:00:00 2001 From: Stephen Fromm Date: Sun, 13 Jan 2013 09:12:40 -0800 Subject: [PATCH] Various cleanup to run_command * Rename fail_on_rc_non_zero to check_rc, much more succinct. * Simplify method defintion * Fix command module and drop shell=shell option; whether to use shell is determined by if args is a list. --- lib/ansible/module_common.py | 26 ++++++++++---------------- library/command | 2 +- library/facter | 2 +- library/git | 6 +++--- library/ohai | 2 +- library/subversion | 2 +- library/supervisorctl | 2 +- 7 files changed, 18 insertions(+), 24 deletions(-) diff --git a/lib/ansible/module_common.py b/lib/ansible/module_common.py index d8e87a8bf4f..b7e750f61b4 100644 --- a/lib/ansible/module_common.py +++ b/lib/ansible/module_common.py @@ -677,40 +677,34 @@ class AnsibleModule(object): self.set_context_if_different(src, context, False) os.rename(src, dest) - def run_command(self, args, **kwargs): + def run_command(self, args, check_rc=False, close_fds=False, executable=None): ''' Execute a command, returns rc, stdout, and stderr. args is the command to run If args is a list, the command will be run with shell=False. Otherwise, the command will be run with shell=True when args is a string. - kwargs is a dict of keyword arguments: - - fail_on_rc_non_zero (boolean) Whether to call fail_json in case of - non zero RC. Default is False. + Other arguments: + - check_rc (boolean) Whether to call fail_json in case of + non zero RC. Default is False. - close_fds (boolean) See documentation for subprocess.Popen(). Default is False. - executable (string) See documentation for subprocess.Popen(). Default is None. ''' if isinstance(args, list): - kwargs['shell'] = False + shell = False elif isinstance(args, basestring): - kwargs['shell'] = True + shell = True else: msg = "Argument 'args' to run_command must be list or string" self.fail_json(rc=257, cmd=args, msg=msg) - if 'fail_on_rc_non_zero' not in kwargs: - kwargs['fail_on_rc_non_zero'] = False - if 'close_fds' not in kwargs: - kwargs['close_fds'] = False - if 'executable' not in kwargs: - kwargs['executable'] = None rc = 0 msg = None try: cmd = subprocess.Popen(args, - executable=kwargs['executable'], - shell=kwargs['shell'], - close_fds=kwargs['close_fds'], + executable=executable, + shell=shell, + close_fds=close_fds, stdout=subprocess.PIPE, stderr=subprocess.PIPE) out, err = cmd.communicate() rc = cmd.returncode @@ -718,7 +712,7 @@ class AnsibleModule(object): self.fail_json(rc=e.errno, msg=str(e), cmd=args) except: self.fail_json(rc=257, msg=traceback.format_exc(), cmd=args) - if rc != 0 and kwargs['fail_on_rc_non_zero']: + if rc != 0 and check_rc: msg = err.rstrip() self.fail_json(cmd=args, rc=rc, stdout=out, stderr=err, msg=msg) return (rc, out, err) diff --git a/library/command b/library/command index c3dc4cdb4d9..13dbb089f63 100644 --- a/library/command +++ b/library/command @@ -99,7 +99,7 @@ def main(): args = shlex.split(args) startd = datetime.datetime.now() - rc, out, err = module.run_command(args, shell=shell, executable=executable) + rc, out, err = module.run_command(args, executable=executable) endd = datetime.datetime.now() delta = endd - startd diff --git a/library/facter b/library/facter index 9f6523e7fe9..9bb807dcff1 100644 --- a/library/facter +++ b/library/facter @@ -44,7 +44,7 @@ def main(): ) cmd = ["/usr/bin/env", "facter", "--json"] - rc, out, err = module.run_command(cmd, fail_on_rc_non_zero=True) + rc, out, err = module.run_command(cmd, check_rc=True) module.exit_json(**json.loads(out)) # this is magic, see lib/ansible/module_common.py diff --git a/library/git b/library/git index b63b3f94d7a..ad3da4fce16 100644 --- a/library/git +++ b/library/git @@ -81,7 +81,7 @@ def clone(module, repo, dest, remote): pass os.chdir(dest_dirname) return module.run_command("git clone -o %s %s %s" % (remote, repo, dest), - fail_on_rc_non_zero=True) + check_rc=True) def has_local_mods(dest): os.chdir(dest) @@ -99,7 +99,7 @@ def reset(module,dest,force): os.chdir(dest) if not force and has_local_mods(dest): module.fail_json(msg="Local modifications exist in repository (force=no).") - return module.run_command("git reset --hard HEAD", fail_on_rc_non_zero=True) + return module.run_command("git reset --hard HEAD", check_rc=True) def get_branches(module, dest): os.chdir(dest) @@ -210,7 +210,7 @@ def switch_version(module, dest, remote, version): if rc != 0: module.fail_json(msg="Failed to checkout branch %s" % branch) cmd = "git reset --hard %s" % remote - return module.run_command(cmd, fail_on_rc_non_zero=True) + return module.run_command(cmd, check_rc=True) # =========================================== diff --git a/library/ohai b/library/ohai index fc5538d2356..fd048a7df2b 100644 --- a/library/ohai +++ b/library/ohai @@ -43,7 +43,7 @@ def main(): argument_spec = dict() ) cmd = ["/usr/bin/env", "ohai"] - rc, out, err = module.run_command(cmd, fail_on_rc_non_zero=True) + rc, out, err = module.run_command(cmd, check_rc=True) module.exit_json(**json.loads(out)) # this is magic, see lib/ansible/module_common.py diff --git a/library/subversion b/library/subversion index 336ab2746c2..c8cd5198b67 100644 --- a/library/subversion +++ b/library/subversion @@ -85,7 +85,7 @@ class Subversion(object): if self.password: bits.append("--password '%s'" % self.password) bits.append(args) - rc, out, err = self.module.run_command(' '.join(bits), fail_on_rc_non_zero=True) + rc, out, err = self.module.run_command(' '.join(bits), check_rc=True) return out.splitlines() def checkout(self): diff --git a/library/supervisorctl b/library/supervisorctl index 89c2e476b19..0882a3a3dde 100644 --- a/library/supervisorctl +++ b/library/supervisorctl @@ -63,7 +63,7 @@ def main(): if state == 'present': if not present: - module.run_command('%s reread' % SUPERVISORCTL, fail_on_rc_non_zero=True) + module.run_command('%s reread' % SUPERVISORCTL, check_rc=True) rc, out, err = module.run_command('%s add %s' % (SUPERVISORCTL, name)) if '%s: added process group' % name in out: