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

libct/int: make TestFdLeaks more robust #3735

Merged
merged 3 commits into from
Mar 20, 2023

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Feb 10, 2023

The purpose of this test is to check that there are no extra file
descriptors left open after repeated calls to runContainer. In fact,
the first call to runContainer leaves a few file descriptors opened,
and this is by design.

Previously, this test relied on two things:

  1. some other tests were run before it (and thus all such opened-once
    file descriptors are already opened);
  2. explicitly excluding fd opened to /sys/fs/cgroup.

Now, if we run this test separately, it will fail (because of 1 above).
The same may happen if the tests are run in a random order.

To fix this, add a container run before collection the initial fd list,
so those fds that are opened once are included and won't be reported.

(plus some minor refactoring commits)

@kolyshkin
Copy link
Contributor Author

Now, this should also hopefully fix the flakiness observed in #3721 (comment)

austinvazquez
austinvazquez previously approved these changes Feb 14, 2023
Copy link
Contributor

@austinvazquez austinvazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

This is to de-duplicate the code that checks that err is nil
and that the exit code is zero.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The purpose of this test is to check that there are no extra file
descriptors left open after repeated calls to runContainer. In fact,
the first call to runContainer leaves a few file descriptors opened,
and this is by design.

Previously, this test relied on two things:
1. some other tests were run before it (and thus all such opened-once
   file descriptors are already opened);
2.  explicitly excluding fd opened to /sys/fs/cgroup.

Now, if we run this test separately, it will fail (because of 1 above).
The same may happen if the tests are run in a random order.

To fix this, add a container run before collection the initial fd list,
so those fds that are opened once are included and won't be reported.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AkihiroSuda AkihiroSuda merged commit efad7a3 into opencontainers:main Mar 20, 2023
@kolyshkin kolyshkin mentioned this pull request Apr 5, 2023
@kolyshkin kolyshkin mentioned this pull request Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants