From d3268d05ec0e186b40ec4604bff369d7a434c5d3 Mon Sep 17 00:00:00 2001 From: Hector Acosta Date: Tue, 24 Jun 2014 01:10:57 -0500 Subject: [PATCH] Correctly handle .repo files in zypper_repository module Before the changes, removing a repository required a repo url. This shouldn't be required since zypper allows removing a repo based on its alias (mapped to name in this module). The name variable was always required, which is misleading since repofiles provide their own alias. So a runtime check was added to avoid this confusion. Additionaly, running this module on .repo files weren't idempotent. e.g Before: $ ./hacking/test-module -m library/packaging/zypper_repository -a "repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo name=foo" {"repo": "http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo", "state": "present", "changed": true} $ ./hacking/test-module -m library/packaging/zypper_repository -a "repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo name=foo" {"msg": "Repository named 'devel_languages_python' already exists. Please use another alias.\n", "failed": true} After: $ ./hacking/test-module -m library/packaging/zypper_repository -a "repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo" {"repo": "http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo", "state": "present", "changed": true} $ ./hacking/test-module -m library/packaging/zypper_repository -a "repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo" {"repo": "http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo", "state": "present", "changed": false} Signed-off-by: Hector Acosta --- library/packaging/zypper_repository | 108 ++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 23 deletions(-) diff --git a/library/packaging/zypper_repository b/library/packaging/zypper_repository index 2dc177dc7bf..26b43a66731 100644 --- a/library/packaging/zypper_repository +++ b/library/packaging/zypper_repository @@ -29,15 +29,15 @@ description: - Add or remove Zypper repositories on SUSE and openSUSE options: name: - required: true + required: false default: none description: - - A name for the repository. + - A name for the repository. Not required when adding repofiles. repo: - required: true + required: false default: none description: - - URI of the repository or .repo file. + - URI of the repository or .repo file. Required when state=present. state: required: false choices: [ "absent", "present" ] @@ -68,15 +68,53 @@ EXAMPLES = ''' # Remove NVIDIA repository - zypper_repository: name=nvidia-repo repo='ftp://download.nvidia.com/opensuse/12.2' state=absent + +# Add python development repository +- zypper_repository: repo=http://download.opensuse.org/repositories/devel:/languages:/python/SLE_11_SP3/devel:languages:python.repo ''' +from xml.dom.minidom import parseString as parseXML +REPO_OPTS = ['alias', 'name', 'priority', 'enabled', 'autorefresh', 'gpgcheck'] -def repo_exists(module, repo): - """Return (rc, stdout, stderr, found) tuple""" - cmd = ['/usr/bin/zypper', 'lr', '--uri'] - rc, stdout, stderr = module.run_command(cmd, check_rc=False) - return (rc, stdout, stderr, repo in stdout) +def _parse_repos(module): + """parses the output of zypper -x lr and returns a parse repo dictionary""" + cmd = ['/usr/bin/zypper', '-x', 'lr'] + repos = [] + + rc, stdout, stderr = module.run_command(cmd, check_rc=True) + dom = parseXML(stdout) + repo_list = dom.getElementsByTagName('repo') + for repo in repo_list: + opts = {} + for o in REPO_OPTS: + opts[o] = repo.getAttribute(o) + opts['url'] = repo.getElementsByTagName('url')[0].firstChild.data + # A repo can be uniquely identified by an alias + url + repos.append(opts) + + return repos + + +def repo_exists(module, **kwargs): + + def repo_subset(realrepo, repocmp): + for k in repocmp: + if k not in realrepo: + return False + + for k, v in realrepo.items(): + if k in repocmp: + if v != repocmp[k]: + return False + return True + + repos = _parse_repos(module) + + for repo in repos: + if repo_subset(repo, kwargs): + return True + return False def add_repo(module, repo, alias, description, disable_gpg_check): @@ -95,15 +133,27 @@ def add_repo(module, repo, alias, description, disable_gpg_check): rc, stdout, stderr = module.run_command(cmd, check_rc=False) changed = rc == 0 - return (rc, stdout, stderr, changed) + if rc == 0: + changed = True + elif 'already exists. Please use another alias' in stderr: + changed = False + else: + module.fail_json(msg=stderr if stderr else stdout) + return changed -def remove_repo(module, repo): - cmd = ['/usr/bin/zypper', 'rr', repo] - rc, stdout, stderr = module.run_command(cmd, check_rc=False) +def remove_repo(module, repo, alias): + + cmd = ['/usr/bin/zypper', 'rr'] + if alias: + cmd.append(alias) + else: + cmd.append(repo) + + rc, stdout, stderr = module.run_command(cmd, check_rc=True) changed = rc == 0 - return (rc, stdout, stderr, changed) + return changed def fail_if_rc_is_null(module, rc, stdout, stderr): @@ -114,8 +164,8 @@ def fail_if_rc_is_null(module, rc, stdout, stderr): def main(): module = AnsibleModule( argument_spec=dict( - name=dict(required=True), - repo=dict(required=True), + name=dict(required=False), + repo=dict(required=False), state=dict(choices=['present', 'absent'], default='present'), description=dict(required=False), disable_gpg_check = dict(required=False, default='no', type='bool'), @@ -132,22 +182,34 @@ def main(): def exit_unchanged(): module.exit_json(changed=False, repo=repo, state=state, name=name) - rc, stdout, stderr, exists = repo_exists(module, repo) - fail_if_rc_is_null(module, rc, stdout, stderr) + # Check run-time module parameters + if state == 'present' and not repo: + module.fail_json(msg='Module option state=present requires repo') + if state == 'absent' and not repo and not name: + module.fail_json(msg='Alias or repo parameter required when state=absent') + + if repo and repo.endswith('.repo'): + if name: + module.fail_json(msg='Incompatible option: \'name\'. Do not use name when adding repo files') + else: + if not name: + module.fail_json(msg='Name required when adding non-repo files:') + + if repo and repo.endswith('.repo'): + exists = repo_exists(module, url=repo, alias=name) + else: + exists = repo_exists(module, alias=name) if state == 'present': if exists: exit_unchanged() - result = add_repo(module, repo, name, description, disable_gpg_check) + changed = add_repo(module, repo, name, description, disable_gpg_check) elif state == 'absent': if not exists: exit_unchanged() - result = remove_repo(module, repo) - - rc, stdout, stderr, changed = result - fail_if_rc_is_null(module, rc, stdout, stderr) + changed = remove_repo(module, repo, name) module.exit_json(changed=changed, repo=repo, state=state)