From cd6f67ef95a36720babe663fdd3b2b1f438f5796 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Dec 2014 14:31:22 -0800 Subject: [PATCH] When repository services are available, use them when creating a new repository Summary: Ref T2783. When creating a new repository, test for cluster services. If cluster services are available, allocate on a random open service. Show the service that repositories are allocated on. Test Plan: Created a new repository, saw it allocate onto an available cluster service. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2783 Differential Revision: https://secure.phabricator.com/D11003 --- .../DiffusionRepositoryCreateController.php | 43 ++++++++++++++++++- .../DiffusionRepositoryEditMainController.php | 15 +++++++ ...ffusionRepositoryEditStorageController.php | 25 ++++++++--- .../editor/PhabricatorRepositoryEditor.php | 8 ++++ .../PhabricatorRepositoryTransaction.php | 30 ++++++++++++- 5 files changed, 112 insertions(+), 9 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionRepositoryCreateController.php b/src/applications/diffusion/controller/DiffusionRepositoryCreateController.php index fcb5579110..d7cb62b17e 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryCreateController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryCreateController.php @@ -20,6 +20,7 @@ final class DiffusionRepositoryCreateController // the latter two cases, we show only a few of the pages. $repository = null; + $service = null; switch ($this->edit) { case 'remote': case 'policy': @@ -40,6 +41,38 @@ final class DiffusionRepositoryCreateController $this->requireApplicationCapability( DiffusionCreateRepositoriesCapability::CAPABILITY); + // Pick a random open service to allocate this repository on, if any + // exist. If there are no services, we aren't in cluster mode and + // will allocate locally. If there are services but none permit + // allocations, we fail. + $services = id(new AlmanacServiceQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withServiceClasses( + array( + 'AlmanacClusterRepositoryServiceType', + )) + ->execute(); + if ($services) { + // Filter out services which do not permit new allocations. + foreach ($services as $key => $possible_service) { + if ($possible_service->getAlmanacPropertyValue('closed')) { + unset($services[$key]); + } + } + + if (!$services) { + throw new Exception( + pht( + 'This install is configured in cluster mode, but all '. + 'available repository cluster services are closed to new '. + 'allocations. At least one service must be open to allow '. + 'new allocations to take place.')); + } + + shuffle($services); + $service = head($services); + } + $cancel_uri = $this->getApplicationURI('new/'); break; default: @@ -110,6 +143,7 @@ final class DiffusionRepositoryCreateController $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY; $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY; $type_push = PhabricatorRepositoryTransaction::TYPE_PUSH_POLICY; + $type_service = PhabricatorRepositoryTransaction::TYPE_SERVICE; $xactions = array(); @@ -141,8 +175,13 @@ final class DiffusionRepositoryCreateController ->getControl('activate')->getValue(); $xactions[] = id(clone $template) ->setTransactionType($type_activate) - ->setNewValue( - ($activate == 'start')); + ->setNewValue(($activate == 'start')); + + if ($service) { + $xactions[] = id(clone $template) + ->setTransactionType($type_service) + ->setNewValue($service->getPHID()); + } $default_local_path = PhabricatorEnv::getEnvConfig( 'repository.default-local-path'); diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php index d50fe53860..128a1f8f89 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php @@ -575,6 +575,21 @@ final class DiffusionRepositoryEditMainController ->setUser($viewer) ->setActionList($actions); + $service_phid = $repository->getAlmanacServicePHID(); + if ($service_phid) { + $handles = $this->loadViewerHandles(array($service_phid)); + $v_service = $handles[$service_phid]->renderLink(); + } else { + $v_service = phutil_tag( + 'em', + array(), + pht('Local')); + } + + $view->addProperty( + pht('Storage Service'), + $v_service); + $view->addProperty( pht('Storage Path'), $repository->getHumanReadableDetail('local-path')); diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditStorageController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditStorageController.php index 16fec0064a..887b6ae161 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditStorageController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditStorageController.php @@ -33,8 +33,28 @@ final class DiffusionRepositoryEditStorageController $title = pht('Edit %s', $repository->getName()); + $service_phid = $repository->getAlmanacServicePHID(); + if ($service_phid) { + $handles = $this->loadViewerHandles(array($service_phid)); + $v_service = $handles[$service_phid]->renderLink(); + } else { + $v_service = phutil_tag( + 'em', + array(), + pht('Local')); + } + $form = id(new AphrontFormView()) ->setUser($user) + ->appendChild( + id(new AphrontFormMarkupControl()) + ->setLabel(pht('Storage Service')) + ->setValue($v_service)) + ->appendChild( + id(new AphrontFormMarkupControl()) + ->setName('local') + ->setLabel(pht('Storage Path')) + ->setValue($v_local)) ->appendRemarkupInstructions( pht( "You can not adjust the local path for this repository from the ". @@ -42,11 +62,6 @@ final class DiffusionRepositoryEditStorageController " phabricator/ $ ./bin/repository edit %s --as %s --local-path ...", $repository->getCallsign(), $user->getUsername())) - ->appendChild( - id(new AphrontFormMarkupControl()) - ->setName('local') - ->setLabel(pht('Local Path')) - ->setValue($v_local)) ->appendChild( id(new AphrontFormSubmitControl()) ->addCancelButton($edit_uri, pht('Done'))); diff --git a/src/applications/repository/editor/PhabricatorRepositoryEditor.php b/src/applications/repository/editor/PhabricatorRepositoryEditor.php index a563a0a7e9..5335fa2a31 100644 --- a/src/applications/repository/editor/PhabricatorRepositoryEditor.php +++ b/src/applications/repository/editor/PhabricatorRepositoryEditor.php @@ -40,6 +40,7 @@ final class PhabricatorRepositoryEditor $types[] = PhabricatorRepositoryTransaction::TYPE_CREDENTIAL; $types[] = PhabricatorRepositoryTransaction::TYPE_DANGEROUS; $types[] = PhabricatorRepositoryTransaction::TYPE_CLONE_NAME; + $types[] = PhabricatorRepositoryTransaction::TYPE_SERVICE; $types[] = PhabricatorTransactions::TYPE_EDGE; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; @@ -95,6 +96,8 @@ final class PhabricatorRepositoryEditor return $object->shouldAllowDangerousChanges(); case PhabricatorRepositoryTransaction::TYPE_CLONE_NAME: return $object->getDetail('clone-name'); + case PhabricatorRepositoryTransaction::TYPE_SERVICE: + return $object->getAlmanacServicePHID(); } } @@ -127,6 +130,7 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_CREDENTIAL: case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: case PhabricatorRepositoryTransaction::TYPE_CLONE_NAME: + case PhabricatorRepositoryTransaction::TYPE_SERVICE: return $xaction->getNewValue(); case PhabricatorRepositoryTransaction::TYPE_NOTIFY: case PhabricatorRepositoryTransaction::TYPE_AUTOCLOSE: @@ -198,6 +202,9 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_CLONE_NAME: $object->setDetail('clone-name', $xaction->getNewValue()); return; + case PhabricatorRepositoryTransaction::TYPE_SERVICE: + $object->setAlmanacServicePHID($xaction->getNewValue()); + return; case PhabricatorRepositoryTransaction::TYPE_ENCODING: // Make sure the encoding is valid by converting to UTF-8. This tests // that the user has mbstring installed, and also that they didn't type @@ -306,6 +313,7 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_CREDENTIAL: case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: case PhabricatorRepositoryTransaction::TYPE_CLONE_NAME: + case PhabricatorRepositoryTransaction::TYPE_SERVICE: PhabricatorPolicyFilter::requireCapability( $this->requireActor(), $object, diff --git a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php index 34be869273..ff162a4ad0 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php +++ b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php @@ -24,6 +24,7 @@ final class PhabricatorRepositoryTransaction const TYPE_CREDENTIAL = 'repo:credential'; const TYPE_DANGEROUS = 'repo:dangerous'; const TYPE_CLONE_NAME = 'repo:clone-name'; + const TYPE_SERVICE = 'repo:service'; // TODO: Clean up these legacy transaction types. const TYPE_SSH_LOGIN = 'repo:ssh-login'; @@ -52,8 +53,13 @@ final class PhabricatorRepositoryTransaction switch ($this->getTransactionType()) { case self::TYPE_PUSH_POLICY: - $phids[] = $old; - $phids[] = $new; + case self::TYPE_SERVICE: + if ($old) { + $phids[] = $old; + } + if ($new) { + $phids[] = $new; + } break; } @@ -367,6 +373,26 @@ final class PhabricatorRepositoryTransaction $old, $new); } + case self::TYPE_SERVICE: + if (strlen($old) && !strlen($new)) { + return pht( + '%s moved storage for this repository from %s to local.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($old)); + } else if (!strlen($old) && strlen($new)) { + // TODO: Possibly, we should distinguish between automatic assignment + // on creation vs explicit adjustment. + return pht( + '%s set storage for this repository to %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($new)); + } else { + return pht( + '%s moved storage for this repository from %s to %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($old), + $this->renderHandleLink($new)); + } } return parent::getTitle();