Hide Forgot
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%
Maybe this one too >>> sysconfig.is_configured(m.getRandomKey(5)) OK (0) - {u'status': 'unknown'}
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.
Upstream patches: https://fedorahosted.org/pipermail/matahari/2011-September/001844.html
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.
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?
All the comments make sense and I believe the new behaviour is the right approach.
good 2 go in 0.4.4-6
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.
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