diff mbox series

rpc_lib.sh: fix portmapper detection in case of socket activation

Message ID 20220120143727.27057-1-nikita.yushchenko@virtuozzo.com
State Rejected
Headers show
Series rpc_lib.sh: fix portmapper detection in case of socket activation | expand

Commit Message

Nikita Yushchenko Jan. 20, 2022, 2:37 p.m. UTC
On systemd-based linux hosts, rpcbind service is typically started via
socket activation, when the first client connects. If no client has
connected before LTP rpc test starts, rpcbind process will not be
running at the time of check_portmap_rpcbind() execution, causing
check_portmap_rpcbind() to report TCONF error.

Fix that by adding a quiet invocation of 'rpcinfo' before checking for
rpcbind.

For portmap, similar step is likely not needed, because portmap is used
only on old systemd and those don't use systemd.

Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
---
 testcases/network/rpc/basic_tests/rpc_lib.sh | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Petr Vorel Jan. 20, 2022, 9:01 p.m. UTC | #1
Hi Nikita,

[ Cc: Steve as user-space maintainer, also Neil and whole linux-nfs ]

> On systemd-based linux hosts, rpcbind service is typically started via
> socket activation, when the first client connects. If no client has
> connected before LTP rpc test starts, rpcbind process will not be
> running at the time of check_portmap_rpcbind() execution, causing
> check_portmap_rpcbind() to report TCONF error.

> Fix that by adding a quiet invocation of 'rpcinfo' before checking for
> rpcbind.

Looks reasonable, but I'd prefer to have confirmation from NFS experts.

> For portmap, similar step is likely not needed, because portmap is used
> only on old systemd and those don't use systemd.

> Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
> ---
>  testcases/network/rpc/basic_tests/rpc_lib.sh | 6 ++++++
>  1 file changed, 6 insertions(+)

> diff --git a/testcases/network/rpc/basic_tests/rpc_lib.sh b/testcases/network/rpc/basic_tests/rpc_lib.sh
> index c7c868709..e882e41b3 100644
> --- a/testcases/network/rpc/basic_tests/rpc_lib.sh
> +++ b/testcases/network/rpc/basic_tests/rpc_lib.sh
> @@ -8,6 +8,12 @@ check_portmap_rpcbind()
>  	if pgrep portmap > /dev/null; then
>  		PORTMAPPER="portmap"
>  	else
> +		# In case of systemd socket activation, rpcbind could be
> +		# not started until somebody tries to connect to it's socket.
> +		#
> +		# To handle that case properly, run a client now.
> +		rpcinfo >/dev/null 2>&1
nit: Shouldn't we keep stderr? In LTP we put required commands into
$TST_NEEDS_CMDS. It'd be better not require rpcinfo (not a hard dependency),
and thus it'd be better to see "command not found" when rpcinfo missing and test
fails.

Kind regards,
Petr

> +
>  		pgrep rpcbind > /dev/null && PORTMAPPER="rpcbind" || \
>  			tst_brk TCONF "portmap or rpcbind is not running"
>  	fi
Nikita Yushchenko Jan. 21, 2022, 4:57 a.m. UTC | #2
21.01.2022 00:01, Petr Vorel wrote:
> Hi Nikita,
> 
> [ Cc: Steve as user-space maintainer, also Neil and whole linux-nfs ]
> 
>> On systemd-based linux hosts, rpcbind service is typically started via
>> socket activation, when the first client connects. If no client has
>> connected before LTP rpc test starts, rpcbind process will not be
>> running at the time of check_portmap_rpcbind() execution, causing
>> check_portmap_rpcbind() to report TCONF error.
> 
>> Fix that by adding a quiet invocation of 'rpcinfo' before checking for
>> rpcbind.
> 
> Looks reasonable, but I'd prefer to have confirmation from NFS experts.

NFS is not involved here, this is about sunrpc tests.

I had to add this patch to make 'runltp -f net.rpc' pass just after container is started - that happens 
in container autotests here.

Nikita
Petr Vorel Jan. 21, 2022, 5:29 a.m. UTC | #3
Hi Nikita,

> 21.01.2022 00:01, Petr Vorel wrote:
> > Hi Nikita,

> > [ Cc: Steve as user-space maintainer, also Neil and whole linux-nfs ]

> > > On systemd-based linux hosts, rpcbind service is typically started via
> > > socket activation, when the first client connects. If no client has
> > > connected before LTP rpc test starts, rpcbind process will not be
> > > running at the time of check_portmap_rpcbind() execution, causing
> > > check_portmap_rpcbind() to report TCONF error.

> > > Fix that by adding a quiet invocation of 'rpcinfo' before checking for
> > > rpcbind.

> > Looks reasonable, but I'd prefer to have confirmation from NFS experts.

