[RFE] Conflict response cleanup

Bug #1651447 reported by Joanna Taryma
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ironic
Triaged
Wishlist
Unassigned
python-ironicclient
Triaged
Wishlist
Unassigned

Bug Description

Conflict exception is currently raised by Ironic in case of:
1) Exceptions related to duplicate name/UUID/address
2) InvalidState
3) PowerStateFailure
4) NodeAssociated
5) InstanceAssociated
6) Lack of standalone port support in porgroup
7) Disabling standalone_ports_supported if vif_port_id exists or pxe_enabled
8) NodeLocked

While situation 1 is a role model for 409 Conflict usage, other ones seem to improperly raise Conflict exception.
What is more, such overuse has consequences in client behavior;

Currently ironic client handles retries on its side. Client retires on following exception types:
* exc.Conflict
* exc.ServiceUnavailable,
* exc.ConnectionRefused,
* kexc.RetriableConnectionFailure

While it is true, that retry should be executed when e.g. node is locked (because it's state may change), there is no use is retrying with same request body with e.g. name duplication.

Cleaning up conflict exceptions, would enable:
* Lack of retries in case of duplicates (+ fail fast, + reduced load)
* Cleaner code by replacing non-retry usages with generic 400
* Retry-After header usage: retry usages could be replaces with e.g. 412 Precondition Failed, then agent could use this exception type for retries. Retry-After header can be set for such exception type (while it wouldn't make sense to set it for Conflict).

Tags: needs-spec rfe
Revision history for this message
Joanna Taryma (jtaryma) wrote :
Joanna Taryma (jtaryma)
Changed in ironic:
assignee: nobody → Joanna Taryma (jtaryma)
Changed in python-ironicclient:
assignee: nobody → Joanna Taryma (jtaryma)
Revision history for this message
Ruby Loo (rloo) wrote :

Thanks for looking into this. Please add a spec that contains details about which ones you propose to change from 409 to 400. How are we going to handle backwards compatibility (or maybe we don't)?

Changed in ironic:
status: New → Triaged
importance: Undecided → Wishlist
tags: added: rfe
tags: added: needs-spec
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ironic-specs (master)

Fix proposed to branch: master
Review: https://review.openstack.org/414194

Changed in ironic:
status: Triaged → In Progress
Changed in python-ironicclient:
importance: Undecided → Wishlist
status: New → Triaged
Joanna Taryma (jtaryma)
Changed in python-ironicclient:
status: Triaged → In Progress
Vladyslav Drok (vdrok)
Changed in ironic:
status: In Progress → Triaged
Changed in python-ironicclient:
status: In Progress → Triaged
Changed in ironic:
assignee: Joanna Taryma (jtaryma) → nobody
Changed in python-ironicclient:
assignee: Joanna Taryma (jtaryma) → nobody
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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