Bug #1511574 “[RFE] Support cleanup of all resources associated ...” : Bugs : python-neutronclient

Revision history for this message

John Davidge (john-davidge) wrote on 2015-10-30:

#1

I'd be happy to volunteer for this once the RFE is approved.

Revision history for this message

Cedric Brandily (cbrandily) wrote on 2015-10-30:

#2

Could ospurge[1] address (at least partially) your needs?

[1] https://github.com/openstack/ospurge

Henry Gessau (gessau) on 2015-10-30

Changed in neutron:
status:	New → Confirmed

Cedric Brandily (cbrandily) on 2015-10-30

tags:

added: usability

Henry Gessau (gessau) on 2015-10-30

tags:

added: ops

Revision history for this message

John Davidge (john-davidge) wrote on 2015-10-30:

#3

Yes, ospurge seems to meet the exact needs described by operators in the session. Some mentioned having scripts in place to do this already (although I don't remember ospurge being named specifically). The desire was that these functions become a part of the neutron API itself. Functionality similar to ospurge's --dry-run option was asked for as well.

The features already offered in ospurge would be a great starting point to discuss the scope of this RFE. Thanks for pointing it out Cedric.

Satyanarayana Patibandla (satya-patibandla) on 2015-10-30

Changed in neutron:
assignee:	nobody → Satyanarayana Patibandla (satya-patibandla)

Revision history for this message

John Davidge (john-davidge) wrote on 2015-10-30:

#4

@Satyanarayana This RFE requires approval before an assignee is decided. I'll assign myself for now until the discussion can take place.

Changed in neutron:
assignee:	Satyanarayana Patibandla (satya-patibandla) → nobody
assignee:	nobody → John Davidge (john-davidge)

Revision history for this message

Henry Gessau (gessau) wrote on 2015-11-01:

#5

See also https://bugs.launchpad.net/neutron/+bug/967832

Armando Migliaccio (armando-migliaccio) on 2015-11-11

Changed in neutron:
importance:	Undecided → Wishlist

John Davidge (john-davidge) on 2015-11-20

summary:

- Support cleanup of all resources associated with a given tenant with a
- single API call
+ [RFE] Support cleanup of all resources associated with a given tenant
+ with a single API call

Revision history for this message

Assaf Muller (amuller) wrote on 2015-11-20: Re: [RFE] Support cleanup of all resources associated with a given tenant with a single API call

#6

This is a cross-project concern and should be addressed as such. I don't want to introduce a new API endpoint to Neutron without getting cross-project consensus on that approach.

Revision history for this message

Armando Migliaccio (armando-migliaccio) wrote on 2015-11-24:

#7

I am not sure we'll ever get cross-project consensus to be honest.

We can rely on something like ospurge, that cleans up tenant resources from a project by using the project's API, but this may lead to a lot of chattiness, and to potential unrecoverable errors.

If we assumed that we can have an API exposed by Neutron that purges the resources from a single tenant, this can be done a lot more effectively. It's handy to have it, but not strictly necessary. If someone were to propose an implementation (as a service plugin perhaps), we could look at its merits.

Changed in neutron:
status:	Confirmed → Triaged

Revision history for this message

Armando Migliaccio (armando-migliaccio) wrote on 2015-11-24:

#8

To be discussed at the drivers meeting.

Revision history for this message

Assaf Muller (amuller) wrote on 2015-11-24:

#9

I don't want to merge a new project-purge API to Neutron without first at least having a discussion with the other major projects.

Revision history for this message

Henry Gessau (gessau) wrote on 2015-11-24:

#10

We have a volunteer (John Davidge). It might be that neutron is the first to add a "purge_resources_owned_by(tenant_id)" API, but that shouldn't stop us. We should definitely get cross-project agreement on how such an API should look/behave so that other projects can align when they get around to their implementations. I am willing to be the approver for this effort if we can agree to go for it. I got the sense that operators will love the feature.

Revision history for this message

Armando Migliaccio (armando-migliaccio) wrote on 2015-12-01:

#11

We should allow this work to proceed, but with the caveat that we do so by ensuring that we don't paint ourselves in the corner should a cross-project initiative take off. I'll let Assaf chime in on the rest. More discussion in [1].

[1] http://eavesdrop.openstack.org/meetings/neutron_drivers/2015/neutron_drivers.2015-12-01-15.00.log.html

tags:

added: rfe-approved
removed: rfe

Revision history for this message

Assaf Muller (amuller) wrote on 2015-12-01:

#12

Download full text (5.2 KiB)

First I'd like to link my previous work on this subject:
https://review.openstack.org/#/q/topic:bp/tenant-delete,n,z

As Armando said very nicely in today's drivers meeting, it was 'ahead of its time'. Although that effort was abandoned, we can learn a few things from it. First, on the practical level, here's the code that actually deleted resources:

https://review.openstack.org/#/c/92600/5/neutron/services/identity/project_deleted.py

If you scroll all the way down you can see (Roughly) the correct order in which we would delete resources. One thing to note is shared resources: If the tenant being deleted has a shared network with ports on it from other tenants, you wouldn't be able to delete that network. You should fail gracefully and skip it, and continue to delete other resources (The code doesn't currently do that).

The more interesting discussion is about the high level approach taken by that patch series, and that is:
1) Listen on the RPC bus for keystone tenant deletion messages (Back when I worked on it, those were actually not emitted by default, you had to configure Keystone to do that. I don't know if this has changed since)
2) In case neutron-server(s) was/were not up when that notification was sent, provide a CLI script that invokes the (Same) tenant-deletion code.
3) The same script would optionally reach out to Keystone, and write a list of resources that existed in Neutron that were owned by tenants that no longer exist according to Keystone. The script could also take such a list (Persisted to a file) and actually delete those resources. Persisting this information to files is useful for ops for auditing reasons, and for integrating with billing systems for example.

There's alternatives to the approach taken by that patch series:
1) One such alternative is to offer a script (os-purge, already mentioned in this bug) that would contain all code by all projects that offer tenant-deletion code.
2) All projects will offer an API that accepts a tenant-id and deletes all resources that belong to that tenant. Ops would have to create a small script that accepts a tenant-id, then calls nova project-purge <id>, neutron project-purge <id>, etc. We could of course modify os-purge to use each project's project-purge as those are implemented (i.e. os-purge would call neutron project-purge instead of implementing that logic on its own). Side note: *aaS and other Neutron stadium projects that maintain resources are an interesting problem. Would they introduce their code to the main Neutron repo, or would Neutron offer pluggability to project-purge?

Ultimately both 1 and 2 are simpler than my patch series and I would advise against listening on the RPC bus. It's a nice idea in theory but it's too complicated because you need to write the sync code anyway.

One advantage to the 2nd approach over the 1st is that each project is more likely to maintain higher quality and up to date code over having it out of repo.

I propose an alteration on the 2nd approach. Call it approach 2'. I'm afraid that until someone (John?) proposes a cross-project spec and follows up on the discussion, we might find ourselves in a position where we're the first ...

First I'd like to link my previous work on this subject:
https://review.openstack.org/#/q/topic:bp/tenant-delete,n,z

As Armando said very nicely in today's drivers meeting, it was 'ahead of its time'. Although that effort was abandoned, we can learn a few things from it. First, on the practical level, here's the code that actually deleted resources:

https://review.openstack.org/#/c/92600/5/neutron/services/identity/project_deleted.py

If you scroll all the way down you can see (Roughly) the correct order in which we would delete resources. One thing to note is shared resources: If the tenant being deleted has a shared network with ports on it from other tenants, you wouldn't be able to delete that network. You should fail gracefully and skip it, and continue to delete other resources (The code doesn't currently do that).

The more interesting discussion is about the high level approach taken by that patch series, and that is:
1) Listen on the RPC bus for keystone tenant deletion messages (Back when I worked on it, those were actually not emitted by default, you had to configure Keystone to do that. I don't know if this has changed since)
2) In case neutron-server(s) was/were not up when that notification was sent, provide a CLI script that invokes the (Same) tenant-deletion code.
3) The same script would optionally reach out to Keystone, and write a list of resources that existed in Neutron that were owned by tenants that no longer exist according to Keystone. The script could also take such a list (Persisted to a file) and actually delete those resources. Persisting this information to files is useful for ops for auditing reasons, and for integrating with billing systems for example.

There's alternatives to the approach taken by that patch series:
1) One such alternative is to offer a script (os-purge, already mentioned in this bug) that would contain all code by all projects that offer tenant-deletion code.
2) All projects will offer an API that accepts a tenant-id and deletes all resources that belong to that tenant. Ops would have to create a small script that accepts a tenant-id, then calls nova project-purge <id>, neutron project-purge <id>, etc. We could of course modify os-purge to use each project's project-purge as those are implemented (i.e. os-purge would call neutron project-purge instead of implementing that logic on its own). Side note: *aaS and other Neutron stadium projects that maintain resources are an interesting problem. Would they introduce their code to the main Neutron repo, or would Neutron offer pluggability to project-purge?

Ultimately both 1 and 2 are simpler than my patch series and I would advise against listening on the RPC bus. It's a nice idea in theory but it's too complicated because you need to write the sync code anyway.

One advantage to the 2nd approach over the 1st is that each project is more likely to maintain higher quality and up to date code over having it out of repo.

I propose an alteration on the 2nd approach. Call it approach 2'. I'm afraid that until someone (John?) proposes a cross-project spec and follows up on the discussion, we might find ourselves in a position where we're the first to merge a new API for this, an API that we'd have to commit to, and other projects wouldn't follow suit. Fast forward a year and you'll see yet another blog post bashing OpenStack consistency '3 out of X OpenStack projects delete their resources when a tenant is deleted. Swift and Neutron offer to do this via the API, although their clients of course do this using a different syntax: swift tenant-delete <id>, neutron project-purge <id>, Nova offers a CLI tool that does the same, and other projects don't bother'. (By the way the unified OpenStack CLI client is sounding pretty good right now). I'd like to *try* to get it right. Thus I propose that the assignee goes ahead with writing the code, and exposing this code via a CLI tool (More on this in a bit). In parallel the assignee would file a cross-project spec, and if that effort succeeds Neutron would offer an API, reusing the same code already merged, with other projects at least in agreement to follow.

Looking at what needs to be done, you'd need to implement essentially a function that accepts a tenant_id and deletes anything owned by that tenant. This can be *implemented internally* via either the plugin layer or the API layer. This is explained via some pseudo code:

def purge_project(tenant_id):
    core_plugin.delete_routers(tenant_id=tenant_id)

Or:

def purge_project(tenant_id):
    neutron_client.delete_routers(tenant_id=tenant_id)

(Or even just use the normal user's context [Not admin] and delete everything that user has access to, i.e. not filter on tenant_id).

The first option probably makes more sense, just because you don't need to go through the API layer to do this (But this isn't a very strong argument IMO, it'd be nice to have, but not required). However, the advantage of the second option is that you could write that code once, and then expose it either via a CLI tool or a new API verb without modifying the implementation.

Summary:
1) File a cross-project spec.
2) Write the code, expose it via CLI, not API.
3) If 1 succeeds (I hope and think it will with enough persistence) expose the already merged code via the API (This should be trivial).

Revision history for this message

Armando Migliaccio (armando-migliaccio) wrote on 2015-12-01:

#13

Assaf, thanks for the nicely written comment.

So to try, and distill it a bit to ensure we're on the same page. I'd like to say that for the sake of this effort:

* We should not pursue an end-to-end integration strategy: Neutron should provide the ability to purge project resources that is invoked by an external party.
* The smallest unit of cleanup is a project, aka tenant in existing Neutron parlance, rather than domain, users and whatnot.

To your summary:

1) File a cross-project spec.
2) Write the code, expose it via CLI, not API.
3) If 1 succeeds, expose the already merged code via the API.

I see the following open issues:

* If no REST API is going to be provided to start with, the tool must be invoked from the node where a single server is running, no? This sounds pretty limiting to me, but if acceptable, we should make this clear upfront. Not sure what'd gain by not exposing the API since operators would still use this in interim and switching to something different is still going to be painful. The difference in this case is that we'd make their life miserable asking them to ssh into one of boxes, when they could neatly do that from their home laptop connected to the admin url. It's either that, or I am grossly missing something and I am making a fool of myself :)
* We can't expect that the submitter of the purge code neutron side to be the same person that files a cross-project spec. That requires a different set of skills, i.e. we need a seasoned dude like yourself to help champion that through, are you prepared to do the legwork?
* We should be careful about the chain of dependencies between known and unknown (extension) resources, so the framework must be planned to allow for hooks and all.
* Testing: the tool can easily break if we don't test this continuously in that a commit about a dependency change, or a policy rule change, can easily invalidate the dependency chain, and that can prevent the deletion, leading to stale resources. This also bring me to the next point.
* purge is intrinsically an async operation, therefore the API should be task-oriented, where the user can check the operation status, whether it has completed successfully, whether there are some resources still behind etc.

