1:47 p.m.
From: Daniel Henrique Barboza <dhbarboza82(a)gmmail.com>
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
1:47 p.m.
New subject: [PATCH 1/3] Adding yumparser module
From: Daniel Henrique Barboza <dhbarboza82(a)gmmail.com>
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(a)gmmail.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
1:47 p.m.
New subject: [PATCH 2/3] Unit tests for the yumparser module
From: Daniel Henrique Barboza <dhbarboza82(a)gmmail.com>
This unit test module is restricted to YUM distros
Signed-off-by: Daniel Henrique Barboza <dhbarboza82(a)gmmail.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)
+
+
+(a)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
1:47 p.m.
New subject: [PATCH 3/3] Changing repositories and swupdate to use yumparser module
From: Daniel Henrique Barboza <dhbarboza82(a)gmmail.com>
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(a)gmmail.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
9:51 p.m.
Please ignore this patchset and consider the v2 I've sent.
These patches have wrong author email information.
On 06/04/2015 08:47 AM, Daniel Henrique Barboza wrote:
From: Daniel Henrique Barboza <dhbarboza82(a)gmmail.com>
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
3242
days inactive
3242
days old
4 comments
1 participants
participants (1)
-
Daniel Henrique Barboza