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.

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

Detect pending upgrades using dist-upgrade #634

Closed
wants to merge 1 commit into from

Conversation

smortex
Copy link
Collaborator

@smortex smortex commented Oct 23, 2016

After changing some apt::source and apt::pin'ing some packages, I was surprised to not see any upgrades in the $::apt_package_updates fact.

Relying on dist-upgrade instead of a bare upgrade made them visible.

Before After
['linux-libc-dev'] ['e2fslibs', 'e2fsprogs', 'libapparmor1', 'libseccomp2', 'libsystemd0', 'libudev1', 'systemd', 'ifupdown', 'udev', 'libfuse2', 'fuse2fs', 'linux-libc-dev', 'ruby-rgen', 'ruby-safe-yaml', 'puppet', 'puppet-common']
['libapr1', 'libconfuse-common', 'libconfuse0', 'libhiredis0.10', 'libvarnishapi1', 'linux-image-3.16.0-4-amd64', 'libserialport0', 'libzip2', 'libsigrok2', 'collectd-core', 'collectd', 'libganglia1', 'librdkafka1', 'linux-compiler-gcc-4.8-x86', 'linux-headers-3.16.0-4-amd64', 'linux-headers-3.16.0-4-common', 'linux-libc-dev', 'ruby-rgen', 'ruby-safe-yaml', 'puppet', 'puppet-common']

@smortex
Copy link
Collaborator Author

smortex commented Nov 1, 2016

Rebasing on top of master now that #633 is merged made TravisCI happy 😄

@daenney
Copy link

daenney commented Jan 14, 2017

I'm not sure this is a great idea... It's normal for dist-upgrade to surface much more packages because of the difference between what upgrade and dist-upgrade do:

upgrade
   upgrade is used to install the newest versions of all packages
   currently installed on the system from the sources enumerated in
   /etc/apt/sources.list. Packages currently installed with new
   versions available are retrieved and upgraded; under no
   circumstances are currently installed packages removed, or packages
   not already installed retrieved and installed. New versions of
   currently installed packages that cannot be upgraded without
   changing the install status of another package will be left at
   their current version. An update must be performed first so that
   apt-get knows that new versions of packages are available.

dist-upgrade
   dist-upgrade in addition to performing the function of upgrade,
   also intelligently handles changing dependencies with new versions
   of packages; apt-get has a "smart" conflict resolution system, and
   it will attempt to upgrade the most important packages at the
   expense of less important ones if necessary. So, dist-upgrade
   command may remove some packages. The /etc/apt/sources.list file
   contains a list of locations from which to retrieve desired package
   files. See also apt_preferences(5) for a mechanism for overriding
   the general settings for individual packages.

dist-upgrade can modify the system in ways that can potentially break things due to how it deals with dependency resolution in this case. It's far more common to run upgrade instead of dist-upgrade and upgrade will still correctly surface security updates and the likes. If you want apt to use these types of packages and essentially upgrade beyond the release's original version boundaries (e.g by using backports) you should change the priorities in order to reflect that which will make upgrade surface the updates.

That said, I think seeing these packages could be useful but they should probably be a separate fact. Especially since the current fact might be used in places to automate updates which up until now has been safe to do based on this fact's output. Your suggested change risks breaking a system.

@nhinds
Copy link

nhinds commented Jan 15, 2017

Prior to puppetlabs-apt 2.3 (which switched away from the apt-check binary to using apt-get in ebf9d9f), packages that are only upgradable via apt-get dist-upgrade were already returned. The old command apt-check -p listed all packages that could be upgraded, whereas the current method of apt-get -s upgrade only lists packages that will be upgraded if the user runs apt-get upgrade.

Example: I have a 16.04 VM which has just been booted from an old cloud image. Running apt-get -s upgrade shows that some vim packages are being held back, and these packages do not show up with Inst lines.

# /usr/bin/apt-get -s upgrade
Reading package lists...
Building dependency tree...
Reading state information...
Calculating upgrade...
The following packages have been kept back:
  vim vim-common vim-runtime vim-tiny
...

The apt_package_updates fact would not list these vim packages as being available for update. However, the previous apt-check -p command does list these packages:

# /usr/lib/update-notifier/apt-check -p 2>&1 | grep vim
vim-common
vim-runtime
vim
vim-tiny

In my case, the results of running apt-get -s dist-upgrade exactly match the set of packages that would previously have been returned from apt-check -p - i.e. the apt dist-upgrade method is actually more compatible with the pre-2.3 behaviour of this puppet module than apt upgrade:

root@app-proxy:~# /usr/lib/update-notifier/apt-check -p 2>&1 | sort > /tmp/apt-check
root@app-proxy:~# /usr/bin/apt-get -s upgrade | grep Inst | awk '{ print $2 }'|sort > /tmp/apt-get-upgrade
root@app-proxy:~# /usr/bin/apt-get -s dist-upgrade | grep Inst | awk '{ print $2 }'|sort > /tmp/apt-get-dist-upgrade
root@app-proxy:~# diff /tmp/apt-check /tmp/apt-get-dist-upgrade 
root@app-proxy:~# diff /tmp/apt-get-upgrade /tmp/apt-get-dist-upgrade 
47a48
> libpython3.5
78a80,83
> vim
> vim-common
> vim-runtime
> vim-tiny
root@app-proxy:~# diff /tmp/apt-check /tmp/apt-get-upgrade 
48d47
< libpython3.5
80,83d78
< vim
< vim-common
< vim-runtime
< vim-tiny

I would prefer this puppet module go back to the previous behaviour of listing all packages that can be upgraded, either by using apt-check (if it's present) or by using apt-get -s dist-upgrade to get the full list of packages that can be upgraded.

@smortex
Copy link
Collaborator Author

smortex commented Jan 16, 2017

@daenney, since we are only listing outdated packages into a fact (without taking any action), I would expect the apt_package_updates fact to report any pending update.

Relying on upgrade rather on dist-upgrade only makes sense for me in regard to actually blindly applying upgrades (changing the system's state).

Do you have in mind some use-cases where having only a part of the available updates (those that will be installed with apt-get upgrade) is more valuable than having all available updates?

@daenney
Copy link

daenney commented Jan 16, 2017

What I want to guard against is that by changing this fact you can cause problems for others. If this was in any way relied upon by people to, for example, automate the upgrades of their hosts this could cause issue. Since all these facts are queriable from PuppetDB for example there's no knowing what other tools have been built on top of it.

However since it seems that this used to be the behaviour we had before I don't think it's a problem to change it again. But, it should be an explicit decision to do so and to ensure that in the future that is kept. We can't keep doing this where we change an implementation of a fact in point releases which changes the packages it will consider as out of date versus not.

@smortex
Copy link
Collaborator Author

smortex commented Jan 17, 2017

OK, I understand your point: I considered this to be a regression when updating from 2.2.2 to 2.3.0, that only deserved a minor bump to 2.4.0 according to this semver faq item, and as I use cron-apt(8) to install updates, I did not considered applying them through the manifest.

Pinging @robinelfrink, the author of #581 where this behavior has changed, in case he has anything to say in regard to this.

If everyone agrees, maybe this PR can be labelled e.g. "3.0.0"?

The `upgrade` action only reports updated packages that have no changed
dependencies.  You must use `dist-upgrade` to see such upgrades.
@smortex
Copy link
Collaborator Author

smortex commented Sep 6, 2017

This PR becomes oldish…

I just rebased this on top of master. I had no news since I proposed to add this as part of a future major release, and 2 major releases have shipped since then 😞

@daenney
Copy link

daenney commented Sep 6, 2017

Ping @hunner

@tphoney
Copy link

tphoney commented Sep 26, 2017

i am late to the show here, does it make sense that this is a different fact ? so people can pick and choose ?

@willmeek
Copy link

willmeek commented Nov 9, 2017

@smortex Thank you for your contribution.

Based on your suggestion, dist-upgrade has been added as a separate fact as per PR #719 , and should be released during the next release.

A separate fact will allow end users to pick and choose between either.

I'm therefore closing off this PR.

Thank you
Will

@willmeek willmeek closed this Nov 9, 2017
@smortex
Copy link
Collaborator Author

smortex commented Nov 9, 2017

Excellent! Thanks for the heads-up @willmeek!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants