Re: [PATCH 8/8] test: Add log file tests for pasta plus corresponding layout and setup

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 9749 bytes --]

On Fri, Oct 07, 2022 at 02:47:42AM +0200, Stefano Brivio wrote:
> To test log files on a tmpfs mount, we need to unshare the mount
> namespace, which means using a context for the passt pane is not
> really practical at the moment, as we can't open a shell there, so
> we would have to encapsulate all the commands under 'unshare -rUm',
> plus the "inner" pasta command, running in turn a tcp_rr server.
> 
> It might be worth fixing this by e.g. detecting we are trying to
> spawn an interactive shell and adding a special path in the context
> setup with some form of stdin redirection -- I'm not sure it's doable
> though.
> 
> For this reason, add a new layout, using a context only for the host
> pane, while keeping the old command dispatch mechanism for the passt
> pane.

Blecch.  I really hate going backwards with the test mechanisms like
this.  .. and I don't think you need to, though it will require a
slight rearrangement.

AIUI, you want to run basically the same tests against a logfile on
the main fs (probably ext4 or xfs) and on tmpfs.  So, I'd suggest one
test file that does the core tests, with two different setup functions
for the two cases.  For the normal fs test, set the passt context to
be a second host context, for the tmpfs test, set the passt context to
nsenter an unshare -rUm.

I don't think you need another context to run things within the pasta
environment, because AFAICT you can just use a fixed command for each
pasta invocation rather than a shell (see details below).

> We also need a new setup function that doesn't start pasta: we want
> to start and restart it with different options.
> 
> Further, we need a 'pint' directive, to send an interrupt to the
> passt pane: add that in lib/test.
> 
> All the tests before the one involving tmpfs and a detached mount
> namespace were also tested with the context mechanism. To make an
> eventual conversion easier, pass tcp_crr directly as a command on
> pasta's command line where feasible.
> 
> While at it, fix the comment to the teardown_pasta() function.
> 
> The new test set can be semi-conveniently run as:
> 
>   ./run pasta_options/log_to_file
> 
> and it checks basic log creation, size of the log file after flooding
> it with debug entries, rotations, and basic consistency after
> rotations, on both an existing filesystem and a tmpfs, chosen as
> it doesn't support collapsing data ranges via fallocate(), hence
> triggering the fall-back mechanism for logging rotation.
> 
> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
> ---
>  test/lib/layout                | 39 +++++++++++++++
>  test/lib/setup                 | 15 +++++-
>  test/lib/test                  |  3 ++
>  test/pasta_options/log_to_file | 90 ++++++++++++++++++++++++++++++++++
>  test/run                       |  4 ++
>  5 files changed, 150 insertions(+), 1 deletion(-)
>  create mode 100644 test/pasta_options/log_to_file
> 
> diff --git a/test/lib/layout b/test/lib/layout
> index 79a6c80..fcd1db4 100644
> --- a/test/lib/layout
> +++ b/test/lib/layout
> @@ -43,6 +43,45 @@ layout_host() {
>  	sleep 1
>  }
>  
> +# layout_pasta_simple() - Panes for host and pasta
> +layout_pasta_simple() {
> +	sleep 3
> +
> +	tmux kill-pane -a -t 0
> +	cmd_write 0 clear
> +
> +	tmux split-window -v -t passt_test
> +	tmux split-window -h -t passt_test
> +
> +	PANE_PASST=0
> +	PANE_HOST=1
> +	PANE_INFO=2
> +
> +	get_info_cols
> +
> +	tmux send-keys -l -t ${PANE_INFO} 'while cat '"$STATEBASE/log_pipe"'; do :; done'
> +	tmux send-keys -t ${PANE_INFO} -N 100 C-m
> +	tmux select-pane -t ${PANE_INFO} -T "test log"
> +
> +	if context_exists host; then
> +		pane_watch_contexts ${PANE_HOST} host host
> +	else
> +		tmux pipe-pane -O -t ${PANE_HOST} "cat >> ${LOGDIR}/pane_host.log"
> +		tmux select-pane -t ${PANE_HOST} -T "host"
> +	fi
> +
> +	if context_exists passt; then
> +		pane_watch_contexts ${PANE_PASST} host host
> +	else
> +		tmux pipe-pane -O -t ${PANE_PASST} "cat >> ${LOGDIR}/pane_passt.log"
> +		tmux select-pane -t ${PANE_PASST} -T "pasta"
> +	fi
> +
> +	info_layout "single pasta instance"
> +
> +	sleep 1
> +}
> +
>  # layout_pasta() - Panes for host, pasta, and separate one for namespace
>  layout_pasta() {
>  	sleep 3
> diff --git a/test/lib/setup b/test/lib/setup
> index 071e11d..e2d0ff0 100755
> --- a/test/lib/setup
> +++ b/test/lib/setup
> @@ -106,6 +106,13 @@ setup_pasta() {
>  	wait_for [ -f "${STATESETUP}/passt.pid" ]
>  }
>  
> +# setup_pasta_options() - Set up layout and host context without starting pasta
> +setup_pasta_options() {
> +	context_setup_host host
> +
> +	layout_pasta_simple
> +}
> +
>  # setup_passt_in_ns() - Set up namespace (with pasta), run qemu and passt into it
>  setup_passt_in_ns() {
>  	context_setup_host host
> @@ -294,7 +301,7 @@ teardown_passt() {
>  	teardown_context_watch ${PANE_GUEST} qemu guest
>  }
>  
> -# teardown_passt() - Exit namespace, kill pasta process
> +# teardown_pasta() - Exit namespace, kill pasta process
>  teardown_pasta() {
>  	${NSHOLDER} "${STATESETUP}/ns.hold" stop
>  	context_wait unshare
> @@ -304,6 +311,12 @@ teardown_pasta() {
>  	teardown_context_watch ${PANE_NS} unshare ns
>  }
>  
> +# teardown_pasta_options() - Tear down pasta and host context, no namespace
> +teardown_pasta_options() {
> +	teardown_context_watch ${PANE_HOST} host
> +	teardown_context_watch ${PANE_PASST} passt
> +}
> +
>  # teardown_passt_in_ns() - Exit namespace, kill qemu and pasta, remove pid file
>  teardown_passt_in_ns() {
>  	context_run ns kill $(cat "${STATESETUP}/qemu.pid")
> diff --git a/test/lib/test b/test/lib/test
> index 558d0f0..4c271a5 100755
> --- a/test/lib/test
> +++ b/test/lib/test
> @@ -137,6 +137,9 @@ test_one_line() {
>  	"passtw")
>  		pane_or_context_wait passt || TEST_ONE_nok=1
>  		;;
> +	"pint")
> +		tmux send-keys -t ${PANE_PASST} "C-c"
> +		;;
>  	"pout")
>  		__varname="${__arg%% *}"
>  		__output="$(pane_or_context_output passt "${__arg#* }")"
> diff --git a/test/pasta_options/log_to_file b/test/pasta_options/log_to_file
> new file mode 100644
> index 0000000..587bf8e
> --- /dev/null
> +++ b/test/pasta_options/log_to_file
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: AGPL-3.0-or-later
> +#
> +# PASST - Plug A Simple Socket Transport
> +#  for qemu/UNIX domain socket mode
> +#
> +# PASTA - Pack A Subtle Tap Abstraction
> +#  for network namespace/tap device mode
> +#
> +# test/pasta_options/log_to_file - Check log creation, rotations and consistency
> +#
> +# Copyright (c) 2022 Red Hat GmbH
> +# Author: Stefano Brivio <sbrivio(a)redhat.com>
> +
> +htools	wc tcp_rr tail cut tr sort
> +
> +def	flood_log_server
> +passtb	tcp_crr --nolog -P 10001 -C 10002 -6
> +sleep	1
> +endef
> +
> +def	flood_log_client
> +host	tcp_crr --nolog -P 10001 -C 10002 -6 -c -H ::1
> +endef
> +
> +def	check_log_size_mountns
> +pout	SIZE cat __LOG_FILE__ | wc -c
> +check	[ __SIZE__ -gt $((50 * 1024)) ]
> +check	[ __SIZE__ -lt $((100 * 1024)) ]
> +endef
> +
> +test	Log creation
> +
> +set	PORTS -t 10001,10002 -u 10001,10002
> +set	LOG_FILE __STATEDIR__/pasta.log
> +
> +passt	./pasta -l __LOG_FILE__

Here you can use 'true' instead of starting a shell then exiting.

> +passt	exit
> +check	[ -s __LOG_FILE__ ]
> +
> +test	Log truncated on creation
> +passt	./pasta -l __LOG_FILE__
> +passt	exit

Same here.

> +check	[ $(cat __LOG_FILE__ | wc -l) -eq 1 ]
> +
> +test	Maximum log size
> +passtb	./pasta --config-net -d -f -l __LOG_FILE__ --log-size $((100 * 1024)) -- sh -c 'while true; do tcp_crr --nolog -P 10001 -C 10002 -6; done'

Here you're already using a command.

> +sleep	1
> +
> +flood_log_client
> +check	[ $(cat __LOG_FILE__ | wc -c) -gt $((50 * 1024)) ]
> +check	[ $(cat __LOG_FILE__ | wc -c) -lt $((100 * 1024)) ]
> +
> +flood_log_client
> +check	[ $(cat __LOG_FILE__ | wc -c) -gt $((50 * 1024)) ]
> +check	[ $(cat __LOG_FILE__ | wc -c) -lt $((100 * 1024)) ]
> +
> +flood_log_client
> +check	[ $(cat __LOG_FILE__ | wc -c) -gt $((50 * 1024)) ]
> +check	[ $(cat __LOG_FILE__ | wc -c) -lt $((100 * 1024)) ]
> +
> +pint
> +
> +test	Timestamp consistency after rotations
> +check	tail -n +2 __LOG_FILE__ | cut -f1 -d' ' | tr -d [.:] | sort -c
> +
> +test	Maximum log size on tmpfs (no FALLOC_FL_COLLAPSE_RANGE)
> +passt	unshare -rUm
> +passt	mkdir __STATEDIR__/t
> +passt	mount -t tmpfs none __STATEDIR__/t
> +set	LOG_FILE __STATEDIR__/t/log
> +passt	./pasta --config-net -d -l __LOG_FILE__ --log-size $((100 * 1024))

Here I think you can use the same while true trick as above, rather
than starting the server repeatedly.

> +flood_log_server
> +flood_log_client
> +check_log_size_mountns
> +
> +flood_log_server
> +flood_log_client
> +check_log_size_mountns
> +
> +flood_log_server
> +flood_log_client
> +check_log_size_mountns
> +
> +test	Timestamp consistency after rotations (no FALLOC_FL_COLLAPSE_RANGE)
> +check	tail -n +2 __LOG_FILE__ | cut -f1 -d' ' | tr -d [.:] | sort -c
> +
> +passt	exit
> +passt	umount __STATEDIR__/t
> +passt	exit
> diff --git a/test/run b/test/run
> index 1ae270e..0bf79d4 100755
> --- a/test/run
> +++ b/test/run
> @@ -80,6 +80,10 @@ run() {
>  	test passt/shutdown
>  	teardown pasta
>  
> +	setup pasta_options
> +	test pasta_options/log_to_file
> +	teardown pasta_options
> +
>  	setup passt
>  	test passt/ndp
>  	test passt/dhcp

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]