You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by dm...@apache.org on 2013/10/22 18:12:04 UTC
git commit: 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)
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 <dl...@hortonworks.com>
Authored: Tue Oct 22 19:06:38 2013 +0300
Committer: Lisnichenko Dmitro <dl...@hortonworks.com>
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