Merge pull request #265 from pelican-dev/issue/222

Simplify node deployment service, add filtering with tags instead of locations

.github/workflows/ci.yaml

@@ -99,7 +99,7 @@ jobs:
       QUEUE_CONNECTION: sync
       HASHIDS_SALT: alittlebitofsalt1234
       DB_CONNECTION: sqlite
-      DB_DATABASE: ${{ github.workspace }}/database/testing.sqlite
+      DB_DATABASE: testing.sqlite
     steps:
       - name: Code Checkout
         uses: actions/checkout@v4

app/Exceptions/Service/Deployment/NoViableNodeException.php

@@ -1,9 +0,0 @@
-<?php
-
-namespace App\Exceptions\Service\Deployment;
-
-use App\Exceptions\DisplayException;
-
-class NoViableNodeException extends DisplayException
-{
-}

app/Http/Controllers/Admin/Servers/CreateServerController.php

@@ -53,7 +53,6 @@ class CreateServerController extends Controller
      * @throws \Illuminate\Validation\ValidationException
      * @throws \App\Exceptions\DisplayException
      * @throws \App\Exceptions\Service\Deployment\NoViableAllocationException
-     * @throws \App\Exceptions\Service\Deployment\NoViableNodeException
      * @throws \Throwable
      */
     public function store(ServerFormRequest $request): RedirectResponse

app/Http/Controllers/Api/Application/Nodes/NodeDeploymentController.php

@@ -9,9 +9,6 @@ use App\Http\Requests\Api\Application\Nodes\GetDeployableNodesRequest;
 
 class NodeDeploymentController extends ApplicationApiController
 {
-    /**
-     * NodeDeploymentController constructor.
-     */
     public function __construct(private FindViableNodesService $viableNodesService)
     {
         parent::__construct();
@@ -21,17 +18,17 @@ class NodeDeploymentController extends ApplicationApiController
      * Finds any nodes that are available using the given deployment criteria. This works
      * similarly to the server creation process, but allows you to pass the deployment object
      * to this endpoint and get back a list of all Nodes satisfying the requirements.
-     *
-     * @throws \App\Exceptions\Service\Deployment\NoViableNodeException
      */
     public function __invoke(GetDeployableNodesRequest $request): array
     {
         $data = $request->validated();
-        $nodes = $this->viableNodesService
+
-            ->setMemory($data['memory'])
+        $nodes = $this->viableNodesService->handle(
-            ->setDisk($data['disk'])
+            $data['disk'] ?? 0,
-            ->setCpu($data['cpu'] ?? 0)
+            $data['memory'] ?? 0,
-            ->handle((int) $request->query('per_page'), (int) $request->query('page'));
+            $data['cpu'] ?? 0,
+            $data['tags'] ?? $data['location_ids'] ?? [],
+        );
 
         return $this->fractal->collection($nodes)
             ->transformWith($this->getTransformer(NodeTransformer::class))

app/Http/Controllers/Api/Application/Servers/ServerController.php

@@ -50,7 +50,6 @@ class ServerController extends ApplicationApiController
      * @throws \App\Exceptions\DisplayException
      * @throws \App\Exceptions\Model\DataValidationException
      * @throws \App\Exceptions\Service\Deployment\NoViableAllocationException
-     * @throws \App\Exceptions\Service\Deployment\NoViableNodeException
      */
     public function store(StoreServerRequest $request): JsonResponse
     {

app/Http/Requests/Api/Application/Nodes/GetDeployableNodesRequest.php

@@ -11,6 +11,10 @@ class GetDeployableNodesRequest extends GetNodesRequest
             'memory' => 'required|integer|min:0',
             'disk' => 'required|integer|min:0',
             'cpu' => 'sometimes|integer|min:0',
+            'tags' => 'sometimes|array',
+
+            /** @deprecated use tags instead */
+            'location_ids' => 'sometimes|array',
         ];
     }
 }

app/Http/Requests/Api/Application/Servers/StoreServerRequest.php

@@ -123,6 +123,15 @@ class StoreServerRequest extends ApplicationApiRequest
             return !$input->deploy;
         });
 
+        /** @deprecated use tags instead */
+        $validator->sometimes('deploy.locations', 'present', function ($input) {
+            return $input->deploy;
+        });
+
+        $validator->sometimes('deploy.tags', 'present', function ($input) {
+            return $input->deploy;
+        });
+
         $validator->sometimes('deploy.port_range', 'present', function ($input) {
             return $input->deploy;
         });
@@ -139,6 +148,7 @@ class StoreServerRequest extends ApplicationApiRequest
 
         $object = new DeploymentObject();
         $object->setDedicated($this->input('deploy.dedicated_ip', false));