> NFS is not involved here, this is about sunrpc tests.
Sure. Just tirpc (in libtirpc or the the old SUN-RPC already removed from glibc)
are used in NFS. Steve is the libtirpc maintainer.

> I had to add this patch to make 'runltp -f net.rpc' pass just after
> container is started - that happens in container autotests here.
Yep, I suspected this. Because on normal linux distro it's working right after
boot (tested on rpc01.sh). Can't this be a setup issue?

Kind regards,
Petr

> Nikita
Nikita Yushchenko Jan. 21, 2022, 5:41 a.m. UTC | #4
> 
>> I had to add this patch to make 'runltp -f net.rpc' pass just after
>> container is started - that happens in container autotests here.
> Yep, I suspected this. Because on normal linux distro it's working right after
> boot (tested on rpc01.sh). Can't this be a setup issue?

This depends on what is installed and how it is configured.

But definitely the state with rpcbind process not running and systemd is listening on rpcbind sockets - 
is valid.

In the setup where the issue was caught, the test harness creates a container with minimal centos8 setup 
inside, boots it, and starts ltp inside.

Just reproduced manually:

[root@vz8 ~]# vzctl start 1000
Starting Container ...
...
[root@vz8 ~]# vzctl enter 1000
entered into CT 1000
CT-1000 /# pidof rpcbind
CT-1000 /# rpcinfo > /dev/null 2>&1
CT-1000 /# pidof rpcbind
678
CT-1000 /#
Petr Vorel Jan. 21, 2022, 6:30 a.m. UTC | #5
> > > I had to add this patch to make 'runltp -f net.rpc' pass just after
> > > container is started - that happens in container autotests here.
> > Yep, I suspected this. Because on normal linux distro it's working right after
> > boot (tested on rpc01.sh). Can't this be a setup issue?

> This depends on what is installed and how it is configured.

> But definitely the state with rpcbind process not running and systemd is
> listening on rpcbind sockets - is valid.

> In the setup where the issue was caught, the test harness creates a
> container with minimal centos8 setup inside, boots it, and starts ltp
> inside.

> Just reproduced manually:

> [root@vz8 ~]# vzctl start 1000
> Starting Container ...
> ...
> [root@vz8 ~]# vzctl enter 1000
> entered into CT 1000
> CT-1000 /# pidof rpcbind
> CT-1000 /# rpcinfo > /dev/null 2>&1
> CT-1000 /# pidof rpcbind
> 678
> CT-1000 /#

Thanks for info. I'm asking because if it's a setup bug it should not be hidden
by workaround but reported. I suppose normal Centos8 VM works.

Kind regards,
Petr
Nikita Yushchenko Jan. 21, 2022, 6:50 a.m. UTC | #6
>> Just reproduced manually:
> 
>> [root@vz8 ~]# vzctl start 1000
>> Starting Container ...
>> ...
>> [root@vz8 ~]# vzctl enter 1000
>> entered into CT 1000
>> CT-1000 /# pidof rpcbind
>> CT-1000 /# rpcinfo > /dev/null 2>&1
>> CT-1000 /# pidof rpcbind
>> 678
>> CT-1000 /#
> 
> Thanks for info. I'm asking because if it's a setup bug it should not be hidden
> by workaround but reported. I suppose normal Centos8 VM works.

I's say this is starting service on demand and this is exactly what socket activation is designed for.
NeilBrown Jan. 21, 2022, 8:44 p.m. UTC | #7
On Fri, 21 Jan 2022, Petr Vorel wrote:
> Hi Nikita,
> 
> [ Cc: Steve as user-space maintainer, also Neil and whole linux-nfs ]
> 
> > On systemd-based linux hosts, rpcbind service is typically started via
> > socket activation, when the first client connects. If no client has
> > connected before LTP rpc test starts, rpcbind process will not be
> > running at the time of check_portmap_rpcbind() execution, causing
> > check_portmap_rpcbind() to report TCONF error.
> 
> > Fix that by adding a quiet invocation of 'rpcinfo' before checking for
> > rpcbind.
> 
> Looks reasonable, but I'd prefer to have confirmation from NFS experts.
> 
> > For portmap, similar step is likely not needed, because portmap is used
> > only on old systemd and those don't use systemd.
> 
> > Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
> > ---
> >  testcases/network/rpc/basic_tests/rpc_lib.sh | 6 ++++++
> >  1 file changed, 6 insertions(+)
> 
> > diff --git a/testcases/network/rpc/basic_tests/rpc_lib.sh b/testcases/network/rpc/basic_tests/rpc_lib.sh
> > index c7c868709..e882e41b3 100644
> > --- a/testcases/network/rpc/basic_tests/rpc_lib.sh
> > +++ b/testcases/network/rpc/basic_tests/rpc_lib.sh
> > @@ -8,6 +8,12 @@ check_portmap_rpcbind()
> >  	if pgrep portmap > /dev/null; then
> >  		PORTMAPPER="portmap"
> >  	else
> > +		# In case of systemd socket activation, rpcbind could be
> > +		# not started until somebody tries to connect to it's socket.
> > +		#
> > +		# To handle that case properly, run a client now.
> > +		rpcinfo >/dev/null 2>&1

