From 3857f3f06e0a61dd7d76c9c22e8034096c36477c Mon Sep 17 00:00:00 2001 From: Tom Elliott Date: Wed, 19 Aug 2026 05:48:56 -0500 Subject: [PATCH] Notifications: name the image, and tell a capture from a deploy The other half of fogproject#1202. Core now fires HOST_IMAGEUP_COMPLETE for a capture, HOST_IMAGE_COMPLETE for a deploy and HOST_IMAGE_FAIL when imaging did not finish, and it sends the image and the failure reason along with the host name. All three of those events reached listeners that could not say anything useful with them. Every one of these six listeners registered HOST_IMAGEUP_COMPLETE or HOST_IMAGE_FAIL and then ignored which event had arrived, because until now neither name was ever fired. So a finished capture said "This host has finished imaging", identically to a deploy, and a failure said "This host has failed to image" with no hint as to why. They now say which image, and for a failure, what FOG rejected. Deliberately defensive about every added key. A plugin release is not tied to a FOG release, so these have to keep working against a server whose core still sends nothing but HostName -- reading a missing key directly would turn the notification into a PHP warning, in an event that only fires when something has already gone wrong. That is also why fog_min is untouched. The image name is substituted outside _(), not inside it: a msgid that mixes literal text with a variable extracts nothing and never translates. This repository has no gettext gate of its own, so tests/imaging-notification- detail.test.php pins that shape along with the defensive reads and the capture/deploy split. Mutation-verified. Co-authored-by: Claude --- ntfy/events/imagecomplete_ntfy.event.php | 29 +++- ntfy/events/imagefail_ntfy.event.php | 21 ++- .../events/imagecomplete_pushbullet.event.php | 29 +++- .../events/imagefail_pushbullet.event.php | 21 ++- slack/events/imagecomplete_slack.event.php | 23 ++- slack/events/imagefail_slack.event.php | 21 ++- tests/imaging-notification-detail.test.php | 148 ++++++++++++++++++ 7 files changed, 278 insertions(+), 14 deletions(-) create mode 100644 tests/imaging-notification-detail.test.php 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);