+        $object->setTags($this->input('deploy.tags', $this->input('deploy.locations', [])));
         $object->setPorts($this->input('deploy.port_range', []));
 
         return $object;

app/Models/Objects/DeploymentObject.php

@@ -6,6 +6,8 @@ class DeploymentObject
 {
     private bool $dedicated = false;
 
+    private array $tags = [];
+
     private array $ports = [];
 
     public function isDedicated(): bool
@@ -31,4 +33,17 @@ class DeploymentObject
 
         return $this;
     }
+
+    public function getTags(): array
+    {
+        return $this->tags;
+    }
+
+    public function setTags(array $tags): self
+    {
+        $this->tags = $tags;
+
+        return $this;
+    }
+
 }

app/Services/Deployment/AllocationSelectionService.php

@@ -90,11 +90,9 @@ class AllocationSelectionService
      */
     private function getRandomAllocation(array $nodes = [], array $ports = [], bool $dedicated = false): ?Allocation
     {
-        $query = Allocation::query()->whereNull('server_id');
+        $query = Allocation::query()
-
+            ->whereNull('server_id')
-        if (!empty($nodes)) {
+            ->whereIn('node_id', $nodes);
-            $query->whereIn('node_id', $nodes);
-        }
 
         if (!empty($ports)) {
             $query->where(function ($inner) use ($ports) {

app/Services/Deployment/FindViableNodesService.php

@@ -3,54 +3,12 @@
 namespace App\Services\Deployment;
 
 use App\Models\Node;
-use Webmozart\Assert\Assert;
 use Illuminate\Support\Collection;
-use Illuminate\Contracts\Pagination\LengthAwarePaginator;
-use App\Exceptions\Service\Deployment\NoViableNodeException;
 
 class FindViableNodesService
 {
-    protected ?int $memory = null;
-    protected ?int $disk = null;
-    protected ?int $cpu = null;
-
     /**
-     * Set the amount of memory that this server will be using. As with disk space, nodes that
+     * Returns a collection of nodes that meet the provided requirements and can then
-     * do not have enough free memory will be filtered out.
-     */
-    public function setMemory(int $memory): self
-    {
-        $this->memory = $memory;
-
-        return $this;
-    }
-
-    /**
-     * Set the amount of disk that will be used by the server being created. Nodes will be
-     * filtered out if they do not have enough available free disk space for this server
-     * to be placed on.
-     */
-    public function setDisk(int $disk): self
-    {
-        $this->disk = $disk;
-
-        return $this;
-    }
-
-    /**
-     * Set the amount of cpu that will be used by the server being created. Nodes will be
-     * filtered out if they do not have enough available free cpu for this server
-     * to be placed on.
-     */
-    public function setCpu(int $cpu): self
-    {
-        $this->cpu = $cpu;
-
-        return $this;
-    }
-
-    /**
-     * Returns an array of nodes that meet the provided requirements and can then
      * be passed to the AllocationSelectionService to return a single allocation.
      *
      * This functionality is used for automatic deployments of servers and will
@@ -58,42 +16,20 @@ class FindViableNodesService
      * and cpu availability requirements. Any nodes not meeting those requirements
      * are tossed out, as are any nodes marked as non-public, meaning automatic
      * deployments should not be done against them.
-     *
-     * @param int|null $page If provided the results will be paginated by returning
-     *                       up to 50 nodes at a time starting at the provided page.
-     *                       If "null" is provided as the value no pagination will
-     *                       be used.
-     *
-     * @throws \App\Exceptions\Service\Deployment\NoViableNodeException
      */
-    public function handle(int $perPage = null, int $page = null): LengthAwarePaginator|Collection
+    public function handle(int $disk = 0, int $memory = 0, int $cpu = 0, $tags = []): Collection
     {
-        Assert::integer($this->memory, 'Memory usage must be an int, got %s');
+        $nodes = Node::query()
-        Assert::integer($this->disk, 'Disk space must be an int, got %s');
+            ->withSum('servers', 'disk')
-        Assert::integer($this->cpu, 'CPU must be an int, got %s');
+            ->withSum('servers', 'memory')
+            ->withSum('servers', 'cpu')
+            ->where('public', true)
+            ->get();
 
-        $query = Node::query()->select('nodes.*')
+        return $nodes
-            ->selectRaw('IFNULL(SUM(servers.memory), 0) as sum_memory')
+            ->filter(fn (Node $node) => !$tags || collect($node->tags)->intersect($tags))
-            ->selectRaw('IFNULL(SUM(servers.disk), 0) as sum_disk')
+            ->filter(fn (Node $node) => $node->servers_sum_disk + $disk <= $node->disk * (1 + $node->disk_overallocate / 100))
-            ->selectRaw('IFNULL(SUM(servers.cpu), 0) as sum_cpu')
+            ->filter(fn (Node $node) => $node->servers_sum_memory + $memory <= $node->memory * (1 + $node->memory_overallocate / 100))
-            ->leftJoin('servers', 'servers.node_id', '=', 'nodes.id')
+            ->filter(fn (Node $node) => $node->servers_sum_cpu + $cpu <= $node->cpu * (1 + $node->cpu_overallocate / 100));
-            ->where('nodes.public', 1);
-
-        $results = $query->groupBy('nodes.id')
-            ->havingRaw('(IFNULL(SUM(servers.memory), 0) + ?) <= (nodes.memory * (1 + (nodes.memory_overallocate / 100)))', [$this->memory])
-            ->havingRaw('(IFNULL(SUM(servers.disk), 0) + ?) <= (nodes.disk * (1 + (nodes.disk_overallocate / 100)))', [$this->disk])
-            ->havingRaw('(IFNULL(SUM(servers.cpu), 0) + ?) <= (nodes.cpu * (1 + (nodes.cpu_overallocate / 100)))', [$this->cpu]);
-
-        if (!is_null($page)) {
-            $results = $results->paginate($perPage ?? 50, ['*'], 'page', $page);
-        } else {
-            $results = $results->get()->toBase();
-        }
-
-        if ($results->isEmpty()) {
-            throw new NoViableNodeException(trans('exceptions.deployment.no_viable_nodes'));
-        }
-
-        return $results;
     }
 }

app/Services/Servers/ServerCreationService.php

@@ -42,7 +42,6 @@ class ServerCreationService
      * @throws \Throwable
      * @throws \App\Exceptions\DisplayException
      * @throws \Illuminate\Validation\ValidationException
-     * @throws \App\Exceptions\Service\Deployment\NoViableNodeException
      * @throws \App\Exceptions\Service\Deployment\NoViableAllocationException
      */
     public function handle(array $data, DeploymentObject $deployment = null): Server
@@ -105,16 +104,16 @@ class ServerCreationService
      *
      * @throws \App\Exceptions\DisplayException
      * @throws \App\Exceptions\Service\Deployment\NoViableAllocationException
-     * @throws \App\Exceptions\Service\Deployment\NoViableNodeException
      */
     private function configureDeployment(array $data, DeploymentObject $deployment): Allocation
     {
-        /** @var \Illuminate\Support\Collection $nodes */
+        /** @var Collection<\App\Models\Node> $nodes */
-        $nodes = $this->findViableNodesService
+        $nodes = $this->findViableNodesService->handle(
-            ->setMemory(Arr::get($data, 'memory'))
+            Arr::get($data, 'disk', 0),
-            ->setDisk(Arr::get($data, 'disk'))
+            Arr::get($data, 'memory', 0),
-            ->setCpu(Arr::get($data, 'cpu'))
+            Arr::get($data, 'cpu', 0),
-            ->handle();
+            Arr::get($data, 'tags', []),
+        );
 
         return $this->allocationSelectionService->setDedicated($deployment->isDedicated())
             ->setNodes($nodes->pluck('id')->toArray())

tests/Integration/Services/Deployment/FindViableNodesServiceTest.php

@@ -1,50 +0,0 @@
-<?php
-
-namespace App\Tests\Integration\Services\Deployment;
-
-use App\Models\Node;
-use App\Models\Server;
-use App\Models\Database;
-use App\Tests\Integration\IntegrationTestCase;
-use App\Services\Deployment\FindViableNodesService;
-
-class FindViableNodesServiceTest extends IntegrationTestCase
-{
-    protected function setUp(): void
-    {
-        parent::setUp();
-
-        Database::query()->delete();
-        Server::query()->delete();
-        Node::query()->delete();
-    }
-
-    public function testExceptionIsThrownIfNoMemoryHasBeenSet(): void
-    {
-        $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('Memory usage must be an int, got NULL');
-
-        $this->getService()->setDisk(10)->setCpu(10)->handle();
-    }
-
-    public function testExceptionIsThrownIfNoDiskSpaceHasBeenSet(): void
-    {
-        $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('Disk space must be an int, got NULL');
-
-        $this->getService()->setMemory(10)->setCpu(10)->handle();
-    }
-
-    public function testExceptionIsThrownIfNoCpuHasBeenSet(): void
-    {
-        $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('CPU must be an int, got NULL');
-
-        $this->getService()->setMemory(10)->setDisk(10)->handle();
-    }
-
-    private function getService(): FindViableNodesService
-    {
-        return $this->app->make(FindViableNodesService::class);
-    }
-}