[PATCH 0/3 v2] Removing Yum API from Kimchi
The use of Yum API is causing a memory leak each time the Host tab is loaded. This can be verified by seeing the use of memory in $top. To remove the Yum API, a new module named 'yumparser' was created to manipulate Yum repositories directly from the filesystem and to return the software update list by parsing $yum check-update command. A new unit test file was created to verify the proper behavior of the yumparser module. Daniel Henrique Barboza (3): Adding yumparser module Unit tests for the yumparser module Changing repositories and swupdate to use yumparser module src/kimchi/repositories.py | 65 ++++------- src/kimchi/swupdate.py | 16 +-- src/kimchi/yumparser.py | 271 +++++++++++++++++++++++++++++++++++++++++++++ tests/test_yumparser.py | 109 ++++++++++++++++++ 4 files changed, 406 insertions(+), 55 deletions(-) create mode 100644 src/kimchi/yumparser.py create mode 100644 tests/test_yumparser.py -- 2.1.0
This module contains: - functions to write and delete YUM repositories using the .repo files from the filesystem directly, without the need of an external API. - a function to return the software update package list by parsing the output of $yum check-update Signed-off-by: Daniel Henrique Barboza <dhbarboza82@gmail.com> --- src/kimchi/yumparser.py | 271 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 271 insertions(+) create mode 100644 src/kimchi/yumparser.py diff --git a/src/kimchi/yumparser.py b/src/kimchi/yumparser.py new file mode 100644 index 0000000..fcbdc55 --- /dev/null +++ b/src/kimchi/yumparser.py @@ -0,0 +1,271 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2015 +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import subprocess + +from os import listdir +from os.path import getmtime, isfile, join, splitext + + +class YumRepoObject(object): + + def _set_base_URL(self, strvalue): + self.baseurl = strvalue + + def _set_enabled(self, strvalue): + self.enabled = (strvalue == '1') + + def _set_gpgcheck(self, strvalue): + self.gpgcheck = (strvalue == '1') + + def _set_gpgkey(self, strvalue): + self.gpgkey = strvalue + + def _set_metalink(self, strvalue): + self.metalink = strvalue + + def _set_mirrorlist(self, strvalue): + self.mirrorlist = strvalue + + def _set_name(self, strvalue): + self.name = strvalue + + def __init__(self, repo_id, repofile): + self.repo_id = repo_id + self.name = None + self.baseurl = None + self.enabled = True + self.gpgcheck = True + self.gpgkey = None + self.metalink = None + self.mirrorlist = None + self.repofile = repofile + self.attr_setters = {'baseurl': self._set_base_URL, + 'enabled': self._set_enabled, + 'gpgcheck': self._set_gpgcheck, + 'gpgkey': self._set_gpgkey, + 'name': self._set_name, + 'metalink': self._set_metalink, + 'mirrorlist': self._set_mirrorlist} + + def set_attribute(self, key, value): + attr_setter = self.attr_setters.get(key) + if attr_setter: + return attr_setter(value) + + def get_attribute_str(self, key): + if key not in self.attr_setters.keys(): + return None + if key == 'enabled' or key == 'gpgcheck': + str_value = '1' if getattr(self, key) is True else '0' + else: + str_value = getattr(self, key) + + if str_value is None: + return None + + return key + '=' + str_value + + def get_attributes(self): + return self.attr_setters.keys() + + def enable(self): + self.enabled = True + + def disable(self): + self.enabled = False + + def __str__(self): + str_obj = '[' + self.repo_id + ']' + '\n' + for key in self.get_attributes(): + if self.get_attribute_str(key) is not None: + str_obj += self.get_attribute_str(key) + '\n' + return str_obj + + +def get_repo_files(): + def _is_repository_file(f): + _, f_extension = splitext(f) + return isfile(f) and (f_extension == '.repo') + + YUM_REPO_DIR = '/etc/yum.repos.d' + return [YUM_REPO_DIR+'/'+f for f in listdir(YUM_REPO_DIR) + if _is_repository_file(YUM_REPO_DIR+'/'+f)] + + +def _ignore_line_repo_file(line): + return line.startswith("#") or '=' not in line + + +def _get_repos_from_file(repo_file): + repos_from_file = {} + current_repo = None + current_repo_id = None + with open(repo_file) as f: + for line in f.readlines(): + line = line.strip() + if line.startswith("["): + if current_repo is not None: + repos_from_file[current_repo_id] = current_repo + current_repo_id = line.strip('[]') + current_repo = YumRepoObject(current_repo_id, repo_file) + continue + if _ignore_line_repo_file(line): + continue + key, value = line.split('=', 1) + key = key.strip() + value = value.strip() + current_repo.set_attribute(key, value) + + # add the last repo from file + repos_from_file[current_repo_id] = current_repo + + return repos_from_file + + +def get_yum_repositories(): + repo_files = get_repo_files() + repos = {} + for yum_repo in repo_files: + repos.update(_get_repos_from_file(yum_repo)) + + return repos + + +def _retrieve_repo_line_index(data, repo): + repo_entry = '[' + repo.repo_id + ']\n' + try: + repo_index = data.index(repo_entry) + except: + return None + return repo_index + + +def _update_repo_file_data(data, repo, repo_index): + remaining_repo_attrs = repo.get_attributes() + + for i in range(repo_index + 1, len(data)): + line = data[i].strip() + if line.startswith('['): + break + if _ignore_line_repo_file(line): + continue + key, _ = line.split('=', 1) + key = key.strip() + attr_str = repo.get_attribute_str(key) + if attr_str is None: + continue + remaining_repo_attrs.remove(key) + data[i] = attr_str + '\n' + + for attr in remaining_repo_attrs: + attr_str = repo.get_attribute_str(attr) + if attr_str is None: + continue + data.insert(repo_index+1, attr_str + '\n') + + return data + + +def write_repo_to_file(repo): + with open(repo.repofile) as f: + data = f.readlines() + + repo_index = _retrieve_repo_line_index(data, repo) + if repo_index is None: + return + + data = _update_repo_file_data(data, repo, repo_index) + + with open(repo.repofile, 'w') as f: + f.writelines(data) + + +def _get_last_line_repo(data, repo_index): + stop_delete_index = None + for i in range(repo_index+1, len(data)): + line = data[i].strip() + if line.startswith('['): + stop_delete_index = i - 1 + break + if stop_delete_index is None: + stop_delete_index = len(data) - 1 + + return stop_delete_index + + +def _remove_repo_file_data(data, repo_index): + last_line_repo = _get_last_line_repo(data, repo_index) + for i in range(last_line_repo, repo_index - 1, -1): + data.pop(i) + return data + + +def delete_repo_from_file(repo): + with open(repo.repofile) as f: + data = f.readlines() + + repo_index = _retrieve_repo_line_index(data, repo) + if repo_index is None: + return + + data = _remove_repo_file_data(data, repo_index) + + with open(repo.repofile, 'w') as f: + f.writelines(data) + + +class YumUpdatePackageObject(object): + + def __init__(self, name, arch, version, repo): + self.name = name + self.arch = arch + self.version = version + self.ui_from_repo = repo + + +def _get_yum_checkupdate_output(): + cmd = ['yum', 'check-update', '-d0'] + yum_update_cmd = subprocess.Popen(cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + out, error = yum_update_cmd.communicate() + if error != '': + return None + + return out.split() + + +def get_yum_packages_list_update(): + yum_checkupdate_output = _get_yum_checkupdate_output() + if yum_checkupdate_output is None: + return None + + packages = [] + index = 0 + while index < len(yum_checkupdate_output): + name_arch = yum_checkupdate_output[index] + index += 1 + version = yum_checkupdate_output[index] + index += 1 + repo = yum_checkupdate_output[index] + index += 1 + name, arch = name_arch.rsplit('.', 1) + packages.append(YumUpdatePackageObject(name, arch, version, repo)) + + return packages -- 2.1.0
This unit test module is restricted to YUM distros Signed-off-by: Daniel Henrique Barboza <dhbarboza82@gmail.com> --- tests/test_yumparser.py | 109 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 tests/test_yumparser.py diff --git a/tests/test_yumparser.py b/tests/test_yumparser.py new file mode 100644 index 0000000..bdccfdc --- /dev/null +++ b/tests/test_yumparser.py @@ -0,0 +1,109 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2015 +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +import os +import tempfile +import unittest + +from kimchi.model import model +from kimchi.yumparser import delete_repo_from_file, get_repo_files +from kimchi.yumparser import get_yum_repositories, write_repo_to_file +from kimchi.yumparser import YumRepoObject + + +TEMP_REPO_FILE = '' + + +def _is_yum_distro(): + inst = model.Model('test:///default') + repo_type = inst.capabilities_lookup()['repo_mngt_tool'] + return repo_type == 'yum' + + +def _create_fake_repos(repo_file_name): + repo1 = YumRepoObject('fake-repo-1', repo_file_name) + repo2 = YumRepoObject('fake-repo-2', repo_file_name) + repo3 = YumRepoObject('fake-repo-3', repo_file_name) + repo4 = YumRepoObject('fake-repo-4', repo_file_name) + repos = [repo1, repo2, repo3, repo4] + return repos + + +def _create_fake_repos_file(): + _, tmp_file_name = tempfile.mkstemp(suffix='.repo', + dir='/etc/yum.repos.d') + + fake_repos = _create_fake_repos(tmp_file_name) + file_data = '' + for repo in fake_repos: + file_data += str(repo) + '\n' + + with open(tmp_file_name, 'w') as f: + f.writelines(file_data) + + return tmp_file_name + + +def setUpModule(): + global TEMP_REPO_FILE + TEMP_REPO_FILE = _create_fake_repos_file() + + +def tearDownModule(): + os.remove(TEMP_REPO_FILE) + + +@unittest.skipIf(not _is_yum_distro(), 'Skipping: YUM exclusive test') +class YumParserTests(unittest.TestCase): + + def test_get_yum_repositories(self): + repo_files = get_repo_files() + repo_objects = get_yum_repositories() + self.assertGreaterEqual(len(repo_objects), len(repo_files)) + + def test_update_repo_attributes(self): + repos = get_yum_repositories() + fake_repo_2 = repos['fake-repo-2'] + fake_repo_2.disable() + fake_repo_2.name = 'This is a fake repo' + fake_repo_2.baseurl = 'http://a.fake.repo.url' + fake_repo_2.gpgkey = 'file://a/fake/gpg/key.fake' + fake_repo_2.gpgcheck = False + fake_repo_2.metalink = 'this is not a true metalink' + fake_repo_2.mirrorlist = 'fake mirrorlist' + write_repo_to_file(fake_repo_2) + + repos = get_yum_repositories() + fake_repo_2 = repos['fake-repo-2'] + self.assertEqual(False, fake_repo_2.enabled) + self.assertEqual(False, fake_repo_2.gpgcheck) + self.assertEqual('This is a fake repo', fake_repo_2.name) + self.assertEqual('http://a.fake.repo.url', fake_repo_2.baseurl) + self.assertEqual('file://a/fake/gpg/key.fake', fake_repo_2.gpgkey) + self.assertEqual('this is not a true metalink', fake_repo_2.metalink) + self.assertEqual('fake mirrorlist', fake_repo_2.mirrorlist) + + def test_delete_repo_from_file(self): + repos = get_yum_repositories() + fake_repo_3 = repos['fake-repo-3'] + delete_repo_from_file(fake_repo_3) + + repos = get_yum_repositories() + repos_id = repos.keys() + self.assertNotIn('fake-repo-3', repos_id) -- 2.1.0
Removing YUM API from the code avoids a memory leak in the Host tab each time it is loaded. Signed-off-by: Daniel Henrique Barboza <dhbarboza82@gmail.com> --- src/kimchi/repositories.py | 65 ++++++++++++++-------------------------------- src/kimchi/swupdate.py | 16 +++++------- 2 files changed, 26 insertions(+), 55 deletions(-) diff --git a/src/kimchi/repositories.py b/src/kimchi/repositories.py index e9fc958..20b7311 100644 --- a/src/kimchi/repositories.py +++ b/src/kimchi/repositories.py @@ -29,6 +29,7 @@ from kimchi.config import kimchiLock from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import OperationFailed, NotFoundError, MissingParameter from kimchi.utils import validate_repo_url +from kimchi.yumparser import get_yum_repositories, write_repo_to_file class Repositories(object): @@ -115,25 +116,17 @@ class YumRepo(object): CONFIG_ENTRY = ('repo_name', 'mirrorlist', 'metalink') def __init__(self): - self._yb = getattr(__import__('yum'), 'YumBase') - self._conf = getattr(__import__('yum'), 'config') - self._confdir = self.DEFAULT_CONF_DIR - reposdir = self._yb().conf.reposdir - for d in reposdir: - if os.path.isdir(d): - self._confdir = d - break def _get_repos(self, errcode): try: - yb = self._yb() - yb.doLock() - repos = yb.repos - yb.doUnlock() + kimchiLock.acquire() + repos = get_yum_repositories() except Exception, e: kimchiLock.release() raise OperationFailed(errcode, {'err': str(e)}) + finally: + kimchiLock.release() return repos @@ -141,37 +134,28 @@ class YumRepo(object): """ Return a list of repositories IDs """ - kimchiLock.acquire() repos = self._get_repos('KCHREPOS0024E') - kimchiLock.release() - return repos.repos.keys() + return repos.keys() def getRepo(self, repo_id): """ Return a dictionary in the repositories.Repositories() of the given repository ID format with the information of a YumRepository object. """ - kimchiLock.acquire() repos = self._get_repos('KCHREPOS0025E') - kimchiLock.release() - if repo_id not in repos.repos.keys(): + if repo_id not in repos.keys(): raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id}) - entry = repos.getRepo(repo_id) + entry = repos.get(repo_id) info = {} info['enabled'] = entry.enabled - - baseurl = '' - if entry.baseurl: - baseurl = entry.baseurl[0] - - info['baseurl'] = baseurl + info['baseurl'] = entry.baseurl or '' info['config'] = {} - info['config']['repo_name'] = entry.name + info['config']['repo_name'] = entry.name or '' info['config']['gpgcheck'] = entry.gpgcheck - info['config']['gpgkey'] = entry.gpgkey + info['config']['gpgkey'] = entry.gpgkey or '' info['config']['mirrorlist'] = entry.mirrorlist or '' info['config']['metalink'] = entry.metalink or '' return info @@ -205,10 +189,8 @@ class YumRepo(object): if repo_id is None: repo_id = "kimchi_repo_%s" % str(int(time.time() * 1000)) - kimchiLock.acquire() repos = self._get_repos('KCHREPOS0026E') - kimchiLock.release() - if repo_id in repos.repos.keys(): + if repo_id in repos.keys(): raise InvalidOperation("KCHREPOS0022E", {'repo_id': repo_id}) repo_name = config.get('repo_name', repo_id) @@ -235,13 +217,11 @@ class YumRepo(object): return repo_id def toggleRepo(self, repo_id, enable): - kimchiLock.acquire() repos = self._get_repos('KCHREPOS0011E') - kimchiLock.release() - if repo_id not in repos.repos.keys(): + if repo_id not in repos.keys(): raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id}) - entry = repos.getRepo(repo_id) + entry = repos.get(repo_id) if enable and entry.enabled: raise InvalidOperation("KCHREPOS0015E", {'repo_id': repo_id}) @@ -255,9 +235,8 @@ class YumRepo(object): else: entry.disable() - self._conf.writeRawRepoFile(entry) + write_repo_to_file(entry) except: - kimchiLock.release() if enable: raise OperationFailed("KCHREPOS0020E", {'repo_id': repo_id}) @@ -271,13 +250,11 @@ class YumRepo(object): """ Update a given repository in repositories.Repositories() format """ - kimchiLock.acquire() repos = self._get_repos('KCHREPOS0011E') - kimchiLock.release() - if repo_id not in repos.repos.keys(): + if repo_id not in repos.keys(): raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id}) - entry = repos.getRepo(repo_id) + entry = repos.get(repo_id) baseurl = params.get('baseurl', None) config = params.get('config', {}) @@ -316,7 +293,7 @@ class YumRepo(object): entry.gpgcheck = config.get('gpgcheck', entry.gpgcheck) entry.gpgkey = config.get('gpgkey', entry.gpgkey) kimchiLock.acquire() - self._conf.writeRawRepoFile(entry) + write_repo_to_file(entry) kimchiLock.release() return repo_id @@ -324,13 +301,11 @@ class YumRepo(object): """ Remove a given repository """ - kimchiLock.acquire() repos = self._get_repos('KCHREPOS0027E') - kimchiLock.release() - if repo_id not in repos.repos.keys(): + if repo_id not in repos.keys(): raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id}) - entry = repos.getRepo(repo_id) + entry = repos.get(repo_id) parser = ConfigParser() with open(entry.repofile) as fd: parser.readfp(fd) diff --git a/src/kimchi/swupdate.py b/src/kimchi/swupdate.py index 40af8f3..4869235 100644 --- a/src/kimchi/swupdate.py +++ b/src/kimchi/swupdate.py @@ -24,6 +24,7 @@ from kimchi.basemodel import Singleton from kimchi.config import kimchiLock from kimchi.exception import NotFoundError, OperationFailed from kimchi.utils import kimchi_log, run_command +from kimchi.yumparser import get_yum_packages_list_update class SoftwareUpdate(object): @@ -150,14 +151,12 @@ class YumUpdate(object): Update the list of packages to be updated in the system. """ try: - yb = getattr(__import__('yum'), 'YumBase')() - yb.doLock() - self._pkgs = yb.doPackageLists('updates') - yb.doUnlock() - del yb + kimchiLock.acquire() + self._pkgs = get_yum_packages_list_update() except Exception, e: - kimchiLock.release() raise OperationFailed('KCHPKGUPD0003E', {'err': str(e)}) + finally: + kimchiLock.release() def getPackagesList(self): """ @@ -166,13 +165,10 @@ class YumUpdate(object): package = {'package_name': <string>, 'version': <string>, 'arch': <string>, 'repository': <string>} """ - kimchiLock.acquire() self._refreshUpdateList() - kimchiLock.release() pkg_list = [] for pkg in self._pkgs: - package = {'package_name': pkg.name, - 'version': "%s-%s" % (pkg.version, pkg.release), + package = {'package_name': pkg.name, 'version': pkg.version, 'arch': pkg.arch, 'repository': pkg.ui_from_repo} pkg_list.append(package) return pkg_list -- 2.1.0
'make check-local' is failing as below: Checking for invalid i18n string... Checking for invalid i18n string successfully find . -path './.git' -prune -type f -o \ -name '*.py' -o -name '*.py.in' | xargs /usr/bin/pyflakes | \ grep -w -v "\./src/kimchi/websocket\.py" | \ while read LINE; do echo "$LINE"; false; done ./src/kimchi/yumparser.py:22: 'join' imported but unused ./src/kimchi/yumparser.py:22: 'getmtime' imported but unused Makefile:909: recipe for target 'check-local' failed make: *** [check-local] Error 1 And 'make check' is failing on Ubuntu: ====================================================================== ERROR: setUpModule (test_yumparser) [05/Jun/2015:14:44:45] ENGINE Waiting for child threads to terminate... ---------------------------------------------------------------------- Traceback (most recent call last): File "test_yumparser.py", line 65, in setUpModule TEMP_REPO_FILE = _create_fake_repos_file() File "test_yumparser.py", line 50, in _create_fake_repos_file dir='/etc/yum.repos.d') File "/usr/lib/python2.7/tempfile.py", line 308, in mkstemp return _mkstemp_inner(dir, prefix, suffix, flags) File "/usr/lib/python2.7/tempfile.py", line 239, in _mkstemp_inner fd = _os.open(file, flags, 0600) OSError: [Errno 2] No such file or directory: '/etc/yum.repos.d/tmp_Ml11K.repo' On 04/06/2015 16:49, Daniel Henrique Barboza wrote:
The use of Yum API is causing a memory leak each time the Host tab is loaded. This can be verified by seeing the use of memory in $top.
To remove the Yum API, a new module named 'yumparser' was created to manipulate Yum repositories directly from the filesystem and to return the software update list by parsing $yum check-update command.
A new unit test file was created to verify the proper behavior of the yumparser module.
Daniel Henrique Barboza (3): Adding yumparser module Unit tests for the yumparser module Changing repositories and swupdate to use yumparser module
src/kimchi/repositories.py | 65 ++++------- src/kimchi/swupdate.py | 16 +-- src/kimchi/yumparser.py | 271 +++++++++++++++++++++++++++++++++++++++++++++ tests/test_yumparser.py | 109 ++++++++++++++++++ 4 files changed, 406 insertions(+), 55 deletions(-) create mode 100644 src/kimchi/yumparser.py create mode 100644 tests/test_yumparser.py
participants (2)
-
Aline Manera -
Daniel Henrique Barboza