Don't stop and delete VMs that failed to clone

[edigaryev]

Resolves https://github.com/cirruslabs/orchard/issues/120.

[fkorotkov]

Should delete of non-existing VM return 0 from tart delete command? What do you think? Or return exit code 2 like tart run for an already running VM.

[edigaryev]

Or return exit code 2 like tart run for an already running VM.

Added in https://github.com/cirruslabs/tart/pull/597, but the thing is that we still need to avoid calling "tart stop" and "tart delete" to avoid errors in logs 🤷

[ruimarinho]

This is one of those cases where both approaches have merit.

If delete returns exit code 0:

Pros:

• Idempotency: it can be especially useful for scripts or automation tools where the delete operation can be repeated.
• User Experience: the user might just want to ensure the VM is deleted. If it's already deleted, the command has technically succeeded in ensuring the VM is not present.

Cons:

• Lack of Information: users might not be aware that the VM did not exist in the first place, and this might hide potential bugs.

If delete returns exit code 2:

Pros:

• Consistency: if the tart run command returns an error when a VM is already running, it would be consistent to return an error when trying to delete a non-existing VM.
• Clear Feedback: it informs the user that the VM they tried to delete wasn't there, which can be useful for troubleshooting or ensuring correctness.

Cons:

• Breaks Idempotency: running the delete command multiple times can yield different outcomes.
• Potentially Interruptive: if integrated into scripts or automation workflows, this behavior might break them unless the exit code is explicitly handled.

Since orchard's objective is to maintain the state of VMs, maybe keeping the user always informed of the actual state sounds more reasonable.

[edigaryev]

@ruimarinho this is a pretty good analysis, but it's more applicable to the Tart itself rather than Orchard.

Since Orchard is supervising the Tart, it has a knowledge that some VM had never existed (in case we failed to clone it), so it's counterproductive to even try to delete that VM because this generates noise in the logs and makes users less likely to follow them.

P.S. "idempotency" issue can be easily alleviated by ignoring the exit code, e.g. in Shell scripts: tart delete <VM> || true.

[ruimarinho]

Indeed, my comment was more targeted at tart rather than orchard. Orchard should simply handle it gracefully, I agree.