Code review comment for lp://qastaging/~johannes.erdfelt/nova/deferred-delete-instance

55 + "Restore a previously deleted instance."
56 +

I think you need more quotes to make this a docstring, and line 56 should be removed. Same goes for:

62 + "Force delete of instance before deferred cleanup."
63 +

Maybe this is pedantic, but I think these are too far indented, 4 spaces should suffice but it doesn't mess with pep8 so feel free to ignore me:

85 + actions = [
86 + extensions.ActionExtension("servers", "restore",
87 + self._restore),
88 + extensions.ActionExtension("servers", "forceDelete",
89 + self._force_delete),
90 + ]

I'd argue that instead of having:

vm_state = vm_states.DELETED
task_state = task_states.QUEUED_DELETE

...that we have something like this:

vm_state = vm_states.SOFT_DELETE

or

vm_state = vm_states.SCHEDULED_FOR_DELETION

I really go back and forth on this because I feel like task_states should be short-lived tasks and that vm_states are longer and potentially 'more permanent' if that makes sense. Having a vm_state of DELETED when it's not actually deleted seems confusing to me (although I understand for all intents and purposes that it's deleted).

My other question/comment is slightly larger, but wouldn't require a lot more work to implement. Would it be better to have a Compute API schedule_delete() method which can be called separately of delete()? The reason I ask is that I like methods to do one thing and do it well. If someone calls Compute API delete() they might expect the instance to be deleted. There is no indication given in the function call that the instance will simply be shut down.

nova.compute.api:API.schedule_delete(self, context, instance_id, datetime/seconds_in_the_future)

might be more explicit.

Overall I love the tests and code, just want to make sure the interface is correct.