When a Drydock host based on an Almanac blueprint has its binding disabled, stop handing out leases
ClosedPublic
Actions

Authored by epriestley on Oct 25 2018, 12:48 PM.

Details

Reviewers

Maniphest Tasks

T12145: Resource allocator does not create new host resources when one is already active
T13210: Plans: 2018 Week 41-44 Bonus Content

Commits

rP57b4b5981947: When a Drydock host based on an Almanac blueprint has its binding disabled…

Summary

Ref T13210. Ref T12145. The "Almanac Host" blueprint currently hands out new leases on a given host even if the binding has been disabled.

Although there are some more complicated cases here (e.g., involving cleanup of the existing resource and existing leases), this one seems clear cut: if the binding has been disabled, we should stop handing out new leases on it.

Test Plan

Created a service with two hosts.
Requested a lease, got host A.
Requested more leases, always got host A (we never build a new host when we don't have to, and we currently never have to).
Disabled the binding to host A.
Requested a lease.
- Before patch: got host A.
- After patch: got host B.
Also disabled the other binding to host B, requested a lease, got an indefinite wait for resources (which is expected and reasonable).

Diff Detail

Repository

rP Phabricator

Branch

drydock6

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 21056
Build 28612: Run Core Tests
Build 28611: arc lint + arc unit

Event Timeline

epriestley created this revision.Oct 25 2018, 12:48 PM

Owners added a subscriber: Restricted Owners Package.Oct 25 2018, 12:48 PM

Herald added a subscriber: yelirekim. · View Herald TranscriptOct 25 2018, 12:48 PM

Harbormaster completed remote builds in B21040: Diff 47195.Oct 25 2018, 12:49 PM

epriestley requested review of this revision.Oct 25 2018, 12:49 PM

epriestley mentioned this in D19762: Allow Drydock Blueprints to control "supplemental allocation" behavior so all hosts in an Almanac pool get used.Oct 25 2018, 1:58 PM

amckinley accepted this revision.Oct 26 2018, 9:30 PM

amckinley added inline comments.

src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
309	This message doesn't make sense now that it's been decoupled from the `DrydockCommandInterface::INTERFACE_TYPE` case statement in `getInterface()`.