From 16f49f8dc1be84ed06441dc72eb8f4ced86c6e6b Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 29 Nov 2020 12:50:22 -0800 Subject: [PATCH] Close cleanup; only try to run power actions against non-suspended & installed servers; closes #2760 --- CHANGELOG.md | 4 + .../Server/BulkPowerActionCommand.php | 81 ++++++++----------- .../Repository/ServerRepositoryInterface.php | 19 ----- .../Eloquent/ServerRepository.php | 39 --------- .../Server/BulkPowerActionCommandTest.php | 4 +- 5 files changed, 40 insertions(+), 107 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 936c2b6a0..cfc6c0f9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ This file is a running track of new features and fixes to each version of the pa This project follows [Semantic Versioning](http://semver.org) guidelines. +## v1.1.3 +### Fixed +* Server bulk power actions command will no longer attempt to run commands against installing or suspended servers. + ## v1.1.2 ### Fixed * Fixes an exception thrown while trying to validate IP access for the client API. diff --git a/app/Console/Commands/Server/BulkPowerActionCommand.php b/app/Console/Commands/Server/BulkPowerActionCommand.php index 383879902..32d9868b5 100644 --- a/app/Console/Commands/Server/BulkPowerActionCommand.php +++ b/app/Console/Commands/Server/BulkPowerActionCommand.php @@ -2,30 +2,15 @@ namespace Pterodactyl\Console\Commands\Server; +use Pterodactyl\Models\Server; use Illuminate\Console\Command; -use GuzzleHttp\Exception\RequestException; use Illuminate\Validation\ValidationException; use Illuminate\Validation\Factory as ValidatorFactory; use Pterodactyl\Repositories\Wings\DaemonPowerRepository; -use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; +use Pterodactyl\Exceptions\Http\Connection\DaemonConnectionException; class BulkPowerActionCommand extends Command { - /** - * @var \Pterodactyl\Repositories\Wings\DaemonPowerRepository - */ - private $powerRepository; - - /** - * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface - */ - private $repository; - - /** - * @var \Illuminate\Validation\Factory - */ - private $validator; - /** * @var string */ @@ -39,37 +24,20 @@ class BulkPowerActionCommand extends Command */ protected $description = 'Perform bulk power management on large groupings of servers or nodes at once.'; - /** - * BulkPowerActionCommand constructor. - * - * @param \Pterodactyl\Repositories\Wings\DaemonPowerRepository $powerRepository - * @param \Pterodactyl\Contracts\Repository\ServerRepositoryInterface $repository - * @param \Illuminate\Validation\Factory $validator - */ - public function __construct( - DaemonPowerRepository $powerRepository, - ServerRepositoryInterface $repository, - ValidatorFactory $validator - ) { - parent::__construct(); - - $this->repository = $repository; - $this->validator = $validator; - $this->powerRepository = $powerRepository; - } - /** * Handle the bulk power request. * + * @param \Pterodactyl\Repositories\Wings\DaemonPowerRepository $powerRepository + * @param \Illuminate\Validation\Factory $validator * @throws \Illuminate\Validation\ValidationException */ - public function handle() + public function handle(DaemonPowerRepository $powerRepository, ValidatorFactory $validator) { $action = $this->argument('action'); $nodes = empty($this->option('nodes')) ? [] : explode(',', $this->option('nodes')); $servers = empty($this->option('servers')) ? [] : explode(',', $this->option('servers')); - $validator = $this->validator->make([ + $validator = $validator->make([ 'action' => $action, 'nodes' => $nodes, 'servers' => $servers, @@ -89,23 +57,18 @@ class BulkPowerActionCommand extends Command throw new ValidationException($validator); } - $count = $this->repository->getServersForPowerActionCount($servers, $nodes); + $count = $this->getQueryBuilder($servers, $nodes)->count(); if (! $this->confirm(trans('command/messages.server.power.confirm', ['action' => $action, 'count' => $count])) && $this->input->isInteractive()) { return; } $bar = $this->output->createProgressBar($count); - $servers = $this->repository->getServersForPowerAction($servers, $nodes); - - $servers->each(function ($server) use ($action, &$bar) { + $this->getQueryBuilder($servers, $nodes)->each(function (Server $server) use ($action, $powerRepository, &$bar) { $bar->clear(); try { - $this->powerRepository - ->setNode($server->node) - ->setServer($server) - ->send($action); - } catch (RequestException $exception) { + $powerRepository->setServer($server)->send($action); + } catch (DaemonConnectionException $exception) { $this->output->error(trans('command/messages.server.power.action_failed', [ 'name' => $server->name, 'id' => $server->id, @@ -120,4 +83,28 @@ class BulkPowerActionCommand extends Command $this->line(''); } + + /** + * Returns the query builder instance that will return the servers that should be affected. + * + * @param array $servers + * @param array $nodes + * @return \Illuminate\Database\Eloquent\Builder + */ + protected function getQueryBuilder(array $servers, array $nodes) + { + $instance = Server::query() + ->where('suspended', false) + ->where('installed', Server::STATUS_INSTALLED); + + if (! empty($nodes) && ! empty($servers)) { + $instance->whereIn('id', $servers)->orWhereIn('node_id', $nodes); + } else if (empty($nodes) && ! empty($servers)) { + $instance->whereIn('id', $servers); + } else if (! empty($nodes) && empty($servers)) { + $instance->whereIn('node_id', $nodes); + } + + return $instance->with('node'); + } } diff --git a/app/Contracts/Repository/ServerRepositoryInterface.php b/app/Contracts/Repository/ServerRepositoryInterface.php index d7e3dfc5d..55cf79a39 100644 --- a/app/Contracts/Repository/ServerRepositoryInterface.php +++ b/app/Contracts/Repository/ServerRepositoryInterface.php @@ -95,25 +95,6 @@ interface ServerRepositoryInterface extends RepositoryInterface */ public function getByUuid(string $uuid): Server; - /** - * Return all of the servers that should have a power action performed against them. - * - * @param int[] $servers - * @param int[] $nodes - * @param bool $returnCount - * @return int|\Illuminate\Support\LazyCollection - */ - public function getServersForPowerAction(array $servers = [], array $nodes = [], bool $returnCount = false); - - /** - * Return the total number of servers that will be affected by the query. - * - * @param int[] $servers - * @param int[] $nodes - * @return int - */ - public function getServersForPowerActionCount(array $servers = [], array $nodes = []): int; - /** * Check if a given UUID and UUID-Short string are unique to a server. * diff --git a/app/Repositories/Eloquent/ServerRepository.php b/app/Repositories/Eloquent/ServerRepository.php index fdfd82fc2..5c16664b6 100644 --- a/app/Repositories/Eloquent/ServerRepository.php +++ b/app/Repositories/Eloquent/ServerRepository.php @@ -194,45 +194,6 @@ class ServerRepository extends EloquentRepository implements ServerRepositoryInt } } - /** - * Return all of the servers that should have a power action performed against them. - * - * @param int[] $servers - * @param int[] $nodes - * @param bool $returnCount - * @return int|\Illuminate\Support\LazyCollection - */ - public function getServersForPowerAction(array $servers = [], array $nodes = [], bool $returnCount = false) - { - $instance = $this->getBuilder(); - - if (! empty($nodes) && ! empty($servers)) { - $instance->whereIn('id', $servers)->orWhereIn('node_id', $nodes); - } else if (empty($nodes) && ! empty($servers)) { - $instance->whereIn('id', $servers); - } else if (! empty($nodes) && empty($servers)) { - $instance->whereIn('node_id', $nodes); - } - - if ($returnCount) { - return $instance->count(); - } - - return $instance->with('node')->cursor(); - } - - /** - * Return the total number of servers that will be affected by the query. - * - * @param int[] $servers - * @param int[] $nodes - * @return int - */ - public function getServersForPowerActionCount(array $servers = [], array $nodes = []): int - { - return $this->getServersForPowerAction($servers, $nodes, true); - } - /** * Check if a given UUID and UUID-Short string are unique to a server. * diff --git a/tests/Unit/Commands/Server/BulkPowerActionCommandTest.php b/tests/Unit/Commands/Server/BulkPowerActionCommandTest.php index d1ba90cf4..92d42ed72 100644 --- a/tests/Unit/Commands/Server/BulkPowerActionCommandTest.php +++ b/tests/Unit/Commands/Server/BulkPowerActionCommandTest.php @@ -53,7 +53,7 @@ class BulkPowerActionCommandTest extends CommandTestCase $this->repository->expects('getServersForPowerAction')->with([], [])->andReturn($servers); for ($i = 0; $i < count($servers); $i++) { - $this->powerRepository->expects('setNode->setServer->send')->with('kill')->andReturnNull(); + $this->powerRepository->expects('setServer->send')->with('kill')->andReturnNull(); } $display = $this->runCommand($this->getCommand(), ['action' => 'kill'], ['yes']); @@ -107,7 +107,7 @@ class BulkPowerActionCommandTest extends CommandTestCase ->andReturn(1); $this->repository->expects('getServersForPowerAction')->with([], [])->andReturn(Collection::make([$server])); - $this->powerRepository->expects('setNode->setServer->send')->with('kill')->andReturnNull(); + $this->powerRepository->expects('setServer->send')->with('kill')->andReturnNull(); $display = $this->runCommand($this->getCommand(), [ 'action' => 'kill',