Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

Conversation

@amshinde
Copy link
Contributor

@amshinde amshinde commented Jan 24, 2018

Close all KSM related fds that we create in our KSM tests.

While these are left open while running the proxy_test.go tests,
where detecting leaking file descriptors is relevant, it looks like
the open ksm fds get closed by the go runtime in the newer version
of Go leading to different before and after file descriptor snapshots in
the proxy_test.go tests.

Fixes #198

Close all KSM related fds that we create in our KSM tests.

While these are left open while running the proxy_test.go tests,
where detecting leaking file descriptors is relevant, it looks like
the open ksm fds get closed by the go runtime in the newer version
of Go leading to different before and after file descriptor snapshots in
the proxy_test.go tests.

Fixes clearcontainers#198

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
This file is opened to check our file descriptor leak detector
works correctly. Close it before exiting the test.

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
Move the defer statements to close fds immediately
after they are opened, to make sure they get closed
in case of error occuring later on in the program flow.

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
@sboeuf
Copy link
Contributor

sboeuf commented Jan 24, 2018

LGTM

Approved with PullApprove

@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@amshinde
Copy link
Contributor Author

@sameo Please take a look.

@jodh-intel
Copy link

jodh-intel commented Jan 24, 2018

Thanks for investigating and fixing this @amshinde.

lgtm

Approved with PullApprove

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

lgtm

@grahamwhaley grahamwhaley merged commit 53be4ae into clearcontainers:master Jan 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit test fail after update to go 1.9.2

5 participants