Bug 709365

Summary: kickstart.deleteProfile does not return 0 on kickstart not found
Product: Red Hat Satellite 5 Reporter: Karl Abbott <kabbott>
Component: APIAssignee: Tomas Lestach <tlestach>
Status: CLOSED ERRATA QA Contact: Šimon Lukašík <slukasik>
Severity: medium Docs Contact:
Priority: medium    
Version: 540CC: cperry, mmello, slukasik
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: spacewalk-java-1.2.39-101 Doc Type: Bug Fix
Doc Text:
Result: Only updating kickstart.deleteProfile API documentation to match the behavior.
 Story Points: ---
Clone Of: Environment:
Last Closed: 2011-10-20 08:21:52 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: 715348    
Attachments:
Description Flags
ks.py
none
ks.py
none
Patch proposed
none
Patch proposed - fixed typo none

Description Karl Abbott 2011-05-31 14:00:05 UTC 
Description of problem:

The documentation for the API method kickstart.deleteProfile states that it will return 1 if the profile is found and deleted and 0 if the kickstart wasn't found or was unable to be deleted. Instead, when this method can't find the kickstart, it returns a traceback:

Traceback (most recent call last):
  File "10-00477148.py", line 13, in ?
    rc = client.kickstart.deleteProfile(key, 'non-existant-kickstart-profile')
  File "/usr/lib64/python2.4/xmlrpclib.py", line 1096, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib64/python2.4/xmlrpclib.py", line 1383, in __request
    verbose=self.__verbose
  File "/usr/lib64/python2.4/xmlrpclib.py", line 1147, in request
    return self._parse_response(h.getfile(), sock)
  File "/usr/lib64/python2.4/xmlrpclib.py", line 1286, in _parse_response
    return u.close()
  File "/usr/lib64/python2.4/xmlrpclib.py", line 744, in close
    raise Fault(**self._stack[0])
xmlrpclib.Fault: <Fault 2753: 'redstone.xmlrpc.XmlRpcFault: Invalid kickstart label: non-existant-kickstart-profile'>


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

Satellite 5.4

How reproducible:

Always.

Steps to Reproduce:

Run the following code for a kickstart profile that you know doesn't exist:

#!/usr/bin/python
import xmlrpclib

SATELLITE_URL = "https://satellite.gsslab.pnq.redhat.com/rpc/api"
SATELLITE_LOGIN = "admin"
SATELLITE_PASSWORD = "redhat"

## BELOW PROBABLY NEEDS NO MODIFICATION ##
client = xmlrpclib.Server(SATELLITE_URL, verbose=0)

key = client.auth.login(SATELLITE_LOGIN, SATELLITE_PASSWORD)

rc = client.kickstart.deleteProfile(key, 'non-existant-kickstart-profile')
# expect rc to be 0, got an exception

client.auth.logout(key)
  
Actual results:

See traceback above.

Expected results:

No traceback. If we did a "print rc" after the kickstart.deleteProfile line, we would get a 0.

Additional info:

Reproducer set up on satellite.gsslab.pnq.redhat.com (root/redhat) as /root/APIs/10-00477148.py
Comment 1 Marcelo Moreira de Mello 2011-05-31 14:27:15 UTC 
Created attachment 502008 [details]
ks.py

Hello, 

Issue reproduced

stheo $> python ks.py 
Label:  CASE_00445449-rhel56-host-plataform
Label:  rhel4as_u5_x86_64-guest-kvm
Label:  rhel4as_u5_x86_64-guest-kvm-step-by-step
Label:  rhel4as_u8_x86_64-guest-kvm-step-by-step
Label:  rhel54-CASE-00431297
Label:  rhel55-32bits-xen-guest
Label:  rhel55-64bits-xen-guest
Label:  rhel55-KVM-guest-x86_64
Label:  rhel55-server-bare-metal-x86_64
Label:  rhel55-server-bare-metal-x86_64-serial-console
Label:  rhel56-CASE-00431297
Label:  rhel56-x64-RT
Label:  rhel6-SAN-case-00411397
Label:  rhel6-server-bare-metal
Label:  rhel6-server-bare-metal-serial-console
Label:  rhel6-server-guest-kvm
Label:  rhel6-x86_64-VM

	Would you like to delete a kickstart? [y/n] y
	Enter the kickstart label to be deleted: asdfasdf
Traceback (most recent call last):
  File "ks.py", line 24, in <module>
    value = client.kickstart.deleteProfile(key,ks2delete)
  File "/usr/lib64/python2.7/xmlrpclib.py", line 1224, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib64/python2.7/xmlrpclib.py", line 1570, in __request
    verbose=self.__verbose
  File "/usr/lib64/python2.7/xmlrpclib.py", line 1264, in request
    return self.single_request(host, handler, request_body, verbose)
  File "/usr/lib64/python2.7/xmlrpclib.py", line 1297, in single_request
    return self.parse_response(response)
  File "/usr/lib64/python2.7/xmlrpclib.py", line 1468, in parse_response
    return u.close()
  File "/usr/lib64/python2.7/xmlrpclib.py", line 793, in close
    raise Fault(**self._stack[0])
xmlrpclib.Fault: <Fault 2753: 'redstone.xmlrpc.XmlRpcFault: Invalid kickstart label: asdfasdf'>

 Cheers, 
Marcelo Moreira de MEllo
Comment 2 Marcelo Moreira de Mello 2011-05-31 14:44:57 UTC 
Created attachment 502014 [details]
ks.py
Comment 3 Marcelo Moreira de Mello 2011-05-31 14:54:00 UTC 
Quick look into the code: 



java/code/src/com/redhat/rhn/frontend/xmlrpc/kickstart/KickstartHandler.java

{SNIP}

    /** 
     * delete a kickstart profile
     * @param sessionKey the session key
     * @param ksLabel the kickstart to remove an ip range from
     * @return 1 on removal, 0 if not found, exception otherwise
     *
     * @xmlrpc.doc Delete a kickstart profile
     * @xmlrpc.param #session_key()
     * @xmlrpc.param #param_desc("string", "ksLabel", "The label of
     * the kickstart profile you want to remove")
     * @xmlrpc.returntype int - 1 on successful deletion, 0 if kickstart wasn't found
     *  or couldn't be deleted.
     */
    public int deleteProfile(String sessionKey, String ksLabel) {
        User user = getLoggedInUser(sessionKey);
        if (!user.hasRole(RoleFactory.CONFIG_ADMIN)) {
            throw new PermissionException(RoleFactory.CONFIG_ADMIN);
        }   
        KickstartData ksdata = lookupKsData(ksLabel, user.getOrg());
        KickstartDeleteCommand com = new KickstartDeleteCommand(ksdata.getId(), user);
        ValidatorError error = com.store();
        if (error == null) {
            return 1;
        }   
        else {
            return 0;
        }   
    }  

{SNIP}
Comment 4 Tomas Lestach 2011-05-31 15:02:26 UTC 
In this case the api documentation shall be fixed.
It's not the satellite way to return 0,  there can be several reasons, why an api fails ... and just reuturning 0 doesn't tell the user much about the reason.
Comment 5 Marcelo Moreira de Mello 2011-05-31 16:33:05 UTC 
Created attachment 502041 [details]
Patch proposed

Hello, 

 Patch sent to approval to spacewalk-devel list. 

    Mail Thread: https://www.redhat.com/archives/spacewalk-devel/2011-May/msg00017.html

 Cheers, 
Marcelo Moreira de Mello
Comment 6 Marcelo Moreira de Mello 2011-05-31 16:48:40 UTC 
Created attachment 502045 [details]
Patch proposed - fixed typo

Hello, 

 Follow a better looking patch. The same were sent to spacewalk-deve mail thread. 

 Cheers, 
Marcelo Moreira de Mello
Comment 7 Tomas Lestach 2011-06-01 11:33:03 UTC 
Patch applied.

spacewalk.git: 2838959597c4660c63c061eaf7435d8fad55f9e6

Thank you Marcelo!
Comment 10 Tomas Lestach 2011-09-27 16:00:46 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:
Result:
Only updating kickstart.deleteProfile API documentation to match the behavior.
Comment 12 Šimon Lukašík 2011-10-05 11:02:42 UTC 
Moving to Verified:

Testing procedure:
 - automated test
 - reading api documentation

Verified against:
spacewalk-java-1.2.39-101
Comment 13 errata-xmlrpc 2011-10-20 08:21:52 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-1388.html