Bug 753021

Summary: spacewalk-repo-sync updates last_modified even if no new packages to download
Product: [Community] Spacewalk Reporter: Marcelo Moreira de Mello <mmello>
Component: ServerAssignee: Jan Pazdziora (Red Hat) <jpazdziora>
Status: CLOSED NOTABUG QA Contact: Red Hat Satellite QA List <satqe-list>
Severity: low Docs Contact:
Priority: medium    
Version: 1.6CC: cperry, jpazdziora, mmello, slukasik, stephan.duehr
Target Milestone: ---Keywords: Patch
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 743833 Environment:
Last Closed: 2011-11-22 09:15:10 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 723481    
Attachments:
Description Flags
patch proposed
none
patch proposed none

Description Marcelo Moreira de Mello 2011-11-11 03:37:52 UTC 
+++ This bug was initially created as a clone of Bug #743833 +++

Description of problem:
spacewalk-repo-sync updates the channels last_modified timestamp even if no new packages were downloaded.

Version-Release number of selected component (if applicable):
Spacewalk 1.6
  
Actual results:
The channels last_modified field is updated to the current time although no modification of the channel occured. In this case the field does not contain the timestamp of the effective last modification of the channel any more.


Expected results:
The channels last_modified field should not change if the channel was not modified. 

  We were able to reproduce the issue were only yumrepo_last_sync should be changed, not last_modified. Please look the steps below: 
  
     --> Python script
     
# cat list-channel-details.py 
import xmlrpclib
import pprint as pp

SATELLITE_URL = "https://HOST/rpc/api" 
SATELLITE_LOGIN = "USER" 
SATELLITE_PASSWORD = "SECRET" 
server = xmlrpclib.Server(SATELLITE_URL, verbose=0) 
token = server.auth.login(SATELLITE_LOGIN, SATELLITE_PASSWORD)

all_channels = server.channel.listAllChannels(token)
for i in all_channels:
    print "\t" + i['label']

ch_id = raw_input("\nEnter the channel label: ")
try:
    channel_details = server.channel.software.getDetails(token, ch_id)
    if channel_details:
        pp.pprint(channel_details)
except:
    print "Error"

server.auth.logout(token)


  ---> Reproducer
  
notebook $> python list-channel-details.py 
	redhat-rhn-satellite-5.4-server-x86_64-6
	rhel-x86_64-server-6
	rhel-x86_64-server-vt-5
	rhel-x86_64-server-5
	rhel-x86_64-server-optional-6
	rhn-tools-rhel-x86_64-server-5
	redhat-rhn-proxy-5.4-server-x86_64-5
	rhn-tools-rhel-x86_64-server-6
	redhat-rhn-satellite-5.4-server-x86_64-5
	rhel-x86_64-server-ha-6
	fedora15_x86_64
	spacewalk-tools-fedora15_x86_64
	rhel5-dir-srv
	test-channel
	rhel-x86_64-server-supplementary-6
	redhat-rhn-proxy-5.4-server-x86_64-6

Enter the channel label: test-channel
{'arch_name': 'x86_64',
 'checksum_label': 'sha1',
 'description': '',
 'end_of_life': '',
 'gpg_key_fp': '',
 'gpg_key_id': '',
 'gpg_key_url': '',
 'id': 131,
 'label': 'test-channel',
 'last_modified': <DateTime '20111110T17:39:27' at 7f79fea43440>,
 'name': 'test-channel',
 'parent_channel_label': '',
 'summary': 'test-channel',
 'yumrepo_label': 'spacewalk-clients-f16',
 'yumrepo_last_sync': <DateTime '20111110T17:39:27' at 7f79fea43488>,
 'yumrepo_source_url': 'http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/'}
 
 
   ---> Forcing sync using spacewalk-repo-sync
   
   # spacewalk-repo-sync -t yum -c test-channel
   Repo http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/ has 20 packages.
   No new packages to download.
   Sync complete

   ---> Checking timestamps
   
notebook $> python list-channel-details.py 
	redhat-rhn-satellite-5.4-server-x86_64-6
	rhel-x86_64-server-6
	rhel-x86_64-server-vt-5
	rhel-x86_64-server-5
	rhel-x86_64-server-optional-6
	rhn-tools-rhel-x86_64-server-5
	redhat-rhn-proxy-5.4-server-x86_64-5
	rhn-tools-rhel-x86_64-server-6
	redhat-rhn-satellite-5.4-server-x86_64-5
	rhel-x86_64-server-ha-6
	fedora15_x86_64
	spacewalk-tools-fedora15_x86_64
	rhel5-dir-srv
	test-channel
	rhel-x86_64-server-supplementary-6
	redhat-rhn-proxy-5.4-server-x86_64-6

Enter the channel label: test-channel
{'arch_name': 'x86_64',
 'checksum_label': 'sha1',
 'description': '',
 'end_of_life': '',
 'gpg_key_fp': '',
 'gpg_key_id': '',
 'gpg_key_url': '',
 'id': 131,
 'label': 'test-channel',
 'last_modified': <DateTime '20111110T18:08:54' at 7fd4e0e3a440>,
 'name': 'test-channel',
 'parent_channel_label': '',
 'summary': 'test-channel',
 'yumrepo_label': 'spacewalk-clients-f16',
 'yumrepo_last_sync': <DateTime '20111110T18:08:54' at 7fd4e0e3a488>,
 'yumrepo_source_url': 'http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/'}
 
 
    Working to provide a patch/workaround to the customer ASAP. 
    
    Best Regards, 
  mmello

--- Additional comment from mmello on 2011-11-10 20:02:14 BRST ---

=== In Red Hat Customer Portal Case 00544311 ===
--- Comment by Moreira de Mello, Marcelo on 11/10/2011 8:02 PM ---

Hello, 
   
 Reading the code we could observe that database have 2 fields to store the timestamp for yumrepo_last_sync and last_modified (LAST_MODIFIED,LAST_SYNCED). 
 
  At the database: 
 
SQL> desc rhnchannel;
 Name														   Null?    Type
 ----------------------------------------------------------------------------------------------------------------- -------- --------------
 ID														   NOT NULL NUMBER
 PARENT_CHANNEL 													    NUMBER
 ORG_ID 														    NUMBER
 CHANNEL_ARCH_ID												   NOT NULL NUMBER
 LABEL														   NOT NULL VARCHAR2(128)
 BASEDIR													   NOT NULL VARCHAR2(256)
 NAME														   NOT NULL VARCHAR2(256)
 SUMMARY													   NOT NULL VARCHAR2(500)
 DESCRIPTION														    VARCHAR2(4000)
 PRODUCT_NAME_ID													    NUMBER
 GPG_KEY_URL														    VARCHAR2(256)
 GPG_KEY_ID														    VARCHAR2(14)
 GPG_KEY_FP														    VARCHAR2(50)
 END_OF_LIFE														    DATE
 CHECKSUM_TYPE_ID													    NUMBER
 RECEIVING_UPDATES												   NOT NULL CHAR(1)
 LAST_MODIFIED													   NOT NULL DATE
 LAST_SYNCED														    DATE
 CHANNEL_PRODUCT_ID													    NUMBER
 CHANNEL_ACCESS 													    VARCHAR2(10)
 MAINT_NAME														    VARCHAR2(128)
 MAINT_EMAIL														    VARCHAR2(128)
 MAINT_PHONE														    VARCHAR2(128)
 SUPPORT_POLICY 													    VARCHAR2(256)
 CREATED													   NOT NULL DATE
 MODIFIED													   NOT NULL DATE

 
 Looking the code we have:
 
java/code/src/com/redhat/rhn/frontend/xmlrpc/channel/software/ChannelSoftwareHandler.java
----------------------------------------------------------------------------
 467     /**
 468      * Returns the details of the given channel as a map with the following
 469      * keys:
 470      * @param sessionKey WebSession containing User information.
 471      * @param channelLabel Label of channel whose details are sought.
 472      * @throws NoSuchChannelException thrown if no channel is found.
 473      * @return the channel requested.
 474      *
 475      * @xmlrpc.doc Returns details of the given channel as a map
 476      * @xmlrpc.param #session_key()
 477      * @xmlrpc.param #param_desc("string", "channelLabel", "channel to query")
 478      * @xmlrpc.returntype
 479      *     $ChannelSerializer
 480      */
 481     public Channel getDetails(String sessionKey, String channelLabel)
 482         throws NoSuchChannelException {
 483         User user = getLoggedInUser(sessionKey);
 484         return lookupChannelByLabel(user, channelLabel);
 485     }

