diff --git a/CHANGELOG.md b/CHANGELOG.md index dc9cea3b1a1..0722ae60ce8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ follows [Semantic Versioning](https://semver.org) guidelines. server every few seconds and move on once the work is done, with a spinning icon to show they're live; before, they stayed put until the page was refreshed. +### Changed + +- Deleting a server and keeping its virtual machine are now separate actions. **Delete** always removes everything -- + backups (including locked ones), the virtual machine on the node, and the server's entry in Convoy -- and when the + virtual machine is already gone from the node, it simply removes the Convoy entry. **Disconnect**, a new button next + to it, replaces the "Do not purge VM and related files" checkbox: it removes the server from Convoy and leaves the + virtual machine and its backups on the node. The API is unchanged; `DELETE` with `no_purge` still disconnects. + ### Fixed - Fixed every error from Proxmox filling the log with three copies of the same stack trace -- about 256 lines per error, @@ -27,6 +35,12 @@ follows [Semantic Versioning](https://semver.org) guidelines. - Fixed a failed reinstall of an already-failed server staying on "Installing" forever instead of going back to "Install failed". The failure handler wrote through the copy of the server captured when the reinstall started, and when that copy already read "install_failed" the write was skipped as a no-op. +- Fixed deleting a server failing, over and over, when its virtual machine was already gone from the node or it had a + locked backup. A failed delete can now also be retried, or the server disconnected, straight from the "Failed to + Delete" screen, and a server stuck on "Deleting" can be disconnected. +- Fixed a delete treating any error from Proxmox as "the virtual machine is gone". Only Proxmox's own "does not exist" + counts now, so a node that is briefly unreachable can no longer get a server removed from Convoy while its virtual + machine keeps running. ## v4.7.0-rc.2 diff --git a/app/Http/Controllers/Admin/ServerController.php b/app/Http/Controllers/Admin/ServerController.php index 354268d34df..7a9951c1ce4 100644 --- a/app/Http/Controllers/Admin/ServerController.php +++ b/app/Http/Controllers/Admin/ServerController.php @@ -2,7 +2,6 @@ namespace Convoy\Http\Controllers\Admin; -use Convoy\Enums\Server\Status; use Convoy\Enums\Server\SuspensionAction; use Convoy\Exceptions\Repository\Proxmox\ProxmoxConnectionException; use Convoy\Http\Controllers\ApiController; @@ -138,13 +137,14 @@ public function unsuspend(Server $server) return $this->returnNoContent(); } + /** + * Deletes the server: its backups, its VM, then the Convoy entry. With + * `no_purge`, only disconnects it -- the Convoy entry goes and everything on + * the node stays. + */ public function destroy(Request $request, Server $server) { - $this->connection->transaction(function () use ($server, $request) { - $server->update(['status' => Status::DELETING->value]); - - $this->deletionService->handle($server, $request->input('no_purge', false)); - }); + $this->deletionService->handle($server, disconnect: $request->boolean('no_purge')); return $this->returnNoContent(); } diff --git a/app/Jobs/Server/PurgeBackupsJob.php b/app/Jobs/Server/PurgeBackupsJob.php index 6d9a6bcbbf2..86012581bce 100644 --- a/app/Jobs/Server/PurgeBackupsJob.php +++ b/app/Jobs/Server/PurgeBackupsJob.php @@ -21,7 +21,10 @@ class PurgeBackupsJob implements ShouldQueue public int $timeout = 300; - public function __construct(protected int $serverId) + /** + * @param bool $force also delete locked backups, as deleting the server does + */ + public function __construct(protected int $serverId, protected bool $force = false) { // } @@ -37,6 +40,6 @@ public function handle(PurgeBackupsService $service): void { $server = Server::findOrFail($this->serverId); - $service->handle($server); + $service->handle($server, $this->force); } } diff --git a/app/Jobs/Server/WaitUntilVmIsDeletedJob.php b/app/Jobs/Server/WaitUntilVmIsDeletedJob.php index d8e3a890ebb..be492b0c87b 100644 --- a/app/Jobs/Server/WaitUntilVmIsDeletedJob.php +++ b/app/Jobs/Server/WaitUntilVmIsDeletedJob.php @@ -35,10 +35,19 @@ public function handle(ServerBuildService $service): void { $server = Server::findOrFail($this->serverId); - $isDeleted = $service->isVmDeleted($server); - - if (!$isDeleted) { + // Only Proxmox's "does not exist" counts as deleted. Any other error + // is thrown and retried: treating an outage as "gone" would let the + // chain drop the server from Convoy while its VM is still there. + if ($service->vmExists($server)) { $this->release(3); } } + + /** + * Wait between retries after an error instead of re-polling at once. + */ + public function backoff(): int + { + return 5; + } } diff --git a/app/Services/Backups/BackupDeletionService.php b/app/Services/Backups/BackupDeletionService.php index 63d77c3f33c..ccf25c701d3 100644 --- a/app/Services/Backups/BackupDeletionService.php +++ b/app/Services/Backups/BackupDeletionService.php @@ -15,9 +15,12 @@ public function __construct(private ConnectionInterface $connection, { } - public function handle(Backup $backup) + /** + * @param bool $force delete even a locked backup, as deleting its server does + */ + public function handle(Backup $backup, bool $force = false) { - if ($backup->is_locked && ($backup->is_successful && !is_null($backup->completed_at))) { + if (! $force && $backup->is_locked && ($backup->is_successful && !is_null($backup->completed_at))) { throw new BackupLockedException(); } diff --git a/app/Services/Backups/PurgeBackupsService.php b/app/Services/Backups/PurgeBackupsService.php index 32f6dcd8efb..fe543fe44a0 100644 --- a/app/Services/Backups/PurgeBackupsService.php +++ b/app/Services/Backups/PurgeBackupsService.php @@ -15,12 +15,12 @@ public function __construct( { } - public function handle(Server $server) + public function handle(Server $server, bool $force = false) { $backups = $this->backupRepository->getNonFailedBackups($server)->get(); - $backups->each(function (Backup $backup) { - $this->backupDeletionService->handle($backup); + $backups->each(function (Backup $backup) use ($force) { + $this->backupDeletionService->handle($backup, $force); }); } } diff --git a/app/Services/Servers/ServerBuildService.php b/app/Services/Servers/ServerBuildService.php index a17fb522b1d..134efb71e69 100644 --- a/app/Services/Servers/ServerBuildService.php +++ b/app/Services/Servers/ServerBuildService.php @@ -7,6 +7,8 @@ use Convoy\Repositories\Proxmox\Server\ProxmoxConfigRepository; use Convoy\Repositories\Proxmox\Server\ProxmoxServerRepository; use Convoy\Exceptions\Repository\Proxmox\ProxmoxConnectionException; +use Illuminate\Http\Client\RequestException; +use Illuminate\Http\Client\Response; /** * Class SnapshotService @@ -56,4 +58,45 @@ public function isVmDeleted(Server $server): bool return false; } + + /** + * Whether the server's VM exists on its node. + * + * Unlike isVmDeleted(), only Proxmox's own "does not exist" answer counts as + * missing. Any other failure -- the node unreachable, a bad token, a 5xx -- + * is rethrown, so an outage can never be mistaken for a VM that's gone and + * lead to a server being dropped from Convoy while its VM keeps running. + * + * @throws ProxmoxConnectionException + */ + public function vmExists(Server $server): bool + { + try { + $this->configRepository->setServer($server)->getConfig(); + } catch (ProxmoxConnectionException $e) { + if (self::isVmMissingError($e)) { + return false; + } + + throw $e; + } + + return true; + } + + /** + * Proxmox answers any request about a VMID it doesn't have with a 500 and + * "Configuration file 'nodes//qemu-server/.conf' does not exist". + */ + public static function isVmMissingError(\Throwable $e): bool + { + // The response sits on the exception itself, or -- before it kept one -- + // on the RequestException it wrapped. + $response = $e->response + ?? ($e->getPrevious() instanceof RequestException ? $e->getPrevious()->response : null); + + return $response instanceof Response + && $response->status() === 500 + && str_contains((string) $response->json('message'), 'does not exist'); + } } diff --git a/app/Services/Servers/ServerDeletionService.php b/app/Services/Servers/ServerDeletionService.php index ff42bf91437..3d1e502a348 100644 --- a/app/Services/Servers/ServerDeletionService.php +++ b/app/Services/Servers/ServerDeletionService.php @@ -10,45 +10,80 @@ class ServerDeletionService { - public function __construct(private ServerBuildDispatchService $buildDispatchService) + /** + * States in which a job chain is still working on the server. Deleting + * then would race it; disconnecting is still allowed, as the way out when + * one of those chains has died and left the server stuck. + */ + private const BUSY_STATES = [ + Status::INSTALLING, + Status::RESTORING_BACKUP, + Status::RESTORING_SNAPSHOT, + Status::DELETING, + ]; + + public function __construct( + private ServerBuildDispatchService $buildDispatchService, + private ServerBuildService $buildService, + ) { } - public function handle(Server $server, bool $noPurge = false) + /** + * @param bool $disconnect remove the server from Convoy only, leaving the VM + * and its backups on the node untouched + */ + public function handle(Server $server, bool $disconnect = false): void { + if ($disconnect) { + $server->delete(); + + return; + } + $this->validateStatus($server); + // Checked now rather than in the chain so the admin hears straight away + // when the node can't be asked: vmExists() throws on anything other than + // Proxmox's "does not exist", and the server is left as it was. + $vmExists = $this->buildService->vmExists($server); + $server->update(['status' => Status::DELETING->value]); - if (! $noPurge) { - Bus::chain([ - new PurgeBackupsJob($server->id), - ...$this->buildDispatchService->getChainedDeleteJobs($server), - function () use ($server) { - Server::findOrFail($server->id)->delete(); - }, - ]) - ->catch(fn () => $server->update(['status' => Status::DELETION_FAILED->value])) - ->dispatch(); + $serverId = $server->id; - return; - } - - $server->delete(); + Bus::chain([ + // Force: locked backups belong to the server being deleted too. + new PurgeBackupsJob($serverId, force: true), + // Already gone from the node -- a failed install, or deleted by hand + // in Proxmox -- so only the Convoy entry is left to remove. + ...($vmExists ? $this->buildDispatchService->getChainedDeleteJobs($server) : []), + static function () use ($serverId) { + Server::whereKey($serverId)->first()?->delete(); + }, + ]) + // By ID, not the captured model: see ServerBuildDispatchService. + ->catch(static fn () => Server::whereKey($serverId) + ->update(['status' => Status::DELETION_FAILED->value])) + ->dispatch(); } - public function validateStatus(Server $server, bool $verifyStatusOnly = false) + /** + * A delete can start from any state except one where a chain is still + * running -- including deletion_failed, so a failed delete can be retried -- + * and not while a backup is being taken, since Proxmox holds a lock on the + * VM for the backup and would refuse to destroy it. + * + * @throws ServerStatusConflictException + */ + public function validateStatus(Server $server): void { - if ( - ! is_null($server->status) && $server->status !== Status::DELETING->value - ) { + if (in_array($server->status, array_map(fn (Status $s) => $s->value, self::BUSY_STATES), true)) { throw new ServerStatusConflictException($server); } - if (! $verifyStatusOnly) { - if ($server->backups()->whereNull('completed_at')->exists()) { - throw new ServerStatusConflictException($server); - } + if ($server->backups()->whereNull('completed_at')->exists()) { + throw new ServerStatusConflictException($server); } } } diff --git a/lang/en_US/admin/servers/settings.php b/lang/en_US/admin/servers/settings.php index 57e0420e064..ae9809f3814 100644 --- a/lang/en_US/admin/servers/settings.php +++ b/lang/en_US/admin/servers/settings.php @@ -24,15 +24,18 @@ ], 'deletion' => [ 'title' => 'Delete Server', - 'description' => 'The server will be deleted from Convoy. Backups and other associated data will be - destroyed. However, you can tick the checkbox below to keep the virtual machine and data - on the Proxmox node.', + 'description' => 'Delete removes the server completely: its backups, its virtual machine on the Proxmox node, and its entry in Convoy. If the virtual machine is already gone from the node, only the Convoy entry is removed. Disconnect removes the server from Convoy only, and leaves the virtual machine and its backups on the node untouched.', 'deleting_status' => 'This server is currently being deleted.', - 'no_purge' => 'Do not purge VM and related files', + 'failed_status' => 'The last attempt to delete this server failed. You can delete it again, or disconnect it to remove it from Convoy only.', + 'disconnect' => 'Disconnect', 'confirmation' => [ 'title' => 'Delete :name', - 'description' => 'Are you sure you want to delete :name?' - ] + 'description' => 'Are you sure you want to delete :name? Its virtual machine and backups will be destroyed on the Proxmox node.' + ], + 'disconnect_confirmation' => [ + 'title' => 'Disconnect :name', + 'description' => 'Remove :name from Convoy? Its virtual machine and backups stay on the Proxmox node, and Convoy will no longer manage them.' + ], ], 'build' => [ 'title' => 'Server Build', diff --git a/resources/scripts/components/admin/servers/settings/partials/general/DeleteServerActions.tsx b/resources/scripts/components/admin/servers/settings/partials/general/DeleteServerActions.tsx new file mode 100644 index 00000000000..b3d31ad489a --- /dev/null +++ b/resources/scripts/components/admin/servers/settings/partials/general/DeleteServerActions.tsx @@ -0,0 +1,129 @@ +import { AdminServerContext } from '@/state/admin/server' +import { useFlashKey } from '@/util/useFlash' +import { useState } from 'react' +import { useTranslation } from 'react-i18next' +import { useNavigate } from 'react-router-dom' + +import deleteServer from '@/api/admin/servers/deleteServer' + +import Button from '@/components/elements/Button' +import FlashMessageRender from '@/components/elements/FlashMessageRenderer' +import Modal from '@/components/elements/Modal' + + +type Action = 'delete' | 'disconnect' + +interface Props { + className?: string +} + +// Delete and Disconnect, with their confirmations. Shared by the settings card +// and the admin's "Deleting"/"Failed to Delete" screen, where the settings +// aren't reachable. +const DeleteServerActions = ({ className }: Props) => { + const server = AdminServerContext.useStoreState(state => state.server.data!) + const setServer = AdminServerContext.useStoreActions( + actions => actions.server.setServer + ) + const { clearFlashes, clearAndAddHttpError } = useFlashKey( + `admin.servers.${server.uuid}.settings.general.delete` + ) + const { t: tStrings } = useTranslation('strings') + const { t } = useTranslation('admin.servers.settings') + const navigate = useNavigate() + const [confirming, setConfirming] = useState(null) + const [submitting, setSubmitting] = useState(false) + + const submit = async (action: Action) => { + clearFlashes() + setSubmitting(true) + try { + // Disconnecting is the API's no_purge: only the Convoy entry goes. + await deleteServer(server.uuid, action === 'disconnect') + + if (action === 'disconnect') { + navigate('/admin/servers') + return + } + + setServer({ ...server, status: 'deleting' }) + setConfirming(null) + } catch (error) { + clearAndAddHttpError(error as any) + setConfirming(null) + } finally { + setSubmitting(false) + } + } + + return ( + <> + +
+ + +
+ + setConfirming(null)}> + + + {confirming === 'disconnect' + ? t('deletion.disconnect_confirmation.title', { + name: server.name, + }) + : t('deletion.confirmation.title', { + name: server.name, + })} + + + + + {confirming === 'disconnect' + ? t('deletion.disconnect_confirmation.description', { + name: server.name, + }) + : t('deletion.confirmation.description', { + name: server.name, + })} + + + + setConfirming(null)}> + {tStrings('cancel')} + + confirming && submit(confirming)} + > + {confirming === 'disconnect' + ? t('deletion.disconnect') + : tStrings('delete')} + + + + + ) +} + +export default DeleteServerActions diff --git a/resources/scripts/components/admin/servers/settings/partials/general/DeleteServerCard.tsx b/resources/scripts/components/admin/servers/settings/partials/general/DeleteServerCard.tsx index 0dfc6125a96..2a9ac7d3761 100644 --- a/resources/scripts/components/admin/servers/settings/partials/general/DeleteServerCard.tsx +++ b/resources/scripts/components/admin/servers/settings/partials/general/DeleteServerCard.tsx @@ -1,147 +1,41 @@ import { AdminServerContext } from '@/state/admin/server' -import { useFlashKey } from '@/util/useFlash' -import { zodResolver } from '@hookform/resolvers/zod' -import { FormEvent, useState } from 'react' -import { FormProvider, useForm } from 'react-hook-form' import { useTranslation } from 'react-i18next' -import { z } from 'zod' -import deleteServer from '@/api/admin/servers/deleteServer' - -import Button from '@/components/elements/Button' -import FlashMessageRender from '@/components/elements/FlashMessageRenderer' import FormCard from '@/components/elements/FormCard' import MessageBox from '@/components/elements/MessageBox' -import Modal from '@/components/elements/Modal' -import CheckboxForm from '@/components/elements/forms/CheckboxForm' + +import DeleteServerActions from '@/components/admin/servers/settings/partials/general/DeleteServerActions' const DeleteServerCard = () => { const server = AdminServerContext.useStoreState(state => state.server.data!) - const setServer = AdminServerContext.useStoreActions( - actions => actions.server.setServer - ) - const { clearFlashes, clearAndAddHttpError } = useFlashKey( - `admin.servers.${server.uuid}.settings.general.delete` - ) - const { t: tStrings } = useTranslation('strings') const { t } = useTranslation('admin.servers.settings') - const [showConfirmation, setShowConfirmation] = useState(false) - - const schema = z.object({ - noPurge: z.boolean(), - }) - - const form = useForm({ - resolver: zodResolver(schema), - defaultValues: { - noPurge: false, - }, - }) - - const submit = async ({ noPurge }: z.infer) => { - clearFlashes() - try { - await deleteServer(server.uuid, noPurge) - - setServer({ - ...server, - status: 'deleting', - }) - } catch (error) { - clearAndAddHttpError(error as any) - } - } - - const triggerConfirmation = (e: FormEvent) => { - e.preventDefault() - setShowConfirmation(true) - } return ( - <> - - -
- - - {t('deletion.title')} - -
- - -

- {t('deletion.description')} -

- {server.status === 'deleting' && ( - - {t('deletion.deleting_status')} - - )} - - -
- - - - -
- - - - setShowConfirmation(false)} - > - - - {t('deletion.confirmation.title', { - name: server.name, - })} - - - - - {t('deletion.confirmation.description', { - name: server.name, - })} - - - - setShowConfirmation(false)} - > - {tStrings('cancel')} - - - {tStrings('delete')} - - - - + + + {t('deletion.title')} +
+

