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".

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 ...

Are you proposing fixing the destroy workflow:

• to refetch the VM's server_uuid after acquiring the ticket when the active
  provision is complete; and
• to just mark as destroyed if there isn't a server_uuid after acquiring
  the ticket?

such as overwriting a value previously set by e.g a CN being back up again

I don't think that applies here because "a VM with a server_uuid that
references a CN that cannot be found via CNAPI's GetServer" should only ever be
a CN that was manually DELETE'd from CNAPI by an operator. A CN that goes down
will still be there, but will have status=unknown.

and released as version "2.0".

And then start working through all VMAPI client usage in Triton to start
requesting Accept-Version: ~2. :)

LGTM. I'm still a little weirded out by having DeleteVm return a "VM object" (in the case where it is just marked as deleted right away). Is there value in having that response body? The client could do a GetVm if they really needed it.

Q: What happens now on DeleteVm if the state is "destroyed" already? If the state is "failed" already?

Are you proposing fixing the destroy workflow:

• to refetch the VM's server_uuid after acquiring the ticket when the active
  provision is complete; and
• to just mark as destroyed if there isn't a server_uuid after acquiring
  the ticket?

It would require fixing the workflow to refetch the VM's server_uuid after acquiring the ticket and just mark the VM as destroyed if after that refetch the VM still has no server_uuid. I should definitely have mentioned that.

I don't think that applies here because "a VM with a server_uuid that
references a CN that cannot be found via CNAPI's GetServer" should only ever be
a CN that was manually DELETE'd from CNAPI by an operator. A CN that goes down
will still be there, but will have status=unknown.

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?

And then start working through all VMAPI client usage in Triton to start
requesting Accept-Version: ~2. :)

You mean if these clients want the new behavior?

LGTM. I'm still a little weirded out by having DeleteVm return a "VM object" (in the case where it is just marked as deleted right away). Is there value in having that response body? The client could do a GetVm if they really needed it.

That's a good question. I don't know if there's much value in having a response body. Do you suggest then responding with 204 and not including any response body?

Q: What happens now on DeleteVm if the state is "destroyed" already? If the state is "failed" already?

Good question again :)

There would be no special case for a VM with a state failed. I think it makes sense for a VM in a state === 'failed' to transition to a state === 'deleted'.

For a VM that is already in a state 'deleted', I would think that the response should be an error. The rationale is that it doesn't mean anything to delete a VM that is in a state === 'deleted', and that it is possible that it is a symptom of an error from the client.

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.

You mean if these clients want the new behavior?

Yes.

Do you suggest then responding with 204 and not including any response body?

I think that would be fine. Curious on @joshwilsdon's opinion. He might have other experiences with code calling VMAPI DeleteVm.

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.

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. 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.

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.