diff --git a/app/availableplugins/SqlConnector/src/Model/Table/SqlProvisionersTable.php b/app/availableplugins/SqlConnector/src/Model/Table/SqlProvisionersTable.php index 985b374de..58d2c10b0 100644 --- a/app/availableplugins/SqlConnector/src/Model/Table/SqlProvisionersTable.php +++ b/app/availableplugins/SqlConnector/src/Model/Table/SqlProvisionersTable.php @@ -500,6 +500,11 @@ protected function syncEntity( // We have a currently provisioned record and the subject is not Deleted, // patch it with $data and try saving. $patchedEntity = $SpTable->patchEntity($curEntity, $data->toArray(), ['validate' => false]); + + // We always force an update to the modified timestamp in order to allow status() + // to report the last time provisioning ran. Cake has a touch() function as part + // of TimestampBehavior to do this, but our use of auto-tables means we can't use that. + $patchedEntity->modified = new \Cake\I18n\DateTime(); $SpTable->saveOrFail( $patchedEntity, diff --git a/app/config/routes.php b/app/config/routes.php index bd049807f..ca51b6952 100644 --- a/app/config/routes.php +++ b/app/config/routes.php @@ -62,11 +62,20 @@ $builder->applyMiddleware('bodyparser'); // Use setPass to make parameter show up as function parameter // Model specific actions, which will usually have more specific URLs: + // Note that while the UI uses dashes in URL paths, because we use {model} for the generic + // CRUD operations below we use underscores for consistency. Also, actions used here must + // be authorized in the relevant model permissions (even though they'll be used by ApiV2Controller + // and not the model's native controller). $builder->post( '/api_users/generate/{id}', ['controller' => 'ApiV2', 'action' => 'generateApiKey', 'model' => 'api_users']) ->setPass(['id']) ->setPatterns(['id' => '[0-9]+']); + $builder->post( + '/provisioning_targets/provision/{id}', + ['controller' => 'ApiV2', 'action' => 'provision', 'model' => 'provisioning_targets']) + ->setPass(['id']) + ->setPatterns(['id' => '[0-9]+']); // These establish the usual CRUD options on all models: $builder->delete( '/{model}/{id}', ['controller' => 'ApiV2', 'action' => 'delete']) diff --git a/app/config/schema/schema.json b/app/config/schema/schema.json index 152386c32..367d1c0e6 100644 --- a/app/config/schema/schema.json +++ b/app/config/schema/schema.json @@ -443,6 +443,7 @@ "status": {}, "provisioning_group_id": { "type": "integer", "foreignkey": { "table": "groups", "column": "id" } }, "retry_interval": { "type": "integer" }, + "max_retry": { "type": "integer" }, "ordr": {} }, "indexes": { @@ -758,7 +759,9 @@ "parameters": { "type": "text" }, "requeue_interval": { "type": "integer" }, "retry_interval": { "type": "integer" }, + "max_retry": { "type": "integer" }, "requeued_from_job_id": { "type": "integer", "foreignkey": { "table": "jobs", "column": "id" }}, + "retry_count": { "type": "integer" }, "status": {}, "assigned_host": { "type": "string", "size": 64 }, "assigned_pid": { "type": "integer" }, diff --git a/app/plugins/CoreJob/src/Lib/Jobs/ProvisionerJob.php b/app/plugins/CoreJob/src/Lib/Jobs/ProvisionerJob.php index d7ff2356d..a9a6393a0 100644 --- a/app/plugins/CoreJob/src/Lib/Jobs/ProvisionerJob.php +++ b/app/plugins/CoreJob/src/Lib/Jobs/ProvisionerJob.php @@ -33,6 +33,7 @@ use Cake\ORM\TableRegistry; use \App\Lib\Enum\JobStatusEnum; use \App\Lib\Enum\ProvisionerModeEnum; +use \App\Lib\Enum\ProvisioningContextEnum; class ProvisionerJob { /** @@ -97,7 +98,12 @@ protected function processEntity( int &$lastPct ): bool { try { - $EntityTable->requestProvisioning($entityId, $model, $target->id); + $EntityTable->requestProvisioning( + id: $entityId, + context: ProvisioningContextEnum::Queue, + provisioningTargetId: $target->id, + job: $job + ); $JobHistoryRecordsTable->record( jobId: $job->id, @@ -186,9 +192,17 @@ public function run( } if(!empty($parameters['entities'])) { - // We have one or more explicitly specified entities to process + // We have one or more explicitly specified entities to process. + // Entities might be an int (single request) or comma separated string + // (because PHP). + + $ids = []; - $ids = explode(',', $parameters['entities']); + if(is_int($parameters['entities'])) { + $ids[] = $parameters['entities']; + } else { + $ids = explode(',', $parameters['entities']); + } $count = count($ids); diff --git a/app/resources/locales/en_US/command.po b/app/resources/locales/en_US/command.po index 27b33c0ec..efea33afb 100644 --- a/app/resources/locales/en_US/command.po +++ b/app/resources/locales/en_US/command.po @@ -57,12 +57,24 @@ msgstr "Queue runner {0} requesting assignment {1} for CO {2}" msgid "job.run.child.running" msgstr "Queue runner {0} processing job ID {1}" +msgid "job.run.done.empty" +msgstr "No jobs left in the queue, queue runner done processing CO {0}" + +msgid "job.run.done.max" +msgstr "Queue runner reached maximum number of jobs ({0}), exiting" + msgid "job.run.max" msgstr "Reached max queue runner count ({0}), waiting for one to exit" msgid "job.run.piddone" msgstr "Queue runner PID {0} completed" +msgid "job.run.request" +msgstr "Queue runner requesting assignment {0} for CO {1}" + +msgid "job.run.running" +msgstr "Queue runner processing job ID {0}" + msgid "job.run.start" msgstr "Launching queue runner {0} (of {1}) for CO {2}" diff --git a/app/resources/locales/en_US/error.po b/app/resources/locales/en_US/error.po index 9c3511818..85e84512f 100644 --- a/app/resources/locales/en_US/error.po +++ b/app/resources/locales/en_US/error.po @@ -262,6 +262,9 @@ msgstr "Job Plugin not provided" msgid "Jobs.failed.abnormal" msgstr "The Job terminated unexpectedly" +msgid "Jobs.failed.abnormal.count" +msgstr "The Job terminated unexpectedly ({0} open transaction/s)" + msgid "Jobs.plugin.parameter.bool" msgstr "Boolean values may only be 0 or 1" diff --git a/app/resources/locales/en_US/field.po b/app/resources/locales/en_US/field.po index 175b4335d..9421bbfcd 100644 --- a/app/resources/locales/en_US/field.po +++ b/app/resources/locales/en_US/field.po @@ -675,6 +675,12 @@ msgstr "Finish Summary" msgid "Jobs.finish_time" msgstr "Finished" +msgid "Jobs.max_retry" +msgstr "Max Retries" + +msgid "Jobs.max_retry.desc" +msgstr "The maximum number of times this Job will be retried on failure" + msgid "Jobs.percent_complete" msgstr "Percent Complete" @@ -693,6 +699,12 @@ msgstr "After the job successfully completes, it will automatically be requeued msgid "Jobs.requeued_from_job_id" msgstr "Requeued From Job" +msgid "Jobs.retry_count" +msgstr "Retry Count" + +msgid "Jobs.retry_count.desc" +msgstr "The retry count for this Job instance" + msgid "Jobs.retry_interval" msgstr "Retry Interval" @@ -935,6 +947,12 @@ msgstr "Subject Object Type" msgid "ProvisioningHistoryRecords.subjectid" msgstr "Subject Object ID" +msgid "ProvisioningTargets.max_retry" +msgstr "Max Retries" + +msgid "ProvisioningTargets.max_retry.desc" +msgstr "For Queue and Queue On Error modes, the maximum number of times to retry on failure (set to 0 to retry indefinitely)" + msgid "ProvisioningTargets.ordr.desc" msgstr "The order in which this provisioner will be run when provisioning occurs (leave blank to run after all current provisioners)" diff --git a/app/resources/locales/en_US/result.po b/app/resources/locales/en_US/result.po index cd4e14b4f..e53795f90 100644 --- a/app/resources/locales/en_US/result.po +++ b/app/resources/locales/en_US/result.po @@ -250,6 +250,12 @@ msgstr "Sent Update Match Attributes Request" msgid "ProvisioningTargets.queued.ok" msgstr "Reprovisioning for {0} queued for {1} ({2})" +msgid "ProvisioningTargets.queued.error.ok" +msgstr "Provisioning queued for {0} ({1}): {2}" + +msgid "ProvisioningTargets.queued.queue.ok" +msgstr "Provisioning queued for {0} ({1})" + msgid "removed" msgstr "removed" diff --git a/app/src/Command/JobCommand.php b/app/src/Command/JobCommand.php index f5c693144..194c02fae 100644 --- a/app/src/Command/JobCommand.php +++ b/app/src/Command/JobCommand.php @@ -68,14 +68,14 @@ public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionPar 'short' => 'j', 'help' => __d('command', 'opt.job.plugin') ] - )->addOption( +/* )->addOption( 'parallel', [ 'required' => false, 'short' => 'p', 'default' => '1', 'help' => __d('command', 'opt.job.parallel') - ] + ]*/ )->addOption( 'max', [ @@ -143,20 +143,66 @@ public function execute(Arguments $args, ConsoleIo $io) // The total number of actively running children $pidcount = 0; - // The maximum number of runners to run at any one time - $max = (int)$args->getOption('max'); + // The maximum number of jobs a queue runner will process before exiting + $maxjobs = (int)$args->getOption('max'); // The number of parallel runners for each CO - $parallel = (int)$args->getOption('parallel'); + // $parallel = (int)$args->getOption('parallel'); - // The maximum number of jobs a queue runner will process before exiting - $maxjobs = 100; + // The number of jobs launched (across all COs we're processing) + $jobcount = 0; + + // Initialize the CoIdEventListener, which will be used to set the CO ID for + // (eg) tables with CO specific validation rules + $CoIdEventListener = new CoIdEventListener(); + EventManager::instance()->on($CoIdEventListener); foreach($coIds as $coId) { - // We probably need to do something like this (from synchronous running, below) - // $CoIdEventListener = new CoIdEventListener((int)$args->getOption('co_id')); - // EventManager::instance()->on($CoIdEventListener); - // but this wouldn't remove the previous $coId, so for now Sync Jobs can't be run - // via the queue. See CFM-400. + // Update the CoIdEventListener's CO ID. This works because EventManager + // is calling by reference, not copy. + $CoIdEventListener->updateCoId((int)$args->getOption('co_id')); + + while($jobcount <= $maxjobs) { + $jobcount++; + + $io->verbose(__d('command', 'job.run.request', [$jobcount, $coId])); + + // Request a job to run + $job = $JobTable->assignNext($coId); + + if(!$job) { + // Nothing to do, move on to the next CO + $io->verbose(__d('command', 'job.run.done.empty', [$coId])); + continue 2; + } + + $io->verbose(__d('command', 'job.run.running', [$job->id])); + + try { + $JobTable->process($job); + } + catch(\Exception $e) { + // The only Exception would be if the Job is in an invalid state, + // which shouldn't happen because we just ran assignNext() + $io->error($e->getMessage()); + } + + // Confirm the Job was properly finished. This was originally intended + // for use with fork()/wait(), but should work well enough here. + $JobTable->confirmFinished(getmypid()); + } + + if($jobcount > $maxjobs) { + $io->verbose(__d('command', 'job.run.done.max', [$maxjobs])); + break; + } + + /* When calling pcntl_fork(), the child process inherits the same + file descriptors as the parent, including (problematically) the + connections to the database server. There doesn't seem to be a + functional way to generate new file descriptors, and requiring + Job plugins to use an alternate database connection creates an + implausible burden (see how complicated CloneCommand is, for + comparison). So for now we'll just disable $parallel. // We start counting from 1 rather than 0 to simplify console output for($i = 1;$i <= $parallel;$i++) { @@ -179,12 +225,16 @@ public function execute(Arguments $args, ConsoleIo $io) // configuration and create a new configuration on the fly so we don't have // to pollute the database config file. - ConnectionManager::setConfig('plugin', ConnectionManager::getConfig('default')); -// XXX this doesn't seem to work, so plugins must always access the 'plugin' database, at least for now (CFM-253) -// ConnectionManager::alias('plugin', 'default'); - $cxn = ConnectionManager::get('plugin'); + $config = ConnectionManager::getConfig('default'); - $JobTable->setConnection($cxn); + // This doesn't work + // ConnectionManager::drop('default'); + // ConnectionManager::setConfig('default', $config); + + // This doesn't work either, and isn't plausible anyway + // ConnectionManager::alias('plugin', 'default'); + // $cxn = ConnectionManager::get('plugin'); + // $JobTable->setConnection($cxn); for($j = 1;$j <= $maxjobs;$j++) { $io->verbose(__d('command', 'job.run.child.request', [$i, $j, $coId])); @@ -201,7 +251,7 @@ public function execute(Arguments $args, ConsoleIo $io) $io->verbose(__d('command', 'job.run.child.running', [$newPid, $job->id])); try { - $JobTable->process($job); + $JobTable->process($job, 'plugin'); } catch(\Exception $e) { // The only Exception would be if the Job is in an invalid state, @@ -232,9 +282,10 @@ public function execute(Arguments $args, ConsoleIo $io) $io->verbose(__d('command', 'job.run.piddone', [$pid])); $pidcount--; } - } + }*/ } + /* // We are the parent, and we're done launching queue runners. wait() for them. while($pidcount > 0) { $io->out(__d('command', 'job.run.waiting', $pidcount)); @@ -248,7 +299,7 @@ public function execute(Arguments $args, ConsoleIo $io) $io->verbose(__d('command', 'job.run.piddone', [$pid])); $pidcount--; - } + }*/ } else { // We have a specific job to process. Note that JobCommand can't require -j // since the -r usage doesn't need it, so we have to check for it manually. @@ -292,6 +343,33 @@ public function execute(Arguments $args, ConsoleIo $io) $JobTable->process($job); } + + // Check that the plugin terminated correctly without leaving any open + // transactions. We don't need to do this when running the queue because + // confirmFinished() will check for stragglers. + + $cxn = ConnectionManager::get('default'); + + $txnCount = 0; + + while($cxn->inTransaction()) { + // We need to clear out any transactions in order to properly close the job + + $txnCount++; + $cxn->rollback(); + } + + // Note we'e overwriting $job + $job = $JobTable->get($job->id); + + if(!$job->isFinished()) { + // Terminate the job + $JobTable->finish( + job: $job, + summary: __d('error', 'Jobs.failed.abnormal.count', [$txnCount]), + result: JobStatusEnum::Failed + ); + } } } } \ No newline at end of file diff --git a/app/src/Controller/ApiV2Controller.php b/app/src/Controller/ApiV2Controller.php index 1dfe0a323..faf9fdd35 100644 --- a/app/src/Controller/ApiV2Controller.php +++ b/app/src/Controller/ApiV2Controller.php @@ -40,6 +40,13 @@ use \App\Lib\Enum\ProvisioningContextEnum; use \App\Lib\Enum\SuspendableStatusEnum; +// This controller is a bit of a special case in that it combines the functionality +// of StandardController (add, edit, view) with model specific functionality +// (generateApiKey, provision). Access to these specific functions is defined via +// routes.php, and enabled via permissions in the model's Table file. Given the +// relatively few model specific API extensions, this is probably OK, but if we end +// up with significantly more of these we might need to consider some refactoring. + class ApiV2Controller extends AppController { use \App\Lib\Traits\LabeledLogTrait; use \App\Lib\Traits\IndexQueryTrait; @@ -379,6 +386,49 @@ public function index() { $this->dispatchIndex(); } + /** + * Provision an entity. + * + * @since COmanage Registry v5.2.0 + * @param string $id Provisioning Target ID + */ + + public function provision(string $id) { + // we require a provisioning target ID in order to simplify primary key lookup. + // (To accept "all" or embed the ID into the JSON request would require custom + // logic to map the request to a CO... possible, but more complicated.) + + $json = $this->request->getData(); // Parsed by BodyParserMiddleware + + if(empty($json['provisioningRequest']['entityType']) + || empty($json['provisioningRequest']['entityId'])) { + throw new \InvalidArgumentException(__d('error', 'invalid.request')); + } + + // We need to find the table for the entity type being provisioned in order to + // call requestProvisioning() on that table. We'll indirectly validate the + // requested entity type by checking for that function. + + $entityType = $json['provisioningRequest']['entityType']; + $entityId = (int)$json['provisioningRequest']['entityId']; + + $Table = TableRegistry::getTableLocator()->get($entityType); + + if(!method_exists($Table, 'requestProvisioning')) { + throw new \InvalidArgumentException(__d('error', 'invalid.request')); + } + + $Table->requestProvisioning( + id: $entityId, + context: \App\Lib\Enum\ProvisioningContextEnum::Manual, + provisioningTargetId: (int)$id + ); + + // Let the view render + $this->viewBuilder()->setLayout('rest'); + $this->render('/Standard/api/v2/json/add-edit'); + } + /** * Generate a view for a set of Standard Objects. * diff --git a/app/src/Lib/Enum/ProvisioningContextEnum.php b/app/src/Lib/Enum/ProvisioningContextEnum.php index d16d9e26c..ee31c69d5 100644 --- a/app/src/Lib/Enum/ProvisioningContextEnum.php +++ b/app/src/Lib/Enum/ProvisioningContextEnum.php @@ -33,4 +33,5 @@ class ProvisioningContextEnum extends StandardEnum { const Automatic = 'A'; // Triggered as a side effect, eg by StandardController const Enrollment = 'E'; // Triggered during enrollment const Manual = 'M'; // Triggered by request of an admin + const Queue = 'Q'; // Triggered by ProvisionerJob processing the queue } \ No newline at end of file diff --git a/app/src/Lib/Traits/ProvisionableTrait.php b/app/src/Lib/Traits/ProvisionableTrait.php index 367dfbec6..7d32a6697 100644 --- a/app/src/Lib/Traits/ProvisionableTrait.php +++ b/app/src/Lib/Traits/ProvisionableTrait.php @@ -31,6 +31,7 @@ use Cake\ORM\TableRegistry; use \App\Lib\Util\StringUtilities; +use \App\Model\Entity\Job; trait ProvisionableTrait { // We use a trait and not a behavior so method_exists($table, "requestProvisioning"). @@ -46,13 +47,16 @@ trait ProvisionableTrait { * @param int $id This table's entity ID to provision * @param ProvisioningContextEnum $context Context in which provisioning is being requested * @param int $provisioningTargetId If set, the Provisioning Target ID to request provisioning for (otherwise all) + * @param Job $job If called from a Job, the current Job entity * @throws InvalidArgumentException */ public function requestProvisioning( int $id, string $context, - ?int $provisioningTargetId=null) { + ?int $provisioningTargetId=null, + ?Job $job=null, + ) { if(method_exists($this, 'marshalProvisioningData')) { // The model specific marshalProvisioningData implementations are expected // to properly handle deleted records. @@ -65,7 +69,8 @@ public function requestProvisioning( data: $data['data'], eligibility: $data['eligibility'], context: $context, - id: $provisioningTargetId + id: $provisioningTargetId, + job: $job ); } else { // This is a secondary model, eg Names. We need to figure out the primary model diff --git a/app/src/Model/Entity/Job.php b/app/src/Model/Entity/Job.php index 617b352ab..190714674 100644 --- a/app/src/Model/Entity/Job.php +++ b/app/src/Model/Entity/Job.php @@ -54,6 +54,19 @@ public function canCancel(): bool { JobStatusEnum::Queued]); } + /** + * Determine if this entity is finished. + * + * @since COmanage Registry v5.2.0 + * @return bool true if the entity is in a finished state, false otherwise + */ + + public function isFinished(): bool { + return in_array($this->status, [JobStatusEnum::Canceled, + JobStatusEnum::Complete, + JobStatusEnum::Failed]); + } + /** * Determine if this entity is Read Only. * diff --git a/app/src/Model/Table/ApiUsersTable.php b/app/src/Model/Table/ApiUsersTable.php index d9861d87c..1276cee3a 100644 --- a/app/src/Model/Table/ApiUsersTable.php +++ b/app/src/Model/Table/ApiUsersTable.php @@ -90,6 +90,8 @@ public function initialize(array $config): void { 'delete' => ['platformAdmin', 'coAdmin'], 'edit' => ['platformAdmin', 'coAdmin'], 'generate' => ['platformAdmin', 'coAdmin'], + // Used by ApiV2Controller + 'generateApiKey' => ['platformAdmin', 'coAdmin'], 'view' => ['platformAdmin', 'coAdmin'] ], // Actions that operate over a table (ie: do not require an $id) diff --git a/app/src/Model/Table/JobsTable.php b/app/src/Model/Table/JobsTable.php index 666a85fa7..e7b9cb998 100644 --- a/app/src/Model/Table/JobsTable.php +++ b/app/src/Model/Table/JobsTable.php @@ -286,6 +286,16 @@ public function confirmFinished(int $pid) { ->first(); if(!empty($job)) { + // Make sure we don't have any open transactions + + $cxn = ConnectionManager::get('default'); + + while($cxn->inTransaction()) { + // We need to clear out any transactions in order to properly close the job + + $cxn->rollback(); + } + // Terminate the job $this->finish( job: $job, @@ -335,38 +345,54 @@ public function finish(Job $job, string $summary="", string $result=JobStatusEnu // on failure if a retry_interval is specified. We need to do this after we // update the status of $id to avoid issues with concurrent jobs. + // We let any exceptions (including OverflowExceptions) bubble up so the + // error gets reported to whatever called the Job (typically JobCommand). + if($result == JobStatusEnum::Complete && !empty($job->requeue_interval) && $job->requeue_interval > 0) { // The new job will be substantially the same as the last one... - $this->register($job->co_id, - $job->plugin, - json_decode($job->parameters, true), - $job->register_summary, - false, - // we only support serialized jobs, not concurrent - false, - $job->requeue_interval, - $job->requeue_interval, - $job->retry_interval, - $job->id); + $this->register( + coId: $job->co_id, + plugin: $job->plugin, + parameters: json_decode($job->parameters, true), + registerSummary: $job->register_summary, + synchronous: false, + // we only support serialized jobs, not concurrent + concurrent: false, + delay: $job->requeue_interval, + requeueInterval: $job->requeue_interval, + retryInterval: $job->retry_interval, + maxRetry: $job->max_retry, + requeuedFrom: $job->id, + // We reset retry_count if max_retry was set, otherwise we leave it null + retryCount: !empty($job->max_retry) ? 0 : null + ); } elseif($result == JobStatusEnum::Failed && !empty($job->retry_interval) && $job->retry_interval > 0) { // The new job will be substantially the same as the last one... - $this->register(job->co_id, - $job->plugin, - json_decode($job->parameters, true), - $job->register_summary, - false, - // we only support serialized jobs, not concurrent - false, - $job->retry_interval, - $job->requeue_interval, - $job->retry_interval, - $job->id); + $this->register( + coId: job->co_id, + plugin: $job->plugin, + parameters: json_decode($job->parameters, true), + registerSummary: $job->register_summary, + synchronous: false, + // we only support serialized jobs, not concurrent + concurrent: false, + // Note the delay parameter is different from when the Job completed + delay: $job->retry_interval, + requeueInterval: $job->requeue_interval, + retryInterval: $job->retry_interval, + maxRetry: $job->max_retry, + requeuedFrom: $job->id, + // If retry_count was not null, increment it. We don't check to see + // if it exceeds max_retry here because register() will do that and + // throw an OverflowException on error. + retryCount: $job->retry_count + 1 + ); } } @@ -391,11 +417,12 @@ public function isCanceled(int $id): bool { * Process a Job. Jobs must be in Ready status (ie: assigned) in order to be processed. * * @since COmanage Registry v5.0.0 - * @param Job $job Job to process + * @param Job $job Job to process + * @param string $dbcxn Database connection name to use * @throws InvalidArgumentException */ - public function process(Job $job) { + public function process(Job $job, string $dbcxn='default') { // The Job must be Assigned to be processed if($job->status != JobStatusEnum::Assigned) { throw new \InvalidArgumentException( @@ -416,20 +443,24 @@ public function process(Job $job) { $JobHistoryRecords = TableRegistry::getTableLocator()->get('JobHistoryRecords'); - // Maybe set the connection on the JobHistoryTable (if we were run via - // the queue runner). - try { - $cxn = ConnectionManager::get('plugin'); + // Maybe set the connection on the JobHistoryTable + + if($dbcxn != 'default') { + try { + $cxn = ConnectionManager::get($dbcxn); - if(!empty($cxn)) { - $JobHistoryRecords->setConnection($cxn); + if(!empty($cxn)) { + $JobHistoryRecords->setConnection($cxn); + } + } + catch(\Cake\Datasource\Exception\MissingDatasourceConfigException $e) { + // plugin datasource not defined (we previously used this to determine + // if we were in the queue runner, now it's just an error) + $this->finish($job, $e->getMessage(), JobStatusEnum::Failed); + } + catch(\Exception $e) { + $this->finish($job, $e->getMessage(), JobStatusEnum::Failed); } - } - catch(\Cake\Datasource\Exception\MissingDatasourceConfigException $e) { - // plugin datasource not defined, so we're not in the queue runner - } - catch(\Exception $e) { - $this->finish($job, $e->getMessage(), JobStatusEnum::Failed); } // We need a separate try block here because we want to specially handle @@ -460,9 +491,12 @@ public function process(Job $job) { * @param int $delay Minimum number of seconds to delay the start of this Job * @param int $requeueInterval If non-zero, number of seconds after successful completion to requeue the same Job * @param int $retryInterval If non-zero, number of seconds after failed completion to requeue the same Job + * @param int $maxRetry If non-zero, the maximum number of times this Job may be retried * @param int $requeuedFrom If requeued, the ID of the Job that created this Job + * @param int $retryCount If non-zero, the current retry count (ie: 0 the first time a Job is queued, 1 the first time it is retried) * @return Job Job entity * @throws InvalidArgumentException + * @throws OverflowException */ public function register( @@ -475,8 +509,14 @@ public function register( int $delay=0, ?int $requeueInterval=null, ?int $retryInterval=null, - ?int $requeuedFrom=null + ?int $maxRetry=null, + ?int $requeuedFrom=null, + ?int $retryCount=null ): Job { + if($maxRetry > 0 && $retryCount > $maxRetry) { + throw new \OverflowException("Maximum retry count reached (" . $maxRetry . ")"); // XXX I18n + } + // Start a transaction. In addition to ruleAlreadyRegistered needing a read lock, // if we're synchronous we need to make sure the current caller gets assigned the Job. @@ -512,7 +552,9 @@ public function register( 'status' => JobStatusEnum::Queued, 'requeue_interval' => $requeueInterval, 'retry_interval' => $retryInterval, + 'max_retry' => $maxRetry, 'requeued_from_job_id' => $requeuedFrom, + 'retry_count' => $retryCount, 'start_after_time' => date('Y-m-d H:i:s', time()+$delay) // We don't set percent_complete here since not all jobs might use that field, // and then a null vs 0 can be used to distinguish. @@ -521,7 +563,14 @@ public function register( // If $concurrent is true, we want to disable AR-Job-1. Right now, since this // is the only application rule we can simply disable rule checking, but if // another rule is added this won't work. - $this->saveOrFail($entity, ['checkRules' => !$concurrent]); + try { + $this->saveOrFail($entity, ['checkRules' => !$concurrent]); + } + catch(\Exception $e) { + $cxn->rollback(); + + throw $e; + } if($synchronous) { // Assign the job within the transaction to make sure it doesn't get @@ -671,7 +720,7 @@ protected function validateJobParameters(string $plugin, int $coId, array $param break; case 'int': case 'integer': - if(!preg_match('/^[0-9.+-]*$/', $val)) { + if(!is_int($val) && !preg_match('/^[0-9.+-]*$/', $val)) { $ret[$p] = __d('error', 'Jobs.plugin.parameter.int'); } break; @@ -741,11 +790,21 @@ public function validationDefault(Validator $validator): Validator { ]); $validator->allowEmptyString('retry_interval'); + $validator->add('max_retry', [ + 'content' => ['rule' => 'isInteger'] + ]); + $validator->allowEmptyString('max_retry'); + $validator->add('requeued_from_job_id', [ 'content' => ['rule' => 'isInteger'] ]); $validator->allowEmptyString('requeued_from_job_id'); + $validator->add('retry_count', [ + 'content' => ['rule' => 'isInteger'] + ]); + $validator->allowEmptyString('retry_count'); + $validator->add('status', [ 'content' => ['rule' => ['inList', JobStatusEnum::getConstValues()]] ]); diff --git a/app/src/Model/Table/ProvisioningTargetsTable.php b/app/src/Model/Table/ProvisioningTargetsTable.php index 0a22054f2..d44955927 100644 --- a/app/src/Model/Table/ProvisioningTargetsTable.php +++ b/app/src/Model/Table/ProvisioningTargetsTable.php @@ -40,6 +40,7 @@ use App\Lib\Enum\ProvisioningStatusEnum; use App\Lib\Enum\SuspendableStatusEnum; use App\Lib\Util\StringUtilities; +use App\Model\Entity\Job; class ProvisioningTargetsTable extends Table { use \App\Lib\Traits\AutoViewVarsTrait; @@ -89,7 +90,7 @@ public function initialize(array $config): void { $this->setPrimaryLink(['co_id', 'group_id', 'person_id']); $this->setRequiresCO(true); - $this->setAllowLookupPrimaryLink(['reprovision']); + $this->setAllowLookupPrimaryLink(['provision', 'reprovision']); $this->setAllowUnkeyedPrimaryLink(['status']); $this->setAutoViewVars([ @@ -113,6 +114,8 @@ public function initialize(array $config): void { 'configure' => ['platformAdmin', 'coAdmin'], 'delete' => ['platformAdmin', 'coAdmin'], 'edit' => ['platformAdmin', 'coAdmin'], + // Used by ApiV2Controller + 'provision' => ['platformAdmin', 'coAdmin'], 'reprovision' => ['platformAdmin', 'coAdmin'], 'view' => ['platformAdmin', 'coAdmin'] ], @@ -150,13 +153,15 @@ public function buildRules(RulesChecker $rules): RulesChecker { * @param ProvisioningEligibilityEnum $eligibility Provisioning eligibility * @param ProvisioningContextEnum $context Provisioning context * @param int $id Provisioning Target ID, or null to provision all targets + * @param Job $job If called from a Job, the current Job entity */ public function provision( - mixed $data, + mixed $data, string $eligibility, string $context, - ?int $id=null + ?int $id=null, + ?Job $job=null ) { // Convert the primary data object to the primary provisioned object name // (eg: People or Cous) @@ -165,7 +170,6 @@ public function provision( $query = $this->find() ->where([ 'ProvisioningTargets.co_id' => $data->co_id, -// XXX how do we know which mode's worth of provisioners we want? 'ProvisioningTargets.status <>' => ProvisionerModeEnum::Disabled ]); @@ -178,12 +182,12 @@ public function provision( ->all(); foreach($targets as $t) { - // Compare our $context against the target's $status. There are three possible + // Compare our $context against the target's $status. There are four possible // contexts, with their corresponding provisionable statuses: // Automatic: Immediate, Queue, QuueOnError // Enrollment: Enrollment, Immediate, Queue, QueueOnError // Manual: Enrollment, Immediate, Manual, Queue, QueueOnError -// XXX do we need ARs or PARs for this? add appropriate logging along with ARs + // Queue: Enrollment, Immediate, Manual, Queue, QueueOnError switch($context) { case ProvisioningContextEnum::Automatic: @@ -193,6 +197,7 @@ public function provision( ProvisionerModeEnum::QueueOnError ])) { $this->llog('trace', "Skipping Provisioning Target " . $t->id . " with mode " . $t->status . " (automatic context)", $t->id); + // Note "continue 2" is correct here, since continue acts like a break within a switch continue 2; } break; @@ -205,6 +210,9 @@ public function provision( case ProvisioningContextEnum::Manual: // Manual provisioning is permitted regardless of target status break; + case ProvisioningContextEnum::Queue: + // Queue provisioning is permitted regardless of target status + break; } $pluginModel = StringUtilities::pluginModel($t->plugin); @@ -217,68 +225,168 @@ public function provision( continue; } - try { - $this->llog('trace', "Provisioning $provisionedModel for $pluginModel (context: $context)", $t->id); + $this->llog('trace', "Provisioning $provisionedModel for $pluginModel (context: $context)", $t->id); + + $requeue = false; - $result = $this->$pluginModel->provision($t, $provisionedModel, $data, $eligibility); + // We immediately run the requested Job, unless the Provisioner is in Queue mode + // _and_ the Provisioning Context is _not_ Queue (which would indicate we are processing + // the queue, so we shouldn't immedately requeue the job) + if($t->status != ProvisionerModeEnum::Queue + || $context == ProvisioningContextEnum::Queue) { + try { + $result = $this->$pluginModel->provision($t, $provisionedModel, $data, $eligibility); - $this->alog('trace', $result); + $this->alog('trace', $result); - $this->ProvisioningHistoryRecords->record( - provisioningTargetId: $t->id, - comment: $result['comment'], - status: $result['status'], - subjectModel: $provisionedModel, - subjectId: $data->id - ); + // The plugin can report failure by throwing an Exception, which we catch below. + // Otherwise, the plugin can return Provisioned (success), NotProvisioned (also + // success, eg the result of a delete operation), or Unknown (error). The only + // situation we do not requeue is \InvalidArgumentException. - if(!empty($result['identifier']) && in_array($provisionedModel, ['People', 'Groups'])) { - $this->llog('trace', "Obtained Provisioning Key " . $result['identifier'] . " for $pluginModel", $t->id); - - // Upsert the identifier - $Identifiers = TableRegistry::getTableLocator()->get('Identifiers'); + if($result['status'] == ProvisioningStatusEnum::Unknown) { + $requeue = $result['comment']; + } - $typeId = $Identifiers->Types->getTypeId( - coId: $data->co_id, - attribute: 'Identifiers.type', - // Although we now call these "Provisioning Keys", we reuse the database value from v4 - value: 'provisioningtarget' + $this->ProvisioningHistoryRecords->record( + provisioningTargetId: $t->id, + comment: $result['comment'], + status: $result['status'], + subjectModel: $provisionedModel, + subjectId: $data->id ); - $pkey = [ - 'type_id' => $typeId, - 'identifier' => $result['identifier'], - 'status' => SuspendableStatusEnum::Active, - 'provisioning_target_id' => $t->id, - 'login' => false, - 'frozen' => false - ]; - - $whereClause = [ - 'type_id' => $typeId - ]; - - if($provisionedModel == 'Group') { - $pkey['group_id'] = $data->id; - $whereClause['group_id'] = $data->id; - } else { - $pkey['person_id'] = $data->id; - $whereClause['person_id'] = $data->id; + if(!empty($result['identifier']) && in_array($provisionedModel, ['People', 'Groups'])) { + // We check for Provisioning Keys when provisioning People or Groups. + // Other models (Services, etc) could support Provisioning Keys, + // just currently they don't. + $this->llog('trace', "Obtained Provisioning Key " . $result['identifier'] . " for $pluginModel", $t->id); + + // Upsert the identifier + $Identifiers = TableRegistry::getTableLocator()->get('Identifiers'); + + $typeId = $Identifiers->Types->getTypeId( + coId: $data->co_id, + attribute: 'Identifiers.type', + // Although we now call these "Provisioning Keys", we reuse the database value from v4 + value: 'provisioningtarget' + ); + + $pkey = [ + 'type_id' => $typeId, + 'identifier' => $result['identifier'], + 'status' => SuspendableStatusEnum::Active, + 'provisioning_target_id' => $t->id, + 'login' => false, + 'frozen' => false + ]; + + $whereClause = [ + 'type_id' => $typeId + ]; + + if($provisionedModel == 'Group') { + $pkey['group_id'] = $data->id; + $whereClause['group_id'] = $data->id; + } else { + $pkey['person_id'] = $data->id; + $whereClause['person_id'] = $data->id; + } + + if(!$Identifiers->upsert($pkey, $whereClause)) { + // We successfully provisioned, but for some reason we failed to store + // the Provisioning Key. We shouldn't throw an Exception because that + // will mask the fact that the target is provisioned, so we'll just log + // an error. + + $this->llog('error', "Provisioning successfully completed, but failed to store Provisioning Key " . $result['identifier']); + } } + } + catch(\InvalidArgumentException $e) { + // The plugin has determined its configuration is invalid, so we do not requeue. + + $this->llog('error', "Provisioning failure due to invalid configuration: " . $e->getMessage()); + + $this->ProvisioningHistoryRecords->record( + provisioningTargetId: $t->id, + comment: $e->getMessage(), + status: ProvisioningStatusEnum::Unknown, + subjectModel: $provisionedModel, + subjectId: $data->id + ); + } + catch(\Exception $e) { + $requeue = $e->getMessage(); - $Identifiers->upsertOrFail($pkey, $whereClause); + $this->llog('error', "Provisioning failure: " . $e->getMessage()); + + $this->ProvisioningHistoryRecords->record( + provisioningTargetId: $t->id, + comment: $e->getMessage(), + status: ProvisioningStatusEnum::Unknown, + subjectModel: $provisionedModel, + subjectId: $data->id + ); } } - catch(\Exception $e) { - $this->llog('error', "Provisioning failure: " . $e->getMessage()); - - $this->ProvisioningHistoryRecords->record( - provisioningTargetId: $t->id, - comment: $e->getMessage(), - status: ProvisioningStatusEnum::NotProvisioned, - subjectModel: $provisionedModel, - subjectId: $data->id - ); + + if(($t->status == ProvisionerModeEnum::QueueOnError && $requeue) + || ($t->status == ProvisionerModeEnum::Queue && $context != ProvisioningContextEnum::Queue)) { + // We either failed or are in Queue mode, so queue the job for later processing. + // The max retry limit is implemented by register(), we'll just catch the + // Exception and log it if register() fails, + + if($t->max_retry > 0) { + try { + $Jobs = TableRegistry::getTableLocator()->get("Jobs"); + + $rqjob = $Jobs->register( + coId: $t->co_id, + plugin: 'CoreJob.ProvisionerJob', + parameters: [ + 'model' => $provisionedModel, + 'provisioning_target_id' => $t->id, + // entities is a comma separated string of $provisionedModel subject IDs + 'entities' => $data->id + ], + registerSummary: + $requeue + ? __d('result', 'ProvisioningTargets.queued.error.ok', [$t->description, $t->id, $requeue]) + : __d('result', 'ProvisioningTargets.queued.queue.ok', [$t->description, $t->id]), + synchronous: false, + // When requeueing we need to allow concurrent jobs because the + // job we're replacing (as defined by having the same CO, plugin, + // and parameters) hasn't technically finished yet. + concurrent: true, + delay: $t->retry_interval, + // Provisioning Jobs should not automatically requeue on success + requeueInterval: null, + retryInterval: $t->retry_interval, + maxRetry: $t->max_retry, + requeuedFrom: $job ? $job->id : null, + retryCount: !empty($job->retry_count) ? $job->retry_count + 1 : 1 + ); + + $this->llog('trace', "Requeued provisioning request as Job " . $rqjob->id); + } + catch(\Exception $e) { + // This will be an OverflowException is max_retry was reached + + $this->llog('error', "Could not requeue provisioning request: " . $e->getMessage()); + } + } else { + $this->llog('trace', "Not requeueing provisioning request because max_retry is not set"); + } + + if($requeue) { + // If we got an error message from the original provisioning request + // (regardless of whether or not we then queued the job for processing) + // we want to throw that error back up the stack so ProvisionerJob can + // record it correctly. + + throw new \RuntimeException($requeue); + } } } } @@ -434,6 +542,11 @@ public function validationDefault(Validator $validator): Validator { ]); $validator->allowEmptyString('retry_interval'); + $validator->add('max_retry', [ + 'content' => ['rule' => 'isInteger'] + ]); + $validator->allowEmptyString('max_retry'); + $validator->add('ordr', [ 'content' => ['rule' => 'isInteger'] ]); diff --git a/app/templates/Jobs/fields.inc b/app/templates/Jobs/fields.inc index 375a50798..0752c6c76 100644 --- a/app/templates/Jobs/fields.inc +++ b/app/templates/Jobs/fields.inc @@ -96,7 +96,9 @@ if($vv_action == 'add') { ], 'finish_summary', 'requeue_interval', - 'retry_interval' + 'retry_interval', + 'max_retry', + 'retry_count' ]); if(!empty($vv_obj->requeued_from_job->id)) { @@ -111,6 +113,12 @@ if($vv_action == 'add') { 'status' => (string)$vv_obj->requeued_from_job->id, 'link' => $link, ]; + } else { + // Make sure the field always renders, even if empty + $fields['requeued_from_job_id'] = [ + 'type' => 'text', + 'value' => __d('result', 'set.not') + ]; } } diff --git a/app/templates/ProvisioningTargets/fields.inc b/app/templates/ProvisioningTargets/fields.inc index 827d866ba..e33cd9de2 100644 --- a/app/templates/ProvisioningTargets/fields.inc +++ b/app/templates/ProvisioningTargets/fields.inc @@ -33,6 +33,7 @@ $fields = [ 'retry_interval' => [ 'default' => 900 ], + 'max_retry', 'plugin', // todo: Not yet implemented (CFM-26) // 'provisioning_group_id', @@ -56,7 +57,6 @@ $subnav = [ if(status == "" || status == "") { - alert("Queue modes not yet implemented"); // XXX CFM-26 showFields(['retry-interval'], isPageLoad); } else { hideFields(['retry-interval'], isPageLoad);