1753     private Channel lookupChannelByLabel(User user, String label)
1754         throws NoSuchChannelException {
1755 
1756         Channel channel = ChannelFactory.lookupByLabelAndUser(label, user);
1757         if (channel == null) {
1758             throw new NoSuchChannelException(label);
1759         }
1760 
1761         return channel;
1762     }

java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java
-----------------------------------------------------------------------
  93     /**
  94      * Lookup a Channel by label and User
  95      * @param label the label to search for
  96      * @param userIn User who is doing the looking
  97      * @return the Server found (null if not or not member if userIn)
  98      */
  99     public static Channel lookupByLabelAndUser(String label, User userIn) {
 100         Map params = new HashMap();
 101         params.put("label", label);
 102         params.put("userId", userIn.getId());
 103         return (Channel) singleton.lookupObjectByNamedQuery(
 104                                        "Channel.findByLabelAndUserId", params);
 105     }


java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/ChannelSerializer.java
-----------------------------------------------------------------------
 77     public void serialize(Object value, Writer output, XmlRpcSerializer builtInSerializer)
 78         throws XmlRpcException, IOException {
 79         SerializerHelper helper = new SerializerHelper(builtInSerializer);
 80         Channel c = (Channel) value;
 81 
 82 
 83         helper.add("id", c.getId());
 84         helper.add("label", c.getLabel());
 85         helper.add("name", c.getName());
 86         helper.add("arch_name",
 87                 StringUtils.defaultString(c.getChannelArch().getName()));
 88         helper.add("summary", StringUtils.defaultString(c.getSummary()));
 89         helper.add("description",
 90                 StringUtils.defaultString(c.getDescription()));
 91         helper.add("checksum_label", c.getChecksumTypeLabel());
 92         helper.add("last_modified", c.getLastModified());
 93         helper.add("maintainer_name", c.getMaintainerName());
 94         helper.add("maintainer_email", c.getMaintainerEmail());
 95         helper.add("maintainer_phone", c.getMaintainerPhone());
 96         helper.add("support_policy", c.getSupportPolicy());
 97 
 98         helper.add("gpg_key_url",
 99                 StringUtils.defaultString(c.getGPGKeyUrl()));
100         helper.add("gpg_key_id",
101                 StringUtils.defaultString(c.getGPGKeyId()));
102         helper.add("gpg_key_fp",
103                 StringUtils.defaultString(c.getGPGKeyFp()));
104 
105         List<ContentSource> csList = new ArrayList<ContentSource>(c.getSources().size());
106         if (!c.getSources().isEmpty()) {
107             for (Iterator itr = c.getSources().iterator(); itr.hasNext();) {
108                 ContentSource cs = (ContentSource) itr.next();
109                 csList.add(cs);
110             }
111             if (c.getLastSynced() != null) {
112                 helper.add("yumrepo_last_sync", c.getLastSynced());
113             }
114             else {
115                 helper.add("yumrepo_last_sync", "");
116             }
117         }

 
backend/satellite_tools/reposync.py
--------------------------
113     def update_date(self):
114         """ Updates the last sync time"""
115         h = rhnSQL.prepare( """update rhnChannel set LAST_SYNCED = sysdate
116                              where label = :channel""")
117         h.execute(channel=self.channel['label'])

 43     def main(self):
 44         initCFG('server')
 45         db_string = CFG.DEFAULT_DB #"rhnsat/rhnsat@rhnsat"
 46         rhnSQL.initDB(db_string)
 
 { ... SNIP .. }
 
 97         if not self.channel or not rhnChannel.isCustomChannel(self.channel['id']):
 98             print "Channel does not exist or is not custom"
 99             sys.exit(1)
100 
101         for url in self.urls:
102             plugin = self.load_plugin()(url, self.channel_label)
103             self.import_packages(plugin, url)
104         if self.regen:
105             taskomatic.add_to_repodata_queue_for_channel_package_subscription(
106                 [self.channel_label], [], "server.app.yumreposync")
107             taskomatic.add_to_erratacache_queue(self.channel_label)
108         self.update_date()
109         rhnSQL.commit()........
110         self.print_msg("Sync complete")

 
 
schema/spacewalk/oracle/triggers/rhnChannel.sql
---------------------------------------------------
 19 create or replace trigger
 20 rhn_channel_mod_trig
 21 before insert or update on rhnChannel
 22 for each row
 23 begin
 24 >---:new.last_modified := sysdate;
 25 >----- this is a really bad way of saying "if all we''re.
 26 >----- changing is the date"
 27 >---if updating then
 28 >--->---if (:old.id != :new.id) or
 29 >--->---  (:old.parent_channel != :new.parent_channel) or
 30 >--->---  (:old.org_id != :new.org_id) or
 31 >--->---  (:old.channel_arch_id != :new.channel_arch_id) or
 32 >--->---  (:old.label != :new.label) or
 33 >--->---  (:old.basedir != :new.basedir) or
 34 >--->---  (:old.name != :new.name) or
 35 >--->---  (:old.summary != :new.summary) or
 36 >--->---  (:old.description != :new.description) then
 37 >--->--->---:new.modified := sysdate;
 38 >--->---end if;
 39 >---end if;
 40 end;
 41 /
 42 show errors
 43 

  During my tests,  if we change the trigger to not automatically update last_modified if last_synced was the only field updated will address the issue, since function update_date() will update the yumrepo_last_sync API field. 
  
  I'll reach some DB experts to help us to modify the trigger to make some tests. 
  
  The issue also happens into upstream verson (Spacewalk). 
  
  Still working into this. 
  
  Best, 
mmello

--- Additional comment from mmello on 2011-11-11 01:30:53 BRST ---

=== In Red Hat Customer Portal Case 00544311 ===
--- Comment by Moreira de Mello, Marcelo on 11/11/2011 1:30 AM ---

Hello, 

 As a POC, we modified the PL/SQL and we were able to make the LAST_MODIFIED get updated only when updating the channel contents. Look below: 

   --- Changing the trigger rhn_channel_mod_trig

notebook $> git diff
diff --git a/schema/spacewalk/oracle/triggers/rhnChannel.sql b/schema/spacewalk/oracle/triggers/rhnChannel.sql
index 3a357fa..58ef396 100644
--- a/schema/spacewalk/oracle/triggers/rhnChannel.sql
+++ b/schema/spacewalk/oracle/triggers/rhnChannel.sql
@@ -21,7 +21,9 @@ rhn_channel_mod_trig
 before insert or update on rhnChannel
 for each row
 begin
-       :new.last_modified := sysdate;
+    if inserting then
+           :new.last_modified := sysdate;
+    end if;
        -- this is a really bad way of saying "if all we''re 
        -- changing is the date"
        if updating then

  *** Trigger

create or replace trigger       
rhn_channel_mod_trig
before insert or update on rhnChannel
for each row
begin
    if inserting then
            :new.last_modified := sysdate;
    end if;
        -- this is a really bad way of saying "if all we''re 
        -- changing is the date"
        if updating then
                if (:old.id != :new.id) or
                  (:old.parent_channel != :new.parent_channel) or
                  (:old.org_id != :new.org_id) or
                  (:old.channel_arch_id != :new.channel_arch_id) or
                  (:old.label != :new.label) or
                  (:old.basedir != :new.basedir) or
                  (:old.name != :new.name) or
                  (:old.summary != :new.summary) or
                  (:old.description != :new.description) then
                        :new.modified := sysdate;
                end if;
        end if;
end;
/
show errors


  *** Testing

notebook $> python list-channel-details.py 
{'arch_name': 'x86_64',
 'checksum_label': 'sha1',
 'description': '',
 'end_of_life': '',
 'gpg_key_fp': '',
 'gpg_key_id': '',
 'gpg_key_url': '',
 'id': 131,
 'label': 'test-channel',
 'last_modified': <DateTime '20111111T01:08:45' at 7fe56bf94440>,
 'name': 'test-channel',
 'parent_channel_label': '',
 'summary': 'test-channel',
 'yumrepo_label': 'spacewalk-clients-f16',
 'yumrepo_last_sync': <DateTime '20111111T01:08:45' at 7fe56bf94488>,
 'yumrepo_source_url': 'http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/'}


[root@sun-x4200-1 reposync]# spacewalk-repo-sync -t yum -c test-channel
Repo http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/ has 20 packages.
1/4 : rhncfg-client-5.10.19-1.fc16-0.noarch
2/4 : rhncfg-actions-5.10.19-1.fc16-0.noarch
3/4 : rhncfg-5.10.19-1.fc16-0.noarch
4/4 : osad-5.10.18.2-1.fc15-0.noarch
Sync complete


notebook $> python list-channel-details.py 
{'arch_name': 'x86_64',
 'checksum_label': 'sha1',
 'description': '',
 'end_of_life': '',
 'gpg_key_fp': '',
 'gpg_key_id': '',
 'gpg_key_url': '',
 'id': 131,
 'label': 'test-channel',
 'last_modified': <DateTime '20111111T01:18:48' at 7f0191151440>,
 'name': 'test-channel',
 'parent_channel_label': '',
 'summary': 'test-channel',
 'yumrepo_label': 'spacewalk-clients-f16',
 'yumrepo_last_sync': <DateTime '20111111T01:18:48' at 7f0191151488>,
 'yumrepo_source_url': 'http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/'}
 
[root@sun-x4200-1 reposync]# spacewalk-repo-sync -t yum -c test-channel
Repo http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/ has 20 packages.
No new packages to download.
Sync complete

notebook $> python list-channel-details.py 
{'arch_name': 'x86_64',
 'checksum_label': 'sha1',
 'description': '',
 'end_of_life': '',
 'gpg_key_fp': '',
 'gpg_key_id': '',
 'gpg_key_url': '',
 'id': 131,
 'label': 'test-channel',
 'last_modified': <DateTime '20111111T01:18:48' at 7fcf81a38440>,
 'name': 'test-channel',
 'parent_channel_label': '',
 'summary': 'test-channel',
 'yumrepo_label': 'spacewalk-clients-f16',
 'yumrepo_last_sync': <DateTime '20111111T01:19:15' at 7fcf81a38488>,
 'yumrepo_source_url': 'http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/'}

[root@sun-x4200-1 reposync]# spacewalk-repo-sync -t yum -c test-channel
Repo http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/ has 20 packages.
No new packages to download.
Sync complete

notebook $> python list-channel-details.py 
{'arch_name': 'x86_64',
 'checksum_label': 'sha1',
 'description': '',
 'end_of_life': '',
 'gpg_key_fp': '',
 'gpg_key_id': '',
 'gpg_key_url': '',
 'id': 131,
 'label': 'test-channel',
 'last_modified': <DateTime '20111111T01:18:48' at 7f18b9253440>,
 'name': 'test-channel',
 'parent_channel_label': '',
 'summary': 'test-channel',
 'yumrepo_label': 'spacewalk-clients-f16',
 'yumrepo_last_sync': <DateTime '20111111T01:29:55' at 7f18b9253488>,
 'yumrepo_source_url': 'http://spacewalk.redhat.com/yum/nightly-client/Fedora/16/x86_64/'}


  I'm making further tests before sending this patch for approval. 

  Best Regards, 
mmello
Comment 1 Marcelo Moreira de Mello 2011-11-11 03:40:05 UTC 
Taking..
Comment 2 Marcelo Moreira de Mello 2011-11-11 17:03:24 UTC 
Created attachment 533131 [details]
patch proposed
Comment 3 Marcelo Moreira de Mello 2011-11-11 17:04:35 UTC 
Created attachment 533132 [details]
patch proposed


  Hello, 

   Patch already submitted to approval at spacewalk-devel maillist 

  Mail thread: https://www.redhat.com/archives/spacewalk-devel/2011-November/msg00020.html

  Best, 
mmello
Comment 4 Jan Pazdziora (Red Hat) 2011-11-15 07:17:23 UTC 
Making bugzilla public.
Comment 5 Jan Pazdziora (Red Hat) 2011-11-21 14:47:55 UTC 
I don't agree with the proposed patch -- it would change the semantics not only for the narrow spacewalk-repo-sync case but for other updates that might go to the rhnChannel table as well.

In fact, could you please elaborate why the modification of the last_modified column is seen as a problem? Clearly, upon the spacewalk-repo-sync, the last_synced value is updated (so the channel entity got changed), and that is also reflected in the last_modified column.

I'm leaning towards closing this bugzilla s NOTABUG.