From 1207182662d1e4b6d1c032b4d7989c14da8755c4 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 1 Aug 2026 18:53:11 +0200 Subject: [PATCH] Ein angehefteter Host umging die Reservierung MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Heftet ein Betreiber eine geparkte Bestellung an einen Host, prüfte der Anheft-Zweig nur die Kapazität — nicht, ob die Maschine jemand anderem reserviert ist. Die frisch gebaute Reservierung ließ sich damit über einen ganz normalen Weg durch die Oberfläche aushebeln, denn das Auswahlfeld auf der Kapazitätsseite bot weiterhin alle aktiven Hosts an. Der Schritt stellt jetzt dieselbe Bedingung wie die freie Platzierung — unreserviert oder dem Kunden dieser Bestellung reserviert — über Host::scopeUnreserved(), statt sie ein zweites Mal zu formulieren. Ein Pin auf eine fremde Maschine wird nicht umgeleitet, sondern wartet (awaiting_pinned_host): zu korrigieren ist die Wahl, nicht die Maschine. Und die Ursache eine Ebene höher: das Auswahlfeld stellt fremd reservierte Hosts gar nicht mehr zur Wahl, pin() lehnt sie zusätzlich ab, und ein Pin, der seit dem Anheften reserviert wurde, sagt das in der Zeile — ein Auswahlfeld, das dann kommentarlos wieder "Automatisch" zeigte, verschwiege, warum die Bestellung weiter wartet. Co-Authored-By: Claude Opus 5 --- app/Livewire/Admin/Capacity.php | 40 +++++++- .../Steps/Customer/ReserveResources.php | 29 +++++- lang/de/capacity.php | 1 + lang/en/capacity.php | 1 + .../views/livewire/admin/capacity.blade.php | 35 +++++-- tests/Feature/Admin/CapacityQueueTest.php | 95 ++++++++++++++++++- 6 files changed, 190 insertions(+), 11 deletions(-) diff --git a/app/Livewire/Admin/Capacity.php b/app/Livewire/Admin/Capacity.php index 3403daa..014797a 100644 --- a/app/Livewire/Admin/Capacity.php +++ b/app/Livewire/Admin/Capacity.php @@ -76,7 +76,16 @@ class Capacity extends Component return; } - $host = Host::query()->where('status', 'active')->find((int) $hostId); + // Nur ein Host, auf dem diese Bestellung auch landen dürfte: + // unreserviert oder ihrem eigenen Kunden reserviert, über denselben + // Scope, den die Platzierung im Schritt benutzt. Eine reservierte + // Maschine gehört ihrem Kunden — sie hier anheften zu lassen hieße, die + // Zusage über die Konsole wieder aufzumachen, und der Schritt lehnte + // die Bestellung danach ohnehin ab. + $host = Host::query() + ->where('status', 'active') + ->unreserved($run->subject?->customer_id) + ->find((int) $hostId); if ($host === null) { return; @@ -96,9 +105,36 @@ class Capacity extends Component $capacity = app(HostCapacity::class); $demand = $capacity->queueDemand(); + $parked = $capacity->parked(); + + // Die Auswahl je Bestellung, nicht eine Liste für alle: welcher Host in + // Frage kommt, hängt am Kunden dieser Bestellung. Gefragt wird über + // denselben Scope wie in der Platzierung — im eigenen Rechenzentrum + // (der Schritt lehnt einen Pin von anderswo ab) und nicht jemand + // anderem reserviert. Ein Auswahlfeld, das anbietet, was die Aktion + // danach ablehnt, ist eine Falle. + // + // Eine Abfrage je Rechenzentrum/Kunde-Paar statt je Zeile: die + // Warteschlange ist die Liste bezahlter Bestellungen ohne Maschine, also + // kurz, und mehrere davon gehören meist demselben Rechenzentrum. + $choices = []; + + foreach ($parked as $index => $entry) { + $customerId = $entry['order']->customer_id; + $key = $entry['datacenter'].'|'.$customerId; + + $choices[$key] ??= Host::query() + ->where('status', 'active') + ->where('datacenter', $entry['datacenter']) + ->unreserved($customerId) + ->orderBy('name') + ->get(); + + $parked[$index]['choices'] = $choices[$key]; + } return view('livewire.admin.capacity', [ - 'parked' => $capacity->parked(), + 'parked' => $parked, 'demand' => $demand, 'unplaceable' => $capacity->unplaceablePlans(), // reservedFor eager-loaded: die Tafel "was die Hosts noch diff --git a/app/Provisioning/Steps/Customer/ReserveResources.php b/app/Provisioning/Steps/Customer/ReserveResources.php index aa8a170..c668a1a 100644 --- a/app/Provisioning/Steps/Customer/ReserveResources.php +++ b/app/Provisioning/Steps/Customer/ReserveResources.php @@ -91,8 +91,14 @@ class ReserveResources extends CustomerStep // it somewhere else because the chosen host happened to be full // would answer a different question than the one the operator // answered, and they would find out from the finished instance. + // + // Reservierung UND Platz, nicht nur Platz: ein Anheften ist eine + // Wahl unter den erlaubten Hosts, keine Umgehung. Ohne die erste + // Bedingung ließ sich die Zusage "diese Maschine gehört Ihnen" + // über das Auswahlfeld der Kapazitätsseite aushebeln — dieselbe + // Frage, die placeableIn() unten über denselben Scope stellt. $host = $pinned !== null - ? ($pinned->canTake((int) $plan['disk_gb']) ? $pinned : null) + ? ($this->mayPlaceOn($pinned, $order) && $pinned->canTake((int) $plan['disk_gb']) ? $pinned : null) : Host::placeableIn($order->datacenter, (int) $plan['disk_gb'], $order->customer_id); if ($host === null) { @@ -197,6 +203,27 @@ class ReserveResources extends CustomerStep ->find((int) $id); } + /** + * Darf diese Bestellung überhaupt auf diese Maschine? + * + * Unreserviert oder diesem Kunden reserviert — über denselben Scope, den + * die freie Platzierung benutzt, und nicht als zweite Formulierung + * derselben Bedingung: zwei Formulierungen wären zwei Grenzen, und die + * zweite wäre die, die man beim nächsten Mal vergisst. + * + * Ein Pin auf eine fremde Maschine wird damit nicht umgeleitet, sondern + * wartet (`awaiting_pinned_host`) — die Wahl ist zu korrigieren, nicht die + * Maschine, und ein stiller Ausweichplatz wäre wieder eine Antwort auf eine + * Frage, die niemand gestellt hat. + */ + private function mayPlaceOn(Host $host, Order $order): bool + { + return Host::query() + ->whereKey($host->getKey()) + ->unreserved($order->customer_id) + ->exists(); + } + private function putContext(ProvisioningRun $run, Instance $instance): void { $run->mergeContext([ diff --git a/lang/de/capacity.php b/lang/de/capacity.php index f4f46e6..199c7fa 100644 --- a/lang/de/capacity.php +++ b/lang/de/capacity.php @@ -19,6 +19,7 @@ return [ 'col_target' => 'Ziel-Host', 'target_auto' => 'Automatisch platzieren', 'target_too_small' => 'Dieser Host hat derzeit zu wenig Platz — die Bestellung wartet, bis er ihn hat.', + 'target_reserved' => 'Dieser Host ist inzwischen einem anderen Kunden reserviert — bitte einen anderen wählen.', 'pinned' => 'Wird auf :host ausgerollt, sobald dort Platz ist.', 'pin_cleared' => 'Zuweisung aufgehoben — die Bestellung nimmt wieder den ersten passenden Host.', diff --git a/lang/en/capacity.php b/lang/en/capacity.php index d52fd9f..ebc2ee7 100644 --- a/lang/en/capacity.php +++ b/lang/en/capacity.php @@ -19,6 +19,7 @@ return [ 'col_target' => 'Target host', 'target_auto' => 'Place automatically', 'target_too_small' => 'This host has too little room right now — the order waits until it has.', + 'target_reserved' => 'This host is now reserved for another customer — please choose a different one.', 'pinned' => 'Will roll out on :host as soon as it has room.', 'pin_cleared' => 'Assignment cleared — the order takes the first host that fits again.', diff --git a/resources/views/livewire/admin/capacity.blade.php b/resources/views/livewire/admin/capacity.blade.php index f9515aa..cec9fae 100644 --- a/resources/views/livewire/admin/capacity.blade.php +++ b/resources/views/livewire/admin/capacity.blade.php @@ -36,14 +36,26 @@ @foreach ($parked as $entry) @php - // Only hosts in the order's own datacenter: placement - // is a per-datacenter question, and the step refuses a - // pin from anywhere else. Offering one here would be - // offering a choice that silently does nothing. - $choices = $hosts->where('datacenter', $entry['datacenter']); + // Die Hosts, auf denen GENAU DIESE Bestellung landen + // dürfte — eigenes Rechenzentrum und keine fremde + // Reservierung. Zusammengestellt in Capacity::render() + // über denselben Scope wie die Platzierung selbst, + // statt hier ein zweites Mal formuliert. + $choices = $entry['choices']; + // Der angeheftete Host aus dem ganzen Bestand des + // Rechenzentrums, nicht nur aus der Auswahl: eine + // Maschine, die seit dem Anheften reserviert wurde, + // steht nicht mehr zur Wahl — und ein Auswahlfeld, + // das dann kommentarlos wieder „Automatisch" zeigt, + // verschweigt, warum die Bestellung weiter wartet. + // Ein Pin aus einem anderen Rechenzentrum bleibt + // unbeachtet wie bisher; der Schritt ignoriert ihn + // ebenfalls, und „reserviert" wäre dafür der + // falsche Satz. $pinnedHost = $entry['pinned_host_id'] === null ? null - : $choices->firstWhere('id', $entry['pinned_host_id']); + : $hosts->where('datacenter', $entry['datacenter'])->firstWhere('id', $entry['pinned_host_id']); + $pinnable = $pinnedHost !== null && $choices->contains('id', $pinnedHost->id); @endphp {{ $entry['order']->customer?->name ?? '—' }} @@ -65,7 +77,16 @@ @endforeach - @if ($pinnedHost !== null && ! $pinnedHost->canTake($entry['needs'])) + @if ($pinnedHost !== null && ! $pinnable) + {{-- Die Wahl steht noch im Lauf, die Maschine + gehört inzwischen jemandem: der Schritt + lässt die Bestellung darauf nicht landen, + und ohne diesen Satz sähe die Zeile aus + wie jede andere. --}} +

