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

fix directory leak leading to out of disk #161

Merged
merged 3 commits into from Mar 23, 2023
Merged

fix directory leak leading to out of disk #161

merged 3 commits into from Mar 23, 2023

Conversation

ghost
Copy link

@ghost ghost commented Feb 27, 2023

Issue 132

Why

When using stereoscope as a library, for long running processes, the relationship between the providers may cause files in temp to not get cleaned up after image.Image.Cleanup is called.

What

  • enable image.Image.Cleanup to own tmpDirGenerator and perform cleanup any directories related to the image.

How Tested

  • per guide
make bootstrap
make unit
make integration
  • using the basic example
$ docker pull ruby:2.6-alpine3.11
$ ls -l /tmp/ | grep stereo
$ go run basic.go docker:ruby:2.6-alpine3.11 &>/dev/null
$ ls -l /tmp/ | grep stereo

client.go Outdated Show resolved Hide resolved
@kzantow
Copy link
Contributor

kzantow commented Feb 27, 2023

@ghost
Copy link
Author

ghost commented Feb 27, 2023

@kzantow
I thought I did this (dc6bd53) - I guess I am not sure what I am missing. Can you provide me guidance of what I am missing?

@kzantow
Copy link
Contributor

kzantow commented Feb 27, 2023

@slimdevl I'm not entirely sure why DCO isn't passing, it looks right. Did you use git commit -s / git commit --signoff? Maybe try the steps listed here?: https://github.com/anchore/stereoscope/pull/161/checks?check_run_id=11626027320

@ghost
Copy link
Author

ghost commented Feb 27, 2023

'm not entirely sure why DCO isn't passing, it looks right. Did you use

I manually added those fields as recommended by the guide. Let me see if I can get the -s (maybe there is something I need to figure out there)

@ghost
Copy link
Author

ghost commented Feb 27, 2023

@slimdevl I'm not entirely sure why DCO isn't passing, it looks right. Did you use git commit -s / git commit --signoff? Maybe try the steps listed here?: https://github.com/anchore/stereoscope/pull/161/checks?check_run_id=11626027320

This worked like magic, thanks @kzantow !

@wagoodman wagoodman self-assigned this Mar 23, 2023
client.go Outdated Show resolved Hide resolved
Copy link
Contributor

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

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

Great find! I think promoting the temp dir generator to the image makes sense. I've pushed a few of adjustments:

  • update the comment for the package Cleanup() function to clearly indicate that this is deprecated
  • restore the root temp directory generator (to facilitate the package Cleanup function)
  • Added both removal of the contentCacheDir and cleanup of the tmp directory generator in the Image object. This way someone constructing their own image object could in theory provider a temp dir generator that did not create the contentCacheDir and everything would still work.

@wagoodman wagoodman enabled auto-merge (squash) March 23, 2023 14:22
@wagoodman wagoodman added the bug Something isn't working label Mar 23, 2023
@wagoodman wagoodman merged commit 92d7aa6 into anchore:main Mar 23, 2023
gnmahanth pushed a commit to deepfence/stereoscope that referenced this pull request Jun 15, 2023
* fix tmpDirGenerator chain of responsibility associated with anchore#132

Signed-off-by: Joseph Barnett <[email protected]>
Signed-off-by: [email protected] <[email protected]>

* reduce log message to debug
Signed-off-by: Joseph Barnett <[email protected]>

Signed-off-by: [email protected] <[email protected]>

* restore global cleanup function

Signed-off-by: Alex Goodman <[email protected]>

---------

Signed-off-by: Joseph Barnett <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
Co-authored-by: Alex Goodman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants