From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 653671FF2A0 for ; Mon, 15 Jul 2024 16:31:42 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EC1B25500; Mon, 15 Jul 2024 16:32:09 +0200 (CEST) Date: Mon, 15 Jul 2024 16:31:34 +0200 From: Christoph Heiss To: Thomas Lamprecht Message-ID: <7inqakvnzxyqmlrcgodrm6rvh2slger2dftnd3zdije2pizp3p@ehg3p6slrvxt> References: <20240710132756.1149508-1-c.heiss@proxmox.com> <58501401-6e7e-4304-bede-4fe5cee03fb1@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <58501401-6e7e-4304-bede-4fe5cee03fb1@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.017 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH installer 00/14] fix #5536: implement post-(auto-)installation notification mechanism X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Cc: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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@host", > > "ed25519": "ssh-ed25519 [..] root@host", > > "rsa": "ssh-rsa [..] root@host", > > } > > } > > [..] _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel