Bug 223229 - [PATCH] rhn_check reports success if %pre scriptlet fail with exit 1
[PATCH] rhn_check reports success if %pre scriptlet fail with exit 1
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: up2date (Show other bugs)
4.4
All Linux
high Severity high
: ---
: ---
Assigned To: Pradeep Kilambi
Brandon Perkins
:
: 221961 (view as bug list)
Depends On:
Blocks: 240201
  Show dependency treegraph
 
Reported: 2007-01-18 10:57 EST by Jose Plans
Modified: 2010-10-22 03:56 EDT (History)
3 users (show)

See Also:
Fixed In Version: RHBA-2007-0250
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-01 19:19:48 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
up2date-4.4.69-scriplet_exception.patch (1.20 KB, patch)
2007-01-18 10:57 EST, Jose Plans
no flags Details | Diff
up2date-4.4.69-scriplet_exception-error-msg.patch (1.27 KB, patch)
2007-01-22 04:41 EST, Jose Plans
no flags Details | Diff

  None (edit)
Description Jose Plans 2007-01-18 10:57:24 EST
RHN Satellite has an action queue, this action queue has also a
scheduler to manage tasks which are often packages we want to
administrate (Install (deploy), remove, etc.).
 What happened here is that the client, when checking this queue with
the tool rhn_check, it failed silently, reporting a successful
installation to the RHN Satellite server's task scheduler, making this
one believe everything went fine, no failures whatsoever.
 Now, rhn_check is a python script that is very similar to up2date, the
difference is that he checks the scheduler instead of fetching packages,
the common points is that they share the same python foundations and
therefore the same rpm code.
 rhn_check will check first with the scheduler to see if we have
pending tasks, since the admin scheduled an install of apache, it will
then download the package's header (containing this broken %pre script)
and evaluate it, if the syntax is correct, it will then perform an rpm
transaction against it and execute each bit that it has been asked on
that header. If everything went fine, it will then install the package
and report to the Satellite a successful result, unless a failure was
encountered, in which case we will send an error code and message.
 The problem came from the transaction, in fact, rpm contains a section
called PSM, which is the one that will execute each one of the
scriptlets and parts of the header. When trying to install a package,
python (rhn_check) will code it's C wrapper and try to run a batch of
transactions with :
     ts.run() which is in the wrapper rpmts_Run()
  This function returns a list.
  What happened in our case ?
  When running this scriptlet batch, we did :
    if (!rc) rpmpsmStage(transaction we have, PSM_PRE);
    if (!rc) ....etc.
    rpmpsmStage(transaction, PSM_FINI); /* we clear and end */
  So you can see that if one of these fail, we just clear/clean/return.
  
 Within this Stage function we fork a child and execute a script shell,
if we failed or if someone (BAD PRACTICE as we say we failed) exits with
an error code and NOT ZERO, we clean/end fail.

 RPM in command line will fail with main returning -1.
 However, the wrapper will do something different! It will just want to
return a list, so how Paul decided to do this ? Simple :

 rc = rpmts_Run()
 ps = rpmts_Problems()
 ...
 if (rc < 0) { /* Our case... */
    create_a_list
    return this_empty_list
 }
 
 And then in Python :
     rc = ts.Run()
     if (rc)
         thins happen...
 
 And here comes our problem! We never checked whether this list was
empty or not during the batch transaction! therefore I added :

+    elif type(rc) == type([]) and not len(rc):
+       raise up2dateErrors.RpmError(_("Failed running rpm transaction"))

 Which means :
  
   Well, we have a return yes, also if we have this return *and* it is a
list *and* it's empty (our case)
   Then raise a simple generic exception as we cannot fetch (unless I write more
lines)
the name of the package and anyway we did exit (1).
   Then I added this exception to the code so it can return :

+    except up2dateErrors.RpmError, e:
+        return (18, "Failed: packages requested raised "\
+                "dependency or transaction problems", {})

Making satellite aware of the error and sending the host to the right queue
within the scheduler (failed hosts).
Let me know if you need more details,

       Jose
Comment 1 Jose Plans 2007-01-18 10:57:25 EST
Created attachment 145924 [details]
up2date-4.4.69-scriplet_exception.patch
Comment 4 Jose Plans 2007-01-18 11:10:19 EST
Steps to reproduce :
  1. Create a software customised channel.
  2. Push this package in that channel.
  3. Subscribe a host to that channel.
  4. Perform a schedule install of that package to that host.
  5. From the client host, run : rhn_check.
  
Actual results:
  --
[root@dhcp-0-238 ~]# rpm -ivh apache-2.0.55-VIS27.i386.rpm
Preparing...                ########################################### [100%]

User wwwrun is missing. Please create this user prior to installing
this package.

error: %pre(apache-2.0.55-VIS27.i386) scriptlet failed, exit status 1
error:   install: %pre scriptlet failed (2), skipping apache-2.0.55-VIS27
--
Success from the scheduler host status. No host had any failure.

Expected results:

D: Running transaction (final step)...
D: Sending back response (18, 'Failed: packages requested raised dependency or
transaction problems', {})
D: do_call packages.checkNeedUpdate ('rhnsd=1',)
D: local action status:  (0, 'rpm database not modified since last update (or
package list recently updated)', {})

We might need to add a print before the raise so the customer can see an error
or message.

And on the failed hosts: 
  "Failed: packages requested raised dependency or transaction problems".

   Jose
Comment 6 Jose Plans 2007-01-22 04:41:06 EST
Created attachment 146160 [details]
up2date-4.4.69-scriplet_exception-error-msg.patch

Prints error message on error instead of silently report the error and give a
prompt.
Comment 7 RHEL Product and Program Management 2007-01-22 04:43:27 EST
This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux maintenance release.  Product Management has requested
further review of this request by Red Hat Engineering, for potential
inclusion in a Red Hat Enterprise Linux Update release for currently deployed
products.  This request is not yet committed for inclusion in an Update
release.
Comment 15 Fanny Augustin 2007-01-30 11:00:13 EST
There is a patch available, but there is development work needed on the RHN side
to complete this bug, thus it will not be ready for QA until it is development
complete.
Comment 16 Pradeep Kilambi 2007-01-30 15:33:24 EST
this should be fixed now, applied patch and tested. committed to revision 110084
in svn.
Comment 19 Pradeep Kilambi 2007-02-02 10:15:05 EST
yes, thats what i would expect it to raise. We are raising the exception in the
up2date actions code and not in rpm itself and the exception we return is:

+    except up2dateErrors.RpmError, e:
+        return (18, "Failed: packages requested raised "\
+                "dependency or transaction problems", {})

which is an error code 18.
Comment 21 Pradeep Kilambi 2007-02-02 11:50:50 EST
most part of the patch proposed was already existing code that was commented out
for some unkonwn reason, we uncommented that so we actually catch the exception
rather than ignore it and end up as a success as it was doing previously. There
is nothing much to add to the patch except catch the exception. Thats my take on it.
The only decision we were trying to make initially was whether to throw the
exception at the rpm level itself or wait untill it enters the actions code and
as per Paul this cannot happen untill the next major release if we want to do it
in rpm, so we decided to do it at the actions level. As far as the print
statement I feel "packages requested raised dependency or transaction problems",
is appropriate(in this case a transaction problem) as this path is also follwed
while doing the dep solving and the error code needs to be as stated.

  I think i dont understand the concern here. isnt the patch working for you? 
Comment 23 Pradeep Kilambi 2007-02-02 12:09:02 EST
yep I see what you mean, I did consider that, as i explained earlier, we need to
throw same exception in multiple scenerios, so I felt what we had was
appripriate as it covered any dependency case and any transaction scenerio(which
is fits our case or any other transaction issue). 
Comment 25 Pradeep Kilambi 2007-02-02 12:34:03 EST
oh my apologies Jose, i totally missed your new patch. Did not realize that.
Thanks for clearing that up cliff. I will include that immediately and update
you all.
Comment 26 Pradeep Kilambi 2007-02-02 13:11:16 EST
added the logging to replace the print

--- rpmUtils.py (revision 110190)
+++ rpmUtils.py (working copy)
@@ -31,7 +31,7 @@
 
 
 from rhpl.translate import _, N_
-            
+from up2date_client import up2dateLog            
 
 # mainly here to make conflicts resolution cleaner
 def findDepLocal(ts, dep):
@@ -519,5 +519,8 @@
         raise up2dateErrors.TransactionError(_(
             "Failed running transaction of  packages: %s") % errors, deps=rc)
     elif type(rc) == type([]) and not len(rc):
+        # let the user know whats wrong
+        log = up2dateLog.initLog()
+        log.log_me("Failed running rpm transaction - %pre %pro failure ?.")
         raise up2dateErrors.RpmError(_("Failed running rpm transaction"))
 
Comment 27 Pradeep Kilambi 2007-02-02 13:30:03 EST
fyi, the above will log the messages to /var/log/up2date or what ever the config
is pointing to.
Comment 29 Pradeep Kilambi 2007-02-02 16:17:03 EST
yea I included this bug into the errata already, will update it with latest
package and changelog information
Comment 31 Jatin Nansi 2007-02-09 05:59:11 EST
*** Bug 221961 has been marked as a duplicate of this bug. ***
Comment 35 Pradeep Kilambi 2007-03-15 12:55:45 EDT
(In reply to comment #33)
> An interesting finding on this issue. up2date-4.4.5-1.i386.rpm does not
> care about obsoletes list as mentioned earlier in this ticket. So , if I
> use above version of up2date and issue the command up2date apache , it just
> goes ahead and installs the package. However , up2date package that is
> attached to this ticket fails with error as apache package get's added to
> obsoletes list due to httpd being in the base channel. So it looks like
> up2date got this additonal capability somewhere between version 4.4.5-1 and
> the current patched version attached to this ticket.
> 
> Regards ,
> Shashin.
> 
> Internal Status set to 'Waiting on SEG'
> Status set to: Waiting on Tech
> 
> This event sent from IssueTracker by jplans 
>  issue 107834
>               

- Could you paste the exact error message its failing with?

- Also did you try commenting out the patch in this bug and try the same ?

Comment 40 Preethi Thomas 2007-03-26 11:59:40 EDT
marking verified.
this seems to have been fixed with 
up2date-gnome-4.5.5-1.el4
up2date-4.5.5-1.el4
Comment 42 Ed Bailey 2007-04-24 08:43:03 EDT
Has this bug been fixed in RHEL 3? Is so then which release of up2date contains
the fix?
Comment 43 Pradeep Kilambi 2007-04-24 09:56:18 EDT
(In reply to comment #42)
> Has this bug been fixed in RHEL 3? Is so then which release of up2date contains
> the fix?

the fix for rhel-3 will be released as a part of rhel-3.9. This is currently
"on_qa". 
Comment 44 Red Hat Bugzilla 2007-05-01 19:19:48 EDT
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2007-0250.html

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