delete-vm-breakage.md

Introduction

ZAPI-685 and ZAPI-782 were two changes made to the same DeleteVm VMAPI endpoint, and they both broke VMAPI clients in some way.

ZAPI-685 was originally made to avoid starting workflow jobs, and thus potentially flooding the jobs queue, when handling DeleteVm requests, unless necessary.

It broke SAPI, which was fixed with ZAPI-782 by changing the behavior of VMAPI to respond with an error in cases when previously VMAPI would send a response that wasn't consistent with the format of other successful responses. It also caused ZAPI-755, preventing VMs that were actually stuck in 'provisioning' state from being deleted.

Unfortunately, ZAPI-782 broke one of sdc-vm-agent's tests, which was relying on that behavior from ZAPI-685.

At that point it seems clear that:

1. Although obvious in hindsight, changing the behavior of the DeleteVm endpoint, even for what seems like edge cases, can have a significant impact on the rest of the Triton ecosystem

2. The behavior of the DeleteVm endpoint is perfectible, and even if some of the consequences of ZAPI-685 and ZAPI-782 were unfortunate, it seems that it can be improved

Recommended changes

This section recommends making two changes to the current behavior of VMAPI's DeleteVm endpoint.

• DeleteVm on a VM with no server_uuid can happen on a VM actually being provisioned (its server_uuid will be set at the end of the provisioning workflow if that VM is actually provisioned on a CN). To avoid races where a VM would end up being marked as destroyed when it's actually provisioned, these requests need to be serialized by acquiring a ticket. For that, they need to go through a workflow job and their response needs to have a 202 HTTP status code and its data needs to be a "VM job object".

• DeleteVm on a VM with a server_uuid that references a CN that cannot be found via CNAPI's GetServer endpoint could avoid going through a workflow job. That VM would not be being provisioned, so the race previously described would not be a concern. However, in order to prevent other races, such as overwriting a value previously set by e.g a CN being back up again and its vm-agent PUTing the VM with state !== 'destroyed', it would be helpful if it used an etag. In case the deletion would be successful, the response could be a 200 HTTP status code with the response's data being the VM object with the status set to destroyed. I'm not sure it makes sense in this case to set the zone_state status, since there is likely no underlying zone if the VM has no server_uuid associated.

These recommendations are not in line with the guidelines in RFC 2616 about DELETE requests. However, I don't think it necessarily makes sense to follow these to the letter since that section of the RFC assumes that DELETEd objects are made inaccessible or removed from the system, which is not the case for VMAPI. In other words, it seems VMAPI's API deviates enough from a pure REST API that it might not be worthwhile to strictly require sticking to what that RFC mandates for the DeleteVm endpoint.

Versioning

As we've seen with changes made with ZAPI-685 and ZAPI-782, changes to the DeleteVM endpoint' response can be disruptive for the Triton ecosystem. Ideally, VMAPI would introduce a versioning mechanism before integrating these changes.

One of the challenges here is that, since VMAPI currently doesn't have any support for API versioning, all current VMAPI clients always use the latest version. Thus, if we were to introduce API versioning to VMAPI, the default API version would need to be a version that represents the current state of VMAPI at the time versioning mechanisms are introduced. That version could be version 1.0.

sdc-imgapi implemented something similar by considering all clients that do not specify an Accept-Version header implicitly use version 1.0 and then conditionally changing some endpoints' response's format when an accepted version different from the default version is provided.

After these versioning mechanisms are introduced in VMAPI, the changes mentioned in the previous section "Recommended changes" could be made, and released as version "2.0".

OK, the use case I had in mind is: an operator deletes the server and then reconnect it to the same DC later. Maybe that's not a valid use case?

I think that's a pretty crazy use case. I wouldn't be too worried about it... unless it is straightforward to cope with.

The way to deal with it seems to be: passing the etag from the loaded VM to the putObject request, and error if putObject results in an error. That seems straightforward enough, but I won't push for it.

I think it makes sense for a VM in a state === 'failed' to transition to a state === 'deleted'.

Hrm. That feels a little weird. A state=failed VM is where I expect to have an "error" field with error details for why it failed. There might be other expected field differences.

OK, it seems currently that there's no mechanism for guarding against deleting a 'failed' VM, but I could have missed something. Not that it would be a justification to continue doing that. I'd be interesting in @joshwilsdon's feedback.

For a VM that is already in a state 'deleted', I would think that the response should be an error.

Okay. Perhaps "410 Gone" for that.

The problem with using that status code is that the VM object is still available, e.g via GetVm.

I don't have a strong opinion there. I'd be swayed by (a) what other APIs of ours do for "DELETE" of a state=deleted resource and also by (b) what VMAPI DleteVm does now.

Sounds good.

sounds good.

My feedback on the 'failed' issue...

The VM object won't contain any special information if it's 'failed'. And in fact there are a few different ways a VM can be in state failed, including:

• the provision job failed before we even called the CN (no VM exists)
• the provision job failed after we created the VM on the CN (the VM may temporarily be marked failed by the job before going running)
• the provision job failed because the VM was created and started but zoneinit collapsed into its own filth (the VM will exist on the CN and state will be 'failed')

In the 3rd case especially, we want to be able to actually still send the destroy job to the CN and clean up this VM. So I think going from failed->destroyed makes sense. The:

sdc-vmapi /vms/<uuid>jobs | json -Ha uuid name create_timestamp execution

command can be used if you're looking for whether the "job" failed or not.