(PUP-5802) Make the Yum package provider correctly parse epochs

[mveytsman]

Without this patch applied, the Yum package provider assumes all version
epochs are "0", i.e. a package with version "1:1.2.3-4.el5" will be
interpreted as having epoch "0" not "1". This causes version comparisons
made by puppet to be incorrect when comparing packages of different
epoch's.

This bug is caused by the line:

e = String(Bignum(e)) rescue '0'

Bignum is not a function, so the line always throws an exception and the
rescue is always invoked.

This patch uses Integer instead of Bignum, rescues the specific
exception needed (this is so that an epoch like "foo" is coerced into
"0"), and provides a test for non-zero epochs

[whopper]

@mveytsman interesting! Thanks for the contribution!

We've run into lots of trouble around Yum / DNF epoch parsing (fairly recently, even. See https://tickets.puppetlabs.com/browse/PUP-5549). We'll be wanting to have a careful look at this to make sure all is well in both Yum and DNF. From a glance, this seems sane, and we plan to take time to do more review on our end soon.

[MikaelSmith]

So I tried this on CentOS 7 with puppet apply -e "package {'vim-minimal': ensure => '2:7.4.160'}" and got

Error: Could not update: Execution of '/usr/bin/yum -d 0 -e 0 -y install vim-minimal-2:7.4.160' returned 1: Error: Nothing to do
Error: /Stage[main]/Main/Package[vim-minimal]/ensure: change from purged to 2:7.4.160 failed: Could not update: Execution of '/usr/bin/yum -d 0 -e 0 -y install vim-minimal-2:7.4.160' returned 1: Error: Nothing to do
Notice: Applied catalog in 0.38 seconds

That doesn't seem right. Do you get successful behavior?

[whopper]

@MikaelSmith yeah, I thought that might be the case. Yum requires you to specify the arch field when you are providing an epoch, like so:

epoch:name-ver-rel.arch

When we specify the version '2:7.4.160' Puppet doesn't understand this constraint, as it doesn't have a field for architecture. We then end up calling: yum install vim-minimal-2:7.4.160 rather than yum install vim-minimal-2:7.4.160.x86_64. https://tickets.puppetlabs.com/browse/PUP-1364 describes this problem, which has been around forever. The yum provider has never had the ability to understand version differences when epochs are involved for this reason.

The only way to make this work is to sidestep version comparisons and hardcode your desired version like so: package { "firefox-3.0.12-1.el5.centos.x86_64": ensure => "present" }

[MikaelSmith]

I'm fine merging this, but I'd like to understand the use-case it fixes @mveytsman

[mveytsman]

@MikaelSmith

Before this patch puppet's version comparison would ignore epochs. For instance, if a user had package with a version "1.2" installed, and asked to install "3:1.2", we would not recognize these as different versions.

yum_parse_evr is called several times in install (

puppet/lib/puppet/provider/package/yum.rb

Lines 133 to 179 in b1d2551

	def install
	wanted = @resource[:name]
	error_level = self.class.error_level
	# If not allowing virtual packages, do a query to ensure a real package exists
	unless @resource.allow_virtual?
	execute([command(:cmd), '-d', '0', '-e', error_level, '-y', install_options, :list, wanted].compact)
	end
	should = @resource.should(:ensure)
	self.debug "Ensuring => #{should}"
	operation = :install
	case should
	when true, false, Symbol
	# pass
	should = nil
	else
	# Add the package version
	wanted += "-#{should}"
	is = self.query
	if is && yum_compareEVR(yum_parse_evr(should), yum_parse_evr(is[:ensure])) < 0
	self.debug "Downgrading package #{@resource[:name]} from version #{is[:ensure]} to #{should}"
	operation = :downgrade
	end
	end
	# Yum on el-4 and el-5 returns exit status 0 when trying to install a package it doesn't recognize;
	# ensure we capture output to check for errors.
	no_debug = if Facter.value(:operatingsystemmajrelease).to_i > 5 then ["-d", "0"] else [] end
	command = [command(:cmd)] + no_debug + ["-e", error_level, "-y", install_options, operation, wanted].compact
	output = execute(command)
	if output =~ /^No package #{wanted} available\.$/
	raise Puppet::Error, "Could not find package #{wanted}"
	end
	# If a version was specified, query again to see if it is a matching version
	if should
	is = self.query
	raise Puppet::Error, "Could not find package #{self.name}" unless is
	# FIXME: Should we raise an exception even if should == :latest
	# and yum updated us to a version other than @param_hash[:ensure] ?
	vercmp_result = yum_compareEVR(yum_parse_evr(should), yum_parse_evr(is[:ensure]))
	raise Puppet::Error, "Failed to update to version #{should}, got version #{is[:ensure]} instead" if vercmp_result != 0
	end
	end

), and I belive this bug will cause incorrect behavior such as what @whopper mentioned --- https://tickets.puppetlabs.com/browse/PUP-5549)

[whopper]

@mveytsman would you mind pasting a manifest that this fixes? I'm trying to work up a reproducible test case but am having no luck thus far.

[mveytsman]

@whopper I don't have one on hand -- I came across the issue reading the code after reading a blog post that references it http://blog.jasonantman.com/2014/07/how-yum-and-rpm-compare-versions/ and I found the bug through code review.

The unit test I include, https://github.com/mveytsman/puppet/blob/fix/master/yumepochparsing/spec/unit/provider/package/yum_spec.rb#L54-L59, will fail without my patch, which is how i concluded that puppet always sets the epoch to 0.