Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

Thanks for chiming in!

On Fri, Nov 15, 2024 at 10:49:19AM +0100, Thomas Lamprecht wrote:
> Am 15.11.24 um 10:34 schrieb Christoph Heiss:
> > [..]
> > Should it be an enum then? I.e. only allowing certain values such as
> > - network-pre.target
> > - network.target
> > - network-online.target
> > - multi-user.target
>
> Yeah, I would not make it generic, just the two or three most common
> orderings, we can then extend it on potential future user demand.

Alright, that was my thought here too. A slight abstraction layer will
make things lot less confusing for users.

>
> I think before network, post network and finished boot, i.e. multi-user
> target seem enough for now.

Ack!

> [..]
> > Not sure if we could just use multi-user.target as a default target, but
> > systemd *should* pull it in and run it in the right ordering too with
> > e.g. {Before,Wants}=network-pre.target ?
>
> Isn't the WantedBy is more for defining the target the unit itself will
> be part of, or? Adapting that might indeed make sense, but a bit to long
> ago that I looked into systemd unit ordering/dependency semantics more
> closely.
>

Yeah, about the meaning AFAIU myself. I'll test with multi-user.target,
should then work in any case I think. But since we will control the
possible ordering targets anyway, it's not a big problem really after
all.


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

[pve-devel] [RFC PATCH installer 0/5] fix #5579: allow specifying optional first-boot script

[Christoph Heiss]

This implements #5579 [0] as proposed by Thomas [1].

Sending it as an RFC as probably some bike-shedding has still to be done
anyway, in regards of naming/filesystem locations etc. -- but should
give an good idea how it can/should work in the end.

Adds a new (optional) section to the auto-installer answer file called
`first-boot`, which can be used to the configure a script/executable
file to run on the first boot after the installation.

To used the baked-in (via the `proxmox-auto-install-assistant prepare-iso
--on-first-boot`) file from the ISO:

  [first-boot]
  source = "from-iso"

Or fetching it from a URL:

  [first-boot]
  source = "from-url"
  url = "http://example.com/first-boot"
  cert_fingerprint = ".." # if needed

The structure the section is mostly taken from the `network` section to
provide consistency.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=5579
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=5579#c5

Inner workings
==============

This creates a new package `proxmox-first-boot`, which only has a
systemd .service file. The service is conditioned to start based on the 
existence of a flag file in /var/lib/proxmox-first-boot, which will
be created by the installer if such a script is supplied by the user.

Q: The condition could maybe be set directly on the script instead of a
   separate file, since the executable file is only present when
   supplied anyway?

Testing
=======

Tested this with the latest (8.2-2) PVE ISO, both baking it into the ISO
and fetching it from an URL. Did not test with PBS explicitly (yet), but
I see no reason why it shouldn't work, as it is completely
product-agnostic.

Diffstat
========

Christoph Heiss (5):
  common: add function for issuing HTTP GET requests
  fix #5579: first-boot: add initial service packaging
  fix #5579: auto-install-assistant: enable baking in first-boot script
  fix #5579: auto-installer: add optional first-boot hook script
  fix #5579: install: copy over `proxmox-first-boot` script if present

 Makefile                                      | 13 +++-
 Proxmox/Install.pm                            | 20 ++++++
 debian/control                                |  7 ++
 debian/proxmox-first-boot.install             |  1 +
 debian/rules.proxmox-first-boot               | 13 ++++
 proxmox-auto-install-assistant/Cargo.toml     |  1 +
 proxmox-auto-install-assistant/src/main.rs    | 17 +++++
 proxmox-auto-installer/Cargo.toml             |  2 +-
 proxmox-auto-installer/src/answer.rs          | 27 +++++++
 .../src/bin/proxmox-auto-installer.rs         | 42 +++++++++--
 proxmox-auto-installer/src/utils.rs           | 15 +++-
 proxmox-first-boot/Makefile                   | 11 +++
 .../etc/proxmox-first-boot.service            | 16 +++++
 proxmox-installer-common/src/http.rs          | 71 +++++++++++++------
 proxmox-installer-common/src/lib.rs           |  3 +
 15 files changed, 227 insertions(+), 32 deletions(-)
 create mode 100644 debian/proxmox-first-boot.install
 create mode 100644 debian/rules.proxmox-first-boot
 create mode 100644 proxmox-first-boot/Makefile
 create mode 100644 proxmox-first-boot/etc/proxmox-first-boot.service

-- 
2.47.0



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

[pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

While there is the `systemd-first-boot.service`, it uses the
non-existence of `/etc/machine-id` as condition to run. As we already
set up that file in the installer ourselves, we cannot use that.

Instead our service depends on a custom flag file in
/var/lib/proxmox-first-boot and will only run if that is present.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Makefile                                         | 13 ++++++++++---
 debian/control                                   |  7 +++++++
 debian/proxmox-first-boot.install                |  1 +
 debian/rules.proxmox-first-boot                  | 13 +++++++++++++
 proxmox-first-boot/Makefile                      | 11 +++++++++++
 .../etc/proxmox-first-boot.service               | 16 ++++++++++++++++
 6 files changed, 58 insertions(+), 3 deletions(-)
 create mode 100644 debian/proxmox-first-boot.install
 create mode 100644 debian/rules.proxmox-first-boot
 create mode 100644 proxmox-first-boot/Makefile
 create mode 100644 proxmox-first-boot/etc/proxmox-first-boot.service

diff --git a/Makefile b/Makefile
index d85347f..a17f6c5 100644
--- a/Makefile
+++ b/Makefile
@@ -5,6 +5,10 @@ BUILDDIR ?= $(PACKAGE)-$(DEB_VERSION_UPSTREAM)
 
 DEB=$(PACKAGE)_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
 ASSISTANT_DEB=proxmox-auto-install-assistant_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
+FIRST_BOOT_DEB=proxmox-first-boot_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
+
+ALL_DEBS = $(DEB) $(ASSISTANT_DEB) $(FIRST_BOOT_DEB)
+
 DSC=$(PACKAGE)_$(DEB_VERSION).dsc
 
 CARGO ?= cargo
@@ -61,6 +65,7 @@ $(BUILDDIR):
 	  proxmox-tui-installer/ \
 	  proxmox-installer-common/ \
 	  proxmox-post-hook \
+	  proxmox-first-boot \
 	  test/ \
 	  $(SHELL_SCRIPTS) \
 	  $@.tmp
@@ -73,9 +78,10 @@ country.dat: country.pl
 
 deb: $(DEB)
 $(ASSISTANT_DEB): $(DEB)
+$(FIRST_BOOT_DEB): $(DEB)
 $(DEB): $(BUILDDIR)
 	cd $(BUILDDIR); dpkg-buildpackage -b -us -uc
-	lintian $(DEB) $(ASSISTANT_DEB)
+	lintian $(ALL_DEBS)
 
 test-$(DEB): $(INSTALLER_SOURCES)
 	rsync --exclude='test*.img' --exclude='*.deb' --exclude='build' -a * build
@@ -114,6 +120,7 @@ HTMLDIR=$(VARLIBDIR)/html/common
 install: $(INSTALLER_SOURCES) $(COMPILED_BINS)
 	$(MAKE) -C banner install
 	$(MAKE) -C Proxmox install
+	$(MAKE) -C proxmox-first-boot install
 	install -D -m 644 interfaces $(DESTDIR)/etc/network/interfaces
 	install -D -m 755 fake-start-stop-daemon $(VARLIBDIR)/fake-start-stop-daemon
 	install -D -m 755 policy-disable-rc.d $(VARLIBDIR)/policy-disable-rc.d
@@ -143,8 +150,8 @@ cargo-build:
 
 .PHONY: upload
 upload: UPLOAD_DIST ?= $(DEB_DISTRIBUTION)
-upload: $(DEB) $(ASSISTANT_DEB)
-	tar cf - $(DEB) $(ASSISTANT_DEB) | ssh -X repoman@repo.proxmox.com -- upload --product pve,pmg,pbs --dist $(UPLOAD_DIST)
+upload: $(ALL_DEBS)
+	tar cf - $(ALL_DEBS) | ssh -X repoman@repo.proxmox.com -- upload --product pve,pmg,pbs --dist $(UPLOAD_DIST)
 
 %.img:
 	truncate -s 2G $@
diff --git a/debian/control b/debian/control
index ff00cc2..fd0f4df 100644
--- a/debian/control
+++ b/debian/control
@@ -62,3 +62,10 @@ Description: Assistant to help with automated installations
  Provides a helper that can assist with creating an answer file for a automated
  installation of a Proxmox project, and preparing a official ISO image to use
  this answer file.
+
+Package: proxmox-first-boot
+Architecture: any
+Depends: ${misc:Depends},
+Description: Service which runs on the first system boot for additional setup
+ Provides a service which will run on the first boot if the a script was
+ configured through the auto-installer.
diff --git a/debian/proxmox-first-boot.install b/debian/proxmox-first-boot.install
new file mode 100644
index 0000000..52f25d2
--- /dev/null
+++ b/debian/proxmox-first-boot.install
@@ -0,0 +1 @@
+lib/systemd/system/proxmox-first-boot.service
diff --git a/debian/rules.proxmox-first-boot b/debian/rules.proxmox-first-boot
new file mode 100644
index 0000000..5b30d35
--- /dev/null
+++ b/debian/rules.proxmox-first-boot
@@ -0,0 +1,13 @@
+#!/usr/bin/make -f
+# See debhelper(7) (uncomment to enable)
+# output every command that modifies files on the build system.
+#DH_VERBOSE = 1
+
+%:
+	dh $@
+
+override_dh_missing:
+	dh_missing --fail-missing
+
+override_dh_installsystemd:
+	dh_installsystemd --no-stop-on-upgrade --no-start proxmox-first-boot.service
diff --git a/proxmox-first-boot/Makefile b/proxmox-first-boot/Makefile
new file mode 100644
index 0000000..137de23
--- /dev/null
+++ b/proxmox-first-boot/Makefile
@@ -0,0 +1,11 @@
+all:
+
+DESTDIR =
+LIBSYSTEMD_DIR = $(DESTDIR)/lib/systemd/system
+
+.PHONY: install
+install: etc/proxmox-first-boot.service
+	install -D -m 644 etc/proxmox-first-boot.service $(LIBSYSTEMD_DIR)/proxmox-first-boot.service
+
+.PHONY: clean
+clean:
diff --git a/proxmox-first-boot/etc/proxmox-first-boot.service b/proxmox-first-boot/etc/proxmox-first-boot.service
new file mode 100644
index 0000000..046bb24
--- /dev/null
+++ b/proxmox-first-boot/etc/proxmox-first-boot.service
@@ -0,0 +1,16 @@
+[Unit]
+Description=Proxmox First Boot Setup
+After=systemd-remount-fs.service
+Before=network-pre.target
+Wants=network-pre.target
+ConditionPathExists=/var/lib/proxmox-first-boot/pending-first-boot-setup
+ConditionPathIsReadWrite=/var/lib
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=/var/lib/proxmox-first-boot/proxmox-first-boot
+ExecStartPost=/usr/bin/rm -v /var/lib/proxmox-first-boot/pending-first-boot-setup
+
+[Install]
+WantedBy=network.target
-- 
2.47.0



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

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Thomas Lamprecht]

