Bug 998424 - Register RHEL5-Server-GOLD system and notice cdparanoia erratum is not listed as applicable although it is
Register RHEL5-Server-GOLD system and notice cdparanoia erratum is not listed...
Status: CLOSED CURRENTRELEASE
Product: Red Hat Satellite 5
Classification: Red Hat
Component: Server (Show other bugs)
560
Unspecified Unspecified
high Severity medium
: ---
: ---
Assigned To: Stephen Herr
Jan Hutař
:
Depends On:
Blocks: sat560-blockers
  Show dependency treegraph
 
Reported: 2013-08-19 05:49 EDT by Jan Hutař
Modified: 2013-10-01 17:58 EDT (History)
3 users (show)

See Also:
Fixed In Version: spacewalk-schema-2.0.2-10-sat
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-10-01 17:58:44 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jan Hutař 2013-08-19 05:49:40 EDT
Description of problem:
Register RHEL5-Server-GOLD system and notice cdparanoia erratum is not listed as applicable although it is (system have "cdparanoia-alpha9.8-27.2", RHEL5 channel have that one and "cdparanoia-alpha9.8-28.x86_64")


Version-Release number of selected component (if applicable):
Client:
  rhn-client-tools-0.4.13-1.el5
  yum-rhn-plugin-0.4.3-1.el5
  rhnlib-2.2.5-1.el5
Server:
  satellite-schema-5.6.0.8-1.el6sat.noarch


How reproducible:
3 of 3 attempts (but all with one Satellite and one client)


Steps to Reproduce:
1. Register RHEL5-Server-GOLD system with "cdparanoia-alpha9.8-27.2" installed
   into RHEL5 channel with "cdparanoia-alpha9.8-28.x86_64"
2. Go to Systems -> <your_system> -> Software -> Errata
   and filter by "cdparanoia"
3. Go to Systems -> <your_system> -> Software -> Packages -> Upgrade
   and filter by "cdparanoia"


Actual results:
In both 2. and 3., update to "cdparanoia-alpha9.8-28.x86_64" is not listed as available.

I have not noticed anything strange in the logs.


Expected results:
When on Oracle variant, in both 2. and 3. I see available upgrade.


Additional info:
Maybe this is a duplicate or relevant to bug 985947.
Comment 1 Stephen Herr 2013-08-20 16:26:32 EDT
This error is actually a result of a problem in the version comparison sql.

select rpm.rpmstrcmp('0', 'alpha9.8') from dual;

should return -1 (meaning 0 is smaller than alpha9.0. Instead it was returning 1, because the comparing function looked at the first arg, saw that it was entirely numeric, and then assuming that they were both entirely numeric and doing an incorrect comparison.

"But wait!" you say, "There is no '0' version of that rpm, all versions of the rpm should have been alphanumeric instead of merely numeric!" Yes, but the max function for evr_t type objects starts with ('0','0','0') as the initial conditions, leading us to run into this problem.

I suspect that this is a problem with any packages with an alphanumeric version. This incorrect version comparison problem ballooned up to the package not being correctly included in rhnServerNeededCache, which *a lot* of things drive off of, including taskomatic's errata cache task and the sql for generating the list of updates available for your server in the webui.

I would not be surprised if a lot of customer problems were due to this. They would all be of the class "yum tells me I have an updated version of X ready to install but Satellite says I'm up-to-date".

Committing to Spacewalk master:
94d95c3334ef62ade78c850a955a217d9a7df052
Comment 2 Stephen Herr 2013-08-20 16:38:53 EDT
I just noticed comment 0 said this only happens on pgsql, not oracle. I think that makes sense, the max function is only defined specifically for evr_t objects in pgsql, oracle uses a more generic max function. So even though the version compare function would have been wrong in both places, we would only really trip over it in pgsql. Oracle would only have a problem if there were actually two packages, the installed package being "package-1-1" and the "newest" package being "package-beta1-1". Usually version strings go the other way, so I think we wouldn't really run into this on Oracle. I think.
Comment 3 Stephen Herr 2013-08-20 16:50:20 EDT
Also need this commit because I named a file wrong:
c093eb247eb1267ab558780241c66ab240920449
Comment 4 Michael Mráka 2013-08-21 04:42:51 EDT
> This error is actually a result of a problem in the version comparison sql.
> 
> select rpm.rpmstrcmp('0', 'alpha9.8') from dual;
> 
> should return -1 (meaning 0 is smaller than alpha9.0. Instead it was
> returning 1, because the comparing function looked at the first arg, saw
> that it was entirely numeric, and then assuming that they were both entirely
> numeric and doing an incorrect comparison.

This is actually not true. Version 0 is newer (higher) version than alpha9.8 both in version and release.

$ rpmdev-vercmp 1-0 1-alpha9.8
0:1-0 is newer
$ rpmdev-vercmp 0-1 alpha9.8-1
0:0-1 is newer
Comment 5 Michael Mráka 2013-08-21 07:29:06 EDT
There's no bug in rpm.rpmstrcmp() but the issue is in initcond of max(evr_t) definition:

 create aggregate max (
  sfunc=evr_t_larger,
  basetype=evr_t,
  stype=evr_t,
  initcond='(0,0,0)'
 );

evr_t(0,0,0) is not the lowest evr_t member(!) so e.g. 

 select max(x.ver) from (select evr_t('0','0.alpha9.8','0') as ver from dual) x;
 -------
 (0,0,0)

which is clearly nonsense. Correct initcond for max should be (NULL, NULL, NULL) which can be simply omitted in the definition, i.e. 

 create aggregate max (
  sfunc=evr_t_larger,
  basetype=evr_t,
  stype=evr_t
 );
Comment 6 Stephen Herr 2013-08-21 08:38:44 EDT
(In reply to Michael Mráka from comment #4)
> > This error is actually a result of a problem in the version comparison sql.
> > 
> > select rpm.rpmstrcmp('0', 'alpha9.8') from dual;
> > 
> > should return -1 (meaning 0 is smaller than alpha9.0. Instead it was
> > returning 1, because the comparing function looked at the first arg, saw
> > that it was entirely numeric, and then assuming that they were both entirely
> > numeric and doing an incorrect comparison.
> 
> This is actually not true. Version 0 is newer (higher) version than alpha9.8
> both in version and release.
> 
> $ rpmdev-vercmp 1-0 1-alpha9.8
> 0:1-0 is newer
> $ rpmdev-vercmp 0-1 alpha9.8-1
> 0:0-1 is newer

I disagree. We cannot make assumptions about the word "alpha" or "beta" as being higher than or lower than version 0. The only thing we can do is apply a consistent set of rules:
1) if both versions are numeric we will compare them numerically
2) if at least one version is not numeric we will compare them alphanumerically

Not this problem does not only apply to the "0" version, the existing code returns the first package version as being larger *any* time the first compared package version is entirely numeric and the second compared package version is not. Right now the function is argument-order dependent, so to use your own example:
> $ rpmdev-vercmp 1-0 1-alpha9.8
> 0:1-0 is newer
> $ rpmdev-vercmp 1-alpha9.8 1-0
> 1-alpha9.8 is newer
This is clearly not correct behavior, and is what my original patch fixes. 

Removing the init condition from the max function is fine but unnecessary once we have a corrected vercmp method.
Comment 7 Stephen Herr 2013-08-21 10:21:27 EDT
It turns out that Michael is right and my assertion that our version compare function is argument-order-dependant is not correct. It looks like it is, but then there's a complicated if tree at the end that correctly does the right thing in each case. I was assuming things that were not true about how it should work.

So what we *really* should be doing is implementing the version comparison found here: https://fedoraproject.org/wiki/Archive:Tools/RPM/VersionComparison?rd=Tools/RPM/VersionComparison
*Not* implementing our own set of consistent rules. The version compare method apparently does this already, so really the only problem here is that the postgres implementation of max() for evr_t objects uses an incorrect initial state, as Michael said. I will revert my previous commits and fix this the right way now.

The upgrade script will be a little complicated because we'd need to drop and create the aggregate max function and there are a couple of views that depend on it.

Reverting:
85e77f54adb9f83d628c6d7873ad6f17e1876515
87045dea1317ac9d209d43aa1bca997c6f63c3cc

Committing correct patch (removing the init condition as in comment 5):
c52d9433111063915f447e3175dcdf4463354f3e
Comment 12 Clifford Perry 2013-10-01 17:58:44 EDT
Satellite 5.6 has been released. This bug was tracked under the release.  

This bug was either VERIFIED or RELEASE_PENDING (re-verified prior shortly
before release). 

Moving to CLOSED CURRENT_RELEASE. 

Text from Upgrade Erratum follows:

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHEA-2013-1395.html

Note You need to log in before you can comment on or make changes to this bug.