If it were me, I would remove the 'pgrep's and just call "rpcbind -p"
and make sure something responds.

NeilBrown



> nit: Shouldn't we keep stderr? In LTP we put required commands into
> $TST_NEEDS_CMDS. It'd be better not require rpcinfo (not a hard dependency),
> and thus it'd be better to see "command not found" when rpcinfo missing and test
> fails.
> 
> Kind regards,
> Petr
> 
> > +
> >  		pgrep rpcbind > /dev/null && PORTMAPPER="rpcbind" || \
> >  			tst_brk TCONF "portmap or rpcbind is not running"
> >  	fi
> 
>
Petr Vorel Jan. 24, 2022, 6:09 a.m. UTC | #8
> On Fri, 21 Jan 2022, Petr Vorel wrote:
> > Hi Nikita,

> > [ Cc: Steve as user-space maintainer, also Neil and whole linux-nfs ]

> > > On systemd-based linux hosts, rpcbind service is typically started via
> > > socket activation, when the first client connects. If no client has
> > > connected before LTP rpc test starts, rpcbind process will not be
> > > running at the time of check_portmap_rpcbind() execution, causing
> > > check_portmap_rpcbind() to report TCONF error.

> > > Fix that by adding a quiet invocation of 'rpcinfo' before checking for
> > > rpcbind.

> > Looks reasonable, but I'd prefer to have confirmation from NFS experts.

> > > For portmap, similar step is likely not needed, because portmap is used
> > > only on old systemd and those don't use systemd.

> > > Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
> > > ---
> > >  testcases/network/rpc/basic_tests/rpc_lib.sh | 6 ++++++
> > >  1 file changed, 6 insertions(+)

> > > diff --git a/testcases/network/rpc/basic_tests/rpc_lib.sh b/testcases/network/rpc/basic_tests/rpc_lib.sh
> > > index c7c868709..e882e41b3 100644
> > > --- a/testcases/network/rpc/basic_tests/rpc_lib.sh
> > > +++ b/testcases/network/rpc/basic_tests/rpc_lib.sh
> > > @@ -8,6 +8,12 @@ check_portmap_rpcbind()
> > >  	if pgrep portmap > /dev/null; then
> > >  		PORTMAPPER="portmap"
> > >  	else
> > > +		# In case of systemd socket activation, rpcbind could be
> > > +		# not started until somebody tries to connect to it's socket.
> > > +		#
> > > +		# To handle that case properly, run a client now.
> > > +		rpcinfo >/dev/null 2>&1

> If it were me, I would remove the 'pgrep's and just call "rpcbind -p"
> and make sure something responds.

Hi Neil,

I guess you mean: rpcinfo -p

Good idea, thanks!

Kind regards,
Petr

> NeilBrown



> > nit: Shouldn't we keep stderr? In LTP we put required commands into
> > $TST_NEEDS_CMDS. It'd be better not require rpcinfo (not a hard dependency),
> > and thus it'd be better to see "command not found" when rpcinfo missing and test
> > fails.

> > Kind regards,
> > Petr

> > > +
> > >  		pgrep rpcbind > /dev/null && PORTMAPPER="rpcbind" || \
> > >  			tst_brk TCONF "portmap or rpcbind is not running"
> > >  	fi
Petr Vorel Jan. 24, 2022, 9:11 p.m. UTC | #9
For a record, merged alternative solution:
eb786468b ("rpc_lib.sh: Check for running RPC")

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/network/rpc/basic_tests/rpc_lib.sh b/testcases/network/rpc/basic_tests/rpc_lib.sh
index c7c868709..e882e41b3 100644
--- a/testcases/network/rpc/basic_tests/rpc_lib.sh
+++ b/testcases/network/rpc/basic_tests/rpc_lib.sh
@@ -8,6 +8,12 @@  check_portmap_rpcbind()
 	if pgrep portmap > /dev/null; then
 		PORTMAPPER="portmap"
 	else
+		# In case of systemd socket activation, rpcbind could be
+		# not started until somebody tries to connect to it's socket.
+		#
+		# To handle that case properly, run a client now.
+		rpcinfo >/dev/null 2>&1
+
 		pgrep rpcbind > /dev/null && PORTMAPPER="rpcbind" || \
 			tst_brk TCONF "portmap or rpcbind is not running"
 	fi