[LU-9474] tests: register traps in a correct order

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.12.0, Lustre 2.10.7
• Labels: patch
• Severity: 3
• Assignee: Saurabh Tandan (Inactive)
• Reporter: CEA

Description

trap is used in many tests to set actions to perform on cleanup.

IMHO, trap should be registered in this order:

cleanup_test_XX()
{
    undo_something
}

test_XX()
{
    trap cleanup_test_XX EXIT
    do_something
    ...
}

Currently many tests do not respect this template.
Among those, some:

• register the trap after do_something();
• explicitly call cleanup_test_XX() at the end of the test;
• call trap 0 at the start of cleanup_test_XX() (this one may be controversial, I believe the test harness should provide, and probably already provides, trap isolation between tests).

Issue Links

• is related to: LU-10461 sanity 77c failure due to variables called in cleanup routine are out of scope
• is related to: LU-9475 tests: provide utilities to stack traps

Comments

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/30490/
Subject: LU-9474 tests: fix quoting in stack_trap
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 99420a1830b89a8aba6350b095065d65107f7c0f

[Gerrit Updater]

John L. Hammond ([e-mail]) merged in patch https://review.whamcloud.com/30371/
Subject: LU-9474 tests: add the stack_trap() utility function
Project: fs/lustre-release
Branch: b2_10
Current Patch Set:
Commit: eb9c7a25c8d62e549d1c41df0bf047bc2b1d63b4

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/30097/
Subject: LU-9474 tests: rewrite copytool_setup to use stack_trap
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 2042bcea662847c12cacdecd598b0a0d5fa4d805

[Gerrit Updater]

Quentin Bouget ([e-mail]) uploaded a new patch: https://review.whamcloud.com/30490
Subject: LU-9474 tests: fix quoting in stack_trap
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 8e086c102cbaabf3cf2be6645a2891064f057e13

[Gerrit Updater]

James Nunez ([e-mail]) uploaded a new patch: https://review.whamcloud.com/30371
Subject: LU-9474 tests: add the stack_trap() utility function
Project: fs/lustre-release
Branch: b2_10
Current Patch Set: 1
Commit: d4dbfd8ef76ce280fceb5c155cae44277a4fa919

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/29653/
Subject: LU-9474 tests: add the stack_trap() utility function
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 1dfda720c080e39890e5a659a30c8988ca498a7c

[Gerrit Updater]

Quentin Bouget ([e-mail]) uploaded a new patch: https://review.whamcloud.com/30098
Subject: LU-9474 tests: replace import_file with copytool import
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 400c76c39fa2157bcebc4ab3c606a3e6f0942740

[Gerrit Updater]

Quentin Bouget ([e-mail]) uploaded a new patch: https://review.whamcloud.com/30097
Subject: LU-9474 tests: rewrite copytool_setup to use stack_trap
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 85c1b03c86a6e3351e13eb5b8ca8e23e497886af

[Gerrit Updater]

Quentin Bouget ([e-mail]) uploaded a new patch: https://review.whamcloud.com/29653
Subject: LU-9474 tests: add the stack_trap() utility function
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: ec4ea64c9cefc6dd973c3955d18d6ba0255371f7

[Quentin Bouget (Inactive)]

Ultimately, the test is launched in run_one() which itself is called inside a subshell in run_one_logged(), so traps defined inside a test function should not leak to other tests and EXIT traps are already executed at the end of tests.

[Andreas Dilger]

I'm not sure why you think the "call trap 0 at start of trap function", "call trap function at end of test" is bad? That consolidates cleanup code that may be in the trap function, rather than duplicating it at the end of the test and ensures that the trap is cleared for this test. AFAIK, there is not yet a "trap 0" at the end of run_tests() though that is probably a good idea.