app(server): rename oom_disabled to oom_killer and invert existing values

This commit is contained in:
Matthew Penner 2023-01-17 12:11:31 -07:00
parent 450fba00bc
commit 7665eea14d
No known key found for this signature in database
16 changed files with 63 additions and 256 deletions

View file

@ -1,55 +0,0 @@
<?php
namespace Pterodactyl\Http\Controllers\Api\Application\Servers;
use Pterodactyl\Models\Server;
use Pterodactyl\Services\Servers\BuildModificationService;
use Pterodactyl\Services\Servers\DetailsModificationService;
use Pterodactyl\Transformers\Api\Application\ServerTransformer;
use Pterodactyl\Http\Controllers\Api\Application\ApplicationApiController;
use Pterodactyl\Http\Requests\Api\Application\Servers\UpdateServerDetailsRequest;
use Pterodactyl\Http\Requests\Api\Application\Servers\UpdateServerBuildConfigurationRequest;
class ServerDetailsController extends ApplicationApiController
{
/**
* ServerDetailsController constructor.
*/
public function __construct(
private BuildModificationService $buildModificationService,
private DetailsModificationService $detailsModificationService
) {
parent::__construct();
}
/**
* Update the details for a specific server.
*
* @throws \Throwable
*/
public function details(UpdateServerDetailsRequest $request, Server $server): array
{
$server = $this->detailsModificationService->returnUpdatedModel()->handle(
$server,
$request->validated()
);
return $this->fractal->item($server)
->transformWith(ServerTransformer::class)
->toArray();
}
/**
* Update the build details for a specific server.
*
* @throws \Throwable
*/
public function build(UpdateServerBuildConfigurationRequest $request, Server $server): array
{
$server = $this->buildModificationService->handle($server, $request->validated());
return $this->fractal->item($server)
->transformWith(ServerTransformer::class)
->toArray();
}
}

View file

@ -67,7 +67,7 @@ class StoreServerRequest extends ApplicationApiRequest
'io' => array_get($data, 'limits.io'), 'io' => array_get($data, 'limits.io'),
'threads' => array_get($data, 'limits.threads'), 'threads' => array_get($data, 'limits.threads'),
'cpu' => array_get($data, 'limits.cpu'), 'cpu' => array_get($data, 'limits.cpu'),
'oom_disabled' => !array_get($data, 'limits.oom_killer'), 'oom_killer' => array_get($data, 'limits.oom_killer'),
'allocation_limit' => array_get($data, 'feature_limits.allocations'), 'allocation_limit' => array_get($data, 'feature_limits.allocations'),
'backup_limit' => array_get($data, 'feature_limits.backups'), 'backup_limit' => array_get($data, 'feature_limits.backups'),

View file

