Skip to content

Two notification events have never fired: HOST_IMAGE_FAIL and HOST_IMAGEUP_COMPLETE #1202

Closed

Description

@mastacontrola

Split out of the hook/event work (#1194, #1196, #1200, #1201). Recorded as F-19 in docs/refactor-facts.md and listed as H.10 in docs/hook-event-plan.md, deliberately not folded into that sequence: this is making a notification feature work for the first time, not refactoring a subsystem, so it wants its own test and its own review.

The mismatch

Core calls notify() from six call sites with five distinct names. The three bundled notification plugins (slack, ntfy, pushbullet) register listeners for six. They do not line up:

cd packages
grep -rn -e '->notify(' web/lib --include=*.php | wc -l
grep -rh -A1 '>register(' web/lib/plugins/*/events/*.event.php | grep -o "'[A-Z_a-z]*'" | sort -u
grep -rn 'HOST_IMAGE_FAIL\|HOST_IMAGEUP_COMPLETE' web/lib --include=*.php | grep -v plugins   # nothing

HOST_IMAGE_FAIL and HOST_IMAGEUP_COMPLETE have listeners in all three plugins and no notify() anywhere in the tree. So "image deploy failed" and "image upload complete" notifications have never fired on any FOG server, for anyone, since those plugins shipped. Nothing errors — the listener is registered and simply never called, which is why this went unnoticed.

HOST_CHECKIN is the inverse: core notifies it on every task checkin and nothing has ever listened. Harmless, but it means the name exists in notifyEvents on every install as a thing an admin can see and never receive.

What is needed

A core notify() for each of the two dead names, at the point where the outcome is actually known — not at the point that is convenient. HOST_IMAGEUP_COMPLETE in particular has to fire after the upload is genuinely finished and the image row is consistent, or the notification arrives before the image is usable.

Then a test that pins each name to its call site, so the next person who moves that code finds out.

Worth deciding at the same time whether HOST_CHECKIN should keep being notified. It costs a notify() per checkin for something nothing consumes; either give it a listener or stop firing it.

Notes

notify() is now safe to call from anywhere: the name-recording no longer runs before the input is validated, and neither error handler can go fatal on a bad name (#1194, #1200, #1201). So this is additive work with no groundwork left to do.

Activity

  1. mastacontrola commented on Aug 19, 2026

    @mastacontrola
    MemberAuthor

    Done in #1205 (core) and FOGProject/fog-plugins#21 (the three bundled notification plugins). Both merged, both suites green on the merged tips.

    HOST_IMAGEUP_COMPLETE — fully fixed. TaskQueue::checkout() already branched on isDeploy()/isCapture() forty lines above the notify and threw the answer away; it now picks the name from it. A capture announces itself as a capture.

    HOST_IMAGE_FAIL — now fires, from checkout()'s catch: imaging ran and FOG then failed to record it (host update, task save, task log, imaging log). That is a real failure and it previously notified nothing.

    It is not the failure most people mean by "the deploy failed". FOS's handleError() prints to the console and exit 1s — it makes no request to the server at all — so a bad image, a mount failure or a partition error is invisible to FOG. Closing that needs a service endpoint here plus a FOS change and release, and it raises questions this issue is the wrong place for (an unauthenticated endpoint taking free text; whether a failed task needs a state, given taskStates has no Failed). Split out as #1206.

    HOST_CHECKIN — left as it is. Core notifies it and nothing listens; that costs one notify() per checkin and is now cheap, since the name-recording is cached rather than a query per call (#1194).

    Two things found along the way and fixed in the same PRs:

    • The notification did not check the task was imaging. checkout() is reached from Post_Wipe.php as well as Post_Stage2/3.php, so wiping a disk sent "This host has finished imaging."
    • The payload was the host's name and nothing else, which is why every bundled listener could only say "this host has finished imaging" without naming the image. It now carries Host, Task, Image, ImageName, TaskType and Reason alongside the original HostName — additive, so no listener needed editing — and the three bundled plugins use it:
    before after
    deploy Host lab01 completed imaging. Host lab01 finished deploying image Win11-Lab.
    capture Host lab01 completed imaging. Host lab01 finished capturing image Win11-Lab.
    failure Host lab01 imaging failed. Host lab01 failed imaging Win11-Lab: Failed to update host

    One behaviour change worth naming: a capture now fires HOST_IMAGEUP_COMPLETE instead of HOST_IMAGE_COMPLETE, so a third-party listener registered only on the deploy name stops hearing captures. That is the documented intent of having two names, and all three bundled plugins already register both.

    Closing. #1206 carries the remaining half.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugready-for-agentWell-scoped, self-contained; an AI coding agent can produce a PR

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions