Bug 678721

Created attachment 479636 [details]
temporary patch which fixes the bug

Description of problem:

If you use errata.clone() api to clone a specific errata to a cloned/custom channel, it adds instead of only packages from cloned channel also packages from different channels which have the same name and architecture as the ones from the cloned channel.

It can lead to adding e.g. RHEL6 packages altogether with RHEL5 ones into channel which was originally cloned from RHEL5 channel. See the 'Actual results'.


Version-Release number of selected component (if applicable):

spacewalk-java-1.2.39-29.el5sat


How reproducible:

Always


Steps to Reproduce:
===================

1. Clone rhel-x86_64-server-5 rhel base channel using the following script (change server, login, pass):

=== <snip clone_channel.py> ===
#!/usr/bin/python

from xmlrpclib import Server
import xmlrpclib

satServer     = "server"
satUser       = "login"
satPasswd     = "pass"
sourceChannel = "rhel-x86_64-server-5"
satClient = Server('https://'+satServer+'/rpc/api')
satAuth = satClient.auth.login(satUser, satPasswd)

print satClient.channel.software.clone(satAuth, sourceChannel , {'name':'testicek', 'label':'testicek', 'summary':'testicek'}, False)
=== </snip> ===

2. Run the following script which clones 'RHSA-2010:0966' into the cloned channel (change server, login, pass):

=== <snip clone_errata.py> ===
#!/usr/bin/python                                                                                                                                                                                              
                                                                                                                                                                                                               
from xmlrpclib import Server                                                                                                                                                                                   
import xmlrpclib                                                                                                                                                                                               
                                                                                                                                                                                                               
errataDict = {}                                                                                                                                                                                                
errataDict['advisory']  = 'RHSA-2010:0966'                                                                                                                                                                     
satServer    = "server"                                                                                                                                                                     
satUser      = "login"                                                                                                                                                                                         
satPasswd    = "pass"
satClient = Server('https://'+satServer+'/rpc/api')                                                                                                                                                            
satAuth = satClient.auth.login(satUser, satPasswd)                                                                                                                                                             
print satClient.errata.clone(satAuth,"testicek",['RHSA-2010:0966'])
=== </snip> ===

  
Actual results:
===============

$ ./clone_channel.py 
426
$

Now, search for RHSA-2010:0966 errata through WebUI and you can see in 'Packages' folder a list of packages belonging to errata and cloned channel ('testicek'), the following output is correct:

testicek 
md5:4864d00fd39cd36741d0f7b8242cac25 firefox-3.6.13-2.el5-i386 
md5:9d986be77ce9a74845cc58855415ccd2 firefox-3.6.13-2.el5-x86_64 
md5:f54552c008aab66135b691e7349fff43 xulrunner-1.9.2.13-3.el5-i386 
md5:587d16fe99016c0d782acfeb83a96765 xulrunner-1.9.2.13-3.el5-x86_64 
md5:63cfc555bc6f52aafa9d7155de062e05 xulrunner-devel-1.9.2.13-3.el5-i386 
md5:edf71edc51ae45cf66eee9561c7c447e xulrunner-devel-1.9.2.13-3.el5-x86_64 

$ ./clone_errata.py 
[{'date': '12/9/10', 'advisory_name': 'CLA-2010:0966', 'advisory_type': 'Security Advisory', 'id': 9306, 'advisory_synopsis': 'Critical: firefox security update'}]
$

Now, again search for RHSA-2010:0966 errata and you can see in 'Packages' the following:

testicek
md5:4864d00fd39cd36741d0f7b8242cac25 firefox-3.6.13-2.el5-i386 
md5:9d986be77ce9a74845cc58855415ccd2 firefox-3.6.13-2.el5-x86_64 
sha256:03ead8549d8fc95b3aab5501f56429d6682c228de5d013f70a60d5824a96c7a2 firefox-3.6.13-2.el6_0-x86_64 
md5:f54552c008aab66135b691e7349fff43 xulrunner-1.9.2.13-3.el5-i386 
md5:587d16fe99016c0d782acfeb83a96765 xulrunner-1.9.2.13-3.el5-x86_64 
sha256:220b0d00e6b0d198b16e31289c580cd0256c5dfb0d733a96c278fe34e4973ff0 xulrunner-1.9.2.13-3.el6_0-x86_64 
md5:63cfc555bc6f52aafa9d7155de062e05 xulrunner-devel-1.9.2.13-3.el5-i386 
md5:edf71edc51ae45cf66eee9561c7c447e xulrunner-devel-1.9.2.13-3.el5-x86_64 