@ -1,127 +0,0 @@
<?php
namespace Pterodactyl\Http\Requests\Api\Application\Servers;
use Illuminate\Support\Arr;
use Pterodactyl\Models\Server;
use Illuminate\Support\Collection;
class UpdateServerBuildConfigurationRequest extends ServerWriteRequest
{
/**
* Return the rules to validate this request against.
*/
public function rules(): array
{
$rules = Server::getRulesForUpdate($this->route()->parameter('server'));
return [
'allocation' => $rules['allocation_id'],
'oom_disabled' => $rules['oom_disabled'],
'limits' => 'sometimes|array',
'limits.memory' => $this->requiredToOptional('memory', $rules['memory'], true),
'limits.swap' => $this->requiredToOptional('swap', $rules['swap'], true),
'limits.io' => $this->requiredToOptional('io', $rules['io'], true),
'limits.cpu' => $this->requiredToOptional('cpu', $rules['cpu'], true),
'limits.threads' => $this->requiredToOptional('threads', $rules['threads'], true),
'limits.disk' => $this->requiredToOptional('disk', $rules['disk'], true),
// Legacy rules to maintain backwards compatible API support without requiring
// a major version bump.
//
// @see https://github.com/pterodactyl/panel/issues/1500
'memory' => $this->requiredToOptional('memory', $rules['memory']),
'swap' => $this->requiredToOptional('swap', $rules['swap']),
'io' => $this->requiredToOptional('io', $rules['io']),
'cpu' => $this->requiredToOptional('cpu', $rules['cpu']),
'threads' => $this->requiredToOptional('threads', $rules['threads']),
'disk' => $this->requiredToOptional('disk', $rules['disk']),
'add_allocations' => 'bail|array',
'add_allocations.*' => 'integer',
'remove_allocations' => 'bail|array',
'remove_allocations.*' => 'integer',
'feature_limits' => 'required|array',
'feature_limits.databases' => $rules['database_limit'],
'feature_limits.allocations' => $rules['allocation_limit'],
'feature_limits.backups' => $rules['backup_limit'],
];
}
/**
* Convert the allocation field into the expected format for the service handler.
*
* @param string|null $key
* @param string|array|null $default
*
* @return mixed
*/
public function validated($key = null, $default = null)
{
$data = parent::validated();
$data['allocation_id'] = $data['allocation'];
$data['database_limit'] = $data['feature_limits']['databases'] ?? null;
$data['allocation_limit'] = $data['feature_limits']['allocations'] ?? null;
$data['backup_limit'] = $data['feature_limits']['backups'] ?? null;
unset($data['allocation'], $data['feature_limits']);
// Adjust the limits field to match what is expected by the model.
if (!empty($data['limits'])) {
foreach ($data['limits'] as $key => $value) {
$data[$key] = $value;
}
unset($data['limits']);
}
if (!is_null($key)) {
return Arr::get($data, $key, $default);
}
return $data;
}
/**
* Custom attributes to use in error message responses.
*
* @return array
*/
public function attributes()
{
return [
'add_allocations' => 'allocations to add',
'remove_allocations' => 'allocations to remove',
'add_allocations.*' => 'allocation to add',
'remove_allocations.*' => 'allocation to remove',
'feature_limits.databases' => 'Database Limit',
'feature_limits.allocations' => 'Allocation Limit',
'feature_limits.backups' => 'Backup Limit',
];
}
/**
* Converts existing rules for certain limits into a format that maintains backwards
* compatability with the old API endpoint while also supporting a more correct API
* call.
*
* @return array
*
* @see https://github.com/pterodactyl/panel/issues/1500
*/
protected function requiredToOptional(string $field, array $rules, bool $limits = false)
{
if (!in_array('required', $rules)) {
return $rules;
}
return (new Collection($rules))
->filter(function ($value) {
return $value !== 'required';
})
->prepend($limits ? 'required_with:limits' : 'required_without:limits')
->toArray();
}
}

View file

@ -1,55 +0,0 @@
<?php
namespace Pterodactyl\Http\Requests\Api\Application\Servers;
use Illuminate\Support\Arr;
use Pterodactyl\Models\Server;
class UpdateServerDetailsRequest extends ServerWriteRequest
{
/**
* Rules to apply to a server details update request.
*/
public function rules(): array
{
$rules = Server::getRulesForUpdate($this->route()->parameter('server'));
return [
'external_id' => $rules['external_id'],
'name' => $rules['name'],
'user' => $rules['owner_id'],
'description' => array_merge(['nullable'], $rules['description']),
];
}
/**
* Convert the posted data into the correct format that is expected
* by the application.
*
* @param string|null $key
* @param string|array|null $default
*/
public function validated($key = null, $default = null)
{
$data = [
'external_id' => $this->input('external_id'),
'name' => $this->input('name'),
'owner_id' => $this->input('user'),
'description' => $this->input('description'),
];
return is_null($key) ? $data : Arr::get($data, $key, $default);
}
/**
* Rename some of the attributes in error messages to clarify the field
* being discussed.
*/
public function attributes(): array
{
return [
'user' => 'User ID',
'name' => 'Server Name',
];
}
}

View file

@ -61,7 +61,7 @@ class UpdateServerRequest extends ApplicationApiRequest
'io' => array_get($data, 'limits.io'), 'io' => array_get($data, 'limits.io'),
'threads' => array_get($data, 'limits.threads'), 'threads' => array_get($data, 'limits.threads'),
'cpu' => array_get($data, 'limits.cpu'), 'cpu' => array_get($data, 'limits.cpu'),
'oom_disabled' => !array_get($data, 'limits.oom_killer'), 'oom_killer' => array_get($data, 'limits.oom_killer'),
'allocation_limit' => array_get($data, 'feature_limits.allocations'), 'allocation_limit' => array_get($data, 'feature_limits.allocations'),
'backup_limit' => array_get($data, 'feature_limits.backups'), 'backup_limit' => array_get($data, 'feature_limits.backups'),

View file

@ -31,7 +31,7 @@ use Pterodactyl\Exceptions\Http\Server\ServerStateConflictException;
* @property int $io * @property int $io
* @property int $cpu * @property int $cpu
* @property string|null $threads * @property string|null $threads
* @property bool $oom_disabled * @property bool $oom_killer
* @property int $allocation_id * @property int $allocation_id
* @property int $nest_id * @property int $nest_id
* @property int $egg_id * @property int $egg_id
@ -90,7 +90,7 @@ use Pterodactyl\Exceptions\Http\Server\ServerStateConflictException;
* @method static \Illuminate\Database\Eloquent\Builder|Server whereName($value) * @method static \Illuminate\Database\Eloquent\Builder|Server whereName($value)
* @method static \Illuminate\Database\Eloquent\Builder|Server whereNestId($value) * @method static \Illuminate\Database\Eloquent\Builder|Server whereNestId($value)
* @method static \Illuminate\Database\Eloquent\Builder|Server whereNodeId($value) * @method static \Illuminate\Database\Eloquent\Builder|Server whereNodeId($value)
* @method static \Illuminate\Database\Eloquent\Builder|Server whereOomDisabled($value) * @method static \Illuminate\Database\Eloquent\Builder|Server whereOomKiller($value)
* @method static \Illuminate\Database\Eloquent\Builder|Server whereOwnerId($value) * @method static \Illuminate\Database\Eloquent\Builder|Server whereOwnerId($value)
* @method static \Illuminate\Database\Eloquent\Builder|Server whereSkipScripts($value) * @method static \Illuminate\Database\Eloquent\Builder|Server whereSkipScripts($value)
* @method static \Illuminate\Database\Eloquent\Builder|Server whereStartup($value) * @method static \Illuminate\Database\Eloquent\Builder|Server whereStartup($value)
@ -131,7 +131,7 @@ class Server extends Model
*/ */
protected $attributes = [ protected $attributes = [
'status' => self::STATUS_INSTALLING, 'status' => self::STATUS_INSTALLING,
'oom_disabled' => true, 'oom_killer' => false,
'installed_at' => null, 'installed_at' => null,
]; ];
@ -162,7 +162,7 @@ class Server extends Model
'io' => 'required|numeric|between:10,1000', 'io' => 'required|numeric|between:10,1000',
'cpu' => 'required|numeric|min:0', 'cpu' => 'required|numeric|min:0',
'threads' => 'nullable|regex:/^[0-9-,]+$/', 'threads' => 'nullable|regex:/^[0-9-,]+$/',
'oom_disabled' => 'sometimes|boolean', 'oom_killer' => 'sometimes|boolean',
'disk' => 'required|numeric|min:0', 'disk' => 'required|numeric|min:0',
'allocation_id' => 'required|bail|unique:servers|exists:allocations,id', 'allocation_id' => 'required|bail|unique:servers|exists:allocations,id',
'nest_id' => 'required|exists:nests,id', 'nest_id' => 'required|exists:nests,id',
@ -187,7 +187,7 @@ class Server extends Model
'disk' => 'integer', 'disk' => 'integer',
'io' => 'integer', 'io' => 'integer',
'cpu' => 'integer', 'cpu' => 'integer',
'oom_disabled' => 'boolean', 'oom_killer' => 'boolean',
'allocation_id' => 'integer', 'allocation_id' => 'integer',
'nest_id' => 'integer', 'nest_id' => 'integer',
'egg_id' => 'integer', 'egg_id' => 'integer',

View file

@ -46,7 +46,7 @@ class BuildModificationService
// If any of these values are passed through in the data array go ahead and set // If any of these values are passed through in the data array go ahead and set
// them correctly on the server model. // them correctly on the server model.
$merge = Arr::only($data, ['oom_disabled', 'memory', 'swap', 'io', 'cpu', 'threads', 'disk', 'allocation_id']); $merge = Arr::only($data, ['oom_killer', 'memory', 'swap', 'io', 'cpu', 'threads', 'disk', 'allocation_id']);
$server->forceFill(array_merge($merge, [ $server->forceFill(array_merge($merge, [
'allocation_limit' => Arr::get($data, 'allocation_limit', 0) ?? null, 'allocation_limit' => Arr::get($data, 'allocation_limit', 0) ?? null,

View file

@ -59,7 +59,9 @@ class ServerConfigurationStructureService
'cpu_limit' => $server->cpu, 'cpu_limit' => $server->cpu,
'threads' => $server->threads, 'threads' => $server->threads,
'disk_space' => $server->disk, 'disk_space' => $server->disk,
'oom_disabled' => $server->oom_disabled, // TODO: remove oom_disabled and use oom_killer, this requires a Wings update.
'oom_disabled' => !$server->oom_killer,
'oom_killer' => $server->oom_killer,
], ],
'container' => [ 'container' => [
'image' => $server->image, 'image' => $server->image,
@ -105,7 +107,7 @@ class ServerConfigurationStructureService
return $item->pluck('port'); return $item->pluck('port');
})->toArray(), })->toArray(),
'env' => $this->environment->handle($server), 'env' => $this->environment->handle($server),
'oom_disabled' => $server->oom_disabled, 'oom_disabled' => !$server->oom_killer,
'memory' => (int) $server->memory, 'memory' => (int) $server->memory,
'swap' => (int) $server->swap, 'swap' => (int) $server->swap,
'io' => (int) $server->io, 'io' => (int) $server->io,

View file

@ -153,7 +153,7 @@ class ServerCreationService
'io' => Arr::get($data, 'io'), 'io' => Arr::get($data, 'io'),
'cpu' => Arr::get($data, 'cpu'), 'cpu' => Arr::get($data, 'cpu'),
'threads' => Arr::get($data, 'threads'), 'threads' => Arr::get($data, 'threads'),
'oom_disabled' => Arr::get($data, 'oom_disabled') ?? true, 'oom_killer' => Arr::get($data, 'oom_killer') ?? false,
'allocation_id' => Arr::get($data, 'allocation_id'), 'allocation_id' => Arr::get($data, 'allocation_id'),
'nest_id' => Arr::get($data, 'nest_id'), 'nest_id' => Arr::get($data, 'nest_id'),
'egg_id' => Arr::get($data, 'egg_id'), 'egg_id' => Arr::get($data, 'egg_id'),

View file

@ -64,7 +64,7 @@ class ServerTransformer extends Transformer
'disk' => $model->disk, 'disk' => $model->disk,
'io' => $model->io, 'io' => $model->io,
'memory' => $model->memory, 'memory' => $model->memory,
'oom_killer' => !$model->oom_disabled, 'oom_killer' => $model->oom_killer,
'swap' => $model->swap, 'swap' => $model->swap,
'threads' => $model->threads, 'threads' => $model->threads,
], ],

View file

@ -55,7 +55,7 @@ class ServerTransformer extends Transformer
'io' => $server->io, 'io' => $server->io,
'cpu' => $server->cpu, 'cpu' => $server->cpu,
'threads' => $server->threads, 'threads' => $server->threads,
'oom_disabled' => $server->oom_disabled, 'oom_killer' => $server->oom_killer,
], ],
'invocation' => $service->handle($server, !$user->can(Permission::ACTION_STARTUP_READ, $server)), 'invocation' => $service->handle($server, !$user->can(Permission::ACTION_STARTUP_READ, $server)),
'docker_image' => $server->image, 'docker_image' => $server->image,

View file

@ -37,7 +37,7 @@ class ServerFactory extends Factory
'io' => 500, 'io' => 500,
'cpu' => 0, 'cpu' => 0,
'threads' => null, 'threads' => null,
'oom_disabled' => 0, 'oom_killer' => true,
'startup' => '/bin/bash echo "hello world"', 'startup' => '/bin/bash echo "hello world"',
'image' => 'foo/bar:latest', 'image' => 'foo/bar:latest',
'allocation_limit' => null, 'allocation_limit' => null,

View file

@ -0,0 +1,44 @@
<?php
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Migrations\Migration;
return new class () extends Migration {
/**
* Run the migrations.
*/
public function up(): void
{
Schema::table('servers', function (Blueprint $table) {
$table->tinyInteger('oom_killer')->unsigned()->default(1)->after('oom_disabled');
});
DB::table('servers')->select(['id', 'oom_disabled'])->cursor()->each(function ($server) {
DB::table('servers')->where('id', $server->id)->update(['oom_killer' => !$server->oom_disabled]);
});
Schema::table('servers', function (Blueprint $table) {
$table->dropColumn('oom_disabled');
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('servers', function (Blueprint $table) {
$table->tinyInteger('oom_disabled')->unsigned()->default(0)->after('oom_killer');
});
DB::table('servers')->select(['id', 'oom_killer'])->cursor()->each(function ($server) {
DB::table('servers')->where('id', $server->id)->update(['oom_disabled' => !$server->oom_killer]);
});
Schema::table('servers', function (Blueprint $table) {
$table->dropColumn('oom_killer');
});
}
};

View file

@ -52,8 +52,6 @@ export default () => {
io: server.limits.io, io: server.limits.io,
cpu: server.limits.cpu, cpu: server.limits.cpu,
threads: server.limits.threads || '', threads: server.limits.threads || '',
// This value is inverted to have the switch be on when the
// OOM Killer is enabled, rather than when disabled.
oomKiller: server.limits.oomKiller, oomKiller: server.limits.oomKiller,
}, },
featureLimits: { featureLimits: {

View file

@ -114,7 +114,7 @@ class BuildModificationServiceTest extends IntegrationTestCase
$this->daemonServerRepository->expects('sync')->withNoArgs()->andReturnUndefined(); $this->daemonServerRepository->expects('sync')->withNoArgs()->andReturnUndefined();
$response = $this->getService()->handle($server, [ $response = $this->getService()->handle($server, [
'oom_disabled' => false, 'oom_killer' => true,
'memory' => 256, 'memory' => 256,
'swap' => 128, 'swap' => 128,
'io' => 600, 'io' => 600,
@ -126,7 +126,7 @@ class BuildModificationServiceTest extends IntegrationTestCase
'allocation_limit' => 20, 'allocation_limit' => 20,
]); ]);
$this->assertFalse($response->oom_disabled); $this->assertTrue($response->oom_killer);
$this->assertSame(256, $response->memory); $this->assertSame(256, $response->memory);
$this->assertSame(128, $response->swap); $this->assertSame(128, $response->swap);
$this->assertSame(600, $response->io); $this->assertSame(600, $response->io);

View file

@ -145,7 +145,7 @@ class ServerCreationServiceTest extends IntegrationTestCase
$this->assertSame($allocations[4]->id, $response->allocations[1]->id); $this->assertSame($allocations[4]->id, $response->allocations[1]->id);
$this->assertFalse($response->isSuspended()); $this->assertFalse($response->isSuspended());
$this->assertTrue($response->oom_disabled); $this->assertFalse($response->oom_killer);
$this->assertSame(0, $response->database_limit); $this->assertSame(0, $response->database_limit);
$this->assertSame(0, $response->allocation_limit); $this->assertSame(0, $response->allocation_limit);
$this->assertSame(0, $response->backup_limit); $this->assertSame(0, $response->backup_limit);