diff --git a/ntfy/events/imagecomplete_ntfy.event.php b/ntfy/events/imagecomplete_ntfy.event.php index 77ed6be..0797098 100644 --- a/ntfy/events/imagecomplete_ntfy.event.php +++ b/ntfy/events/imagecomplete_ntfy.event.php @@ -67,8 +67,33 @@ public function __construct() */ public function onEvent($event, $data) { - self::$message = 'This host has finished imaging.'; - self::$shortdesc = 'Imaging Complete'; + // One listener, two names: HOST_IMAGE_COMPLETE is a deploy finishing + // and HOST_IMAGEUP_COMPLETE is a capture. Core never fired the capture + // name at all until fogproject#1202, so both outcomes arrived as a + // deploy and this message could not tell them apart. + // + // Composed here rather than left as a bare literal for the parent to + // translate, because the image name has to be substituted OUTSIDE _(). + // The parent's _() then finds no entry and passes the finished string + // through, which is what we want. Read defensively: this plugin has to + // keep working against a server that has not taken #1202 yet. + $image = (string) ($data['ImageName'] ?? ''); + if ('' === $image) { + $image = _('an unnamed image'); + } + if ('HOST_IMAGEUP_COMPLETE' === $event) { + self::$shortdesc = _('Capture Complete'); + self::$message = sprintf( + _('This host has finished capturing image %s.'), + $image + ); + } else { + self::$shortdesc = _('Deploy Complete'); + self::$message = sprintf( + _('This host has finished deploying image %s.'), + $image + ); + } parent::onEvent($event, $data); } } diff --git a/ntfy/events/imagefail_ntfy.event.php b/ntfy/events/imagefail_ntfy.event.php index 87fd49f..049355e 100644 --- a/ntfy/events/imagefail_ntfy.event.php +++ b/ntfy/events/imagefail_ntfy.event.php @@ -63,8 +63,25 @@ public function __construct() */ public function onEvent($event, $data) { - self::$message = 'This host has failed to image'; - self::$shortdesc = 'Failed'; + // HOST_IMAGE_FAIL had no core caller at all until fogproject#1202, so + // this listener has never run on any server. The reason is the part an + // admin can act on, so say it rather than "failed to image". Read + // defensively: this plugin has to keep working against a server that + // has not taken #1202 yet. + $image = (string) ($data['ImageName'] ?? ''); + if ('' === $image) { + $image = _('an unnamed image'); + } + $reason = (string) ($data['Reason'] ?? ''); + if ('' === $reason) { + $reason = _('no reason was reported'); + } + self::$shortdesc = _('Imaging Failed'); + self::$message = sprintf( + _('This host failed imaging %1$s: %2$s'), + $image, + $reason + ); parent::onEvent($event, $data); } } diff --git a/pushbullet/events/imagecomplete_pushbullet.event.php b/pushbullet/events/imagecomplete_pushbullet.event.php index 80f18bd..f7de97f 100644 --- a/pushbullet/events/imagecomplete_pushbullet.event.php +++ b/pushbullet/events/imagecomplete_pushbullet.event.php @@ -67,8 +67,33 @@ public function __construct() */ public function onEvent($event, $data) { - self::$message = 'This host has finished imaging.'; - self::$shortdesc = 'Imaging Complete'; + // One listener, two names: HOST_IMAGE_COMPLETE is a deploy finishing + // and HOST_IMAGEUP_COMPLETE is a capture. Core never fired the capture + // name at all until fogproject#1202, so both outcomes arrived as a + // deploy and this message could not tell them apart. + // + // Composed here rather than left as a bare literal for the parent to + // translate, because the image name has to be substituted OUTSIDE _(). + // The parent's _() then finds no entry and passes the finished string + // through, which is what we want. Read defensively: this plugin has to + // keep working against a server that has not taken #1202 yet. + $image = (string) ($data['ImageName'] ?? ''); + if ('' === $image) { + $image = _('an unnamed image'); + } + if ('HOST_IMAGEUP_COMPLETE' === $event) { + self::$shortdesc = _('Capture Complete'); + self::$message = sprintf( + _('This host has finished capturing image %s.'), + $image + ); + } else { + self::$shortdesc = _('Deploy Complete'); + self::$message = sprintf( + _('This host has finished deploying image %s.'), + $image + ); + } parent::onEvent($event, $data); } } diff --git a/pushbullet/events/imagefail_pushbullet.event.php b/pushbullet/events/imagefail_pushbullet.event.php index 4add6f0..67e17a3 100644 --- a/pushbullet/events/imagefail_pushbullet.event.php +++ b/pushbullet/events/imagefail_pushbullet.event.php @@ -63,8 +63,25 @@ public function __construct() */ public function onEvent($event, $data) { - self::$message = 'This host has failed to image'; - self::$shortdesc = 'Failed'; + // HOST_IMAGE_FAIL had no core caller at all until fogproject#1202, so + // this listener has never run on any server. The reason is the part an + // admin can act on, so say it rather than "failed to image". Read + // defensively: this plugin has to keep working against a server that + // has not taken #1202 yet. + $image = (string) ($data['ImageName'] ?? ''); + if ('' === $image) { + $image = _('an unnamed image'); + } + $reason = (string) ($data['Reason'] ?? ''); + if ('' === $reason) { + $reason = _('no reason was reported'); + } + self::$shortdesc = _('Imaging Failed'); + self::$message = sprintf( + _('This host failed imaging %1$s: %2$s'), + $image, + $reason + ); parent::onEvent($event, $data); } } diff --git a/slack/events/imagecomplete_slack.event.php b/slack/events/imagecomplete_slack.event.php index 21cf52a..d3bedfe 100644 --- a/slack/events/imagecomplete_slack.event.php +++ b/slack/events/imagecomplete_slack.event.php @@ -47,6 +47,15 @@ public function __construct() /** * Perform action * + * One listener, two names: HOST_IMAGE_COMPLETE is a deploy finishing and + * HOST_IMAGEUP_COMPLETE is a capture. Core never fired the capture name + * at all until fogproject#1202, so both outcomes arrived as a deploy and + * the message could not tell them apart. The payload now also carries the + * image, so the notification can say which one. + * + * Every added key is read defensively: this plugin has to keep working + * against a server that has not taken that change yet. + * * @param string $event the event to enact * @param mixed $data the data * @@ -54,15 +63,23 @@ public function __construct() */ public function onEvent($event, $data) { + $image = (string) ($data['ImageName'] ?? ''); + if ('' === $image) { + $image = _('an unnamed image'); + } + $format = ( + 'HOST_IMAGEUP_COMPLETE' === $event ? + _('Host %1$s finished capturing image %2$s.') : + _('Host %1$s finished deploying image %2$s.') + ); $Slacks = Route::getList('slack'); foreach ($Slacks as $Slack) { $args = [ 'channel' => $Slack->name, 'text' => sprintf( - '%s: %s %s.', - _('Host'), + $format, $data['HostName'], - _('completed imaging') + $image ) ]; self::getClass('Slack', $Slack->id)->call('chat.postMessage', $args); diff --git a/slack/events/imagefail_slack.event.php b/slack/events/imagefail_slack.event.php index 7eaeb4f..0d57a41 100644 --- a/slack/events/imagefail_slack.event.php +++ b/slack/events/imagefail_slack.event.php @@ -44,6 +44,13 @@ public function __construct() /** * Perform action * + * HOST_IMAGE_FAIL had no core caller at all until fogproject#1202, so this + * listener has never run on any server. The payload carries the image and + * the reason FOG rejected the task, which is the part an admin can act on. + * + * Every added key is read defensively: this plugin has to keep working + * against a server that has not taken that change yet. + * * @param string $event the event to enact * @param mixed $data the data * @@ -51,15 +58,23 @@ public function __construct() */ public function onEvent($event, $data) { + $image = (string) ($data['ImageName'] ?? ''); + if ('' === $image) { + $image = _('an unnamed image'); + } + $reason = (string) ($data['Reason'] ?? ''); + if ('' === $reason) { + $reason = _('no reason was reported'); + } $Slacks = Route::getList('slack'); foreach ($Slacks as $Slack) { $args = [ 'channel' => $Slack->name, 'text' => sprintf( - '%s: %s %s.', - _('Host'), + _('Host %1$s failed imaging %2$s: %3$s'), $data['HostName'], - _('imaging failed') + $image, + $reason ) ]; self::getClass('Slack', $Slack->id)->call('chat.postMessage', $args); diff --git a/tests/imaging-notification-detail.test.php b/tests/imaging-notification-detail.test.php new file mode 100644 index 0000000..94d02d4 --- /dev/null +++ b/tests/imaging-notification-detail.test.php @@ -0,0 +1,148 @@ + $file) { + $src = file_get_contents($file); + $where = basename($file); + + // The whole point of #1202's core half: one listener, two names. + $check( + false !== strpos($src, "'HOST_IMAGEUP_COMPLETE' === \$event"), + "$where does not distinguish a capture from a deploy, so the name" + . ' core went to the trouble of firing is wasted' + ); + $check( + false !== strpos($src, "'HOST_IMAGEUP_COMPLETE'") + && false !== strpos($src, "'HOST_IMAGE_COMPLETE'"), + "$where no longer registers both completion names" + ); +} + +foreach ($fail as $plugin => $file) { + $src = file_get_contents($file); + $where = basename($file); + + // The reason is the only part of a failure an admin can act on. + $check( + false !== strpos($src, "\$data['Reason'] ?? ''"), + "$where does not report why imaging failed, or reads Reason without a" + . ' default' + ); + $check( + false !== strpos($src, "'HOST_IMAGE_FAIL'"), + "$where no longer registers HOST_IMAGE_FAIL" + ); +} + +if (count($failures) > 0) { + fwrite(STDERR, 'FAIL: ' . count($failures) . " problem(s):\n"); + foreach ($failures as $f) { + fwrite(STDERR, " - $f\n"); + } + exit(1); +} + +printf("ok: %d checks across 6 imaging notification listeners\n", $checks); +exit(0);