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

[CI] Tigron breakout 3: leak detection #4044

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

apostasie
Copy link
Contributor

On top of the two previous breakouts.

This PR adds internal/highk in preparation for the command rewrite.

Currently, the implementation relies on Uber library for go routines, and on parsing the output of lsof for fd.

I have used it intensively while rewriting Command. It has been very useful so far, but it certainly needs more work.

Proposal is to first start using it internally for Tigron own testing (as for the upcoming Command), hammer the kinks and then consider adding it as a feature of the test framework.

For an example on how it can be used at this point:

var (
snapFile *os.File
before, after []byte
)
if os.Getenv("EXPERIMENTAL_HIGHK_FD") != "" {
snapFile, _ = os.CreateTemp("", "fileleaks")
before, _ = highk.SnapshotOpenFiles(snapFile)
}
exitCode = m.Run()
if exitCode != 0 {
return
}
if os.Getenv("EXPERIMENTAL_HIGHK_FD") != "" {
after, _ = highk.SnapshotOpenFiles(snapFile)
diff := highk.Diff(string(before), string(after))
if len(diff) != 0 {
zerolog.SetGlobalLevel(zerolog.ErrorLevel)
log.Error().Msg("Leaking file descriptors")
for _, file := range diff {
log.Error().Str("file", file).Msg("leaked")
}
exitCode = 1
}
}
if err := highk.FindGoRoutines(); err != nil {
log.Error().Msg("Leaking go routines")
_, _ = fmt.Fprint(os.Stderr, err.Error())
exitCode = 1
}
}

@apostasie
Copy link
Contributor Author

apostasie commented Mar 27, 2025

Note: some discussion on the master PR:

#4040 (review)

AkihiroSuda 50 minutes ago
Can we have a unit test?

Contributor
Author
@apostasie apostasie 38 minutes ago
This is a very rough first draft for the fileleak detector that is solely used internally by tigron for its own testing.
It does its job for that limited purpose, but before I turn it on for others, a lot of work should happen on it (probably rewriting parts of lsof in go), so, most of that code is throwable at this point - tests will come later on with better code.

@apostasie apostasie force-pushed the ci-tigron-break-3 branch 2 times, most recently from 9e0e10a to bc8b0b0 Compare March 27, 2025 14:56
@apostasie
Copy link
Contributor Author

Rebased.

@apostasie apostasie force-pushed the ci-tigron-break-3 branch 2 times, most recently from c450d20 to 5d5f5c3 Compare March 27, 2025 15:24
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@apostasie
Copy link
Contributor Author

buildg failure logged in #4046

@AkihiroSuda AkihiroSuda merged commit c57654b into containerd:main Mar 27, 2025
30 checks passed
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

Successfully merging this pull request may close these issues.

2 participants