1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-18 12:52:42 +01:00

Make the "daemons and web have different config" warning more specific

Summary:
I'm hitting this in the cluster and couldn't figure it out after staring at it for a couple minutes. Produce a better error.

This dumps a hash of each configuration key value which is set to a non-default value into the daemon log. This is much more compact than the full config, and doesn't spread secrets around, so it seems like a good balance between providing information and going crazy with it.

Test Plan: {F284139}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11699
This commit is contained in:
epriestley 2015-02-05 14:07:35 -08:00
parent 69f06387cb
commit 74ea59235a
6 changed files with 128 additions and 6 deletions

View file

@ -0,0 +1,2 @@
ALTER TABLE {$NAMESPACE}_daemon.daemon_log
ADD envInfo LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};

View file

@ -92,11 +92,36 @@ final class PhabricatorDaemonsSetupCheck extends PhabricatorSetupCheck {
'At least one daemon is currently running with different '. 'At least one daemon is currently running with different '.
'configuration than the Phabricator web application.'); 'configuration than the Phabricator web application.');
$list_section = null;
$env_info = $daemon->getEnvInfo();
if ($env_info) {
$issues = PhabricatorEnv::compareEnvironmentInfo(
PhabricatorEnv::calculateEnvironmentInfo(),
$env_info);
if ($issues) {
foreach ($issues as $key => $issue) {
$issues[$key] = phutil_tag('li', array(), $issue);
}
$list_section = array(
pht(
'The configurations differ in the following %s way(s):',
new PhutilNumber(count($issues))),
phutil_tag(
'ul',
array(),
$issues),
);
}
}
$message = pht( $message = pht(
'At least one daemon is currently running with a different '. 'At least one daemon is currently running with a different '.
'configuration (config checksum %s) than the web application '. 'configuration (config checksum %s) than the web application '.
'(config checksum %s).'. '(config checksum %s).'.
"\n\n". "\n\n%s".
'This usually means that you have just made a configuration change '. 'This usually means that you have just made a configuration change '.
'from the web UI, but have not yet restarted the daemons. You '. 'from the web UI, but have not yet restarted the daemons. You '.
'need to restart the daemons after making configuration changes '. 'need to restart the daemons after making configuration changes '.
@ -130,6 +155,7 @@ final class PhabricatorDaemonsSetupCheck extends PhabricatorSetupCheck {
phutil_tag('tt', array(), substr($daemon->getEnvHash(), 0, 12)), phutil_tag('tt', array(), substr($daemon->getEnvHash(), 0, 12)),
phutil_tag('tt', array(), substr($environment_hash, 0, 12)), phutil_tag('tt', array(), substr($environment_hash, 0, 12)),
$list_section,
phutil_tag('tt', array(), 'bin/phd restart'), phutil_tag('tt', array(), 'bin/phd restart'),
phutil_tag( phutil_tag(
'a', 'a',

View file

@ -42,6 +42,7 @@ final class PhabricatorDaemonEventListener extends PhabricatorEventListener {
->setPID(getmypid()) ->setPID(getmypid())
->setRunningAsUser($current_user['name']) ->setRunningAsUser($current_user['name'])
->setEnvHash(PhabricatorEnv::calculateEnvironmentHash()) ->setEnvHash(PhabricatorEnv::calculateEnvironmentHash())
->setEnvInfo(PhabricatorEnv::calculateEnvironmentInfo())
->setStatus(PhabricatorDaemonLog::STATUS_RUNNING) ->setStatus(PhabricatorDaemonLog::STATUS_RUNNING)
->setArgv($event->getValue('argv')) ->setArgv($event->getValue('argv'))
->setExplicitArgv($event->getValue('explicitArgv')) ->setExplicitArgv($event->getValue('explicitArgv'))

View file

@ -17,6 +17,7 @@ final class PhabricatorDaemonLog extends PhabricatorDaemonDAO
protected $argv; protected $argv;
protected $explicitArgv = array(); protected $explicitArgv = array();
protected $envHash; protected $envHash;
protected $envInfo;
protected $status; protected $status;
protected function getConfiguration() { protected function getConfiguration() {
@ -24,6 +25,7 @@ final class PhabricatorDaemonLog extends PhabricatorDaemonDAO
self::CONFIG_SERIALIZATION => array( self::CONFIG_SERIALIZATION => array(
'argv' => self::SERIALIZATION_JSON, 'argv' => self::SERIALIZATION_JSON,
'explicitArgv' => self::SERIALIZATION_JSON, 'explicitArgv' => self::SERIALIZATION_JSON,
'envInfo' => self::SERIALIZATION_JSON,
), ),
self::CONFIG_COLUMN_SCHEMA => array( self::CONFIG_COLUMN_SCHEMA => array(
'daemon' => 'text255', 'daemon' => 'text255',

View file

@ -240,19 +240,105 @@ final class PhabricatorEnv {
} }
public static function calculateEnvironmentHash() { public static function calculateEnvironmentHash() {
$keys = array_keys(self::getAllConfigKeys()); $keys = self::getKeysForConsistencyCheck();
sort($keys);
$skip_keys = self::getEnvConfig('phd.variant-config');
$keys = array_diff($keys, $skip_keys);
$values = array(); $values = array();
foreach ($keys as $key) { foreach ($keys as $key) {
$values[$key] = self::getEnvConfigIfExists($key); $values[$key] = self::getEnvConfigIfExists($key);
} }
return PhabricatorHash::digest(json_encode($values)); return PhabricatorHash::digest(json_encode($values));
} }
/**
* Returns a summary of non-default configuration settings to allow the
* "daemons and web have different config" setup check to list divergent
* keys.
*/
public static function calculateEnvironmentInfo() {
$keys = self::getKeysForConsistencyCheck();
$info = array();
$defaults = id(new PhabricatorConfigDefaultSource())->getAllKeys();
foreach ($keys as $key) {
$current = self::getEnvConfigIfExists($key);
$default = idx($defaults, $key, null);
if ($current !== $default) {
$info[$key] = PhabricatorHash::digestForIndex(json_encode($current));
}
}
$keys_hash = array_keys($defaults);
sort($keys_hash);
$keys_hash = implode("\0", $keys_hash);
$keys_hash = PhabricatorHash::digestForIndex($keys_hash);
return array(
'version' => 1,
'keys' => $keys_hash,
'values' => $info,
);
}
/**
* Compare two environment info summaries to generate a human-readable
* list of discrepancies.
*/
public static function compareEnvironmentInfo(array $u, array $v) {
$issues = array();
$uversion = idx($u, 'version');
$vversion = idx($v, 'version');
if ($uversion != $vversion) {
$issues[] = pht(
'The two configurations were generated by different versions '.
'of Phabricator.');
// These may not be comparable, so stop here.
return $issues;
}
if ($u['keys'] !== $v['keys']) {
$issues[] = pht(
'The two configurations have different keys. This usually means '.
'that they are running different versions of Phabricator.');
}
$uval = idx($u, 'values', array());
$vval = idx($v, 'values', array());
$all_keys = array_keys($uval + $vval);
foreach ($all_keys as $key) {
$uv = idx($uval, $key);
$vv = idx($vval, $key);
if ($uv !== $vv) {
if ($uv && $vv) {
$issues[] = pht(
'The configuration key "%s" is set in both configurations, but '.
'set to different values.',
$key);
} else {
$issues[] = pht(
'The configuration key "%s" is set in only one configuration.',
$key);
}
}
}
return $issues;
}
private static function getKeysForConsistencyCheck() {
$keys = array_keys(self::getAllConfigKeys());
sort($keys);
$skip_keys = self::getEnvConfig('phd.variant-config');
return array_diff($keys, $skip_keys);
}
/* -( Reading Configuration )---------------------------------------------- */ /* -( Reading Configuration )---------------------------------------------- */

View file

@ -906,6 +906,11 @@ abstract class PhabricatorBaseEnglishTranslation
'You have an unpaid invoice.', 'You have an unpaid invoice.',
'You have unpaid invoices.', 'You have unpaid invoices.',
), ),
'The configurations differ in the following %s way(s):' => array(
'The configurations differ:',
'The configurations differ in these ways:',
),
); );
} }