Expected results:
=================

No duplicate packages in cloned channel


Additional info:
================

The problem seems to be in a db call, which gets package list belonging to a chosen errata to add corresponding packages to the cloned channel (you can find a codepath leading to this sql hereinafter):

./spacewalk-java-1.2.39/code/src/com/redhat/rhn/common/db/datasource/xml/Errata_queries.xml:
 655 <mode name="find_packages_for_errata_and_channel" class="com.redhat.rhn.frontend.dto.PackageOverview">
 656   <query params="custom_cid, eid, org_id">
 657         select distinct P.id, PN.name package_name, PA.label as package_arch,
 658                 EVR.version
 659        || '-' || EVR.release || (CASE WHEN EVR.epoch IS NULL THEN '' ELSE ':' || EVR.epoch END)
 660          package_nvre
 661     from
 662         rhnErrata E inner join
 663         rhnErrataPackage EP on E.id = EP.errata_id inner join
 664         rhnPackage P on P.id = EP.package_id inner join
 665         rhnPackageArch PA on PA.id = P.package_arch_id inner join
 666         rhnPackageEvr EVR on Evr.id = P.evr_id inner join
 667     rhnPackage P2 on P2.name_id = P.name_id inner join <<<---
 668     rhnChannelPackage CP on CP.package_id = P2.id      <<<---
 669     inner join rhnPackageName PN on  PN.id = P.name_id
 670     inner join rhnChannel CN on CP.channel_id = CN.id
 671     where E.id = :eid
 672          and CP.channel_id = :custom_cid
 673          and P2.Package_arch_id = P.package_arch_id   <<<---
 674          and CN.org_id = :org_id and
 675          (E.org_id is NULL or E.org_id = :org_id)
 676   </query>
 677 </mode>

Problematic are lines #673 and related conditions at line #667,#668 which lead to returning also packages from unrelated channels. It happens because we iterate through all packages belonging to errata from all channels (#663) ...:

=== <snip> ===
SQL> select * from rhnErrataPackage where errata_id = 68;  <<<--- 'RHSA-2010:0966'

 ERRATA_ID PACKAGE_ID CREATED   MODIFIED
---------- ---------- --------- ---------
        68       3644 30-JAN-11 30-JAN-11  <<<--- firefox-3.6.13-2.el6_0.x86_64.rpm
        68       3645 30-JAN-11 30-JAN-11  <<<--- xulrunner-1.9.2.13-3.el6_0.i686.rpm (note this wasn't added)
        68       3646 30-JAN-11 30-JAN-11  <<<--- xulrunner-1.9.2.13-3.el6_0.x86_64.rpm
        68      14319 09-FEB-11 09-FEB-11
        68      14320 09-FEB-11 09-FEB-11  <<<--- firefox-3.6.13-2.el5.x86_64.rpm
        68      14321 09-FEB-11 09-FEB-11
        68      14322 09-FEB-11 09-FEB-11
        68      14323 09-FEB-11 09-FEB-11  <<<--- xulrunner-1.9.2.13-3.el5.x86_64.rpm
        68      14324 09-FEB-11 09-FEB-11

9 rows selected.
=== </snip> ===

... and P can contain a package from one channel (#664, in our case firefox-3.6.13-2.el6_0.x86_64.rpm) and the destination channel can contain a package with the same name (#667, in our case firefox-3.6.13-2.el5.x86_64.rpm) in destination channel (#668) so that it prints the same package from both RHEL5 and RHEL6 channel (correct packages were removed from output):

=== <snip> ===
SQL> select distinct P.id, PN.name package_name, PA.label as package_arch, EVR.version || '-' || EVR.release || (CASE WHEN EVR.epoch IS NULL THEN '' ELSE ':' || EVR.epoch END) package_nvre
 from rhnErrata E inner join
  rhnErrataPackage EP on E.id = EP.errata_id inner join
  rhnPackage P on P.id = EP.package_id inner join                                                                                                                                                              
  rhnPackageArch PA on PA.id = P.package_arch_id inner join                                                                                                                                                    
  rhnPackageEvr EVR on Evr.id = P.evr_id inner join                                                                                                                                                            
  rhnPackage P2 on P2.name_id = P.name_id inner join                                                                                                                                                           
  rhnChannelPackage CP on CP.package_id = P2.id                                                                                                                                                                
  inner join rhnPackageName PN on  PN.id = P.name_id                                                                                                                                                           
  inner join rhnChannel CN on CP.channel_id = CN.id                                                                                                                                                            
 where E.id = 68                                                                                                                                                                                               
  and CP.channel_id = 426                                                                                                                                                                                      
  and P2.Package_arch_id = P.package_arch_id                                                                                                                                                                   
  and CN.org_id = 1 and                                                                                                                                                                                        
  (E.org_id is NULL or E.org_id = 1);  2    3    4    5    6    7    8    9   10   11   12   13   14   15                                                                                                      
                                                                                                                                                                                                               
        ID                                                                                                                                                                                                     
----------                                                                                                                                                                                                     
PACKAGE_NAME                                                                                                                                                                                                   
--------------------------------------------------------------------------------                                                                                                                               
PACKAGE_ARCH
----------------------------------------------------------------
PACKAGE_NVRE
--------------------------------------------------------------------------------
      3646
xulrunner
x86_64
1.9.2.13-3.el6_0

...snip...

        ID
----------
PACKAGE_NAME
--------------------------------------------------------------------------------
PACKAGE_ARCH
----------------------------------------------------------------
PACKAGE_NVRE
--------------------------------------------------------------------------------
      3644
firefox
x86_64
3.6.13-2.el6_0

8 rows selected.
=== </snip> ===


I cannot find out a background reason/intention of lines #667 and #673, but without them it works correctly, see the attached temporary patch (actually I didn't test it enough to be sure it doesn't break something).

The following is a correct output, the changes in the patch won't fix cases where the channel was already corrupted by cloning an errata before applying the patch.

SQL> select distinct P.id, PN.name package_name, PA.label as package_arch, EVR.version || '-' || EVR.release || (CASE WHEN EVR.epoch IS NULL THEN '' ELSE ':' || EVR.epoch END) package_nvre
 from rhnErrata E inner join
  rhnErrataPackage EP on E.id = EP.errata_id inner join
  rhnPackage P on P.id = EP.package_id inner join
  rhnPackageArch PA on PA.id = P.package_arch_id inner join
  rhnPackageEvr EVR on Evr.id = P.evr_id inner join
  rhnChannelPackage CP on CP.package_id = P.id
  inner join rhnPackageName PN on  PN.id = P.name_id
  inner join rhnChannel CN on CP.channel_id = CN.id
 where E.id = 68
  and CP.channel_id = 465
  and CN.org_id = 1 and
  (E.org_id is NULL or E.org_id = 1);  2    3    4    5    6    7    8    9   10   11   12   13  

        ID
----------
PACKAGE_NAME
--------------------------------------------------------------------------------
PACKAGE_ARCH
----------------------------------------------------------------
PACKAGE_NVRE
--------------------------------------------------------------------------------
     14321
xulrunner
i386
1.9.2.13-3.el5


        ID
----------
PACKAGE_NAME
--------------------------------------------------------------------------------
PACKAGE_ARCH
----------------------------------------------------------------
PACKAGE_NVRE
--------------------------------------------------------------------------------
     14320
firefox
x86_64
3.6.13-2.el5


        ID
----------
PACKAGE_NAME
--------------------------------------------------------------------------------
PACKAGE_ARCH
----------------------------------------------------------------
PACKAGE_NVRE
--------------------------------------------------------------------------------
     14319
firefox
i386
3.6.13-2.el5


        ID
----------
PACKAGE_NAME
--------------------------------------------------------------------------------
PACKAGE_ARCH
----------------------------------------------------------------
PACKAGE_NVRE
--------------------------------------------------------------------------------
     14322
xulrunner-devel
i386
1.9.2.13-3.el5


        ID
----------
PACKAGE_NAME
--------------------------------------------------------------------------------
PACKAGE_ARCH
----------------------------------------------------------------
PACKAGE_NVRE
--------------------------------------------------------------------------------
     14324
xulrunner-devel
x86_64
1.9.2.13-3.el5


        ID
----------
PACKAGE_NAME
--------------------------------------------------------------------------------
PACKAGE_ARCH
----------------------------------------------------------------
PACKAGE_NVRE
--------------------------------------------------------------------------------
     14323
xulrunner
x86_64
1.9.2.13-3.el5


6 rows selected.

SQL>


errata.clone() codepath leading to the responsible sql:
=======================================================

./spacewalk-java-1.2.39/code/src/com/redhat/rhn/frontend/xmlrpc/handler-manifest.xml:
...
  5   <template name="errata" classname="com.redhat.rhn.frontend.xmlrpc.errata.ErrataHandler" />
...

./spacewalk-java-1.2.39/code/src/com/redhat/rhn/frontend/xmlrpc/errata/ErrataHandler.java:
 855     public Object[] clone(String sessionKey, String channelLabel,
 856             List advisoryNames) throws InvalidChannelRoleException {
...
 879         for (Iterator itr = errataToClone.iterator(); itr.hasNext();) {
 880             Errata cloned = ErrataManager.createClone(loggedInUser, (Errata)itr.next());
 881             Errata publishedClone = ErrataManager.publish(cloned);
 882 
 883             publishedClone = ErrataFactory.publishToChannel(publishedClone, channel, <<<--- 1.
 884                     loggedInUser, false);
 885             ErrataFactory.save(publishedClone);
 886 
 887             toReturn.add(publishedClone);
 888         }
...

spacewalk-java-1.2.39/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java:
 268     public static Errata publishToChannel(Errata errata, Channel chan, User user,
 269             boolean inheritPackages) {
...
 277         if (inheritPackages) {
...
 284         }
 285         else { 
 286             packs = ErrataManager.lookupPacksFromErrataForChannel(chan, errata, user);  <<<---- 2.
 287         }
 288         for (PackageOverview packOver : packs) {
 289             //lookup the Package object
 290             Package pack = PackageFactory.lookupByIdAndUser(
 291                     packOver.getId().longValue(), user);
 292             packagesToPush.add(pack);
 293         } 
 294         return publishErrataPackagesToChannel(errata, chan, user, packagesToPush);
 295     }

spacewalk-java-1.2.39/code/src/com/redhat/rhn/manager/errata/ErrataManager.java:
1184    public static DataResult<PackageOverview> lookupPacksFromErrataForChannel(
1185                Channel customChan, Errata errata, User user) {
1186        Map params = new HashMap();
1187        //params.put("uid", user.getId());
1188        params.put("eid" , errata.getId());
1189        params.put("org_id" , user.getOrg().getId());
1190        params.put("custom_cid", customChan.getId());
1191        SelectMode m = ModeFactory.getMode(
1192                "Errata_queries",  "find_packages_for_errata_and_channel");  <<<--- 3. the query
1193        return m.execute(params);
1194     
1195    }


Related problem:
================

* I found out during investigation that the same problem happens also when cloning a channel with 'Current state of the channel (all errata)' through WebUI, it creates cloned erratas (with a name in a form CLA-XXXX:XXXX-X) which contains also duplicated packages. I will create a BZ for it soon (currently investigating related perl code, I see the sql responsible for inserting package ids into rhnChannelPackage table, but don't see the culprit yet)


BZ related to this bug:
=======================

BZ #500128 (adding errata to clone channels also adds non-relevant packages from that errata)


Workaround:
===========

* It currently works correctly only when cloning an individual erratas through WebUI and the destination channel was originally created by copying erratas and related packages through channel.software.clone() api as done in first script in 'Steps to Reproduce' instead of cloning the channel which creates a new/cloned errata with a new name in a form CLA-XXXX:XXXX-X which doesn't work:

Cloned RHEL5 64 bit Channel -> Errata -> Add -> Add Red hat Errata -> select 2010:431 Errata -> Confirm -> Clone

The above works because the code in a background simply gathers package list from source channel and compares it with package ids stored in related rhnErrataPackage table (which holds errata and related package ids) and these packages are then copied to destination (cloned) channel.

Also probably good to mention that errata.clone() api was intended to be used only on channels which already contain all the packages related to errata. It means that one has to find out all the packages related to specific errata, then add them to the channel and then call the errata.clone() api to add erratas and connect them with packages.