Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#19587 add yum skip-broken #21475

Merged
merged 9 commits into from
Feb 21, 2017
Merged

#19587 add yum skip-broken #21475

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9e15d31
#19587: refactor to PEP8
Feb 15, 2017
7ac7ddb
Issue: #19587: add skip_broken option to yum module
Feb 15, 2017
06209dc
Issue: #19587: fix line break, attempt to make readble
Feb 15, 2017
d2da1d1
#19587: fixing typo, skip-broken has to be passed as list
Feb 16, 2017
52455b6
PR: #21475: peer review changes
Feb 16, 2017
9b3968f
PR: #21475: peer review changes
Feb 16, 2017
2d0c1da
Issue: #21475: fixing typo in the Man section
Feb 16, 2017
6c8d199
PR: #21475 - Peer review: revert variable a
Feb 18, 2017
8249dd3
Issue: 19587 - small correction
Feb 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 39 additions & 19 deletions lib/ansible/modules/packaging/os/yum.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
options:
name:
description:
- "Package name, or package specifier with version, like C(name-1.0). When using state=latest, this can be '*' which means run: yum -y update. You can also pass a url or a local path to a rpm file (using state=present). To operate on several packages this can accept a comma separated list of packages or (as of 2.0) a list of packages."
- "Package name, or package specifier with version, like C(name-1.0). When using state=latest, this can be '*' which means run: yum -y update.
You can also pass a url or a local path to a rpm file (using state=present). To operate on several packages this can accept a comma separated list
of packages or (as of 2.0) a list of packages."
required: true
default: null
aliases: [ 'pkg' ]
Expand Down Expand Up @@ -94,6 +96,16 @@
choices: ["yes", "no"]
aliases: []