+ {{ __('capacity.target_reserved') }} +

+ @elseif ($pinnedHost !== null && ! $pinnedHost->canTake($entry['needs'])) {{-- A pin is honoured or it waits — never quietly redirected. So say so, or the row sits there looking like every other one diff --git a/tests/Feature/Admin/CapacityQueueTest.php b/tests/Feature/Admin/CapacityQueueTest.php index 380e35b..16e229a 100644 --- a/tests/Feature/Admin/CapacityQueueTest.php +++ b/tests/Feature/Admin/CapacityQueueTest.php @@ -2,11 +2,13 @@ use App\Livewire\Admin\Capacity; use App\Livewire\Admin\Overview; +use App\Models\Customer; use App\Models\Datacenter; use App\Models\Host; use App\Models\Instance; use App\Models\Order; use App\Models\ProvisioningRun; +use App\Models\Subscription; use App\Provisioning\RunRunner; use App\Provisioning\Steps\Customer\ReserveResources; use App\Services\Provisioning\HostCapacity; @@ -159,7 +161,7 @@ it('sizes the queue from the contract the customer signed, not today catalogue', // for the customer who bought the old one. $run = parkedOrder('start'); - App\Models\Subscription::query() + Subscription::query() ->where('order_id', $run->subject_id) ->update(['disk_gb' => 4000]); @@ -246,6 +248,97 @@ it('waits rather than quietly placing a pinned order somewhere else', function ( ->and(Instance::query()->where('order_id', $run->subject_id)->exists())->toBeFalse(); }); +/** + * Nachtrag aus der Durchsicht (Befund 2): ein Anheften ist eine Wahl unter den + * erlaubten Hosts, keine Umgehung der Reservierung. + * + * Der Anheft-Zweig prüfte nur die Kapazität (`canTake`) — nicht, ob die + * Maschine jemand anderem gehört. Die frisch gebaute Reservierung ließ sich + * damit über einen ganz normalen Weg durch die Oberfläche aushebeln: das + * Auswahlfeld auf der Kapazitätsseite bot alle aktiven Hosts an. + */ +it('platziert eine angeheftete Bestellung nicht auf einem fremd reservierten Host', function () { + $run = parkedOrder('start'); + $fremder = Customer::factory()->create(); + + $reserviert = Host::factory()->active()->create([ + 'datacenter' => 'fsn1', 'name' => 'pve-fremd', 'total_gb' => 1000, 'reserve_pct' => 0, + 'reserved_for_customer_id' => $fremder->id, + ]); + + // Ein freier Host daneben: der Pin darf auch nicht still dorthin ausweichen + // — eine Bestellung, die woanders landet, beantwortet eine andere Frage als + // die, die der Betreiber beantwortet hat. + $frei = Host::factory()->active()->create(['datacenter' => 'fsn1', 'total_gb' => 1000, 'reserve_pct' => 0]); + + $run->mergeContext(['preferred_host_id' => $reserviert->id]); + $run->update(['status' => ProvisioningRun::STATUS_RUNNING]); + + $result = app(ReserveResources::class)->execute($run->fresh()); + + expect($result->type)->toBe('poll') + ->and($result->reason)->toBe('awaiting_pinned_host') + ->and(Instance::query()->where('order_id', $run->subject_id)->exists())->toBeFalse() + ->and($frei->instances()->count())->toBe(0); +}); + +it('platziert eine angeheftete Bestellung auf dem Host, der ihrem eigenen Kunden reserviert ist', function () { + // Der ganze Sinn einer Reservierung: die Maschine gehört diesem Kunden, und + // seine eigene Bestellung gehört genau dorthin. + $run = parkedOrder('start'); + + $eigen = Host::factory()->active()->create([ + 'datacenter' => 'fsn1', 'total_gb' => 1000, 'reserve_pct' => 0, + 'reserved_for_customer_id' => $run->subject->customer_id, + ]); + + $run->mergeContext(['preferred_host_id' => $eigen->id]); + $run->update(['status' => ProvisioningRun::STATUS_RUNNING]); + + app(ReserveResources::class)->execute($run->fresh()); + + expect(Instance::query()->where('order_id', $run->subject_id)->first()?->host_id)->toBe($eigen->id); +}); + +it('stellt einen fremd reservierten Host im Anheft-Auswahlfeld gar nicht erst zur Wahl', function () { + // Die Ursache eine Ebene höher. Ein Knopf, der etwas anbietet, das die + // Aktion danach ablehnt, ist eine Falle — und die Aktion lehnt jetzt ab. + $run = parkedOrder('start'); + $fremder = Customer::factory()->create(); + + $reserviert = Host::factory()->active()->create([ + 'datacenter' => 'fsn1', 'name' => 'pve-fremd', 'total_gb' => 1000, 'reserve_pct' => 0, + 'reserved_for_customer_id' => $fremder->id, + ]); + $eigen = Host::factory()->active()->create([ + 'datacenter' => 'fsn1', 'name' => 'pve-eigen', 'total_gb' => 1000, 'reserve_pct' => 0, + 'reserved_for_customer_id' => $run->subject->customer_id, + ]); + + $page = Livewire::actingAs(operator('Owner'), 'operator')->test(Capacity::class); + + $page->assertDontSeeHtml('