Am 13.11.24 um 14:59 schrieb Christoph Heiss:
> diff --git a/proxmox-first-boot/etc/proxmox-first-boot.service b/proxmox-first-boot/etc/proxmox-first-boot.service
> new file mode 100644
> index 0000000..046bb24
> --- /dev/null
> +++ b/proxmox-first-boot/etc/proxmox-first-boot.service
> @@ -0,0 +1,16 @@
> +[Unit]
> +Description=Proxmox First Boot Setup
> +After=systemd-remount-fs.service
> +Before=network-pre.target
> +Wants=network-pre.target

I now I mentioned above ordering in our off-list chat, and it seems correct
for the usecase where one needs to configure networking itself here.
But, when summarizing our chat in the bug report, I re-read the use-cases
and saw that there might be also some users that require the first-boot
script to have the network available, e.g. to pull further automation stuff in.

So it really would be great to allow overriding that ordering.

Simplest way might be to leave it out here, or well go for the default we want
(in doubt -> dice roll), and write out a systemd unit snippet during installation
depending on a additional setting from the answer file.


> +ConditionPathExists=/var/lib/proxmox-first-boot/pending-first-boot-setup
> +ConditionPathIsReadWrite=/var/lib
> +
> +[Service]
> +Type=oneshot
> +RemainAfterExit=yes
> +ExecStart=/var/lib/proxmox-first-boot/proxmox-first-boot
> +ExecStartPost=/usr/bin/rm -v /var/lib/proxmox-first-boot/pending-first-boot-setup
> +
> +[Install]
> +WantedBy=network.target



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

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

On Thu, Nov 14, 2024 at 09:23:48PM +0100, Thomas Lamprecht wrote:
> Am 13.11.24 um 14:59 schrieb Christoph Heiss:
> > diff --git a/proxmox-first-boot/etc/proxmox-first-boot.service b/proxmox-first-boot/etc/proxmox-first-boot.service
> > new file mode 100644
> > index 0000000..046bb24
> > --- /dev/null
> > +++ b/proxmox-first-boot/etc/proxmox-first-boot.service
> > @@ -0,0 +1,16 @@
> > +[Unit]
> > +Description=Proxmox First Boot Setup
> > +After=systemd-remount-fs.service
> > +Before=network-pre.target
> > +Wants=network-pre.target
>
> I now I mentioned above ordering in our off-list chat, and it seems correct
> for the usecase where one needs to configure networking itself here.
> But, when summarizing our chat in the bug report, I re-read the use-cases
> and saw that there might be also some users that require the first-boot
> script to have the network available, e.g. to pull further automation stuff in.
>
> So it really would be great to allow overriding that ordering.

I see, so probably introduce a `first-boot.ordering` (or similar)
key, defaulting to "network-pre"?

Should it be an enum then? I.e. only allowing certain values such as
- network-pre.target
- network.target
- network-online.target
- multi-user.target

Further we could include {local,remote}-fs.target and maybe ceph.target?

(All available can be listed with `systemctl list-units --type target`,
for reference.)

Or just be a freeform text field and let the user decide entirely by
themselves?

If we allow configuring that though, we might need to change WantedBy=
depending on that too.

Not sure if we could just use multi-user.target as a default target, but
systemd *should* pull it in and run it in the right ordering too with
e.g. {Before,Wants}=network-pre.target ?

>
> Simplest way might be to leave it out here, or well go for the default we want
> (in doubt -> dice roll), and write out a systemd unit snippet during installation
> depending on a additional setting from the answer file.
>


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

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Thomas Lamprecht]

Am 15.11.24 um 10:34 schrieb Christoph Heiss:
> I see, so probably introduce a `first-boot.ordering` (or similar)
> key, defaulting to "network-pre"?
> 
> Should it be an enum then? I.e. only allowing certain values such as
> - network-pre.target
> - network.target
> - network-online.target
> - multi-user.target

Yeah, I would not make it generic, just the two or three most common
orderings, we can then extend it on potential future user demand.

I think before network, post network and finished boot, i.e. multi-user
target seem enough for now.

> Further we could include {local,remote}-fs.target and maybe ceph.target?

Maybe, but wouldn't do that for now, ceph.target on a freshly installed
node seems a bit odd, the fs targets might be sensible, but I would wait
for actual user demand.

> Or just be a freeform text field and let the user decide entirely by
> themselves?

No, that is harder to understand IMO than simple "needed for network
communication itself", "needs working network to run"  and "needs fully
booted system to run".

> If we allow configuring that though, we might need to change WantedBy=
> depending on that too.

Changing that might probably make sense.

> Not sure if we could just use multi-user.target as a default target, but
> systemd *should* pull it in and run it in the right ordering too with
> e.g. {Before,Wants}=network-pre.target ?

Isn't the WantedBy is more for defining the target the unit itself will
be part of, or? Adapting that might indeed make sense, but a bit to long
ago that I looked into systemd unit ordering/dependency semantics more
closely.



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

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

On Thu, Nov 14, 2024 at 09:23:48PM +0100, Thomas Lamprecht wrote:
> [..]
> So it really would be great to allow overriding that ordering.
>
> Simplest way might be to leave it out here, or well go for the default we want
> (in doubt -> dice roll), and write out a systemd unit snippet during installation
> depending on a additional setting from the answer file.

Thinking about this again, while implementing - if we need to customize
the unit file, do we want to:

- create an `override.conf` file in
  `/etc/systemd/system/proxmox-first-boot.service.d/`, like systemd
  would do it when using `systemctl edit <service>`?
  Disadvantage is, that that file isn't removed when removing the
  `proxmox-first-boot` package from the system (although we could do it
  via postrm maybe?)

- edit the unit file directly? That would mean that e.g. `debsums -c`
  would complain.

- even bother with a separate package and instead just write the unit
  file directly to /etc/systemd/system?
  Disadvantage here is that it would be a "lost" file, not managed by
  dpkg, and administrators would have to remove the file directly, if
  they want/need to.

I have implemented the first variant for now, for testing. But not sure
if it's the best way to go.


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

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Thomas Lamprecht]

Am 15.11.24 um 14:34 schrieb Christoph Heiss:
> On Thu, Nov 14, 2024 at 09:23:48PM +0100, Thomas Lamprecht wrote:
>> [..]
>> So it really would be great to allow overriding that ordering.
>>
>> Simplest way might be to leave it out here, or well go for the default we want
>> (in doubt -> dice roll), and write out a systemd unit snippet during installation
>> depending on a additional setting from the answer file.
> 
> Thinking about this again, while implementing - if we need to customize
> the unit file, do we want to:
> 
> - create an `override.conf` file in
>   `/etc/systemd/system/proxmox-first-boot.service.d/`, like systemd
>   would do it when using `systemctl edit <service>`?
>   Disadvantage is, that that file isn't removed when removing the
>   `proxmox-first-boot` package from the system (although we could do it
>   via postrm maybe?)

I'd either use above or as an additional alternative: ship the different
variants as separate complete unit files with a common Alias (to convey
that they're the "same" thing) and enable only the one (manually) that is
configured. That would keep every file fully covered by the package
system.

> 
> - edit the unit file directly? That would mean that e.g. `debsums -c`
>   would complain.
> 
> - even bother with a separate package and instead just write the unit
>   file directly to /etc/systemd/system?
>   Disadvantage here is that it would be a "lost" file, not managed by
>   dpkg, and administrators would have to remove the file directly, if
>   they want/need to.
> 
> I have implemented the first variant for now, for testing. But not sure
> if it's the best way to go.
> 

Fine by me, but maybe you find some benefits on the separate unit file
one, not to hard feelings here.


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

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

On Fri, Nov 15, 2024 at 02:39:16PM +0100, Thomas Lamprecht wrote:
> Am 15.11.24 um 14:34 schrieb Christoph Heiss:
> > On Thu, Nov 14, 2024 at 09:23:48PM +0100, Thomas Lamprecht wrote:
> >> [..]
> >> So it really would be great to allow overriding that ordering.
> >>
> >> Simplest way might be to leave it out here, or well go for the default we want
> >> (in doubt -> dice roll), and write out a systemd unit snippet during installation
> >> depending on a additional setting from the answer file.
> >
> > Thinking about this again, while implementing - if we need to customize
> > the unit file, do we want to:
> >
> > - create an `override.conf` file in
> >   `/etc/systemd/system/proxmox-first-boot.service.d/`, like systemd
> >   would do it when using `systemctl edit <service>`?
> >   Disadvantage is, that that file isn't removed when removing the
> >   `proxmox-first-boot` package from the system (although we could do it
> >   via postrm maybe?)
>
> I'd either use above or as an additional alternative: ship the different
> variants as separate complete unit files with a common Alias (to convey
> that they're the "same" thing) and enable only the one (manually) that is
> configured. That would keep every file fully covered by the package
> system.

Didn't think of that variant, good idea! Thanks!
Should really cover all the cases I think and leave no files unmanaged.

(Also, TIL about Alias=)


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