placement API not translating CannotDeleteParentResourceProvider to 409 Conflict

[Jay Pipes]

Something I noticed while reviewing https://review.openstack.org/#/c/546675/14/osc_placement/resources/resource_provider.py is that the code that prevents parent providers from being deleted:

https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/objects/resource_provider.py#L970

raises a CannotDeleteParentResourceProvider exception, but we're not catching that in the handler:

https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/resource_provider.py#L130-L140

Need to add a gabbit func test and catch that exception properly, converting it to an HTTP 409 Conflict.

[Bence Romsics (bence-romsics)]

Likely we should update the api-ref too, because this use of 409 is not documented at the moment:

https://developer.openstack.org/api-ref/placement/#delete-resource-provider

[Chris Dent (cdent)]

Deepak Mourya are you still planning to work on this? If not we need to remove you as the assignee so someone else can make some progress.

[Deepak Mourya (mourya007)]

Hi, Chris Dent yes i am going to start this as was busy in other assignment before.

[Deepak Mourya (mourya007)]

Do we need to add exception for CannotDeleteParentResourceProvider only in https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/resource_provider.py#L130-L140 and write test case or something else ?

[Chris Dent (cdent)]

Yes, basically a test needs to be created that raises the exception (by creating a parent and child and then trying to delete the parent) and the exception needs to be caught in the handler, and turned into a 409

[Deepak Mourya (mourya007)]

@chris Thanks, one more doubt is that where we can place our new test for this because i couldn't find file related here https://github.com/openstack/nova/tree/master/nova/tests/unit/api/openstack/placement/handlers.

[Jay Pipes (jaypipes)]

@moutya007: you *can* put the unit test in there, sure. You could also put it into a functional test since this is deterministic behaviour that you can control the setup for (as opposed to something like a race condition where it is very difficult to reproduce reliably outside of unit tests).

To do the functional test, you can copy one of the "gabbits" in this directory:

https://github.com/openstack/nova/tree/master/nova/tests/functional/api/openstack/placement/gabbits

and adapt appropriately. A good example to copy from would be something like this:

https://github.com/openstack/nova/blob/master/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml#L370-L389

Best,
-jay

[Deepak Mourya (mourya007)]

@jay ok thanks for your suggestion and help, I will make gabbit test as you suggested from the link and will put it under the directory [1]

https://github.com/openstack/nova/blob/master/nova/tests/functional/api/openstack/placement/gabbits/resource-provider.yaml

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/577726
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=1ea3d5ec743838ddb754739a9919a2ecf4c54f9f
Submitter: Zuul
Branch: master

commit 1ea3d5ec743838ddb754739a9919a2ecf4c54f9f
Author: deepak.mourya <email address hidden>
Date: Mon Jun 25 10:05:38 2018 +0530

    Handle CannotDeleteParentResourceProvider to 409 Conflict

    Error handling to raise the exception 409 while deleting
    the parent if it has children.

    Closes-Bug: #1770636
    Change-Id: I87df68992e4e635f009974f5205ca4919a4f2576

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/578804
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=40de025cedffb1aa75aa28388943c2b326266b83
Submitter: Zuul
Branch: stable/queens

commit 40de025cedffb1aa75aa28388943c2b326266b83
Author: deepak.mourya <email address hidden>
Date: Mon Jun 25 10:05:38 2018 +0530

    Handle CannotDeleteParentResourceProvider to 409 Conflict

    Error handling to raise the exception 409 while deleting
    the parent if it has children.

    Closes-Bug: #1770636
    Change-Id: I87df68992e4e635f009974f5205ca4919a4f2576
    (cherry picked from commit 1ea3d5ec743838ddb754739a9919a2ecf4c54f9f)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 18.0.0.0b3 development milestone.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 17.0.6 release.