Re: [pve-devel] applied: [PATCH installer v4 00/12] fix #5536: implement post-(auto-)installation notification mechanism

[Christoph Heiss <c.heiss@proxmox.com>]

On Mon, Nov 11, 2024 at 06:41:15PM +0100, Thomas Lamprecht wrote:
> Am 11.11.24 um 14:14 schrieb Christoph Heiss:
> > This implements a mechanism for post-installation "notifications" via a
> > POST request [0] when using the auto-installer.
> > [..]
> > Feedback on the post-hook information schema is of course also very much
> > appreciated!
>
> What about adding an X.Y format version? So that we can expand this nicely or
> even rework completely.
> [..]
> One naturally can use the product version for deriving the format version,
> but in the long run that needs more client side handling and mapping than
> a separate format version.

Sure, makes sense! The document format will surely change/get
expanded in the future, so definitely good to have something like this
from the get-go.

I'll send a patch asap, fairly trivial to implement for now anyway.

>
> >
> > It should be noted that some information like DMI is generally very
> > depended on the motherboard/firmware, on what information is actually
> > available and filled-in. So the contents are expected to vary wildly
> > between machines and may also be empty, as in the example below from a
> > VM.
>
> Can we also dump the schema so that this can be added to the docs, or
> at least (linked to) in the wiki of the automated installer?

Of course! Glancing over it, the schema for the JSON that
`proxmox-fetch-answer` is also not explicitly documented - so I'll that
one too.

>
> Also, I figure you already planned to document this in the wiki for the
> next ISO release? Just asking to be sure it won't be overlooked.

Yes! Just waited for this to get applied, i.e. having a finalized
format.

>
> >
> > Diffstat
> > ========
> >
> > Christoph Heiss (12):
> >   debian: strip unused library dependencies
> >   fetch-answer: move http-related code to gated module in
> >     installer-common
> >   tree-wide: convert some more crates to use workspace dependencies
> >   auto-install-assistant: replace `PathBuf` parameters with
> >     `AsRef<Path>`
>
> ^- this one would have done well with some short commit message body,
> even if obvious it will never hurt to state the intention of the commit.

Ack!

>
> >   auto-installer: tests: simplify empty disks check
> >   auto-installer: tests: replace `PathBuf` parameters with `AsRef<Path>`
>
> ^- same here w.r.t. the lack of a short commit message body
>
> >   auto-installer: move `SystemDMI` struct to common crate
> >   auto-installer: answer: factor out answer file reading into function
> >   auto-installer: udevinfo: introduce type alias for udev properties
> >   fix #5536: auto-installer: answer: add `posthook` section
> >   fix #5536: post-hook: add utility for sending notifications after
> >     auto-install
>
> ^- above two might have been better off squashed, but I can get how you
> wanted to separate them, so I kept them as is.

Thanks! Since the second patch, adding the `proxmox-post-hook` tool, was
pretty big already, I'd tried to split off things not immediately
relevant to it. (In a sense that the tool is considered "standalone")


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel