Reviewed-by: Daniel Barboza <dhbarboza82@gmail.com> Tested-by: Daniel Barboza <dhbarboza82@gmail.com> This patch also fixes github #630 (test_repository_update in test_model.py). On 03/26/2015 04:49 PM, Aline Manera wrote:

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com>

On 24/03/2015 15:16, Crístian Viana wrote:

...

When the user tries to update a YUM repository and some of the variables are not passed, the code will try to use those variables anyway and an exception will be raised.

Check if some YUM repository variables are empty before using them.

Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/repositories.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/kimchi/repositories.py b/src/kimchi/repositories.py index f1e1eb3..19c5810 100644 --- a/src/kimchi/repositories.py +++ b/src/kimchi/repositories.py @@ -275,10 +275,10 @@ class YumRepo(object): config = params.get('config', {}) mirrorlist = config.get('mirrorlist', None) - if len(baseurl.strip()) == 0: + if baseurl is not None and len(baseurl.strip()) == 0: baseurl = None - if len(mirrorlist.strip()) == 0: + if mirrorlist is not None and len(mirrorlist.strip()) == 0: mirrorlist = None if baseurl is None and mirrorlist is None:

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 24/03/2015 15:16, Crístian Viana wrote:

The current error message when the user provides an invalid mirror list URL implies that the repository is DEB-based, but that error message is used for other repository types as well (e.g. YUM).

