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

[Christoph Heiss]

Thanks for the feedback!

Since this will be externally consumed API surface, changing it
afterwards will be a pain anyway - so comments on the JSON schema are
really appreciated too.

On Mon, Jul 15, 2024 at 12:42:41PM GMT, Thomas Lamprecht wrote:
> Am 10/07/2024 um 15:27 schrieb Christoph Heiss:
> > [..]
> > {
> >   "debian-version": "12.5",
> >   "product-version": "pve-manager/8.2.2/9355359cd7afbae4",
> >   "kernel-version": "proxmox-kernel-6.8.4-2-pve-signed",
>
> no hard feelings, but from a gut feeling I'd probably move this to a
> "version" object and potentially use a more reduced, easier to parse, value.
> Maybe also as an object so that both, a simple X.Y(.Z) release and a full
> string are given, as we changed (kernel) packages name in the past, and I
> could imagine that there will be a LTS and non LTS variant if we ever rework
> on where we base our kernel on, so this might change in the future too.
> While the simple X.Y.Z version will more likely stay as is.

Yeah, that's fair. While writing this I've basically just looked around
what information could potentially be interesting for
users/administrators and then dumped them into a JSON.

Having an actual structured version object here makes sense.

>
> And I do not want to move the goal post here to far, but isn't some of this
> information potentially interesting to have sent to a metric server?

Yeah, at least Prometheus' node_exporter does send such information, so
its quite "standard" practice.

> At least with a low frequency (or just once on every boot) so that one has a
> somewhat recent externally saved set of information that can be used to
> identify machines more easily and be aware of some changes to correlate
> regressions or strange (load) metrics with.

Just to get this right: is the idea to (optionally) send some/all of
this information to a external metrics server, in addition to the
posthook target?
Or just generally to keep in mind where information like this could go
and why it should be uniform?

>
> No need to do that now, but maybe something to keep in mind to allow easier
> reuse of this.
>
> IMO it's a big plus if we manage to keep information laid out the same way,
> or at list in a similar one, wherever it's included. And that doesn't have
> to mean that the whole struct has to be shared, maybe it just could be just
> a collection of types that stem from common crates outside the installer
> (at least in the long run, as said, no need to completely block this on
> scope extension now).
>
> >   "boot-type": "bios",
>
> We call this "mode" in the product APIs [0], might potentially make sense
> to use the same schema here? Else I'd at least name this boot-mode and use
> the same keys.
>
> [0]: https://pve.proxmox.com/pve-docs/api-viewer/index.html#/nodes/{node}/status

Sounds good! Looking at the API, I'd take the entire "boot-info" object
from there and have it be the same.

I will generally try to align the schema of this JSON a bit more with
that returned by the API mentioned above. E.g. also the (kernel) version
object above with the "current-kernel" structure, as far as possible.
And looking at it, we also could include the "cpuinfo" structure for
good measure.

Being consistent (where sensible, as you say) with our existing API
makes very much sense, didn't think of this!

>
> >   "filesystem": "zfs (RAID1)",
> >   "fqdn": "host.domain",
> >   "machine-id": "f4bf9711783248b7aaffe3ccbca3e3dc",
> >   "bootdisk": [
>
> could be also interesting to get all disks and flag the ones used for booting
> with a boolean "bootdisk" flag. E.g. to make additional storage provisioning
> later on slightly more convenient.

With that in mind it definitely could come in handy. Or maybe a separate
object "disks"/"other-disks"/etc. entirely? So as not have to filter out
the (non-)bootdisks again on the receiving end.

While at it, the same would IMO make sense for NICs too, since one might
want to set up additional network devices too.

>
> >     {
> >       "size": 8589934592,
> >       "udev-properties": {
> >         "DEVNAME": "/dev/vda", [..]
> >       }
> >     },
> >     {
> >       "size": 8589934592,
> >       "udev-properties": {
> >         "DEVNAME": "/dev/vdb", [..]
> >       }
> >     }
> >   ],
> >   "management-nic": {
> >     "mac": "de:ad:f0:0d:12:34",
> >     "address": "10.0.0.10/24",
> >     "udev-properties": {
> >       "INTERFACE": "enp6s18", [..]
> >     }
> >   },
> >   "ssh-public-host-keys": {
> >     "ecdsa": "ecdsa-sha2-nistp256 [..] root at host",
> >     "ed25519": "ssh-ed25519 [..] root at host",
> >     "rsa": "ssh-rsa [..] root at host",
> >   }
> > }
> > [..]