Re: [pve-devel] [PATCH v3 00/30] add automated/unattended installation

[Aaron Lauterer <a.lauterer@proxmox.com>]

thanks for looking into this

On  2024-04-02  16:43, Christoph Heiss wrote:
> Rebuild an (PVE) ISO to contain the new auto-installer with an
> appropriate GRUB entry setting `proxauto` on the kernel commandline.
> 
> What I've tried:
> 
> - Using a few different values for `global` options
> - Install on ext4, xfs, Btrfs RAID1 and ZFS RAID1
>    (with different values in multiple runs)
> - Using DHCP and static IP
> - Fetching answer from a HTTP source, getting the URL from DHCP
> - Trying out the `proxmox-autoinst-helper` tool for assembling udev
>    rules and validating files.
> - Using the `post_command` to create some files in the newly installed
>    system.
> 
> I didn't play around all that extensively the udev filters.
> 
> Some notes:
> 
> - When using ext4 or xfs as filesystem, `disk_list` happily takes
>    multiple disks (but really only installs on the first disk, of
>    course). Should probably be another sanity check there.

Good idea. I'll add a check for this.

> 
> - As for the `proxmox-autoinst-helper answer` command, might `validate`
>    or `validate-answer` be a better name?
>    `answer` alone seems a bit confusing at first, as if e.g. the tool
>    would give me an answer file or similar.

Hmm, yeah. I see your point and will change that.

> 
> - `{pre,post}_command` in the answer file should be spelled plural, as
>    it is an array of commands after all.

good point
> 
> - While trying out different configurations, I wondered if for the
>    network something like this would be better for static IPs:
> 
>      [network.manual]
>      cidr = ".."
>      dns = ".."
>      [..]
> 
>    .. keeping the `network.use_dhcp` option as before. Would simplify
>    some checks now and provide good future-proofing for any new options
>    that might get added.
> 
>    Thereby basically modelling
>    `proxmox_auto_installer::answer::NetworkSettings` enum nearly 1:1 to
>    the TOML config.

okay, so that in the DHPC case, it could be
[network]
use_dhcp = true

and in the manual case, either
[network]
manual.cidr = "…"
manual.dns = "…"

and so forth, or, to keep it simpler, like your example with 
[network.manual] defining the overall manual key.

This will make it slightly more elaborate to document, as we need to dig 
deeper into how TOML works and that there are multiple ways to define 
the same hierarchy. But it could be worth it to keep the definition cleaner.

Some more feedback in that regard might be useful, especially since 
changing the format later on will be, as you described it, a PITA :)

> 
> Nitpicking at this point, I know, but changing the answer format
> afterwards would be a quite a PITA :^)
> 
> Overall very nice! and
> 
> Tested-by: Christoph Heiss <c.heiss@proxmox.com>
> 
> in any case.