Updated Branches: refs/heads/trunk 98243a918 -> 08f3991b0
AMBARI-3558. Resource Manager. On resource fail should give actual error messages, not just exceptions and Enable passing lists to Execute() to fix the user escape errors (Andrew Onischuk via dlysnichenko) Project: http://git-wip-us.apache.org/repos/asf/incubator-ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ambari/commit/08f3991b Tree: http://git-wip-us.apache.org/repos/asf/incubator-ambari/tree/08f3991b Diff: http://git-wip-us.apache.org/repos/asf/incubator-ambari/diff/08f3991b Branch: refs/heads/trunk Commit: 08f3991b05c5a54c58aeed2b4a8227dcb9a84a39 Parents: 98243a9 Author: Lisnichenko Dmitro <[email protected]> Authored: Tue Oct 22 19:06:38 2013 +0300 Committer: Lisnichenko Dmitro <[email protected]> Committed: Tue Oct 22 19:06:38 2013 +0300 ---------------------------------------------------------------------- .../python/resource_management/environment.py | 4 +- .../resource_management/providers/accounts.py | 10 ++-- .../resource_management/providers/mount.py | 1 - .../providers/package/yumrpm.py | 8 ++-- .../providers/package/zypper.py | 8 ++-- .../resource_management/providers/system.py | 11 ++--- .../resource_management/resources/system.py | 10 ++++ .../main/python/resource_management/shell.py | 49 ++++++++++++++++++++ .../main/python/resource_management/system.py | 15 +++--- 9 files changed, 82 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/environment.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/environment.py b/ambari-agent/src/main/python/resource_management/environment.py index 89054a0..089a03a 100644 --- a/ambari-agent/src/main/python/resource_management/environment.py +++ b/ambari-agent/src/main/python/resource_management/environment.py @@ -5,9 +5,9 @@ __all__ = ["Environment"] import logging import os import shutil -import subprocess from datetime import datetime +from resource_management import shell from resource_management.exceptions import Fail from resource_management.providers import find_provider from resource_management.utils import AttributeDictionary @@ -92,7 +92,7 @@ class Environment(object): return cond() if isinstance(cond, basestring): - ret = subprocess.call(cond, shell=True) + ret, out = shell.call(cond) return ret == 0 raise Exception("Unknown condition type %r" % cond) http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/providers/accounts.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/providers/accounts.py b/ambari-agent/src/main/python/resource_management/providers/accounts.py index 038e795..45673a7 100644 --- a/ambari-agent/src/main/python/resource_management/providers/accounts.py +++ b/ambari-agent/src/main/python/resource_management/providers/accounts.py @@ -2,7 +2,7 @@ from __future__ import with_statement import grp import pwd -import subprocess +from resource_management import shell from resource_management.providers import Provider @@ -33,14 +33,14 @@ class UserProvider(Provider): command.append(self.resource.username) - subprocess.check_call(command) + shell.checked_call(command) self.resource.updated() self.log.info("Added user %s" % self.resource) def action_remove(self): if self.user: command = ['userdel', self.resource.username] - subprocess.check_call(command) + shell.checked_call(command) self.resource.updated() self.log.info("Removed user %s" % self.resource) @@ -70,7 +70,7 @@ class GroupProvider(Provider): command.append(self.resource.group_name) - subprocess.check_call(command) + shell.checked_call(command) self.resource.updated() self.log.info("Added group %s" % self.resource) @@ -85,7 +85,7 @@ class GroupProvider(Provider): def action_remove(self): if self.user: command = ['groupdel', self.resource.group_name] - subprocess.check_call(command) + shell.checked_call(command) self.resource.updated() self.log.info("Removed group %s" % self.resource) http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/providers/mount.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/providers/mount.py b/ambari-agent/src/main/python/resource_management/providers/mount.py index 8ab8d32..4170d3b 100644 --- a/ambari-agent/src/main/python/resource_management/providers/mount.py +++ b/ambari-agent/src/main/python/resource_management/providers/mount.py @@ -2,7 +2,6 @@ from __future__ import with_statement import os import re -from subprocess import Popen, PIPE, STDOUT, check_call from resource_management.base import Fail from resource_management.providers import Provider http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/providers/package/yumrpm.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/providers/package/yumrpm.py b/ambari-agent/src/main/python/resource_management/providers/package/yumrpm.py index 277e308..652597e 100644 --- a/ambari-agent/src/main/python/resource_management/providers/package/yumrpm.py +++ b/ambari-agent/src/main/python/resource_management/providers/package/yumrpm.py @@ -1,17 +1,15 @@ from resource_management.providers.package import PackageProvider -from subprocess import STDOUT, PIPE, check_call +from resource_management import shell INSTALL_CMD = "/usr/bin/yum -d 0 -e 0 -y install %s" REMOVE_CMD = "/usr/bin/yum -d 0 -e 0 -y erase %s" class YumProvider(PackageProvider): def install_package(self, name): - return 0 == check_call(INSTALL_CMD % (name), - shell=True, stdout=PIPE, stderr=STDOUT) + shell.checked_call(INSTALL_CMD % (name)) def upgrade_package(self, name): return self.install_package(name) def remove_package(self, name): - return 0 == check_call(REMOVE_CMD % (name), - shell=True, stdout=PIPE, stderr=STDOUT) + shell.checked_call(REMOVE_CMD % (name)) http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/providers/package/zypper.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/providers/package/zypper.py b/ambari-agent/src/main/python/resource_management/providers/package/zypper.py index 981b526..fe4cadd 100644 --- a/ambari-agent/src/main/python/resource_management/providers/package/zypper.py +++ b/ambari-agent/src/main/python/resource_management/providers/package/zypper.py @@ -1,17 +1,15 @@ from resource_management.providers.package import PackageProvider -from subprocess import STDOUT, PIPE, check_call +from resource_management import shell INSTALL_CMD = "/usr/bin/zypper --quiet install --auto-agree-with-licenses --no-confirm %s" REMOVE_CMD = "/usr/bin/zypper --quiet remove --no-confirm %s" class ZypperProvider(PackageProvider): def install_package(self, name): - return 0 == check_call(INSTALL_CMD % (name), - shell=True, stdout=PIPE, stderr=STDOUT) + shell.checked_call(INSTALL_CMD % (name)) def upgrade_package(self, name): return self.install_package(name) def remove_package(self, name): - return 0 == check_call(REMOVE_CMD % (name), - shell=True, stdout=PIPE, stderr=STDOUT) + shell.checked_call(REMOVE_CMD % (name)) http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/providers/system.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/providers/system.py b/ambari-agent/src/main/python/resource_management/providers/system.py index ca20774..c97211d 100644 --- a/ambari-agent/src/main/python/resource_management/providers/system.py +++ b/ambari-agent/src/main/python/resource_management/providers/system.py @@ -3,7 +3,7 @@ from __future__ import with_statement import grp import os import pwd -import subprocess +from resource_management import shell from resource_management.base import Fail from resource_management.providers import Provider @@ -184,15 +184,12 @@ class ExecuteProvider(Provider): self.log.info("Executing %s" % self.resource) - ret = subprocess.call(self.resource.command, shell=True, + ret, out = shell.checked_call(self.resource.command, cwd=self.resource.cwd, env=self.resource.environment, preexec_fn=_preexec_fn(self.resource)) - if self.resource.returns and ret not in self.resource.returns: - raise Fail("%s failed, returned %d instead of %s" % ( - self, ret, self.resource.returns)) self.resource.updated() - + class ScriptProvider(Provider): def action_run(self): @@ -204,7 +201,7 @@ class ScriptProvider(Provider): tf.flush() _ensure_metadata(tf.name, self.resource.user, self.resource.group) - subprocess.call([self.resource.interpreter, tf.name], + shell.call([self.resource.interpreter, tf.name], cwd=self.resource.cwd, env=self.resource.environment, preexec_fn=_preexec_fn(self.resource)) self.resource.updated() http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/resources/system.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/resources/system.py b/ambari-agent/src/main/python/resource_management/resources/system.py index 4695cc6..729b351 100644 --- a/ambari-agent/src/main/python/resource_management/resources/system.py +++ b/ambari-agent/src/main/python/resource_management/resources/system.py @@ -37,7 +37,17 @@ class Link(Resource): class Execute(Resource): action = ForcedListArgument(default="run") + + """ + Recommended: + command = ('rm','-f','myfile') + Not recommended: + command = 'rm -f myfile' + + The first one helps to stop escaping issues + """ command = ResourceArgument(default=lambda obj: obj.name) + creates = ResourceArgument() cwd = ResourceArgument() environment = ResourceArgument() http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/shell.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/shell.py b/ambari-agent/src/main/python/resource_management/shell.py new file mode 100644 index 0000000..2b334a0 --- /dev/null +++ b/ambari-agent/src/main/python/resource_management/shell.py @@ -0,0 +1,49 @@ +import logging +import subprocess +from exceptions import Fail + + +def checked_call(command, log_stdout=False, + cwd=None, env=None, preexec_fn=None): + return _call(command, log_stdout, True, cwd, env, preexec_fn) + +def call(command, log_stdout=False, + cwd=None, env=None, preexec_fn=None): + return _call(command, log_stdout, False, cwd, env, preexec_fn) + + +def _call(command, log_stdout=False, throw_on_failure=True, + cwd=None, env=None, preexec_fn=None): + """ + Execute shell command + + @param command: list/tuple of arguments (recommended as more safe - don't need to escape) + or string of the command to execute + @param log_stdout: boolean, whether command output should be logged of not + @param throw_on_failure: if true, when return code is not zero exception is thrown + + @return: retrun_code, stdout, stderr + """ + + if isinstance(command, (list, tuple)): + shell = False + else: + shell = True + + proc = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + cwd=cwd, env=env, shell=shell, + preexec_fn=preexec_fn) + out = proc.communicate()[0] + code = proc.wait() + + if throw_on_failure and code: + err_msg = ("Execution of '%s' returned %d: Error: %s") % (command, code, out) + raise Fail(err_msg) + + if log_stdout: + _log.info("%s.\n%s" % (command, out)) + + return code, out + +def _log(): + return logging.getLogger("resource_management.provider") \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/08f3991b/ambari-agent/src/main/python/resource_management/system.py ---------------------------------------------------------------------- diff --git a/ambari-agent/src/main/python/resource_management/system.py b/ambari-agent/src/main/python/resource_management/system.py index c42d931..89c53cd 100644 --- a/ambari-agent/src/main/python/resource_management/system.py +++ b/ambari-agent/src/main/python/resource_management/system.py @@ -2,9 +2,8 @@ __all__ = ["System"] import os import sys +from resource_management import shell from functools import wraps -from subprocess import Popen, PIPE - def lazy_property(undecorated): name = '_' + undecorated.__name__ @@ -21,7 +20,6 @@ def lazy_property(undecorated): return decorated - class System(object): @lazy_property def os(self): @@ -47,15 +45,15 @@ class System(object): @lazy_property def machine(self): - p = Popen(["/bin/uname", "-m"], stdout=PIPE, stderr=PIPE) - return p.communicate()[0].strip() + code, out = shell.call(["/bin/uname", "-m"]) + return out.strip() @lazy_property def lsb(self): if os.path.exists("/usr/bin/lsb_release"): - p = Popen(["/usr/bin/lsb_release", "-a"], stdout=PIPE, stderr=PIPE) + code, out = shell.call(["/usr/bin/lsb_release", "-a"]) lsb = {} - for l in p.communicate()[0].split('\n'): + for l in out.split('\n'): v = l.split(':', 1) if len(v) != 2: continue @@ -99,8 +97,7 @@ class System(object): @lazy_property def locales(self): - p = Popen("locale -a", shell=True, stdout=PIPE) - out = p.communicate()[0] + code, out = shell.call("locale -a") return out.strip().split("\n") @lazy_property
