bug#60657: Rethinking how service extensions work

[Ludovic Courtès]

Hi Bruno,

Bruno Victal <mirai@makinata.eu> skribis:

> On 2023-02-25 17:46, Ludovic Courtès wrote:

[...]

>> As we once discussed on IRC, the conclusion to me is that some of the
>> code currently implemented as activation snippets should rather be
>> implemented either as part of the ‘start’ method of the corresponding
>> Shepherd service, or as a one-shot Shepherd service that the main
>> service would depend on.
>
> I think moving them into the ‘start’ method is the best course of action.
> I'm considering the following changes:
> * Adding (gnu build activation) to %default-imported-modules + 
> %default-modules in (gnu services shepherd).
>   I expect that mkdir-p/perms is going to be used frequently enough, using 
> the number of activation-service
>   extensions in use as a rough estimate.
> * Refactor the activation extensions into the ‘start’ method, where it makes 
> sense to do so.

OK.  Cosmetic considerations: how about adding a ‘pre-start’ field in
<shepherd-service>?  That would allow us to keep the “setup” bit
visually separate from the actual ‘start’ method, even if under the hood
they get “merged” together:

  (shepherd-service
    ;; …
    (pre-start #~(mkdir-p "/whatever"))
    (start #~(make-forkexec-constructor …)))

> There's one issue I'm somewhat concerned about, consider the following 
> snippet:
>
>
> (define log-directory "/var/log")
> (define username "notroot")
>
> (start
>  #~(lambda _
>     (mkdir-p/perms #$log-directory (getpw #$username) #o750)
>     ...))
>
> This is somewhat pitfall prone since you most likely don't want to chown 
> /var/log to a non-root user.
> I'm unsure what's the best course to take here, would a simple file-exist? 
> check before mkdir-p/perms be sufficient?

We ensure /var/log exists before anything else—see ‘directives’ in (gnu
build install).

If we want an extra safety, we can add a real activation snippet that
does (mkdir-p "/var/log"), with the understanding that it would notably
run at boot time before shepherd is started.

> In either case, with or without refactoring this issue is already present 
> (but in activation-service extensions)
> so it's no worse than the status quo.

Right.

>> Note that this should prolly be declared as a ‘file-system’ rather than
>> as a custom service.  That way, it would get a “standard” Shepherd
>> service.
>> 
>> There are cases where we add explicit dependencies on
>> ‘file-system-/media/foo’ or similar.  <file-system> has a ‘dependencies’
>> field specifically for this purpose (info "(guix) File Systems").
>> 
>> Would that work for you?
>
> Unfortunately OverlayFS is filtered out from fstab by Guix (reported #60246) 
> and the dependencies field IMO is too restrictive,
> there should be a (sane) way to pass shepherd service symbols too. (for cases 
> where a file system depends on 'networking or
> depends on a particular interface e.g. NFS mount that uses a IPv6 link-local 
> address)

Sure, we could make these changes.  Let’s discuss it separately?

Thanks,
Ludo’.