mirror of
https://github.com/tiennm99/coolify.git
synced 2026-10-06 20:15:27 +00:00
fix(servers): isolate cloud status checks from SSH checks
Track provider state independently, skip SSH work for placeholder IPs, and clean up failed cloud server provisioning.
This commit is contained in:
1 parent
e01b8a057e
commit
c8a332a3bc
22 files changed
+954
-180
No files matched your search
@@ -6,6 +6,7 @@ use App\Models\PrivateKey;
|
||||
use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Http\Client\RequestException;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
use Tests\TestCase;
|
||||
|
||||
@@ -44,3 +45,59 @@ it('deletes the DigitalOcean droplet when requested', function () {
|
||||
Http::assertSent(fn ($request) => $request->method() === 'DELETE'
|
||||
&& $request->url() === 'https://api.digitalocean.com/v2/droplets/987');
|
||||
});
|
||||
|
||||
it('retains the server and surfaces a DigitalOcean deletion failure', function () {
|
||||
Http::fake([
|
||||
'https://api.digitalocean.com/v2/droplets/987' => Http::response([
|
||||
'message' => 'deletion failed',
|
||||
], 500),
|
||||
]);
|
||||
|
||||
$team = Team::factory()->create();
|
||||
$token = CloudProviderToken::factory()->create([
|
||||
'team_id' => $team->id,
|
||||
'provider' => 'digitalocean',
|
||||
'token' => 'test-digitalocean-token',
|
||||
]);
|
||||
$privateKey = PrivateKey::factory()->create(['team_id' => $team->id]);
|
||||
$server = Server::factory()->create([
|
||||
'team_id' => $team->id,
|
||||
'private_key_id' => $privateKey->id,
|
||||
'cloud_provider_token_id' => $token->id,
|
||||
'digitalocean_droplet_id' => 987,
|
||||
]);
|
||||
$server->delete();
|
||||
|
||||
expect(fn () => DeleteServer::run(
|
||||
serverId: $server->id,
|
||||
deleteFromDigitalOcean: true,
|
||||
digitalOceanDropletId: 987,
|
||||
cloudProviderTokenId: $token->id,
|
||||
teamId: $team->id,
|
||||
))->toThrow(RequestException::class, 'status code 500');
|
||||
|
||||
expect(Server::withTrashed()->find($server->id))->not->toBeNull();
|
||||
});
|
||||
|
||||
it('retains the server when no DigitalOcean token can delete the droplet', function () {
|
||||
Http::preventStrayRequests();
|
||||
|
||||
$team = Team::factory()->create();
|
||||
$privateKey = PrivateKey::factory()->create(['team_id' => $team->id]);
|
||||
$server = Server::factory()->create([
|
||||
'team_id' => $team->id,
|
||||
'private_key_id' => $privateKey->id,
|
||||
'cloud_provider_token_id' => null,
|
||||
'digitalocean_droplet_id' => 987,
|
||||
]);
|
||||
$server->delete();
|
||||
|
||||
expect(fn () => DeleteServer::run(
|
||||
serverId: $server->id,
|
||||
deleteFromDigitalOcean: true,
|
||||
digitalOceanDropletId: 987,
|
||||
teamId: $team->id,
|
||||
))->toThrow(RuntimeException::class, 'No DigitalOcean token found');
|
||||
|
||||
expect(Server::withTrashed()->find($server->id))->not->toBeNull();
|
||||
});
|
||||
@@ -66,8 +66,8 @@ it('backfills a placeholder IP from the DigitalOcean droplet state', function ()
|
||||
expect($server->fresh()->ip)->toBe('203.0.113.10');
|
||||
});
|
||||
|
||||
it('does not overwrite a real IP from the DigitalOcean droplet state', function () {
|
||||
$server = createDigitalOceanServerForStateTest('active', ['ip' => '198.51.100.20']);
|
||||
it('does not overwrite an administrator configured address from the DigitalOcean droplet state', function (string $configuredAddress) {
|
||||
$server = createDigitalOceanServerForStateTest('active', ['ip' => $configuredAddress]);
|
||||
|
||||
Http::fake([
|
||||
'https://api.digitalocean.com/v2/droplets/987' => Http::response([
|
||||
@@ -84,7 +84,38 @@ it('does not overwrite a real IP from the DigitalOcean droplet state', function
|
||||
]);
|
||||
|
||||
$server->refreshDigitalOceanState();
|
||||
expect($server->fresh()->ip)->toBe('198.51.100.20');
|
||||
expect($server->fresh()->ip)->toBe($configuredAddress);
|
||||
})->with([
|
||||
'public IPv4' => '198.51.100.20',
|
||||
'private IPv4' => '10.10.0.12',
|
||||
'IPv6' => '2001:db8::10',
|
||||
'tunnel hostname' => 'server.internal.example.com',
|
||||
]);
|
||||
|
||||
it('does not overwrite an address configured while DigitalOcean state is loading', function () {
|
||||
$server = createDigitalOceanServerForStateTest('new', ['ip' => Server::PLACEHOLDER_IP]);
|
||||
|
||||
Http::fake(function () use ($server) {
|
||||
Server::query()->findOrFail($server->id)->update([
|
||||
'ip' => 'server.internal.example.com',
|
||||
]);
|
||||
|
||||
return Http::response([
|
||||
'droplet' => [
|
||||
'id' => 987,
|
||||
'status' => 'active',
|
||||
'networks' => [
|
||||
'v4' => [
|
||||
['type' => 'public', 'ip_address' => '203.0.113.10'],
|
||||
],
|
||||
],
|
||||
],
|
||||
], 200);
|
||||
});
|
||||
|
||||
expect($server->refreshDigitalOceanState())->toBe('active')
|
||||
->and($server->fresh()->ip)->toBe('server.internal.example.com')
|
||||
->and($server->digitalocean_droplet_status)->toBe('active');
|
||||
});
|
||||
|
||||
it('does not mark a DigitalOcean droplet as deleted on transient provider errors', function () {
|
||||
|
||||
@@ -127,7 +127,7 @@ describe('shouldSkipDueToBackoff', function () {
|
||||
});
|
||||
|
||||
describe('ServerConnectionCheckJob unreachable_count', function () {
|
||||
it('marks Vultr servers unreachable when provider status is unavailable', function () {
|
||||
it('marks servers unreachable when SSH is unavailable', function () {
|
||||
Event::fake([ServerReachabilityChanged::class]);
|
||||
|
||||
$settings = Mockery::mock();
|
||||
@@ -141,14 +141,12 @@ describe('ServerConnectionCheckJob unreachable_count', function () {
|
||||
$server->shouldReceive('getAttribute')->andReturnUsing(fn (string $key) => match ($key) {
|
||||
'settings' => $settings,
|
||||
'unreachable_notification_sent' => false,
|
||||
'vultr_instance_id' => 'instance-1',
|
||||
'cloudProviderToken' => (object) ['token' => 'test-token'],
|
||||
'ip' => '203.0.113.10',
|
||||
'id' => 1,
|
||||
'name' => 'test-server',
|
||||
'unreachable_count' => 1,
|
||||
default => null,
|
||||
});
|
||||
$server->shouldReceive('refreshVultrState')->once()->andReturn('stopped');
|
||||
$server->shouldReceive('increment')->with('unreachable_count')->once();
|
||||
$server->id = 1;
|
||||
$server->name = 'test-server';
|
||||
@@ -158,22 +156,20 @@ describe('ServerConnectionCheckJob unreachable_count', function () {
|
||||
$job->handle();
|
||||
});
|
||||
|
||||
it('increments unreachable_count on timeout', function () {
|
||||
it('does not change reachability on a job-level timeout', function () {
|
||||
Event::fake([ServerReachabilityChanged::class]);
|
||||
|
||||
$settings = Mockery::mock();
|
||||
$settings->is_reachable = true;
|
||||
$settings->shouldReceive('update')
|
||||
->with(['is_reachable' => false, 'is_usable' => false])
|
||||
->once();
|
||||
$settings->shouldNotReceive('update');
|
||||
|
||||
$server = Mockery::mock(Server::class)->makePartial()->shouldAllowMockingProtectedMethods();
|
||||
$server->shouldReceive('getAttribute')->with('settings')->andReturn($settings);
|
||||
$server->shouldReceive('getAttribute')->with('unreachable_notification_sent')->andReturn(false);
|
||||
$server->shouldReceive('increment')->with('unreachable_count')->once();
|
||||
$server->shouldNotReceive('increment');
|
||||
$server->id = 1;
|
||||
$server->name = 'test-server';
|
||||
$server->unreachable_count = 1; // Will become 2 after increment in real code; mock keeps value as-is
|
||||
$server->unreachable_count = 1;
|
||||
|
||||
$job = new ServerConnectionCheckJob($server);
|
||||
$job->failed(new TimeoutExceededException);
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
<?php
|
||||
|
||||
use App\Jobs\CleanupOrphanedPreviewContainersJob;
|
||||
use App\Jobs\ScheduledJobManager;
|
||||
use App\Models\PrivateKey;
|
||||
use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
@@ -60,8 +62,64 @@ it('ignores a missing IP when backfilling', function () {
|
||||
expect($server->fresh()->ip)->toBe(Server::PLACEHOLDER_IP);
|
||||
});
|
||||
|
||||
it('does not replace a placeholder with another placeholder', function (string $replacementIp) {
|
||||
$server = createServerForPlaceholderIpTest(Server::PLACEHOLDER_IP);
|
||||
|
||||
expect($server->backfillPlaceholderIp($replacementIp))->toBeFalse();
|
||||
expect($server->fresh()->ip)->toBe(Server::PLACEHOLDER_IP);
|
||||
})->with([
|
||||
'unspecified IPv4 address' => '0.0.0.0',
|
||||
'unspecified IPv6 address' => '::',
|
||||
]);
|
||||
|
||||
it('does not overwrite an address configured after the placeholder model was loaded', function () {
|
||||
$staleServer = createServerForPlaceholderIpTest(Server::PLACEHOLDER_IP);
|
||||
$concurrentServer = Server::query()->findOrFail($staleServer->id);
|
||||
|
||||
$concurrentServer->update(['ip' => 'server.internal.example.com']);
|
||||
|
||||
expect($staleServer->backfillPlaceholderIp('203.0.113.10'))->toBeFalse()
|
||||
->and($staleServer->fresh()->ip)->toBe('server.internal.example.com');
|
||||
});
|
||||
|
||||
it('skips servers with a placeholder IP in scheduled jobs', function () {
|
||||
$server = createServerForPlaceholderIpTest(Server::PLACEHOLDER_IP);
|
||||
|
||||
expect($server->skipServer())->toBeTrue();
|
||||
});
|
||||
|
||||
it('excludes every placeholder address from scheduled Docker cleanup', function () {
|
||||
$realServer = createServerForPlaceholderIpTest('203.0.113.10');
|
||||
|
||||
foreach (Server::PLACEHOLDER_IPS as $placeholderIp) {
|
||||
Server::factory()->create([
|
||||
'team_id' => $realServer->team_id,
|
||||
'private_key_id' => $realServer->private_key_id,
|
||||
'ip' => $placeholderIp,
|
||||
]);
|
||||
}
|
||||
|
||||
$method = new ReflectionMethod(ScheduledJobManager::class, 'getServersForCleanupQuery');
|
||||
$servers = $method->invoke(new ScheduledJobManager)->get();
|
||||
|
||||
expect($servers->modelKeys())->toBe([$realServer->id]);
|
||||
});
|
||||
|
||||
it('excludes every placeholder address from orphaned preview cleanup', function () {
|
||||
$realServer = createServerForPlaceholderIpTest('203.0.113.10');
|
||||
$realServer->settings->update(['is_reachable' => true, 'is_usable' => true]);
|
||||
|
||||
foreach (Server::PLACEHOLDER_IPS as $placeholderIp) {
|
||||
$server = Server::factory()->create([
|
||||
'team_id' => $realServer->team_id,
|
||||
'private_key_id' => $realServer->private_key_id,
|
||||
'ip' => $placeholderIp,
|
||||
]);
|
||||
$server->settings->update(['is_reachable' => true, 'is_usable' => true]);
|
||||
}
|
||||
|
||||
$method = new ReflectionMethod(CleanupOrphanedPreviewContainersJob::class, 'getServersToCheck');
|
||||
$servers = $method->invoke(new CleanupOrphanedPreviewContainersJob);
|
||||
|
||||
expect($servers->modelKeys())->toBe([$realServer->id]);
|
||||
});
|
||||
@@ -7,6 +7,7 @@ use App\Models\PrivateKey;
|
||||
use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Http\Client\RequestException;
|
||||
use Illuminate\Support\Facades\Http;
|
||||
use Tests\TestCase;
|
||||
|
||||
@@ -116,3 +117,52 @@ it('does not use another team Vultr token when deleting an instance', function (
|
||||
|
||||
expect($request->header('Authorization'))->toBe(['Bearer test-vultr-token']);
|
||||
});
|
||||
|
||||
it('retains the server and surfaces a Vultr deletion failure', function () {
|
||||
Http::fake([
|
||||
'https://api.vultr.com/v2/instances/instance-1' => Http::response([
|
||||
'error' => 'deletion failed',
|
||||
], 500),
|
||||
]);
|
||||
|
||||
$server = Server::factory()->create([
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $this->privateKey->id,
|
||||
'cloud_provider_token_id' => $this->vultrToken->id,
|
||||
'vultr_instance_id' => 'instance-1',
|
||||
]);
|
||||
$server->delete();
|
||||
|
||||
expect(fn () => DeleteServer::run(
|
||||
serverId: $server->id,
|
||||
cloudProviderTokenId: $this->vultrToken->id,
|
||||
teamId: $this->team->id,
|
||||
deleteFromVultr: true,
|
||||
vultrInstanceId: 'instance-1'
|
||||
))->toThrow(RequestException::class, 'status code 500');
|
||||
|
||||
expect(Server::withTrashed()->find($server->id))->not->toBeNull();
|
||||
});
|
||||
|
||||
it('retains the server when no Vultr token can delete the instance', function () {
|
||||
Http::preventStrayRequests();
|
||||
|
||||
$this->vultrToken->delete();
|
||||
|
||||
$server = Server::factory()->create([
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $this->privateKey->id,
|
||||
'cloud_provider_token_id' => null,
|
||||
'vultr_instance_id' => 'instance-1',
|
||||
]);
|
||||
$server->delete();
|
||||
|
||||
expect(fn () => DeleteServer::run(
|
||||
serverId: $server->id,
|
||||
teamId: $this->team->id,
|
||||
deleteFromVultr: true,
|
||||
vultrInstanceId: 'instance-1'
|
||||
))->toThrow(RuntimeException::class, 'No Vultr token found');
|
||||
|
||||
expect(Server::withTrashed()->find($server->id))->not->toBeNull();
|
||||
});
|
||||
Reference in new issue
Block a user