Bug 737137 - sysconfig run methods return zero on bad data
Summary: sysconfig run methods return zero on bad data
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: matahari
Version: 6.2
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: rc
: ---
Assignee: Russell Bryant
QA Contact: Dave Johnson
URL:
Whiteboard:
Depends On:
Blocks: 743047
TreeView+ depends on / blocked
 
Reported: 2011-09-09 17:48 UTC by Dave Johnson
Modified: 2011-12-06 11:43 UTC (History)
3 users (show)

Fixed In Version: matahari-0.4.4-4.el6
Doc Type: Bug Fix
Doc Text:
No description required.
Clone Of:
Environment:
Last Closed: 2011-12-06 11:43:07 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:1569 normal SHIPPED_LIVE matahari bug fix and enhancement update 2011-12-06 00:39:06 UTC

Description Dave Johnson 2011-09-09 17:48:32 UTC
Description of problem:
==================================
the run_uri and run_string methods seem to always return zero even when bad data is given.  Here is a list of scenarios I would expect a non-zero result.

run_uri -> url not found
run_uri -> file not found
run_uri -> bad schema (any thing not puppet or augeas)
run_url -> empty key given ('')    actually runs, which should not be permitted?
run_string -> bad schema (any thing not puppet or augeas)
run_string -> empty key given ('') actually runs, which should not be permitted?


Version-Release number of selected component (if applicable):
====================================================================
v0.4.4-2


How reproducible:
==================================
100%

Comment 2 Dave Johnson 2011-09-09 18:18:27 UTC
Maybe this one too

>>> sysconfig.is_configured(m.getRandomKey(5))
OK (0) - {u'status': 'unknown'}

Comment 3 Russell Bryant 2011-09-12 13:42:35 UTC
Adam, we have talked about this some before.  The issue here is that puppet is executed asynchronously.  The code doesn't know if puppet succeeds or not.  It's going to have to work more like the services agent works when it executes things and waits for the result in the background.

Comment 4 Russell Bryant 2011-09-13 07:00:02 UTC
Upstream patches:

https://fedorahosted.org/pipermail/matahari/2011-September/001844.html

Comment 5 Russell Bryant 2011-09-13 08:43:17 UTC
There are some different aspects of this report that the changes address.

QMF only provides two options for returning back from a method call.  We can return success (which is when you see a 0) along with additional data, or we can throw an exception.  There will be cases considered a failure that will result in both of these cases.

If the arguments are blatantly wrong up front, we can throw an exception.  If the arguments at least appear valid at first, but something fails during the execution of the request, it will be a successful method return, but the additional data will indicate the details about the failure.

So, to your examples and what I would expect after these changes:

> run_uri -> url not found
> run_uri -> file not found
> run_uri -> bad schema (any thing not puppet or augeas)
> run_url -> empty key given ('')    actually runs, which should not be permitted?
> run_string -> bad schema (any thing not puppet or augeas)
> run_string -> empty key given ('') actually runs, which should not be permitted?

These now all throw an exception.

While going through and working on this, some additional cases have been addressed.  If you use run_uri or run_string, but puppet fails for some reason, that is now reflected in the result.  The QMF method call will return success (we did successfully execute the request), but the status included with return will indicate the failure and the exit code from the puppet process.

There is one use case you provided that doesn't behave how I would like it to as I went through and tested them.

> run_uri -> file not found

This one runs puppet, and returns back a status of "FAILED\n1".  I don't think it should run puppet at all.  curl is returning success, but the file is just the HTML of a 404 page.  Puppet obviously throws an error on that.  I will work on addressing this last use case before calling the patch ready to be put into a build.

Comment 6 Russell Bryant 2011-09-13 17:26:21 UTC
I just posted version 2 of this patch series to the mailing list.  I will merge it into upstream later today.  It includes some additional fixes, including a change that addresses the last use case so that it also results in a QMF exception being thrown.

Dave, do all of my comments make sense?  Does the new behaviour sound good to you?

Comment 7 Dave Johnson 2011-09-13 17:37:22 UTC
All the comments make sense and I believe the new behaviour is the right approach.

Comment 11 Dave Johnson 2011-10-05 01:58:03 UTC
good 2 go in 0.4.4-6

Comment 12 Russell Bryant 2011-11-16 22:25:40 UTC
    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
No description required.

Comment 13 errata-xmlrpc 2011-12-06 11:43:07 UTC
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/RHBA-2011-1569.html


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