recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

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).

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13210, T12145

Differential Revision: https://secure.phabricator.com/D19761

epriestley 57b4b598 65e95365

+37 -17
+37 -17
src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
··· 113 113 DrydockBlueprint $blueprint, 114 114 DrydockResource $resource, 115 115 DrydockLease $lease) { 116 + 117 + // Require the binding to a given host be active before we'll hand out more 118 + // leases on the corresponding resource. 119 + $binding = $this->loadBindingForResource($resource); 120 + if ($binding->getIsDisabled()) { 121 + return false; 122 + } 123 + 116 124 return true; 117 125 } 118 126 ··· 154 162 DrydockLease $lease, 155 163 $type) { 156 164 157 - $viewer = PhabricatorUser::getOmnipotentUser(); 158 - 159 165 switch ($type) { 160 166 case DrydockCommandInterface::INTERFACE_TYPE: 161 167 $credential_phid = $blueprint->getFieldValue('credentialPHID'); 162 - $binding_phid = $resource->getAttribute('almanacBindingPHID'); 163 - 164 - $binding = id(new AlmanacBindingQuery()) 165 - ->setViewer($viewer) 166 - ->withPHIDs(array($binding_phid)) 167 - ->executeOne(); 168 - if (!$binding) { 169 - throw new Exception( 170 - pht( 171 - 'Unable to load binding "%s" to create command interface.', 172 - $binding_phid)); 173 - } 174 - 168 + $binding = $this->loadBindingForResource($resource); 175 169 $interface = $binding->getInterface(); 176 170 177 171 return id(new DrydockSSHCommandInterface()) ··· 213 207 $blueprint->getBlueprintName())); 214 208 } 215 209 216 - $viewer = PhabricatorUser::getOmnipotentUser(); 210 + $viewer = $this->getViewer(); 217 211 $services = id(new AlmanacServiceQuery()) 218 212 ->setViewer($viewer) 219 213 ->withPHIDs($service_phids) ··· 246 240 247 241 private function loadFreeBindings(DrydockBlueprint $blueprint) { 248 242 if ($this->freeBindings === null) { 249 - $viewer = PhabricatorUser::getOmnipotentUser(); 243 + $viewer = $this->getViewer(); 250 244 251 245 $pool = id(new DrydockResourceQuery()) 252 246 ->setViewer($viewer) ··· 293 287 ); 294 288 } 295 289 290 + private function loadBindingForResource(DrydockResource $resource) { 291 + $binding_phid = $resource->getAttribute('almanacBindingPHID'); 292 + if (!$binding_phid) { 293 + throw new Exception( 294 + pht( 295 + 'Drydock resource ("%s") has no Almanac binding PHID, so its '. 296 + 'binding can not be loaded.', 297 + $resource->getPHID())); 298 + } 299 + 300 + $viewer = $this->getViewer(); 301 + 302 + $binding = id(new AlmanacBindingQuery()) 303 + ->setViewer($viewer) 304 + ->withPHIDs(array($binding_phid)) 305 + ->executeOne(); 306 + if (!$binding) { 307 + throw new Exception( 308 + pht( 309 + 'Unable to load Almanac binding ("%s") for resource ("%s").', 310 + $binding_phid, 311 + $resource->getPHID())); 312 + } 313 + 314 + return $binding; 315 + } 296 316 297 317 }