diff --git a/packages/web/commons/schema.php b/packages/web/commons/schema.php index b28f8fdd37..4d2ca22546 100644 --- a/packages/web/commons/schema.php +++ b/packages/web/commons/schema.php @@ -4062,3 +4062,101 @@ function () { . "SET `logType` = 'state' " . "WHERE `logType` = '' OR `logType` IS NULL", ); +// 283 +$this->schema[] = array( + // A report keeps enough identity to be read after its task is gone. + // Ported from 1.6 schema 341 (#1236). + // + // taskLog stores no host and no task type of its own, and reaches both + // through `tasks`. Nothing deletes taskLog rows -- but Host::destroy() + // calls TaskManager->destroy() and taskLog is in no cascade at all, so + // deleting a host destroys its tasks and leaves the reports behind with + // nothing to join to, losing the host name at the same moment the host + // row that could supply it goes. + // + // Host name is the first thing anyone searches a failure by, and this + // branch has no Task Management log pane, so the REST API is the only + // reader there is -- it hands back a report whose taskID points at + // nothing and no way at all to learn which machine it came from. The + // point of GH-1206 is that a failure message is findable later instead + // of arriving as a phone photo of a wrapped console, and a foreign key + // to a routinely-deleted row cannot deliver that. + // + // Blocking deletion of a task that has reports was the alternative. It + // inverts the dependency -- a diagnostic artifact would then constrain + // operational cleanup -- and to be consistent it would have to block + // HOST deletion too, since that is the path that actually removes tasks. + // + // The state a row records is NOT copied: taskLog already stores + // taskStateID itself, so that lookup survives the task. + // + // Written only by the FOS report endpoint. Every other row in this table + // is a state transition written by TaskingElement::taskLog() on every + // transition; they are meaningless without their task anyway, and making + // that path do three extra lookups buys nothing. Same reasoning that + // gave logText no value on a state row in step 280. + // + // Two column shapes on purpose, and they follow what the writer can + // actually produce. FOGController::save() omits an unset OPTIONAL column + // whose key ends in "id" -- so logHostID gets its DEFAULT of NULL -- but + // for every other key an unset value is written as '', never NULL (the + // trap step 282 had to repair for logType). Declaring logHostName NOT + // NULL DEFAULT '' says what the ORM will really store rather than + // describing a NULL the writer cannot produce. + // + // A closure rather than a bare ALTER for the same reason step 280 is: + // ADD COLUMN has no IF NOT EXISTS below MariaDB 10.0.2/MySQL 8.0.29, so + // a re-run has to converge on its own rather than error. + function () { + $have = self::$DB->query( + "SELECT `COLUMN_NAME` AS `c` FROM `information_schema`.`COLUMNS` " + . "WHERE `TABLE_SCHEMA` = DATABASE() AND `TABLE_NAME` = 'taskLog' " + . "AND `COLUMN_NAME` IN " + . "('logHostID','logHostName','logTaskTypeName')" + )->fetch(\PDO::FETCH_ASSOC, 'fetch_all')->get(); + $cols = array(); + foreach ((array)$have as $row) { + if (isset($row['c'])) { + $cols[] = $row['c']; + } + } + $adds = array(); + if (!in_array('logHostID', $cols)) { + $adds[] = "ADD `logHostID` INT(11) NULL DEFAULT NULL"; + } + if (!in_array('logHostName', $cols)) { + // varchar(16) matches hosts.hostName, which is capped at the + // NetBIOS limit and cannot outgrow this copy. + $adds[] = "ADD `logHostName` VARCHAR(16) NOT NULL DEFAULT ''"; + } + if (!in_array('logTaskTypeName', $cols)) { + // varchar(30) matches taskTypes.ttName. + $adds[] = "ADD `logTaskTypeName` VARCHAR(30) NOT NULL DEFAULT ''"; + } + if (count($adds) > 0) { + self::$DB->query( + "ALTER TABLE `taskLog` " . implode(', ', $adds) + ); + } + + // Backfill the reports whose task is still there, so the history is + // not split between rows that know their host and rows that do not. + // Restricted to report rows and to rows not already filled, so a + // re-run is a no-op and a later hand-correction is not overwritten. + self::$DB->query( + "UPDATE `taskLog` " + . "JOIN `tasks` ON `tasks`.`taskID` = `taskLog`.`taskID` " + . "LEFT JOIN `hosts` " + . "ON `hosts`.`hostID` = `tasks`.`taskHostID` " + . "LEFT JOIN `taskTypes` " + . "ON `taskTypes`.`ttID` = `tasks`.`taskTypeID` " + . "SET `taskLog`.`logHostID` = `tasks`.`taskHostID`, " + . "`taskLog`.`logHostName` = COALESCE(`hosts`.`hostName`, ''), " + . "`taskLog`.`logTaskTypeName` = COALESCE(`taskTypes`.`ttName`, '') " + . "WHERE `taskLog`.`logType` <> 'state' " + . "AND `taskLog`.`logHostID` IS NULL" + ); + + return true; + }, +); diff --git a/packages/web/lib/fog/system.class.php b/packages/web/lib/fog/system.class.php index 114522a61f..70a710ba4b 100644 --- a/packages/web/lib/fog/system.class.php +++ b/packages/web/lib/fog/system.class.php @@ -54,7 +54,7 @@ public function __construct() { self::_versionCompare(); define('FOG_VERSION', '1.5.10.2308'); - define('FOG_SCHEMA', 282); + define('FOG_SCHEMA', 283); define('FOG_BCACHE_VER', 143); define('FOG_CLIENT_VERSION', '0.13.0'); // GH-959: iPXE lives in FOGProject/fog-ipxe and its binaries arrive as diff --git a/packages/web/lib/fog/tasklog.class.php b/packages/web/lib/fog/tasklog.class.php index 616137d285..31907c6feb 100644 --- a/packages/web/lib/fog/tasklog.class.php +++ b/packages/web/lib/fog/tasklog.class.php @@ -30,6 +30,20 @@ class TaskLog extends FOGController /** * The task log fields and common names. * + * hostID/hostName/taskTypeName are a copy of who the report was about, + * kept because the row outlives what it points at. taskLog reaches host + * and task type through `tasks`; nothing deletes taskLog rows, but + * Host::destroy() destroys the host's tasks and taskLog is in no cascade + * -- so deleting a host leaves the reports with nothing to join to, + * losing the host name at the same moment the host row that could supply + * it goes. This branch has no Task Management log pane, so the REST API + * is the only reader, and it could not recover the host at all. Schema + * 283 stores it here. + * + * Only the FOS report endpoint fills them. A state row leaves them + * empty: it is an annotation on a task and means nothing without it, + * and TaskingElement::taskLog() runs on every transition. + * * @var array */ protected $databaseFields = array( @@ -41,6 +55,9 @@ class TaskLog extends FOGController 'createdBy' => 'createdBy', 'type' => 'logType', 'text' => 'logText', + 'hostID' => 'logHostID', + 'hostName' => 'logHostName', + 'taskTypeName' => 'logTaskTypeName', ); /** * A row recording a state transition, which is what every row was diff --git a/packages/web/lib/reg-task/taskerror.class.php b/packages/web/lib/reg-task/taskerror.class.php index a29003b3df..a60a26eb7e 100644 --- a/packages/web/lib/reg-task/taskerror.class.php +++ b/packages/web/lib/reg-task/taskerror.class.php @@ -281,12 +281,21 @@ private static function _markFailed($Task) */ private static function _logRow($Task, $type, $text) { + // Host and task type are copied onto the row, not left to the join. + // Host::destroy() destroys the host's tasks, taskLog is in no + // cascade, so the report outlives everything it points at -- and by + // the time the join fails the host row is gone too, which makes this + // the last moment the name can be recorded at all. See TaskLog's + // $databaseFields and schema step 283. self::getClass('TaskLog') ->set('taskID', $Task->get('id')) ->set('stateID', $Task->get('stateID')) ->set('createdBy', 'fos') ->set('type', $type) ->set('text', $text) + ->set('hostID', self::$Host->get('id')) + ->set('hostName', self::$Host->get('name')) + ->set('taskTypeName', $Task->getTaskTypeText()) ->save(); } /** diff --git a/tests/task-error-report.test.php b/tests/task-error-report.test.php index b2193d0f63..bf3cb81561 100644 --- a/tests/task-error-report.test.php +++ b/tests/task-error-report.test.php @@ -339,13 +339,58 @@ class TaskLog // ----------------------------------------------------------- the plumbing $model = file_get_contents($web . '/lib/fog/tasklog.class.php'); -foreach (array('type' => 'logType', 'text' => 'logText') as $key => $column) { +$columns = array( + 'type' => 'logType', + 'text' => 'logText', + 'hostID' => 'logHostID', + 'hostName' => 'logHostName', + 'taskTypeName' => 'logTaskTypeName', +); +foreach ($columns as $key => $column) { if (false === strpos($model, "'$key' => '$column'")) { $fails[] = "TaskLog does not map $key onto $column, so the endpoint's" . ' report is dropped on save'; } } +// ---------------------------------------------------------- the retention + +// The row outlives what it points at. Host::destroy() destroys the host's +// tasks and taskLog is in no cascade, so a report whose host is deleted keeps +// its text and loses the name it would be searched by -- and this branch has +// no log pane, so the REST API is the only reader and cannot recover it from +// anywhere. Ported from 1.6 (GH-1236); the reader half has nothing to port. +foreach (array( + 'hostID' => "set('hostID', self::\$Host->get('id'))", + 'hostName' => "set('hostName', self::\$Host->get('name'))", + 'taskTypeName' => "set('taskTypeName', \$Task->getTaskTypeText())", +) as $field => $needle) { + if (false === strpos($src, $needle)) { + $fails[] = "the endpoint does not store $field on the report, so the" + . ' row loses it the moment its task is deleted'; + } +} +// The columns have to exist before anything can be written to them, and a +// declared field that has no column takes the whole INSERT down -- which is +// silent here, because save() swallows it and _logRow ignores the return. +if (!preg_match( + "#ADD `logHostID` INT\(11\).*?ADD `logHostName` VARCHAR\(16\)" + . ".*?ADD `logTaskTypeName` VARCHAR\(30\)#s", + $schema +)) { + $fails[] = 'no schema step adds the report identity columns, so every' + . ' report insert fails with 1054 and is dropped without a trace'; +} +// Existing reports whose task survived must not be left as the only rows +// that cannot answer the question. +if (!preg_match( + "#UPDATE `taskLog` .*?SET `taskLog`\.`logHostID`#s", + $schema +)) { + $fails[] = 'the schema step adds the columns but does not backfill the' + . ' reports whose task is still there'; +} + // The log directory has to appear in all three lists or the Log Viewer fails // in two different ways -- no entry, or an entry answering "Invalid Folder". $subdir = constant('TaskError::LOG_SUBDIR');