On Fri, 7 Oct 2022 19:37:49 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:On Fri, Oct 07, 2022 at 02:47:42AM +0200, Stefano Brivio wrote:Sure, I can do it, but I wonder: wouldn't it be worth to eventually try and allow spawning an interactive shell from contexts, so that I can have a single setup function -- which looks definitely better to me, especially given the premises of the patch enabling test selection I just posted? Because, if that's feasible, I would rather keep this temporary regression, for simplicity.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).Of course, I tried, and:Sure. This is not even a problem actually, the shell will terminate and I have my small log file.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.Same here.+passt exit +check [ -s __LOG_FILE__ ] + +test Log truncated on creation +passt ./pasta -l __LOG_FILE__ +passt exitSame here.Yes, and it works with the context approach, I tested it until this point.+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.Not really, because I want to check the size of the log file several times during the test, and I can't do it from outside the mount namespace, hence this: pout SIZE cat __LOG_FILE__ | wc -c in check_log_size_mountns. This would be solved by a separate setup function like the one you mentioned, though. Having a separate context/pane to just check that looks a bit bad for usability (in terms of showing what's going on). So the question here is really if we can avoid having a separate setup function, by means of adapting the context mechanism to enable interactive shells. If we can't, fine. But if we can, I'd leave this as a temporary hack, because it's more usable than the alternative -- for example, I don't have to select two separate tests for this. -- Stefano+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.