+ {t('deletion.description')} +

+ {server.status === 'deleting' && ( + + {t('deletion.deleting_status')} + + )} + {server.status === 'deletion_failed' && ( + + {t('deletion.failed_status')} + + )} +
+ + + + + ) } -export default DeleteServerCard \ No newline at end of file +export default DeleteServerCard diff --git a/resources/scripts/routers/AdminServerRouter.tsx b/resources/scripts/routers/AdminServerRouter.tsx index c178957569b..aa2b77e7376 100644 --- a/resources/scripts/routers/AdminServerRouter.tsx +++ b/resources/scripts/routers/AdminServerRouter.tsx @@ -13,6 +13,7 @@ import Spinner from '@/components/elements/Spinner' import { NavigationBarContext } from '@/components/elements/navigation/NavigationBar' import RestoreAccessButton from '@/components/admin/servers/RestoreAccessButton' +import DeleteServerActions from '@/components/admin/servers/settings/partials/general/DeleteServerActions' export const routes: Route[] = [ { @@ -157,7 +158,7 @@ const AdminServerRouter = () => { message={ server.status === 'deleting' ? "This server is being deleted. If you think this is an error, click below to make the server accessible. Clicking the button will not stop the server from being deleted if it's in progress" - : 'This server failed to delete. if you think this is an error, click below to make the server accessible. The server MAY not be in a usable state.' + : 'This server failed to delete. Delete it again, disconnect it to remove it from Convoy only, or make it accessible again if you think this is an error. The server MAY not be in a usable state.' } title={ server.status === 'deleting' @@ -166,6 +167,7 @@ const AdminServerRouter = () => { } > + ) : ( diff --git a/routes/api-admin.php b/routes/api-admin.php index 58455370f0a..a3a2c468fa4 100644 --- a/routes/api-admin.php +++ b/routes/api-admin.php @@ -135,7 +135,11 @@ function () { Route::patch('/', [Admin\ServerController::class, 'update'])->withoutMiddleware( ValidateServerStatusMiddleware::class, ); - Route::delete('/', [Admin\ServerController::class, 'destroy']); + // The deletion service checks the status itself: it has to allow + // deletion_failed (to retry) and disconnecting from any state. + Route::delete('/', [Admin\ServerController::class, 'destroy'])->withoutMiddleware( + ValidateServerStatusMiddleware::class, + ); Route::prefix('/settings')->group(function () { Route::patch('/build', [Admin\ServerController::class, 'updateBuild']); diff --git a/routes/api-application.php b/routes/api-application.php index 1e60198cc63..ed7c61e011a 100644 --- a/routes/api-application.php +++ b/routes/api-application.php @@ -125,7 +125,11 @@ function () { Route::patch('/', [Admin\ServerController::class, 'update'])->withoutMiddleware( ValidateServerStatusMiddleware::class, ); - Route::delete('/', [Admin\ServerController::class, 'destroy']); + // The deletion service checks the status itself: it has to allow + // deletion_failed (to retry) and disconnecting from any state. + Route::delete('/', [Admin\ServerController::class, 'destroy'])->withoutMiddleware( + ValidateServerStatusMiddleware::class, + ); Route::prefix('/settings')->group(function () { Route::patch('/build', [Admin\ServerController::class, 'updateBuild']); diff --git a/tests/Feature/Controllers/Admin/ServerDeletionTest.php b/tests/Feature/Controllers/Admin/ServerDeletionTest.php new file mode 100644 index 00000000000..eefb12a96e4 --- /dev/null +++ b/tests/Feature/Controllers/Admin/ServerDeletionTest.php @@ -0,0 +1,163 @@ + Http::response( + file_get_contents(base_path('tests/Fixtures/Repositories/Server/GetServerConfigData.json')), + 200, + ), + '*' => Http::response(['data' => 'dummy-upid'], 200), + ]); +} + +function fakeVmGone(): void +{ + Http::fake([ + '*/config' => Http::response([ + 'data' => null, + 'message' => "Configuration file 'nodes/proxmox/qemu-server/100.conf' does not exist\n", + ], 500), + '*' => Http::response(['data' => 'dummy-upid'], 200), + ]); +} + +function deleteAsAdmin(Server $server, array $data = []) +{ + return test()->actingAs(User::factory()->create(['root_admin' => true])) + ->deleteJson("/api/admin/servers/{$server->uuid}", $data); +} + +it('deletes the backups, then the VM, then the Convoy entry', function () { + fakeVmPresent(); + [$_, $_, $_, $server] = createServerModel(); + + deleteAsAdmin($server)->assertNoContent(); + + expect($server->fresh()->status)->toBe(Status::DELETING->value); + Queue::assertPushedWithChain(PurgeBackupsJob::class, [ + SendPowerCommandJob::class, + MonitorStateJob::class, + DeleteServerJob::class, + WaitUntilVmIsDeletedJob::class, + CallQueuedClosure::class, + ]); +}); + +it('only removes the Convoy entry when the VM is already gone from the node', function () { + fakeVmGone(); + [$_, $_, $_, $server] = createServerModel(); + + deleteAsAdmin($server)->assertNoContent(); + + Queue::assertPushedWithChain(PurgeBackupsJob::class, [CallQueuedClosure::class]); +}); + +it('leaves the server alone when the node cannot be asked', function () { + Http::fake(['*' => Http::response(['data' => null, 'message' => 'proxy error'], 503)]); + [$_, $_, $_, $server] = createServerModel(); + + deleteAsAdmin($server)->assertServerError(); + + // An outage must not read as "VM already gone" and drop the server. + expect($server->fresh())->not->toBeNull() + ->and($server->fresh()->status)->toBeNull(); + Queue::assertNothingPushed(); +}); + +it('can retry a delete that failed', function () { + fakeVmPresent(); + [$_, $_, $_, $server] = createServerModel(); + $server->update(['status' => Status::DELETION_FAILED->value]); + + deleteAsAdmin($server)->assertNoContent(); + + expect($server->fresh()->status)->toBe(Status::DELETING->value); +}); + +it("won't delete while another operation is running on the server", function (Status $status) { + fakeVmPresent(); + [$_, $_, $_, $server] = createServerModel(); + $server->update(['status' => $status->value]); + + deleteAsAdmin($server)->assertConflict(); + + Queue::assertNothingPushed(); +})->with([Status::INSTALLING, Status::DELETING, Status::RESTORING_BACKUP]); + +it('disconnects without touching the node, from any state', function (?Status $status) { + Http::fake(); + [$_, $_, $_, $server] = createServerModel(); + $server->update(['status' => $status?->value]); + + deleteAsAdmin($server, ['no_purge' => true])->assertNoContent(); + + expect(Server::find($server->id))->toBeNull(); + Http::assertNothingSent(); + Queue::assertNothingPushed(); +})->with([null, Status::DELETING, Status::DELETION_FAILED, Status::INSTALLING]); + +it('marks the server deletion_failed when the chain fails', function () { + fakeVmPresent(); + Queue::fake()->serializeAndRestore(); + [$_, $_, $_, $server] = createServerModel(); + + deleteAsAdmin($server)->assertNoContent(); + + Queue::pushed(PurgeBackupsJob::class)->first() + ->invokeChainCatchCallbacks(new RuntimeException('destroy failed')); + + expect($server->fresh()->status)->toBe(Status::DELETION_FAILED->value); +}); + +it('purges locked backups too when the server is deleted', function () { + Http::fake(['*' => Http::response(['data' => 'dummy-upid'], 200)]); + [$_, $_, $_, $server] = createServerModel(); + $backup = Backup::factory()->for($server)->create([ + 'is_locked' => true, + 'is_successful' => true, + 'completed_at' => now(), + ]); + + app(PurgeBackupsService::class)->handle($server, force: true); + + expect(Backup::find($backup->id))->toBeNull(); +}); + +it('sees a VM that answers as existing', function () { + fakeVmPresent(); + [$_, $_, $_, $server] = createServerModel(); + + expect(app(ServerBuildService::class)->vmExists($server))->toBeTrue(); +}); + +it('sees Proxmox "does not exist" as a missing VM', function () { + fakeVmGone(); + [$_, $_, $_, $server] = createServerModel(); + + expect(app(ServerBuildService::class)->vmExists($server))->toBeFalse(); +}); + +it('throws on any other Proxmox failure rather than guess', function () { + Http::fake(['*' => Http::response(['data' => null, 'message' => 'proxy error'], 503)]); + [$_, $_, $_, $server] = createServerModel(); + + expect(fn () => app(ServerBuildService::class)->vmExists($server)) + ->toThrow(ProxmoxConnectionException::class); +});