In a nutshell, this is no small feat of engineering and a spec (both for the Neutron design internals) and the user facing API must be pursued.

Thoughts?

Assaf, thanks for the nicely written comment.

So to try, and distill it a bit to ensure we're on the same page. I'd like to say that for the sake of this effort:

* We should not pursue an end-to-end integration strategy: Neutron should provide the ability to purge project resources that is invoked by an external party.
* The smallest unit of cleanup is a project, aka tenant in existing Neutron parlance, rather than domain, users and whatnot.

To your summary:

1) File a cross-project spec.
2) Write the code, expose it via CLI, not API.
3) If 1 succeeds, expose the already merged code via the API.

I see the following open issues:

* If no REST API is going to be provided to start with, the tool must be invoked from the node where a single server is running, no? This sounds pretty limiting to me, but if acceptable, we should make this clear upfront. Not sure what'd gain by not exposing the API since operators would still use this in interim and switching to something different is still going to be painful. The difference in this case is that we'd make their life miserable asking them to ssh into one of boxes, when they could neatly do that from their home laptop connected to the admin url. It's either that, or I am grossly missing something and I am making a fool of myself :)
* We can't expect that the submitter of the purge code neutron side to be the same person that files a cross-project spec. That requires a different set of skills, i.e. we need a seasoned dude like yourself to help champion that through, are you prepared to do the legwork?
* We should be careful about the chain of dependencies between known and unknown (extension) resources, so the framework must be planned to allow for hooks and all.
* Testing: the tool can easily break if we don't test this continuously in that a commit about a dependency change, or a policy rule change, can easily invalidate the dependency chain, and that can prevent the deletion, leading to stale resources. This also bring me to the next point.
* purge is intrinsically an async operation, therefore the API should be task-oriented, where the user can check the operation status, whether it has completed successfully, whether there are some resources still behind etc.

In a nutshell, this is no small feat of engineering and a spec (both for the Neutron design internals) and the user facing API must be pursued.

Thoughts?

Revision history for this message

Assaf Muller (amuller) wrote on 2015-12-01:

#14

armax said:
> * If no REST API is going to be provided to start with, the tool must be invoked from the node where a single server is running, no? This sounds pretty limiting to me, but if acceptable, we should make this clear upfront. Not sure what'd gain by not exposing the API since operators would still use this in interim and switching to something different is still going to be painful. The difference in this case is that we'd make their life miserable asking them to ssh into one of boxes, when they could neatly do that from their home laptop connected to the admin url. It's either that, or I am grossly missing something and I am making a fool of myself :)

If the CLI script uses only the Neutron API client as I suggested then it could be used from anywhere the client is installed (That is the reason I suggested it uses the API client and not the plugin layer, I'm sorry if I was not clear).

> * We can't expect that the submitter of the purge code neutron side to be the same person that files a cross-project spec. That requires a different set of skills, i.e. we need a seasoned dude like yourself to help champion that through, are you prepared to do the legwork?

To be honest it's dangerous for me to commit to something like that, it might end up not happening because of my own time constraints. I mean well, but... I would prefer we define something fairly concrete then have John file the spec (If he is willing). Push comes to shove I'll do it.

> * We should be careful about the chain of dependencies between known and unknown (extension) resources, so the framework must be planned to allow for hooks and all.

Extension top level resources? Can you elaborate? Are you talking about *aaS, or something like QoS? Couldn't we use discovery to see if the Neutron server supports the extension and delete conditionally? I am not sure what you mean by 'hooks'.

> * Testing: the tool can easily break if we don't test this continuously in that a commit about a dependency change, or a policy rule change, can easily invalidate the dependency chain, and that can prevent the deletion, leading to stale resources. This also bring me to the next point.

Naturally :) The script should have a functional test, we can discuss this in the implementation phase.

> * purge is intrinsically an async operation, therefore the API should be task-oriented, where the user can check the operation status, whether it has completed successfully, whether there are some resources still behind etc.

I had not considered this. We should discuss this further.