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 79CAC1FF2D3 for ; Fri, 12 Jul 2024 11:11:29 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4E5A73F7AE; Fri, 12 Jul 2024 11:11:52 +0200 (CEST) Date: Fri, 12 Jul 2024 11:11:48 +0200 From: Christoph Heiss To: Stefan Hanreich Message-ID: References: <20240710132756.1149508-1-c.heiss@proxmox.com> <059dcd9d-97cb-4fa7-9cb8-465eab2730d4@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <059dcd9d-97cb-4fa7-9cb8-465eab2730d4@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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [answer.rs, unconfigured.sh, main.rs, http.rs, proxmox.com, proxmox-auto-installer.rs, sysinfo.rs, lib.rs, options.rs, utils.rs, setup.rs, partition.rs, parse-answer.rs] 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" On Thu, Jul 11, 2024 at 06:49:28PM GMT, Stefan Hanreich wrote: > Did a quick smoke test of this series by creating an ISO with an answer > file baked in and checking the response via `nc -l`. Review is inline. Thanks for the review & testing! >From a quick glance all the comments make sense, I'll address them in a v2. > > Consider this: > > Tested-By: Stefan Hanreich > Reviewed-By: Stefan Hanreich > > > On 7/10/24 15:27, Christoph Heiss wrote: > > This implements a mechanism for post-installation "notifications" via a > > POST request [0] when using the auto-installer. > > > > It's implemented as a separate, small utility to facilitate separation > > of concerns and make the information gathering easier by having it > > isolated in one place. > > > > Patches #1 through #10 are simply clean-ups, refactors, etc. that were > > done along the way, which do not impact functionality in any way. > > > > Most interesting here will be patch #12, which adds the actual > > implementation of the post-hook. (Bind-)mounting the installed host > > system is done using the existing `proxmox-chroot` utility, and the HTTP > > POST functionality can fortunately be re-used 1:1 from > > `proxmox-fetch-answer`. > > > > I've also included an example of how the JSON body (pretty-printed for > > readability) of such a post-installation request would look like below, > > for reference. > > > > Tested this with both PVE and PBS ISOs, PMG did not (yet) have a > > release with an auto-installation capable ISO. The only product-specific > > code is the version detection in `proxmox-post-hook`, which - since it > > works the same for PVE and PMG - be no obstacle. > > > > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5536 > > > > { > > "debian-version": "12.5", > > "product-version": "pve-manager/8.2.2/9355359cd7afbae4", > > "kernel-version": "proxmox-kernel-6.8.4-2-pve-signed", > > "boot-type": "bios", > > "filesystem": "zfs (RAID1)", > > "fqdn": "host.domain", > > "machine-id": "f4bf9711783248b7aaffe3ccbca3e3dc", > > "bootdisk": [ > > { > > "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", > > } > > } > > > > Christoph Heiss (14): chroot: print full anyhow message > > tree-wide: fix some typos > > tree-wide: collect hardcoded installer runtime directory strings into > > constant > > common: simplify filesystem type serializing & Display trait impl > > common: setup: serialize `target_hd` as string explicitly > > common: split out installer setup files loading functionality > > 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-installer: tests: replace left/right with got/expected in output > > auto-installer: answer: add `posthook` section > > fix #5536: add post-hook utility for sending notifications after > > auto-install > > fix #5536: post-hook: add some unit tests > > unconfigured.sh: run proxmox-post-hook after successful auto-install > > > > Cargo.toml | 11 + > > Makefile | 8 +- > > debian/control | 1 + > > debian/install | 1 + > > debian/rules | 9 + > > proxmox-auto-install-assistant/Cargo.toml | 14 +- > > proxmox-auto-installer/Cargo.toml | 15 +- > > proxmox-auto-installer/src/answer.rs | 27 +- > > .../src/bin/proxmox-auto-installer.rs | 15 +- > > proxmox-auto-installer/src/sysinfo.rs | 10 +- > > proxmox-auto-installer/src/utils.rs | 15 +- > > proxmox-auto-installer/tests/parse-answer.rs | 42 +- > > proxmox-chroot/Cargo.toml | 8 +- > > proxmox-chroot/src/main.rs | 19 +- > > proxmox-fetch-answer/Cargo.toml | 17 +- > > .../src/fetch_plugins/http.rs | 100 +--- > > .../src/fetch_plugins/partition.rs | 14 +- > > proxmox-installer-common/Cargo.toml | 26 +- > > proxmox-installer-common/src/http.rs | 94 ++++ > > proxmox-installer-common/src/lib.rs | 5 + > > proxmox-installer-common/src/options.rs | 109 ++-- > > proxmox-installer-common/src/setup.rs | 108 +--- > > proxmox-installer-common/src/utils.rs | 2 + > > proxmox-post-hook/Cargo.toml | 19 + > > proxmox-post-hook/src/main.rs | 498 ++++++++++++++++++ > > proxmox-tui-installer/Cargo.toml | 8 +- > > proxmox-tui-installer/src/setup.rs | 2 +- > > unconfigured.sh | 7 +- > > 28 files changed, 862 insertions(+), 342 deletions(-) > > create mode 100644 proxmox-installer-common/src/http.rs > > create mode 100644 proxmox-post-hook/Cargo.toml > > create mode 100644 proxmox-post-hook/src/main.rs > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel