Bug 709365 - kickstart.deleteProfile does not return 0 on kickstart not found
Summary: kickstart.deleteProfile does not return 0 on kickstart not found
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite 5
Classification: Red Hat
Component: API
Version: 540
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tomas Lestach
QA Contact: Šimon Lukašík
URL:
Whiteboard:
Depends On:
Blocks: sat54-blockers
TreeView+ depends on / blocked
 
Reported: 2011-05-31 14:00 UTC by Karl Abbott
Modified: 2018-11-14 12:28 UTC (History)
3 users (show)

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.
Clone Of:
Environment:
Last Closed: 2011-10-20 08:21:52 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
ks.py (886 bytes, text/x-python)
2011-05-31 14:27 UTC, Marcelo Moreira de Mello
no flags Details
ks.py (854 bytes, text/x-python)
2011-05-31 14:44 UTC, Marcelo Moreira de Mello
no flags Details
Patch proposed (1.89 KB, patch)
2011-05-31 16:33 UTC, Marcelo Moreira de Mello
no flags Details | Diff
Patch proposed - fixed typo (1.88 KB, patch)
2011-05-31 16:48 UTC, Marcelo Moreira de Mello
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:1388 0 normal SHIPPED_LIVE Red Hat Network Satellite server spacewalk-java bug fix and enhancement update 2011-10-20 08:21:14 UTC

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


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