cli: implement deployment grace period and...

...refactor/restructure related code to achieve the same behavior on CLI and
via automated job

fixes #2499

application/clicommands/ConfigCommand.php

@@ -5,10 +5,11 @@ namespace Icinga\Module\Director\Clicommands;
 use Icinga\Application\Benchmark;
 use Icinga\Module\Director\Cli\Command;
 use Icinga\Module\Director\Core\Json;
+use Icinga\Module\Director\Deployment\ConditionalDeployment;
+use Icinga\Module\Director\Deployment\DeploymentGracePeriod;
 use Icinga\Module\Director\Deployment\DeploymentStatus;
 use Icinga\Module\Director\IcingaConfig\IcingaConfig;
 use Icinga\Module\Director\Import\SyncUtils;
-use Icinga\Module\Director\Objects\DirectorDeploymentLog;
 
 /**
  * Generate, show and deploy Icinga 2 configuration
@@ -90,17 +91,20 @@ class ConfigCommand extends Command
      * USAGE
      *
      * icingacli director config deploy [--checksum <checksum>] [--force] [--wait <seconds>]
+     *  [--grace-period <seconds>]
      *
      * OPTIONS
      *
      *   --checksum <checksum>  Optionally deploy a specific configuration
-     *   --force                Force a deployment, even when the configuration hasn't
+     *   --force                Force a deployment, even when the configuration
-     *                          changed
+     *                          hasn't changed
-     *   --wait <seconds>       Optionally wait until Icinga completed it's restart
+     *   --wait <seconds>       Optionally wait until Icinga completed it's
+     *                          restart
+     *   --grace-period <seconds>  Do not deploy if a deployment took place
+     *                          less than <seconds> ago
      */
     public function deployAction()
     {
-        $api = $this->api();
         $db = $this->db();
 
         $checksum = $this->params->get('checksum');
@@ -111,32 +115,31 @@ class ConfigCommand extends Command
             $checksum = $config->getHexChecksum();
         }
 
-        $api->wipeInactiveStages($db);
+        $deployer = new ConditionalDeployment($db, $this->api());
-        $current = $api->getActiveChecksum($db);
+        $deployer->force((bool) $this->params->get('force'));
-        if ($current === $checksum) {
+        if ($graceTime = $this->params->get('grace-period')) {
+            $deployer->setGracePeriod(new DeploymentGracePeriod((int) $graceTime, $db));
             if ($this->params->get('force')) {
-                echo "Config matches active stage, deploying anyway\n";
+                fwrite(STDERR, "WARNING: force overrides Grace period\n");
+            }
+        }
+        $deployer->refresh();
+
+        if ($deployment = $deployer->deploy($config)) {
+            if ($deployer->hasBeenForced()) {
+                echo $deployer->getNoDeploymentReason() . ", deploying anyway\n";
+            }
+            printf("Config '%s' has been deployed\n", $checksum);
         } else {
-                echo "Config matches active stage, nothing to do\n";
+            echo $deployer->getNoDeploymentReason() . "\n";
-
-                return;
-            }
         }
 
-        $deploymentLog = $api->dumpConfig($config, $db);
+        if ($timeout = $this->getWaitTime()) {
-        if (! $deploymentLog) {
+            $deployed = $deployer->waitForStartupAfterDeploy($deployment, $timeout);
-            $this->fail("Failed to deploy config '%s'", $checksum);
-        }
-        if ($timeout = $this->params->get('wait')) {
-            if (! ctype_digit($timeout)) {
-                $this->fail("--wait must be the number of seconds to wait'");
-            }
-            $deployed = $this->waitForStartupAfterDeploy($deploymentLog, $timeout);
             if ($deployed !== true) {
                 $this->fail("Failed to deploy config '%s': %s\n", $checksum, $deployed);
             }
         }
-        printf("Config '%s' has been deployed\n", $checksum);
     }
 
     /**
@@ -159,24 +162,16 @@ class ConfigCommand extends Command
         }
     }
 
-    private function waitForStartupAfterDeploy($deploymentLog, $timeout)
+    protected function getWaitTime()
     {
-        $startTime = time();
+        if ($timeout = $this->params->get('wait')) {
-        while ((time() - $startTime) <= $timeout) {
+            if (!ctype_digit($timeout)) {
-            $deploymentFromDB = DirectorDeploymentLog::load($deploymentLog->getId(), $this->db());
+                $this->fail("--wait must be the number of seconds to wait'");
-            $stageCollected = $deploymentFromDB->get('stage_collected');
-            if ($stageCollected === null) {
-                usleep(500000);
-                continue;
             }
-            if ($stageCollected === 'n') {
+
-                return 'stage has not been collected';
+            return (int) $timeout;
         }
-            if ($deploymentFromDB->get('startup_succeeded') === 'y') {
+
-                return true;
+        return null;
-            }
-            return 'deployment failed during startup';
-        }
-        return 'deployment timed out';
     }
 }

doc/60-CLI.md

@@ -456,7 +456,22 @@ Config with checksum b330febd0820493fb12921ad8f5ea42102a5c871 already exists
 
 ### Config deployment
 
-You do not need to explicitely render your config before deploying it to your
+#### Usage
+
+`icingacli director config deploy [options]`
+
+#### Options
+
+| Option                     | Description                                                      |
+|----------------------------|------------------------------------------------------------------|
+| `checksum <checksum>`      | Optionally deploy a specific configuration                       |
+| `--force`                  | Force a deployment, even when the configuration hasn't changed   |
+| `--wait <seconds>`         | Optionally wait until Icinga completed it's restart              |
+| `--grace-period <seconds>` | Do not deploy if a deployment took place less than <seconds> ago |
+
+#### Examples
+
+You do not need to explicitly render your config before deploying it to your
 Icinga 2 master node. Just trigger a deployment, it will re-render the current
 config:
 
@@ -490,6 +505,13 @@ version the `deploy` command allows you to provide a specific checksum:
 icingacli director config deploy --checksum b330febd0820493fb12921ad8f5ea42102a5c871
 ```
 
+When using `icingacli` deployments in an automated way, and want to avoid fast
+consecutive deployments, you can provide a grace period:
+
+```shell
+icingacli director config deploy --grace-period 300
+```
+
 ### Deployments status
 In case you want to fetch the information about the deployments status, 
 you can call the following CLI command:

library/Director/Cli/Command.php

@@ -2,6 +2,7 @@
 
 namespace Icinga\Module\Director\Cli;
 
+use gipfl\Json\JsonString;
 use Icinga\Cli\Command as CliCommand;
 use Icinga\Module\Director\Application\MemoryLimit;
 use Icinga\Module\Director\Core\CoreApi;
@@ -21,7 +22,7 @@ class Command extends CliCommand
 
     protected function renderJson($object, $pretty = true)
     {
-        return json_encode($object, $pretty ? JSON_PRETTY_PRINT : null) . "\n";
+        return JsonString::encode($object, $pretty ? JSON_PRETTY_PRINT : null) . "\n";
     }
 
     /**

library/Director/Deployment/ConditionalConfigRenderer.php

@@ -0,0 +1,64 @@
+<?php
+
+namespace Icinga\Module\Director\Deployment;
+
+use Icinga\Exception\NotFoundError;
+use Icinga\Module\Director\Db;
+use Icinga\Module\Director\IcingaConfig\IcingaConfig;
+use Icinga\Module\Director\Objects\DirectorActivityLog;
+
+class ConditionalConfigRenderer
+{
+    /** @var Db */
+    protected $db;
+
+    protected $forceRendering = false;
+
+    public function __construct(Db $connection)
+    {
+        $this->db = $connection;
+    }
+
+    public function forceRendering($force = true)
+    {
+        $this->forceRendering = $force;
+
+        return $this;
+    }
+
+    public function getConfig()
+    {
+        if ($this->shouldGenerate()) {
+            return IcingaConfig::generate($this->db);
+        }
+
+        return $this->loadLatestActivityConfig();
+    }
+
+    protected function loadLatestActivityConfig()
+    {
+        $db = $this->db;
+
+        return IcingaConfig::loadByActivityChecksum($db->getLastActivityChecksum(), $db);
+    }
+
+    protected function shouldGenerate()
+    {
+        return $this->forceRendering || !$this->configForLatestActivityExists();
+    }
+
+    protected function configForLatestActivityExists()
+    {
+        $db = $this->db;
+        try {
+            $latestActivity = DirectorActivityLog::loadLatest($db);
+        } catch (NotFoundError $e) {
+            return false;
+        }
+
+        return IcingaConfig::existsForActivityChecksum(
+            bin2hex($latestActivity->get('checksum')),
+            $db
+        );
+    }
+}

library/Director/Deployment/ConditionalDeployment.php

@@ -0,0 +1,190 @@
+<?php
+
+namespace Icinga\Module\Director\Deployment;
+
+use Icinga\Exception\IcingaException;
+use Icinga\Module\Director\Core\CoreApi;
+use Icinga\Module\Director\Db;
+use Icinga\Module\Director\IcingaConfig\IcingaConfig;
+use Icinga\Module\Director\Objects\DirectorDeploymentLog;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
+
+class ConditionalDeployment implements LoggerAwareInterface
+{
+    use LoggerAwareTrait;
+
+    /** @var Db */
+    protected $db;
+
+    /** @var CoreApi */
+    protected $api;
+
+    /** @var ?DeploymentGracePeriod */
+    protected $gracePeriod = null;
+
+    protected $force = false;
+
+    protected $hasBeenForced = false;
+
+    /** @var ?string */
+    protected $noDeploymentReason = null;
+
+    public function __construct(Db $connection, CoreApi $api = null)
+    {
+        $this->setLogger(new NullLogger());
+        $this->db = $connection;
+        if ($api === null) {
+            $this->api = $connection->getDeploymentEndpoint()->api();
+        } else {
+            $this->api = $api;
+        }
+        $this->refresh();
+    }
+
+    /**
+     * @param IcingaConfig $config
+     * @return ?DirectorDeploymentLog
+     */
+    public function deploy(IcingaConfig $config)
+    {
+        $this->hasBeenForced = false;
+        if ($this->shouldDeploy($config)) {
+            return $this->reallyDeploy($config);
+        } elseif ($this->force) {
+            $deployment = $this->reallyDeploy($config);
+            $this->hasBeenForced = true;
+
+            return $deployment;
+        }
+
+        return null;
+    }
+
+    /**
+     * @param bool $force
+     * @return $this
+     */
+    public function force($force = true)
+    {
+        $this->force = $force;
+        return $this;
+    }
+
+    public function setGracePeriod(DeploymentGracePeriod $gracePeriod)
+    {
+        $this->gracePeriod = $gracePeriod;
+        return $this;
+    }
+
+    public function refresh()
+    {
+        $this->api->collectLogFiles($this->db);
+        $this->api->wipeInactiveStages($this->db);
+    }
+
+    public function waitForStartupAfterDeploy(DirectorDeploymentLog $deploymentLog, $timeout)
+    {
+        $startTime = time();
+        while ((time() - $startTime) <= $timeout) {
+            $deploymentFromDB = DirectorDeploymentLog::load($deploymentLog->getId(), $this->db);
+            $stageCollected = $deploymentFromDB->get('stage_collected');
+            if ($stageCollected === null) {
+                usleep(500000);
+                continue;
+            }
+            if ($stageCollected === 'n') {
+                return 'stage has not been collected';
+            }
+            if ($deploymentFromDB->get('startup_succeeded') === 'y') {
+                return true;
+            }
+            return 'deployment failed during startup';
+        }
+        return 'deployment timed out';
+    }
+
+    /**
+     * @return string|null
+     */
+    public function getNoDeploymentReason()
+    {
+        return $this->noDeploymentReason;
+    }
+
+    public function hasBeenForced()
+    {
+        return $this->hasBeenForced;
+    }
+
+    protected function shouldDeploy(IcingaConfig $config)
+    {
+        $this->noDeploymentReason = null;
+        if ($this->hasNeverDeployed()) {
+            return true;
+        }
+
+        if ($this->isWithinGracePeriod()) {
+            $this->noDeploymentReason = 'Grace period is active';
+            return false;
+        }
+
+        if ($this->deployedConfigMatches($config)) {
+            $this->noDeploymentReason = 'Config matches last deployed one';
+            return false;
+        }
+
+        if ($this->getActiveChecksum() === $config->getHexChecksum()) {
+            $this->noDeploymentReason = 'Config matches active stage';
+            return false;
+        }
+
+        return true;
+    }
+
+    protected function hasNeverDeployed()
+    {
+        return !DirectorDeploymentLog::hasDeployments($this->db);
+    }
+
+    protected function isWithinGracePeriod()
+    {
+        return $this->gracePeriod && $this->gracePeriod->isActive();
+    }
+
+    protected function deployedConfigMatches(IcingaConfig $config)
+    {
+        if ($deployment = DirectorDeploymentLog::optionalLatest($this->db)) {
+            return $deployment->getConfigHexChecksum() === $config->getHexChecksum();
+        }
+
+        return false;
+    }
+
+    protected function getActiveChecksum()
+    {
+        return DirectorDeploymentLog::getConfigChecksumForStageName(
+            $this->db,
+            $this->api->getActiveStageName()
+        );
+    }
+
+    /**
+     * @param IcingaConfig $config
+     * @return bool|DirectorDeploymentLog
+     * @throws IcingaException
+     * @throws \Icinga\Module\Director\Exception\DuplicateKeyException
+     */
+    protected function reallyDeploy(IcingaConfig $config)
+    {
+        $checksum = $config->getHexChecksum();
+        $this->logger->info(sprintf('Director ConfigJob ready to deploy "%s"', $checksum));
+        if ($deployment = $this->api->dumpConfig($config, $this->db)) {
+            $this->logger->notice(sprintf('Director ConfigJob deployed config "%s"', $checksum));
+            return $deployment;
+        } else {
+            throw new IcingaException('Failed to deploy config "%s"', $checksum);
+        }
+    }
+}

