This is an automated email from the ASF dual-hosted git repository.
hapylestat pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push:
new f277631 [AMBARI-24632] APT/DPKG existence check doesn't work for
system packages (dgrinenko) (#2366)
f277631 is described below
commit f277631dec36a442999327026a030e463e4c67e1
Author: Dmytro Grinenko <[email protected]>
AuthorDate: Thu Sep 27 12:00:07 2018 +0300
[AMBARI-24632] APT/DPKG existence check doesn't work for system packages
(dgrinenko) (#2366)
---
.../ambari_commons/repo_manager/apt_manager.py | 16 ++++--
.../ambari_commons/repo_manager/apt_parser.py | 2 +-
.../ambari_commons/repo_manager/yum_manager.py | 4 +-
.../ambari_commons/repo_manager/zypper_manager.py | 2 +-
.../src/main/python/ambari_commons/shell.py | 12 +++-
.../custom_actions/scripts/install_packages.py | 5 +-
.../python/custom_actions/TestInstallPackages.py | 64 +++++++++++++++++-----
7 files changed, 79 insertions(+), 26 deletions(-)
diff --git
a/ambari-common/src/main/python/ambari_commons/repo_manager/apt_manager.py
b/ambari-common/src/main/python/ambari_commons/repo_manager/apt_manager.py
index 0310af9..5882253 100644
--- a/ambari-common/src/main/python/ambari_commons/repo_manager/apt_manager.py
+++ b/ambari-common/src/main/python/ambari_commons/repo_manager/apt_manager.py
@@ -67,8 +67,6 @@ class AptManagerProperties(GenericManagerProperties):
install_cmd_env = {'DEBIAN_FRONTEND': 'noninteractive'}
- check_cmd = pkg_manager_bin + " --get-selections | grep -v deinstall | awk
'{print $1}' | grep ^%s$"
-
repo_url_exclude = "ubuntu.com"
configuration_dump_cmd = [AMBARI_SUDO_BINARY, "apt-config", "dump"]
@@ -237,7 +235,7 @@ class AptManager(GenericManager):
if not name:
raise ValueError("Installation command was executed with no package
name")
- elif context.is_upgrade or context.use_repos or not
self._check_existence(name):
+ elif not self._check_existence(name) or context.action_force:
cmd = self.properties.install_cmd[context.log_output]
copied_sources_files = []
is_tmp_dir_created = False
@@ -251,7 +249,7 @@ class AptManager(GenericManager):
if use_repos:
is_tmp_dir_created = True
apt_sources_list_tmp_dir =
tempfile.mkdtemp(suffix="-ambari-apt-sources-d")
- Logger.info("Temporary sources directory was created: %s" %
apt_sources_list_tmp_dir)
+ Logger.info("Temporary sources directory was created:
{}".format(apt_sources_list_tmp_dir))
for repo in use_repos:
new_sources_file = os.path.join(apt_sources_list_tmp_dir, repo +
'.list')
@@ -327,6 +325,12 @@ class AptManager(GenericManager):
apt-get in inconsistant state (locked, used, having invalid repo). Once
packages are installed
we should not rely on that.
"""
+ # this method is more optimised than #installed_packages, as here we do
not call available packages(as we do not
+ # interested in repository, from where package come)
+ cmd = self.properties.installed_packages_cmd + [name]
+
+ with shell.process_executor(cmd,
strategy=shell.ReaderStrategy.BufferedChunks, silent=True) as output:
+ for package, version in AptParser.packages_installed_reader(output):
+ return package == name
- r = shell.subprocess_executor(self.properties.check_cmd % name)
- return not bool(r.code)
+ return False
diff --git
a/ambari-common/src/main/python/ambari_commons/repo_manager/apt_parser.py
b/ambari-common/src/main/python/ambari_commons/repo_manager/apt_parser.py
index 03afb86..29f1a65 100644
--- a/ambari-common/src/main/python/ambari_commons/repo_manager/apt_parser.py
+++ b/ambari-common/src/main/python/ambari_commons/repo_manager/apt_parser.py
@@ -137,7 +137,7 @@ class AptParser(GenericParser):
line = line[2:].lstrip()
data = line.partition(" ")
- pkg_name = data[0]
+ pkg_name = data[0].partition(":")[0] # for system packages in format
"libuuid1:amd64"
version = data[2].strip().partition(" ")[0]
if pkg_name and version:
diff --git
a/ambari-common/src/main/python/ambari_commons/repo_manager/yum_manager.py
b/ambari-common/src/main/python/ambari_commons/repo_manager/yum_manager.py
index e3df80e..8e2931e 100644
--- a/ambari-common/src/main/python/ambari_commons/repo_manager/yum_manager.py
+++ b/ambari-common/src/main/python/ambari_commons/repo_manager/yum_manager.py
@@ -208,7 +208,7 @@ class YumManager(GenericManager):
if not name:
raise ValueError("Installation command was executed with no package
name")
- elif context.is_upgrade or context.use_repos or not
self._check_existence(name):
+ elif not self._check_existence(name) or context.action_force:
cmd = self.properties.install_cmd[context.log_output]
if context.use_repos:
enable_repo_option = '--enablerepo=' +
",".join(sorted(context.use_repos.keys()))
@@ -273,6 +273,8 @@ class YumManager(GenericManager):
yum in inconsistant state (locked, used, having invalid repo). Once
packages are installed
we should not rely on that.
"""
+ if not name:
+ raise ValueError("Package name can't be empty")
if os.geteuid() == 0:
return self.yum_check_package_available(name)
diff --git
a/ambari-common/src/main/python/ambari_commons/repo_manager/zypper_manager.py
b/ambari-common/src/main/python/ambari_commons/repo_manager/zypper_manager.py
index 789a3d0..7e1249f 100644
---
a/ambari-common/src/main/python/ambari_commons/repo_manager/zypper_manager.py
+++
b/ambari-common/src/main/python/ambari_commons/repo_manager/zypper_manager.py
@@ -183,7 +183,7 @@ class ZypperManager(GenericManager):
"""
if not name:
raise ValueError("Installation command was executed with no package
name")
- elif context.is_upgrade or context.use_repos or not
self._check_existence(name):
+ elif not self._check_existence(name) or context.action_force:
cmd = self.properties.install_cmd[context.log_output]
if context.use_repos:
diff --git a/ambari-common/src/main/python/ambari_commons/shell.py
b/ambari-common/src/main/python/ambari_commons/shell.py
index 9f3d463..fda1782 100644
--- a/ambari-common/src/main/python/ambari_commons/shell.py
+++ b/ambari-common/src/main/python/ambari_commons/shell.py
@@ -104,10 +104,11 @@ class RepoCallContext(object):
use_repos = None
skip_repos = None
is_upgrade = False
+ action_force = False # currently only for install action
def __init__(self, ignore_errors=True, retry_count=2, retry_sleep=30,
retry_on_repo_unavailability=False,
retry_on_locked=True, log_output=True, use_repos=None,
skip_repos=None,
- is_upgrade=False):
+ is_upgrade=False, action_force=False):
"""
:type ignore_errors bool
:type retry_count int
@@ -128,6 +129,7 @@ class RepoCallContext(object):
self.use_repos = use_repos
self.skip_repos = skip_repos
self.is_upgrade = is_upgrade
+ self.action_force = action_force
@property
def retry_count(self):
@@ -450,7 +452,8 @@ def subprocess_executor(command, timeout=__TIMEOUT_SECONDS,
strategy=ReaderStrat
@contextmanager
-def process_executor(command, timeout=__TIMEOUT_SECONDS, error_callback=None,
strategy=ReaderStrategy.BufferedQueue, env=None):
+def process_executor(command, timeout=__TIMEOUT_SECONDS, error_callback=None,
strategy=ReaderStrategy.BufferedQueue,
+ env=None, silent=False):
"""
Context manager for command execution
@@ -458,12 +461,14 @@ def process_executor(command, timeout=__TIMEOUT_SECONDS,
error_callback=None, st
:param timeout execution time limit in seconds. If None will default to
TIMEOUT_SECONDS, -1 disable feature
:param strategy the way how to process output. Available methods listed in
ReaderStrategy
:param env Environment variable for new spawned process
+ :param silent no error logging if command execution failed, do not affect
`error_callback` param
:type command list|str
:type timeout None|int
:type error_callback func
:type strategy int
:type env dict
+ :type silent bool
:rtype stdout collections.Iterable
@@ -515,7 +520,8 @@ def process_executor(command, timeout=__TIMEOUT_SECONDS,
error_callback=None, st
if error_callback and cmd.returncode and cmd.returncode > 0:
error_callback(command, cmd.stderr.readlines(), cmd.returncode)
except Exception as e:
- _logger.error("Exception during command '{0}' execution:
{1}".format(command, str(e)))
+ if not silent:
+ _logger.error("Exception during command '{0}' execution:
{1}".format(command, str(e)))
if error_callback:
error_callback(command, [str(e)], -1)
diff --git
a/ambari-server/src/main/resources/custom_actions/scripts/install_packages.py
b/ambari-server/src/main/resources/custom_actions/scripts/install_packages.py
index 800bb21..bc4c65a 100644
---
a/ambari-server/src/main/resources/custom_actions/scripts/install_packages.py
+++
b/ambari-server/src/main/resources/custom_actions/scripts/install_packages.py
@@ -389,9 +389,12 @@ class InstallPackages(Script):
# patches installed
repositories = config['repositoryFile']['repositories']
command_repos = CommandRepository(config['repositoryFile'])
- command_repos.items = [x for x in command_repos.items if not
x.applicable_services]
repository_ids = [repository['repoId'] for repository in repositories]
repos_to_use = {}
+
+ if not command_repos.items:
+ raise ValueError("No repositories passed with the command")
+
for repo_id in repository_ids:
if repo_id in self.repo_files:
repos_to_use[repo_id] = self.repo_files[repo_id]
diff --git
a/ambari-server/src/test/python/custom_actions/TestInstallPackages.py
b/ambari-server/src/test/python/custom_actions/TestInstallPackages.py
index a8840f0..018d561 100644
--- a/ambari-server/src/test/python/custom_actions/TestInstallPackages.py
+++ b/ambari-server/src/test/python/custom_actions/TestInstallPackages.py
@@ -106,10 +106,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_lookUpYum
available_packages.side_effect =
TestInstallPackages._add_packages_lookUpYum
installed_packages.side_effect =
TestInstallPackages._add_packages_lookUpYum
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
@@ -226,10 +229,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
@@ -294,10 +300,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_lookUpYum
available_packages.side_effect =
TestInstallPackages._add_packages_lookUpYum
installed_packages.side_effect =
TestInstallPackages._add_packages_lookUpYum
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
list_ambari_managed_repos_mock.return_value=["HDP-UTILS-2.2.0.1-885"]
@@ -453,10 +462,12 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
is_suse_family_mock.return_value = True
@@ -523,10 +534,12 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
is_suse_family_mock.return_value = True
@@ -603,10 +616,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
list_ambari_managed_repos_mock.return_value = []
@@ -688,6 +704,7 @@ class TestInstallPackages(RMFTestCase):
with open(config_file, "r") as f:
command_json = json.load(f)
+
command_json['roleParams']['repository_version'] =
VERSION_STUB_WITHOUT_BUILD_NUMBER
from ambari_commons.os_check import OSConst
@@ -697,10 +714,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
list_ambari_managed_repos_mock.return_value = []
@@ -766,10 +786,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages
available_packages.side_effect = TestInstallPackages._add_packages
installed_packages.side_effect = TestInstallPackages._add_packages
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
@@ -867,10 +890,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
list_ambari_managed_repos_mock.return_value = []
@@ -957,10 +983,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
list_ambari_managed_repos_mock.return_value = []
@@ -1051,10 +1080,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
list_ambari_managed_repos_mock.return_value = []
@@ -1151,10 +1183,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
list_ambari_managed_repos_mock.return_value = []
@@ -1237,10 +1272,13 @@ class TestInstallPackages(RMFTestCase):
with patch.object(pkg_manager, "all_packages") as all_packages, \
patch.object(pkg_manager, "available_packages") as available_packages, \
- patch.object(pkg_manager, "installed_packages") as installed_packages:
+ patch.object(pkg_manager, "installed_packages") as installed_packages, \
+ patch.object(pkg_manager, "_check_existence") as check_existence:
+
all_packages.side_effect = TestInstallPackages._add_packages_available
available_packages.side_effect =
TestInstallPackages._add_packages_available
installed_packages.side_effect =
TestInstallPackages._add_packages_available
+ check_existence.return_value = False
get_provider.return_value = pkg_manager
list_ambari_managed_repos_mock.return_value=[]