Update the repository mirror list error message in order to be valid for all repository types.

Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index e4e1a89..dc0830a 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -292,7 +292,7 @@ messages = { "KCHREPOS0004E": _("Distribution to DEB repository must be a string"), "KCHREPOS0005E": _("Components to DEB repository must be listed in a array"), "KCHREPOS0006E": _("Components to DEB repository must be a string"), - "KCHREPOS0007E": _("Mirror list to DEB repository must be a string"), + "KCHREPOS0007E": _("Mirror list to repository must be a string"), "KCHREPOS0008E": _("YUM Repository name must be string."), "KCHREPOS0009E": _("GPG check must be a boolean value."), "KCHREPOS0010E": _("GPG key must be a URL pointing to the ASCII-armored file."),

On 24/03/2015 15:16, Crístian Viana wrote:

YUM repositories have a field called 'metalink', which is another method to specify how to reach the repository data. Kimchi, however, is unaware of that field.

Allow creating and updating a YUM repository with a metalink value, and return the field 'config > metalink' when listing a YUM repository.

Fix issue #461 (repositories: Kimchi does not display metalink information).

Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/API.json | 5 +++++ src/kimchi/i18n.py | 3 ++- src/kimchi/repositories.py | 21 +++++++++++++++++---- 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/kimchi/API.json b/src/kimchi/API.json index f507251..50de8c7 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -701,6 +701,11 @@ "description": "URL to a file containing a list of baseurls", "type": "string", "error": "KCHREPOS0007E" + }, + "metalink": { + "description": "URL to a metalink file for the repomd.xml", + "type": "string", + "error": "KCHREPOS0029E" } } } diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index dc0830a..472e891 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -298,7 +298,7 @@ messages = { "KCHREPOS0010E": _("GPG key must be a URL pointing to the ASCII-armored file."), "KCHREPOS0011E": _("Could not update repository %(repo_id)s."), "KCHREPOS0012E": _("Repository %(repo_id)s does not exist."), - "KCHREPOS0013E": _("Specify repository base URL or mirror list in order to create or update a YUM repository."), + "KCHREPOS0013E": _("Specify repository base URL, mirror list or metalink in order to create or update a YUM repository."), "KCHREPOS0014E": _("Repository management tool was not recognized for your system."), "KCHREPOS0015E": _("Repository %(repo_id)s is already enabled."), "KCHREPOS0016E": _("Repository %(repo_id)s is already disabled."), @@ -314,6 +314,7 @@ messages = { "KCHREPOS0026E": _("Unable to add repository. Details: '%(err)s'"), "KCHREPOS0027E": _("Unable to remove repository. Details: '%(err)s'"), "KCHREPOS0028E": _("Configuration items: '%(items)s' are not supported by repository manager"), + "KCHREPOS0029E": _("Repository metalink must be an http://, ftp:// or file:// URL."),

"KCHSNAP0001E": _("Virtual machine '%(vm)s' must be stopped before creating a snapshot of it."), "KCHSNAP0002E": _("Unable to create snapshot '%(name)s' on virtual machine '%(vm)s'. Details: %(err)s"), diff --git a/src/kimchi/repositories.py b/src/kimchi/repositories.py index 19c5810..0a69cb1 100644 --- a/src/kimchi/repositories.py +++ b/src/kimchi/repositories.py @@ -112,7 +112,7 @@ class YumRepo(object): """ TYPE = 'yum' DEFAULT_CONF_DIR = "/etc/yum.repos.d" - CONFIG_ENTRY = ('repo_name', 'mirrorlist') + CONFIG_ENTRY = ('repo_name', 'mirrorlist', 'metalink')

def __init__(self): self._yb = getattr(__import__('yum'), 'YumBase') @@ -173,6 +173,7 @@ class YumRepo(object): info['config']['gpgcheck'] = entry.gpgcheck info['config']['gpgkey'] = entry.gpgkey info['config']['mirrorlist'] = entry.mirrorlist or '' + info['config']['metalink'] = entry.metalink or '' return info

def addRepo(self, params): @@ -184,7 +185,8 @@ class YumRepo(object):

config = params.get('config', {}) mirrorlist = config.get('mirrorlist', '') - if not baseurl and not mirrorlist: + metalink = config.get('metalink', '') + if not baseurl and not mirrorlist and not metalink: raise MissingParameter("KCHREPOS0013E")

if baseurl: @@ -193,6 +195,9 @@ class YumRepo(object): if mirrorlist: validate_repo_url(mirrorlist)

+ if metalink: + validate_repo_url(metalink) + repo_id = params.get('repo_id', None) if repo_id is None: repo_id = "kimchi_repo_%s" % str(int(time.time() * 1000)) @@ -206,7 +211,7 @@ class YumRepo(object): repo_name = config.get('repo_name', repo_id) repo = {'baseurl': baseurl, 'mirrorlist': mirrorlist, 'name': repo_name, 'gpgcheck': 1, - 'gpgkey': [], 'enabled': 1} + 'gpgkey': [], 'enabled': 1, 'metalink': metalink}

# write a repo file in the system with repo{} information. parser = ConfigParser()

@@ -274,6 +279,7 @@ class YumRepo(object): baseurl = params.get('baseurl', None) config = params.get('config', {}) mirrorlist = config.get('mirrorlist', None) + metalink = config.get('metalink', None)

if baseurl is not None and len(baseurl.strip()) == 0: baseurl = None @@ -281,7 +287,10 @@ class YumRepo(object): if mirrorlist is not None and len(mirrorlist.strip()) == 0: mirrorlist = None

- if baseurl is None and mirrorlist is None: + if metalink is not None and len(metalink.strip()) == 0: + metalink = None + + if baseurl is None and mirrorlist is None and metalink is None: raise MissingParameter("KCHREPOS0013E")

We can use '' (empty string) as the default value while getting the information from the data sent and simplify that code in a similar way we did on addRepo() config = params.get('config', {}) mirrorlist = config.get('mirrorlist', '') - if not baseurl and not mirrorlist: + metalink = config.get('metalink', '') + if not baseurl and not mirrorlist and not metalink: raise MissingParameter("KCHREPOS0013E")

if baseurl is not None: @@ -292,6 +301,10 @@ class YumRepo(object): validate_repo_url(mirrorlist) entry.mirrorlist = mirrorlist

+ if metalink is not None: + validate_repo_url(metalink) + entry.metalink = metalink + entry.id = params.get('repo_id', repo_id) entry.name = config.get('repo_name', entry.name) entry.gpgcheck = config.get('gpgcheck', entry.gpgcheck)