Skip to content

libct/int: make TestFdLeaks more robust#3735

Merged
AkihiroSuda merged 3 commits intoopencontainers:mainfrom
kolyshkin:int-fix-flake
Mar 20, 2023
Merged

libct/int: make TestFdLeaks more robust#3735
AkihiroSuda merged 3 commits intoopencontainers:mainfrom
kolyshkin:int-fix-flake

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