Hello Maxim,

thank you for the review. :)  Responses below.

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Tomas,
>
> Tomas Volf <~@wolfsden.cz> writes:
>
>> Depending on networking is not enough in some setups, so a language clarifying
>> that and an example of network-online service.
>>
>> * doc/guix.texi (File Systems): Document the possible need for network-online.
>>
>> Change-Id: I8abe07cc9d6dc61f28eeea7ffa785eb8c9e8fd09
>> ---
>>  doc/guix.texi | 28 +++++++++++++++++++++++++++-
>>  1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/guix.texi b/doc/guix.texi
>> index 26488b41c8..861e78e6d2 100644
>> --- a/doc/guix.texi
>> +++ b/doc/guix.texi
>> @@ -18018,7 +18018,33 @@ File Systems
>>  met before mounting the file system.
>>
>>  As an example, an NFS file system would typically have a requirement for
>> -@code{networking}.
>> +@code{networking}.  However be aware that depending on @code{networking}
>> +might not be sufficient in some setups and a variant of custom
>> +@code{network-online} service might be necessary.
>
> Could we be more precise and detail a simple case or example of when
> requiring 'network-online is needed over just 'network for NFS?

It would be the case for any setup where 'networking being marked as
started does not mean the network is fully configured.  I personally
encountered the issue with dhcp-client-service-type.

I will send a v2 that will go into more details than "in some setups".

>
>> Example:
>> +
>> +@lisp
>> +(simple-service 'network-online shepherd-root-service-type
>> +  (list
>> +   (shepherd-service
>> +    (requirement '(networking))
>> +    (provision '(network-online))
>> +    (documentation "Wait for the network to come up.")
>> +    (start #~(lambda _
>> +               (let* ((cmd
>> +                       "set -eux
>> +c=0
>> +while ! /run/setuid-programs/ping -qc1 -W1 example.org; do
>> +	sleep 1
>> +	[ \"$((c += 1))\" -lt 30 ] || exit 1  # Limit the wait time
>> +done
>> +")
>> +                    (status (system cmd)))
>> +               (= 0 (status:exit-val status)))))
>
> I'm pretty sure we have connectivity tests already in the Guix test
> suite that must make use of Guile; that would be nicer, I think; for
> example, the (guix tests) module has:
>
> (define (network-reachable?)
>   "Return true if we can reach the Internet."
>   (false-if-exception (getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)))
>

This procedure however (despite the name) does not test whether network
is reachable, it tests whether single specific host name is resolvable.
That is not an equivalent of the example 'network-online service.

1. The procedure succeeds on completely offline machine, assuming there
   is a record for the host in /etc/hosts.  This is not an hypothetical
   problem, in my configuration the actual host name I ping (instead of
   example.org above) *is* in /etc/hosts.

2. Even if the DNS server works, that does not mean that the route to
   the host being checked is configured (yet).  That could happen when
   the route is (for example) created by VPN, so it needs some extra
   time to come up.

I do not see a way to do a "ping" using Guile's standard library, and
while I *could* implement it using raw sockets (the service is running
as a root after all), that seems like an overkill for just an example
snippet of code.

I mean, I am not happy about shelling to the /run/setuid-programs/ping.
But I did not figure out a Guile-only way to test what I want to test
(specific host is reachable) that would fit in similar amount of code.

What I could do is to replace the example with something like
"Implementing the 'network-online service is left as an exercise to the
reader.".  Would that work for you?

>> +   ;; Ordering for one-shot? services does not currently work.
>> +   ;; https://issues.guix.gnu.org/74284
>> +   ;; (one-shot? #t)
>> +   )))
>> +@end lisp
>
> Otherwise, it looks like a useful addition.

Will send a v2 once we clarify what to do about the ping.

Thanks,
Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.