Cleanup & fix server deployment (#1497)

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

@@ -55,6 +55,7 @@ class StoreServerRequest extends ApplicationApiRequest
 
             // Automatic deployment rules
             'deploy' => 'sometimes|required|array',
+            // Locations are deprecated, use tags
             'deploy.locations' => 'sometimes|array',
             'deploy.locations.*' => 'required_with:deploy.locations|integer|min:1',
             'deploy.tags' => 'array',
@@ -176,7 +177,6 @@ class StoreServerRequest extends ApplicationApiRequest
         $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', []));
-        $object->setNode($this->input('deploy.node_id'));
 
         return $object;
     }

app/Models/Objects/DeploymentObject.php

@@ -2,12 +2,8 @@
 
 namespace App\Models\Objects;
 
-use App\Models\Node;
-
 class DeploymentObject
 {
-    private ?Node $node = null;
-
     private bool $dedicated = false;
 
     /** @var string[] */
@@ -16,18 +12,6 @@ class DeploymentObject
     /** @var array<int|string> */
     private array $ports = [];
 
-    public function getNode(): ?Node
-    {
-        return $this->node;
-    }
-
-    public function setNode(Node $node): self
-    {
-        $this->node = $node;
-
-        return $this;
-    }
-
     public function isDedicated(): bool
     {
         return $this->dedicated;

app/Services/Servers/ServerCreationService.php

@@ -67,19 +67,35 @@ class ServerCreationService
         $data['image'] = $data['image'] ?? collect($egg->docker_images)->first();
         $data['startup'] = $data['startup'] ?? $egg->startup;
 
-        // If a deployment object has been passed we need to get the allocation
+        // If a deployment object has been passed we need to get the allocation and node that the server should use.
-        // that the server should use, and assign the node from that allocation.
         if ($deployment) {
-            $allocation = $this->configureDeployment($data, $deployment);
+            $nodes = $this->findViableNodesService->handle(
+                Arr::get($data, 'memory', 0),
+                Arr::get($data, 'disk', 0),
+                Arr::get($data, 'cpu', 0),
+                $deployment->getTags(),
+            )->pluck('id');
+
+            if ($nodes->isEmpty()) {
+                throw new NoViableNodeException(trans('exceptions.deployment.no_viable_nodes'));
+            }
+
+            $ports = $deployment->getPorts();
+            if (!empty($ports)) {
+                $allocation = $this->allocationSelectionService->setDedicated($deployment->isDedicated())
+                    ->setNodes($nodes->toArray())
+                    ->setPorts($ports)
+                    ->handle();
 
-            if ($allocation) {
                 $data['allocation_id'] = $allocation->id;
-                // Auto-configure the node based on the selected allocation
-                // if no node was defined.
                 $data['node_id'] = $allocation->node_id;
             }
-            $data['node_id'] ??= $deployment->getNode()->id;
+
+            if (empty($data['node_id'])) {
+                $data['node_id'] = $nodes->first();
             }
+        }
+
         Assert::false(empty($data['node_id']), 'Expected a non-empty node_id in server creation data.');
 
         $eggVariableData = $this->validatorService
@@ -118,39 +134,6 @@ class ServerCreationService
         return $server;
     }
 
-    /**
-     * Gets an allocation to use for automatic deployment.
-     *
-     * @param  array{memory?: ?int, disk?: ?int, cpu?: ?int, tags?: ?string[]}  $data
-     *
-     * @throws \App\Exceptions\Service\Deployment\NoViableAllocationException
-     * @throws \App\Exceptions\Service\Deployment\NoViableNodeException
-     */
-    private function configureDeployment(array $data, DeploymentObject $deployment): ?Allocation
-    {
-        $nodes = $this->findViableNodesService->handle(
-            Arr::get($data, 'memory', 0),
-            Arr::get($data, 'disk', 0),
-            Arr::get($data, 'cpu', 0),
-            $deployment->getTags(),
-        );
-
-        $availableNodes = $nodes->pluck('id');
-
-        if ($availableNodes->isEmpty()) {
-            throw new NoViableNodeException(trans('exceptions.deployment.no_viable_nodes'));
-        }
-
-        if (!$deployment->getPorts()) {
-            return null;
-        }
-
-        return $this->allocationSelectionService->setDedicated($deployment->isDedicated())
-            ->setNodes($availableNodes->toArray())
-            ->setPorts($deployment->getPorts())
-            ->handle();
-    }
-
     /**
      * Store the server in the database and return the model.
      *

tests/Integration/Services/Servers/ServerCreationServiceTest.php

@@ -116,6 +116,7 @@ class ServerCreationServiceTest extends IntegrationTestCase
         $this->assertInstanceOf(Server::class, $response);
         $this->assertNotNull($response->uuid);
         $this->assertSame($response->uuid_short, substr($response->uuid, 0, 8));
+        $this->assertSame($node->id, $response->node_id);
         $this->assertSame($egg->id, $response->egg_id);
         $this->assertCount(2, $response->variables);
         $this->assertSame('123', $response->variables()->firstWhere('env_variable', 'BUNGEE_VERSION')->server_value);
@@ -153,7 +154,7 @@ class ServerCreationServiceTest extends IntegrationTestCase
         /** @var \App\Models\Node $node */
         $node = Node::factory()->create();
 
-        $deployment = (new DeploymentObject())->setNode($node);
+        $deployment = new DeploymentObject();
 
         $egg = $this->cloneEggAndVariables($this->bungeecord);
         // We want to make sure that the validator service runs as an admin, and not as a regular
@@ -204,6 +205,7 @@ class ServerCreationServiceTest extends IntegrationTestCase
         $this->assertInstanceOf(Server::class, $response);
         $this->assertNotNull($response->uuid);
         $this->assertSame($response->uuid_short, substr($response->uuid, 0, 8));
+        $this->assertSame($node->id, $response->node_id);
         $this->assertSame($egg->id, $response->egg_id);
         $this->assertCount(2, $response->variables);
         $this->assertSame('123', $response->variables()->firstWhere('env_variable', 'BUNGEE_VERSION')->server_value);