Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not clean up when initialization failed on FreeBSD. #246

Open
Rin0913 opened this issue Dec 18, 2024 · 11 comments
Open

Not clean up when initialization failed on FreeBSD. #246

Rin0913 opened this issue Dec 18, 2024 · 11 comments

Comments

@Rin0913
Copy link

Rin0913 commented Dec 18, 2024

I am running Kyua on my FreeBSD VM.

One testing program would launch a jail and operate the jail network, and before so, it would create epair interfaces.

I found that it did clean up when the test fails or successes. However, when the test fails in initialization stage, the epair interfaces would not be destroyed.

I guess this is a bug on Kyua.

@ngie-eign
Copy link
Contributor

@ihoro : fyi

@markjdb
Copy link
Member

markjdb commented Dec 18, 2024

kyua doesn't create epair interfaces. Tests do. They're responsible for cleaning up; if they don't, it's a test bug.

@ihoro
Copy link
Member

ihoro commented Dec 18, 2024

@Rin0913, thanks for reporting. More details are needed to proceed with the analysis. According to the previous comments it may turn out to be a defect in the test itself.

@Rin0913
Copy link
Author

Rin0913 commented Dec 18, 2024

@ihoro @markjdb

Sorry for making confusing.
What I meant is Kyua didn't call the clean up method in the test, and I guess it should do so?

@markjdb
Copy link
Member

markjdb commented Dec 18, 2024

It's impossible to say without knowing which test you're talking about. Please let us know how to reproduce the problem you're seeing.

@Rin0913
Copy link
Author

Rin0913 commented Dec 18, 2024

The test failed because the jail already exists.

$ ifconfig 
...
epair60b: flags=1008842<BROADCAST,RUNNING,SIMPLEX,MULTICAST,LOWER_UP> metric 0 mtu 1500
        options=8<VLAN_MTU>
        ether 02:aa:e9:13:fb:0b
        groups: epair
        media: Ethernet 10Gbase-T (10Gbase-T <full-duplex>)
        status: active
        maclabel biba/equal(equal-equal)
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>

$ sudo kyua test test_routing_l3.py:TestIfOps::test_change_prefix_route[inet6]
test_routing_l3.py:TestIfOps::test_change_prefix_route[inet6]  ->  failed: /usr/tests/atf_python/sys/net/vnet.py:293: ValueError  [0.238s]

$ ifconfig 
...
epair62b: flags=1008842<BROADCAST,RUNNING,SIMPLEX,MULTICAST,LOWER_UP> metric 0 mtu 1500
        options=8<VLAN_MTU>
        ether 02:7d:d2:49:0d:0b
        groups: epair
        media: Ethernet 10Gbase-T (10Gbase-T <full-duplex>)
        status: active
        maclabel biba/equal(equal-equal)
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>

After removing the existing jail:

$ sudo kyua test test_routing_l3.py:TestIfOps::test_change_prefix_route[inet6]
test_routing_l3.py:TestIfOps::test_change_prefix_route[inet6]  ->  passed  [0.279s]

Results file id is usr_tests_sys_net_routing.20241218-211919-533112
Results saved to /root/.kyua/store/results.usr_tests_sys_net_routing.20241218-211919-533112.db

1/1 passed (0 broken, 0 failed, 0 skipped)

$ ifconfig
...
epair62b: flags=1008842<BROADCAST,RUNNING,SIMPLEX,MULTICAST,LOWER_UP> metric 0 mtu 1500
        options=8<VLAN_MTU>
        ether 02:7d:d2:49:0d:0b
        groups: epair
        media: Ethernet 10Gbase-T (10Gbase-T <full-duplex>)
        status: active
        maclabel biba/equal(equal-equal)
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>

I was running the tests under /usr/tests/sys/net/routing, with a generic_cleanup.sh inside:

srcdir=`dirname $0`
. ${srcdir}/../../common/vnet.subr

vnet_cleanup

@Rin0913
Copy link
Author

Rin0913 commented Dec 19, 2024

Sorry, let me be clear: when setting up part fails, the cleaning up part seems not being executed.
However, the cleaning up part is well-implemented, and would be executed when the test finished.
Therefore, I think it may be a bug of Kyua instead of a defect of the test program.

@ihoro
Copy link
Member

ihoro commented Dec 19, 2024

I've tried to reproduce it in my env, no luck so far. All epair interfaces are cleared as expected with the following result for /usr/tests/sys/net/routing:

test_routing_l3.py:TestIfOps::test_change_prefix_route[inet]  ->  failed: /usr/tests/sys/net/routing/test_routing_l3.py:42: AssertionError  [0.369s]
test_routing_l3.py:TestIfOps::test_change_prefix_route_same_iface[inet6]  ->  skipped: /usr/tests/sys/net/routing/test_routing_l3.py:88: AssertionError  [0.372s]
71/73 passed (0 broken, 1 failed, 1 skipped)

Actually, I had to do pkg upgrade for my test host as all *.py tests were not even listable (*:__test_cases_list__ is reported by Kyua). Probably, it's due to some recent regression related to python/pytest/py-something version update. It's just a clue in case if you need the same.

I guess you are working on that AssertionError (thanks for that!), and presumably the test has changes on your side -- it could be one of the reasons for the issue you are experiencing. I would recommend to take step back to the original version of the test (and revert to a clean environment if possible) to make sure that it was working as expected before your patching. It may help to understand the reproduction steps.

@Rin0913
Copy link
Author

Rin0913 commented Dec 20, 2024

@ihoro It seems that the test executed the setup part successfully so that you could see the "assertion failed".
What I want to say is when the test fails in setup part, the cleanup seems not to be successfully executed.

@ngie-eign
Copy link
Contributor

I wonder if this is an issue with how kyua works..

The XUnit paradigm is:

  • Run setup:

    • If setup fails, bail.
  • Run test.

  • Irrespective of the result, run the Teardown routines.

  • kyua doesn't have a notion of "setup" like XUnit does.

  • kyua doesn't have atomic "cleanup" routines (what pytest calls finalizers), so if a series of steps are executed in the Teardown routine and the routine terminates early, some of the cleanup steps might not be executed.

Also: are jail names generated in a deterministic manner? If so, that's potentially problematic: jail names should really be done in the format:

kyua-${kyua_process_id}-${unique_hash}

Using a format like this would better avoid collisions between tests.

@ihoro
Copy link
Member

ihoro commented Dec 20, 2024

Also: are jail names generated in a deterministic manner? If so, that's potentially problematic: jail names should really be done in the format:

kyua-${kyua_process_id}-${unique_hash}

Using a format like this would better avoid collisions between tests.

@ngie-eign
Currently, the net/routing tests are usual exclusive tests. In addition, they are not opted-in yet to use execenv=jail. Thus, the only jails in question are the ones spawned by the tests themselves. Nevertheless, as it was discussed a year ago the execenv=jail implementation left room for jail naming improvements, and I like your idea to add kyua pid to the existing formula -- it should help for the cases when multiple kyuaS are running in parallel to execute the same tests, which is not expected but possible, e.g. by human mistake or maybe there are leftovers from the previous totally failed or crashed run. Added to my TODO list. Thanks!

@Rin0913
Could you please provide the reproduction steps and more details of the environment used? If the test has been patched then the patch would be also helpful. Or you simply run the unchanged code from the official repo? By the way, what is the name of the jail you had to remove?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants