GET on /plugins/ginger/network/cfginterfaces/<interfacename> fails with 500

[pkulkark]

Architecture: s390x, OS: KVM for IBM z Systems The api fails with 500 when tried to retrieve details of interface that is not configured (no persistent file exists). Instead it should returning 404: Resource not found.

[abhiramkulkarni]

I guess 500 is ok to return as interface cfg file is not found for the interface name passed

bash-4.3# curl -k -u root:passw0rd -H "Content-Type: application/json" -H "Accept: application/json" -X GET 'https://127.0.0.1:8001/plugins/ginger/network/cfginterfaces/bond0a' { "reason":"GINNET0057E: Persistent file is not available for an interface, 'bond0a'", "code":"500 Internal Server Error" } bash-4.3#

[sureshab]

@abhiramkulkarni With HTTP Status code - 404, it is meant for resource not found. If I understand correctly, when /cfginterfaces api is hit with some interface which doesn't have ifcfg file you are simply raising exception which results in 500, instead you should raise NotFoundError from wok exception.

Example: with /network/interfaces api [root@zfwcec103 ~]# curl -k -u root -H "Content-Type: application/json" -H "Accept: application/json" -X GET 'https://127.0.0.1:8001/plugins/ginger/network/interfaces/invalid' Enter host password for user 'root': { "reason":"GINNET0014E: invalid is not valid network interface", "code":"404 Not Found" }

With /network/cfginterfaces api [root@zfwcec103 ~]# curl -k -u root -H "Content-Type: application/json" -H "Accept: application/json" -X GET 'https://127.0.0.1:8001/plugins/ginger/network/cfginterfaces/invalid' Enter host password for user 'root': { "reason":"GINNET0057E: Persistent file is not available for an interface, 'invalid'", "code":"500 Internal Server Error" }

hope you got the difference.

[danielhb]

The HTTP 500 code should be issued only in an unexpected server/backend failure that prevented the backend from completing the request.

For situations that we can predict, the one described in this issue for example, we must return the proper error code (400, 404 ...).

[abhiramkulkarni]

Suresh/Daniel i agree with your points. Will investigate how to handle it.

Below are further observations and any suggestions are welcome.

The problem is to implement the right logic of identifying a valid resource or not. Interface resource deals only with physical interfaces and hence there is no confusion. cfginterface resource as deals with physical and ifcfg files, definitely needs a lot of fine tuning :).

Considering below use cases, 1)Valid interface and ifcfg file not present. ---> Resource :- Check if interface is present in ethtool.get_devices() and check if file exist. File not exist, send 500 error file not exist. ---> Collection :- Ignore this error in getlist as we dont want to block other interfaces which have ifcfg file.

2)Valid interface and ifcfg file present. ---> Check if interface is present in ethtool.get_devices() and check if file exist. File exists, return correct lookup.

3)Not a physical interface and ifcfg file present. --->This can happen for bond,vlan,bridge files which are in inactive state. Probably we have to write a parser of /etc/sysconfig/network-scripts to get this devices with valid physical interface. ---> If the interface passed is neither bond/vlan/bridge and still ifcfg file exists, we can still send resource not found error? Still i am unclear of all the possibilites here(Need more investigation)

4)Not a physical interface and not ifcfg file present. ---> Resource not found, return 400.

Current code is addressing step 1 and step2. step 4 we can handle. step 3 will need investigation/discussion.

Let me know your thoughts.

On Wed, 2015-12-30 at 10:18 -0800, Suresh Babu Angadi wrote:

@abhiramkulkarni With HTTP Status code - 404, it is meant for resource not found. If I understand correctly, when /cfginterfaces api is hit with some interface which doesn't have ifcfg file you are simply raising exception which results in 500, instead you should raise NotFoundError from wok exception. This is considering the scenario, valid physical interface which do not have ifcfg file.

Example: with /network/interfaces api [root@zfwcec103 ~]# curl -k -u root -H "Content-Type: application/json" -H "Accept: application/json" -X GET 'https://127.0.0.1:8001/plugins/ginger/network/interfaces/invalid' Enter host password for user 'root': { "reason":"GINNET0014E: invalid is not valid network interface", "code":"404 Not Found" }

With /network/cfginterfaces api [root@zfwcec103 ~]# curl -k -u root -H "Content-Type: application/json" -H "Accept: application/json" -X GET 'https://127.0.0.1:8001/plugins/ginger/network/cfginterfaces/invalid' Enter host password for user 'root': { "reason":"GINNET0057E: Persistent file is not available for an interface, 'invalid'", "code":"500 Internal Server Error" }

hope you got the difference.

— Reply to this email directly or view it on GitHub.

[sureshab]

@abhiramkulkarni I would go with what is supported from /cfginterfaces, so all the interfaces returned from /cfginterfaces collection should have resource and user should be able to get details using /cfginterfaces/:name. For other requests it should be 404 .

if the api supports only the interfaces which has ifcfg files & part of ethtool.get_devices(), then its safe to return "400 Bad Request" or "404 Resource Not Found" for any other requests.

From my point of view: Case 1) from api doc, it doesn't satisfy the requirement - so it should go with 404 resource not found, error message can include - ifcfg file is missing .

Case 2) Success case - return interface details.

Case 3) what would be returned from ethtool.get_devices() for not a physical interface and ifcfg file present? if it doesn't list those, then you can safely return 404.

3)Not a physical interface and ifcfg file present. --->This can happen for bond,vlan,bridge files which are in inactive state. ---> Will it be part of your collection /cfginterfaces? Also I guess currently, activate/deactiveate is not part of /cfginterfaces, it comes as part of /interfaces. Probably we have to write a parser of /etc/sysconfig/network-scripts to get this devices with valid physical interface. ---> If the interface passed is neither bond/vlan/bridge and still ifcfg file exists, we can still send resource not found error? Still i am unclear of all the possibilites here(Need more investigation) ---> 404 for simply files and not part of ethtool.get_devices()

Case 4) 404 - Not Found