conflict (HTTP 409) incorrect for some cases

Bug #1461140 reported by Ruby Loo
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Ironic
Triaged
Medium
Unassigned

Bug Description

I think we need to revisit when we should be raising exception.Conflict. This causes an HTTP 409 status in the response. In an IRC chat with Lucas about it [1], Lucas mentioned "usually CONFLICT means another operation is in progress so the request conflicted".

I've encountered two cases recently where we probably should not be returning it:
- when a node is destroyed (deleted) [2], it makes sense for NodeLocked, but not if an instance is associated with a node or the node is in an unexpected state
- if a node is created (or updated) with the name of an existing node, the DuplicateName exception is percolated up.

I think we need to:
1. understand what is a Conflict (to the user) and what isn't
2. find out where we're 'abusing' it and make the appropriate changes
3. this is unclear to me -- if we change the HTTP status, it could break existing deployments that eg, are keying on 'HTTP 409'. How do we deal with this? (Maybe a new microversion?)

For example, using python-ironicclient, if I try to create a node with the name of an existing node, the client handles conflicts by retrying 6 times (so issues the request 7 times) to the server, before failing.

$ ironic node-create -d fake_ipmitool -n foo
+--------------+--------------------------------------+
| Property | Value |
+--------------+--------------------------------------+
| uuid | 20374cd3-e191-4685-910f-d117121643c0 |
| driver_info | {} |
| extra | {} |
| driver | fake_ipmitool |
| chassis_uuid | |
| properties | {} |
| name | foo |
+--------------+--------------------------------------+

$ ironic node-create -d fake_ipmitool -n foo
A node with name foo already exists. (HTTP 409)

[1] around 2015-06-02T14:59:53, http://eavesdrop.openstack.org/irclogs/%23openstack-ironic/%23openstack-ironic.2015-06-02.log

[2] https://github.com/openstack/ironic/blob/b0840884a14f9efdee89c7c45e77d1d602e9616f/ironic/conductor/manager.py#L1262

Tags: api needs-spec
Ruby Loo (rloo)
Changed in ironic:
assignee: nobody → Ruby Loo (rloo)
Dmitry Tantsur (divius)
Changed in ironic:
status: New → Confirmed
importance: Undecided → Medium
tags: added: api
Changed in ironic:
assignee: Ruby Loo (rloo) → Lucas Alvares Gomes (lucasagomes)
Revision history for this message
Ruby Loo (rloo) wrote :

We discussed this a bit at our weekly meeting on July 13, starting at 17:23:54:
    http://eavesdrop.openstack.org/meetings/ironic/2015/ironic.2015-07-13-17.00.log.html

Changed in ironic:
assignee: Lucas Alvares Gomes (lucasagomes) → nobody
Michael Turek (mjturek)
Changed in ironic:
assignee: nobody → Michael Turek (mjturek)
Revision history for this message
Ruby Loo (rloo) wrote :

Hi Michael, are you still working on this? It has been more than 6 months...

Revision history for this message
Jay Faulkner (jason-oldos) wrote :

Unassigning Michael since there's been no comment/patch in months, and didn't respond to Ruby's ping in the bug in September.

Changed in ironic:
status: Confirmed → Triaged
assignee: Michael Turek (mjturek) → nobody
Joanna Taryma (jtaryma)
Changed in ironic:
assignee: nobody → Joanna Taryma (jtaryma)
Joanna Taryma (jtaryma)
Changed in ironic:
status: Triaged → In Progress
Revision history for this message
Joanna Taryma (jtaryma) wrote :

After reading the mentioned discussion, I implemented a new exception type Locked, that shall set code to 423 (http_client.LOCKED) in case a resource is locked.

However, in in pecan does not accept 423 as valid response - it is not contained in six.moves.http_client.responses and it returns 500 instead (with a valid message, though).

Maybe 412 can do the job?

Full list of supported 400 codes:
400: 'Bad Request',
401: 'Unauthorized',
402: 'Payment Required',
403: 'Forbidden',
404: 'Not Found',
405: 'Method Not Allowed',
406: 'Not Acceptable',
407: 'Proxy Authentication Required',
408: 'Request Timeout',
409: 'Conflict',
410: 'Gone',
411: 'Length Required',
412: 'Precondition Failed',
413: 'Request Entity Too Large',
414: 'Request-URI Too Long',
415: 'Unsupported Media Type',
416: 'Requested Range Not Satisfiable',
417: 'Expectation Failed'

Dmitry Tantsur (divius)
Changed in ironic:
status: In Progress → Triaged
assignee: Joanna Taryma (jtaryma) → nobody
tags: added: needs-spec
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.