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('