library/Director/Deployment/DeploymentGracePeriod.php

@@ -0,0 +1,61 @@
+<?php
+
+namespace Icinga\Module\Director\Deployment;
+
+use Icinga\Module\Director\Db;
+use Icinga\Module\Director\Objects\DirectorDeploymentLog;
+
+class DeploymentGracePeriod
+{
+    /** @var int */
+    protected $graceTimeSeconds;
+
+    /** @var Db */
+    protected $db;
+
+    /**
+     * @param int $graceTimeSeconds
+     * @param Db $db
+     */
+    public function __construct($graceTimeSeconds, Db $db)
+    {
+        $this->graceTimeSeconds = $graceTimeSeconds;
+        $this->db = $db;
+    }
+
+    /**
+     * Whether we're still within a grace period
+     * @return bool
+     */
+    public function isActive()
+    {
+        if ($deployment = $this->lastDeployment()) {
+            return $deployment->getDeploymentTimestamp() > $this->getGracePeriodStart();
+        }
+
+        return false;
+    }
+
+    protected function getGracePeriodStart()
+    {
+        return time() - $this->graceTimeSeconds;
+    }
+
+    public function getRemainingGraceTime()
+    {
+        if ($this->isActive()) {
+            if ($deployment = $this->lastDeployment()) {
+                return $deployment->getDeploymentTimestamp() - $this->getGracePeriodStart();
+            } else {
+                return null;
+            }
+        }
+
+        return 0;
+    }
+
+    protected function lastDeployment()
+    {
+        return DirectorDeploymentLog::optionalLatest($this->db);
+    }
+}

library/Director/Hook/JobHook.php

@@ -83,30 +83,4 @@ abstract class JobHook
     {
         return $this->db;
     }