skip_broken:
description:
- Resolve depsolve problems by removing packages that are causing problems from the trans‐
action.
required: false
version_added: "2.3"
default: "no"
choices: ["yes, "no"]
aliases: []

update_cache:
description:
- Force yum to check if cache is out of date and redownload if needed.
Expand Down Expand Up @@ -631,7 +643,18 @@ def list_stuff(module, repoquerybin, conf_file, stuff, installroot='/'):
elif stuff == 'repos':
return [ dict(repoid=name, state='enabled') for name in sorted(repolist(module, repoq)) if name.strip() ]
else:
return [ pkg_to_dict(p) for p in sorted(is_installed(module, repoq, stuff, conf_file, qf=is_installed_qf, installroot=installroot) + is_available(module, repoq, stuff, conf_file, qf=qf, installroot=installroot)) if p.strip() ]
return [ pkg_to_dict(p) for p in sorted(is_installed(module,
repoq,
stuff,
conf_file,
qf=is_installed_qf,
installroot=installroot) +
is_available(module,
repoq,
stuff,
conf_file,
qf=qf,
installroot=installroot)) if p.strip() ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is horrible formatting ;-) If you want to break this up, make logical divisions more like this:

return [ pkg_to_dict(p) for p in sorted(
                  is_installed(module, repoq, stuff, conf_file, qf=is_installed_qf, installroot=installroot) +
                  is_available(module, repoq, stuff, conf_file, qf=qf, installroot=installroot))
            if p.strip() ]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack, will do was not sure how do format this 100 organizations 100 formatting styles :-D


def install(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos, installroot='/'):

Expand Down Expand Up @@ -988,10 +1011,13 @@ def latest(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos, in
for w in will_update:
if w.startswith('@'):
to_update.append((w, None))
msg = '%s will be updated' % w
res['msg'] += '%s will be updated' % w
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not even sure why that previous msg = '%s will be updated' % w was there, just tried to 'guess'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Judging from the other things in this conditional, it probably should be appended to to_update?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case I think it should be removed at all cause to_updated get's updated on previous line

elif w not in updates:
other_pkg = will_update_from_other_package[w]
to_update.append((w, 'because of (at least) %s-%s.%s from %s' % (other_pkg, updates[other_pkg]['version'], updates[other_pkg]['dist'], updates[other_pkg]['repo'])))
to_update.append((w, 'because of (at least) %s-%s.%s from %s' % (other_pkg,
updates[other_pkg]['version'],
updates[other_pkg]['dist'],
updates[other_pkg]['repo'])))
else:
to_update.append((w, '%s.%s from %s' % (updates[w]['version'], updates[w]['dist'], updates[w]['repo'])))

Expand Down Expand Up @@ -1038,7 +1064,7 @@ def latest(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos, in
return res

def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo,
disable_gpg_check, exclude, repoq, installroot='/'):
disable_gpg_check, exclude, repoq, skip_broken, installroot='/'):

# fedora will redirect yum to dnf, which has incompatibilities
# with how this module expects yum to operate. If yum-deprecated
Expand All @@ -1058,6 +1084,10 @@ def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo,

dis_repos =[]
en_repos = []

if skip_broken:
yum_basecmd.extend(['--skip-broken'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is skip-broken allowed for all of the commands that we run? (I've used it with yum update but never with yum remove or yum install).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep https://linux.die.net/man/8/yum --skip-broken is in the general section meaning it can be added to any other action.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use it quite often during Maintenance windows and mass server updates, we don't want to waste time to solve problems on failed machines, we just --skip-update and later follow-up what are the deps issues.


if disablerepo:
dis_repos = disablerepo.split(',')
r_cmd = ['--disablerepo=%s' % disablerepo]
Expand Down Expand Up @@ -1095,7 +1125,7 @@ def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo,
for i in new_repos:
if not i in current_repos:
rid = my.repos.getRepo(i)
a = rid.repoXML.repoid
a = rid.repoXML.repoid ## if no one complains remove this on next MR
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is one of the var complaints, the var is assigned but not really used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the yum code for sideeffects (repoid might be a property and referencing it might cause some action to happen). If this is a simply attribute rather than something producing side-effects, feel free to get rid of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  3 import yum                                                                      
  4 from json import dumps                                                          
  5                                                                                 
  6 instance = yum.YumBase()                                                        
  7 print "###################################"                                     
  8 for repo in instance.repos.listEnabled():                                       
  9   print  type(repo.repoXML.repoid)

says

###################################
Loaded plugins: product-id
Repo rhel-7-server-satellite-tools-6.2-rpms forced skip_if_unavailable=True due to: /etc/pki/entitlement/7092233073807600698-key.pem
Repo rhel-server-rhscl-7-eus-source-rpms forced skip_if_unavailable=True due to: /etc/pki/entitlement/7092233073807600698-key.pem
Repo rhel-7-server-extras-rpms forced skip_if_unavailable=True due to: /etc/pki/entitlement/7092233073807600698-key.pem
Repo rhel-7-server-optional-rpms forced skip_if_unavailable=True due to: /etc/pki/entitlement/7092233073807600698-key.pem
Repo rhel-7-server-rpms forced skip_if_unavailable=True due to: /etc/pki/entitlement/7092233073807600698-key.pem
Repo rhel-7-fast-datapath-rpms forced skip_if_unavailable=True due to: /etc/pki/entitlement/7092233073807600698-key.pem
Repo rhel-7-server-tus-rpms forced skip_if_unavailable=True due to: /etc/pki/entitlement/7092233073807600698-key.pem
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
<type 'str'>
[vmindru@vmutil python]$

so i guess it's a string.

~

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will remove that, it's basically repo name string.

current_repos = new_repos
except yum.Errors.YumBaseError:
e = get_exception()
Expand All @@ -1117,23 +1147,11 @@ def ensure(module, state, pkgs, conf_file, enablerepo, disablerepo,
# should be caught by AnsibleModule argument_spec
module.fail_json(msg="we should never get here unless this all"
" failed", changed=False, results='', errors='unexpected state')

return res


def main():

# state=installed name=pkgspec
# state=removed name=pkgspec
# state=latest name=pkgspec
#
# informational commands:
# list=installed
# list=updates
# list=available
# list=repos
# list=pkgspec

module = AnsibleModule(
argument_spec = dict(
name=dict(aliases=['pkg'], type="list"),
Expand All @@ -1145,6 +1163,7 @@ def main():
list=dict(),
conf_file=dict(default=None),
disable_gpg_check=dict(required=False, default="no", type='bool'),
skip_broken=dict(required=False, default="no", aliases=[], type='bool'),
update_cache=dict(required=False, default="no", aliases=['expire-cache'], type='bool'),
validate_certs=dict(required=False, default="yes", type='bool'),
installroot=dict(required=False, default="/", type='str'),
Expand Down Expand Up @@ -1201,8 +1220,9 @@ def main():
enablerepo = params.get('enablerepo', '')
disablerepo = params.get('disablerepo', '')
disable_gpg_check = params['disable_gpg_check']
skip_broken = params['skip_broken']
results = ensure(module, state, pkg, params['conf_file'], enablerepo,
disablerepo, disable_gpg_check, exclude, repoquery,
disablerepo, disable_gpg_check, exclude, repoquery, skip_broken,
params['installroot'])
if repoquery:
results['msg'] = '%s %s' % (results.get('msg',''),
Expand Down