Re: [PATCH v3] expand-on-first-boot: Ensure that /tmp is writable

["'Florian Bezdeka' via isar-users" <isar-users@googlegroups.com>]

On Tue, 2024-09-03 at 11:05 +0200, Jan Kiszka wrote:
> On 03.09.24 09:20, 'MOESSBAUER, Felix' via isar-users wrote:
> > On Thu, 2024-08-15 at 07:07 +0300, Uladzimir Bely wrote:
> > > On Tue, 2024-08-13 at 13:32 +0300, Uladzimir Bely wrote:
> > > > On Tue, 2024-08-13 at 09:24 +0000, MOESSBAUER, Felix wrote:
> > > > > On Tue, 2024-08-13 at 12:17 +0300, Uladzimir Bely wrote:
> > > > > > On Thu, 2024-07-25 at 16:17 +0200, 'Clara Kowalsky' via isar-
> > > > > > users
> > > > > > wrote:
> > > > > > > By setting PrivateTmp, a new file system namespace is created
> > > > > > > for
> > > > > > > this
> > > > > > > service and private /tmp/<service>/tmp and
> > > > > > > /var/tmp/<service>/tmp
> > > > > > > subdirectories are mounted, which are only used for processes
> > > > > > > of
> > > > > > > this
> > > > > > > namespace. The service unit receives a mount unit dependency
> > > > > > > for
> > > > > > > all
> > > > > > > mounts required to access /tmp and /var/tmp.
> > > > > > > This ensures that the /tmp directory is writable for the
> > > > > > > service,
> > > > > > > as
> > > > > > > mktemp is used in expand-last-partition.sh and creates a
> > > > > > > temporary
> > > > > > > file.
> > > > > > > 
> > > > > > > Signed-off-by: Clara Kowalsky <clara.kowalsky@siemens.com>
> > > > > > > ---
> > > > > > >  .../expand-on-first-boot/files/expand-on-first-
> > > > > > > boot.service     
> > > > > > > > 
> > > > > > > 1
> > > > > > > +
> > > > > > >  1 file changed, 1 insertion(+)
> > > > > > > 
> > > > > > > diff --git a/meta/recipes-support/expand-on-first-
> > > > > > > boot/files/expand-
> > > > > > > on-first-boot.service b/meta/recipes-support/expand-on-first-
> > > > > > > boot/files/expand-on-first-boot.service
> > > > > > > index 90c92a39..8e76998b 100644
> > > > > > > --- a/meta/recipes-support/expand-on-first-boot/files/expand-
> > > > > > > on-
> > > > > > > first-boot.service
> > > > > > > +++ b/meta/recipes-support/expand-on-first-boot/files/expand-
> > > > > > > on-
> > > > > > > first-boot.service
> > > > > > > @@ -16,6 +16,7 @@ Type=oneshot
> > > > > > >  ExecStart=/usr/share/expand-on-first-boot/expand-last-
> > > > > > > partition.sh
> > > > > > >  ExecStartPost=-/bin/systemctl disable expand-on-first-
> > > > > > > boot.service
> > > > > > >  ExecStopPost=-/bin/systemctl disable expand-on-first-
> > > > > > > boot.service
> > > > > > > +PrivateTmp=true
> > > > > > >  
> > > > > > >  [Install]
> > > > > > >  WantedBy=sysinit.target
> > > > > > > -- 
> > > > > > > 2.45.2
> > > > > > > 
> > > > > > 
> > > > > > Hello all.
> > > > > > 
> > > > > > After few days having this patch merged we at least twice faced
> > > > > > the
> > > > > > issue in CI with running qemuamd64 machine, probably related to
> > > > > > the
> > > > > > applied patch.
> > > > > > 
> > > > > > Error message is "ERROR| No resize output while expected".
> > > > > > E.g.,
> > > > > > there
> > > > > > is no btrfs resize output in VM boot log.
> > > > > > 
> > > > > > The reason of non-running expand-on-first-boot serivce is:
> > > > > > 
> > > > > > ```
> > > > > > [    5.578636] systemd[1]: local-fs-pre.target: Job expand-on-
> > > > > > first-
> > > > > > boot.service/start deleted to break ordering cycle starting
> > > > > > with
> > > > > > local-
> > > > > > fs-pre.target/start
> > > > > > ```
> > > > > 
> > > > > Interesting, I observed this same issue as well, but thought it
> > > > > comes
> > > > > from a downstream part. You're right, this cannot work:
> > > > > 
> > > > > Citing systemd.exec:
> > > > > 
> > > > > Similarly, units with PrivateTmp= enabled automatically get mount
> > > > > unit
> > > > > dependencies for all mounts required to access /tmp/ and
> > > > > /var/tmp/.
> > > > > They will also gain an automatic After= dependency on systemd-
> > > > > tmpfiles-
> > > > > setup.service(8). 
> > > > > 
> > > > > If /var is the partition to be resized, this will break.
> > > > > 
> > > > > Felix
> > > > > 
> > > > 
> > > > The dependency conflict seems to be here:
> > > > 
> > > > - expand-on-first-boot.service
> > > > Before=local-fs-pre.target
> > > > PrivateTmp=true # This means implicit "After=systemd-tmpfiles-
> > > > setup.service" dependency0, according to 
> > > > https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html
> > > > 
> > > > - systemd-tmpfiles-setup.service 
> > > > After=local-fs.target
> > > > 
> > > > - local-fs.target
> > > > After=local-fs-pre.target
> > > > 
> > > > 
> > > 
> > > Finally, does this all mean we need to revert this v3 patch and get
> > > back to "[PATCH v2] expand-on-first-boot: Add /tmp to
> > > ConditionPathIsReadWrite" variant?
> > 
> > The conditions are evaluated right before the service starts. By that,
> > we might have non-deterministic behavior, depending on which service
> > mounts /tmp (if at all) and when it is started relative to the expand-
> > on-first-boot.
> > 
> > I'm wondering if we should better create our own tmpfs in combination
> > with TMPDIR, just for that service (and drop it after execution). For
> > obvious reasons, the expanding needs to happen VERY early, but at that
> > point in time not much can be assumed about the rootfs.
> > 
> > CC'ing Florian.
> 
> In many cases, expansion can ran also after the system is fully booted
> and operational - unless it is then already complaining about too little
> disk space ;)

Not sure if I can help here. expand-on-first-boot tends to explode
every time we touch it. It seems that we don't have tests for most of
the use cases (like encrypted disks, ...) which makes it hard to prove
the correctness.

Couple of things I noticed while scanning / looking at the code:

- Why do we require /etc to be writable? We only read /etc/fstab
  right?

- I think all relevant file systems support growing the file system
  while being active / online / mounted. Maybe we don't have to run 
  so early? Might help to reduce rootfs requirements.

- Do we still need expand-on-first-boot as it is right now? Seems
  systemd provides x-systemd.growfs flags in /etc/fstab. Which use
  cases are not supported by that systemd feature? Could we migrate?

- According to the patch description we need /tmp (or any tmpfs) 
  so that mktemp is working. 

  We use the generated tmp directory as mount point only. We never 
  add / write any files to it. Can't we just create any "random" 
  (in terms of hardcoded...) directory for that to get rid of the
  systemd dependency?

Best regards,
Florian

> 
> Jan
> 
> -- 
> Siemens AG, Technology
> Linux Expert Center

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/9a103f6ae148f7183bea97c3493ee86e30da6b71.camel%40siemens.com.