-
-    /**
-     * printf helper method
-     *
-     * @param  string $message Format string
-     * @param  mixed  ...$arg  Format string argument
-     *
-     * @return self
-     */
-    protected function info($message)
-    {
-        call_user_func_array(array('Icinga\\Application\\Logger', 'info'), func_get_args());
-        return $this;
-    }
-
-    protected function warning($message)
-    {
-        call_user_func_array(array('Icinga\\Application\\Logger', 'warn'), func_get_args());
-        return $this;
-    }
-
-    protected function error($message)
-    {
-        call_user_func_array(array('Icinga\\Application\\Logger', 'error'), func_get_args());
-        return $this;
-    }
 }

library/Director/Job/ConfigJob.php

@@ -2,182 +2,32 @@
 
 namespace Icinga\Module\Director\Job;
 
-use Exception;
+use Icinga\Module\Director\Deployment\ConditionalConfigRenderer;
-use Icinga\Exception\IcingaException;
+use Icinga\Module\Director\Deployment\ConditionalDeployment;
-use Icinga\Module\Director\IcingaConfig\IcingaConfig;
+use Icinga\Module\Director\Deployment\DeploymentGracePeriod;
 use Icinga\Module\Director\Hook\JobHook;
-use Icinga\Module\Director\Objects\DirectorActivityLog;
-use Icinga\Module\Director\Objects\DirectorDeploymentLog;
-use Icinga\Module\Director\Util;
 use Icinga\Module\Director\Web\Form\QuickForm;
 
 class ConfigJob extends JobHook
 {
-    protected $lastDeployment;
-
-    protected $api;
-
     public function run()
     {
         $db = $this->db();
-        $this->clearLastDeployment();
+        $deployer = new ConditionalDeployment($db);
-
+        $renderer = new ConditionalConfigRenderer($db);
-        if ($this->shouldGenerate()) {
+        if ($grace = $this->getSetting('grace_period')) {
-            $config = IcingaConfig::generate($db);
+            $deployer->setGracePeriod(new DeploymentGracePeriod((int) $grace, $db));
-        } else {
+        }
-            $config = $this->loadLatestActivityConfig();
+        if ($this->getSetting('force_generate') === 'y') {
+            $renderer->forceRendering();
         }
 
-        if ($this->shouldDeploy($config)) {
+        $deployer->deploy($renderer->getConfig());
-            $this->deploy($config);
-        }
-
-        $this->clearLastDeployment();
-    }
-
-    protected function api()
-    {
-        if ($this->api === null) {
-            $this->api = $this->db()->getDeploymentEndpoint()->api();
-        }
-
-        return $this->api;
-    }
-
-    protected function loadLatestActivityConfig()
-    {
-        $db = $this->db();
-
-        return IcingaConfig::loadByActivityChecksum(
-            $db->getLastActivityChecksum(),
-            $db
-        );
-    }
-
-    protected function shouldGenerate()
-    {
-        return $this->getSetting('force_generate') === 'y'
-            || ! $this->configForLatestActivityExists();
-    }
-
-    protected function configForLatestActivityExists()
-    {
-        $db = $this->db();
-
-        return IcingaConfig::existsForActivityChecksum(
-            bin2hex(DirectorActivityLog::loadLatest($db)->checksum),
-            $db
-        );
-    }
-
-    protected function shouldDeploy(IcingaConfig $config)
-    {
-        $db = $this->db();
-        if ($this->getSetting('deploy_when_changed') !== 'y') {
-            return false;
-        }
-        $api = $this->api();
-        $api->collectLogFiles($db);
-
-        if (! DirectorDeploymentLog::hasDeployments($db)) {
-            return true;
-        }
-
-        if ($this->isWithinGracePeriod()) {
-            return false;
-        }
-
-        if (DirectorDeploymentLog::loadLatest($db)->getConfigHexChecksum()
-            === $config->getHexChecksum()
-        ) {
-            return false;
-        }
-
-        if ($this->getActiveChecksum() === $config->getHexChecksum()) {
-            return false;
-        }
-
-        return true;
-    }
-
-    protected function deploy(IcingaConfig $config)
-    {
-        $db = $this->db();
-        $api = $this->api();
-        $api->wipeInactiveStages($db);
-
-        $checksum = $config->getHexChecksum();
-        $this->info('Director ConfigJob ready to deploy "%s"', $checksum);
-        if ($api->dumpConfig($config, $db)) {
-            $this->info('Director ConfigJob deployed config "%s"', $checksum);
-
-            // TODO: Loop and try multiple times?
-            sleep(2);
-            try {
-                $api->collectLogFiles($db);
-            } catch (Exception $e) {
-                // Ignore those errors, Icinga may be reloading
-            }
-        } else {
-            throw new IcingaException('Failed to deploy config "%s"', $checksum);
-        }
-    }
-
-    protected function getGracePeriodStart()
-    {
-        return time() - $this->getSetting('grace_period');
-    }
-
-    public function getRemainingGraceTime()
-    {
-        if ($this->isWithinGracePeriod()) {
-            if ($deployment = $this->lastDeployment()) {
-                return $deployment->getDeploymentTimestamp()
-                + $this->getSetting('grace_period')
-                - time();
-            } else {
-                return null;
-            }
-        }
-
-        return 0;
-    }
-
-    protected function isWithinGracePeriod()
-    {
-        if ($deployment = $this->lastDeployment()) {
-            return $deployment->getDeploymentTimestamp() > $this->getGracePeriodStart();
-        }
-
-        return false;
-    }
-
-    protected function getActiveChecksum()
-    {
-        return DirectorDeploymentLog::getConfigChecksumForStageName(
-            $this->db(),
-            $this->api()->getActiveStageName()
-        );
-    }
-
-    protected function lastDeployment()
-    {
-        if ($this->lastDeployment === null) {
-            $this->lastDeployment = DirectorDeploymentLog::loadLatest($this->db());
-        }
-
-        return $this->lastDeployment;
-    }
-
-    protected function clearLastDeployment()
-    {
-        $this->lastDeployment = null;
-        return $this;
     }
 
     public static function addSettingsFormFields(QuickForm $form)
     {
-        $form->addElement('select', 'force_generate', array(
+        $form->addElement('select', 'force_generate', [
             'label'        => $form->translate('Force rendering'),
             'description'  => $form->translate(
                 'Whether rendering should be forced. If not enforced, this'
@@ -185,23 +35,23 @@ class ConfigJob extends JobHook
                 . ' activities since the last rendered config'
             ),
             'value'        => 'n',
-            'multiOptions' => array(
+            'multiOptions' => [
                 'y'  => $form->translate('Yes'),
                 'n'  => $form->translate('No'),
-            )
+            ]
-        ));
+        ]);
 
-        $form->addElement('select', 'deploy_when_changed', array(
+        $form->addElement('select', 'deploy_when_changed', [
             'label'        => $form->translate('Deploy modified config'),
             'description'  => $form->translate(
                 'This allows you to immediately deploy a modified configuration'
             ),
             'value'        => 'n',
-            'multiOptions' => array(
+            'multiOptions' => [
                 'y'  => $form->translate('Yes'),
                 'n'  => $form->translate('No'),
-            )
+            ]
-        ));
+        ]);
 
         $form->addElement('text', 'grace_period', array(
             'label' => $form->translate('Grace period'),

library/Director/Objects/DirectorActivityLog.php

@@ -84,6 +84,11 @@ class DirectorActivityLog extends DbObject
         }
     }
 
+    /**
+     * @param Db $connection
+     * @return DirectorActivityLog
+     * @throws \Icinga\Exception\NotFoundError
+     */
     public static function loadLatest(Db $connection)
     {
         $db = $connection->getDbAdapter();

library/Director/Objects/DirectorDeploymentLog.php

@@ -120,6 +120,19 @@ class DirectorDeploymentLog extends DbObject
         return static::load($db->fetchOne($query), $connection);
     }
 
+    /**
+     * @param Db $connection
+     * @return ?DirectorDeploymentLog
+     */
+    public static function optionalLatest(Db $connection)
+    {
+        try {
+            return static::loadLatest($connection);
+        } catch (NotFoundError $exception) {
+            return null;
+        }
+    }
+
     /**
      * @param CoreApi $api
      * @param Db $connection