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

All jobs should be run with --rm now which makes this not necessary. #80

Merged
merged 1 commit into from
Jan 13, 2016

Conversation

tfoote
Copy link
Member

@tfoote tfoote commented Jan 6, 2016

@tfoote
Copy link
Member Author

tfoote commented Jan 8, 2016

@dirk-thomas please review

@dirk-thomas
Copy link
Member

If after numerous builds no containers accumulate on the host machine it looks good to me.

@tfoote
Copy link
Member Author

tfoote commented Jan 8, 2016

We still leak the occasional container. Introspecting one of our active slaves there are well over 100 leaked in the last 48 hours. https://gist.github.com/tfoote/ba1e8d608e98df9f1af7

I think it's worth leaving this cleanup. We just have to be sure that the minimum cleanup age is longer than any timeouts. I haven't seen any failures recently since we added the minimum age.

@dirk-thomas
Copy link
Member

While fixing the symptom with the cleanup script might work currently I think we should investigate why these containers remain i the first place and try to fix the source of the problem.

@tfoote
Copy link
Member Author

tfoote commented Jan 11, 2016

This is removing the cleanup script which is dealing with a synptom. @esteve is looking into the underlying issue. I'm going to close this removal of the cleanup, as this symptom is important to clean up in case it happens again as it can be catastrophic. (aka slaves run out of disk space)

@tfoote tfoote closed this Jan 11, 2016
@tfoote tfoote deleted the remote_container_cleanup branch January 11, 2016 20:18
@dirk-thomas
Copy link
Member

If we do fix the actual problem I would recommend to merge this. Even if the result would be "catastrophic" otherwise any problem in the future in this area would go unnoticed. I think a hard failure is better then hiding a problem.

@tfoote tfoote restored the remote_container_cleanup branch January 11, 2016 21:28
@tfoote tfoote reopened this Jan 11, 2016
@tfoote tfoote self-assigned this Jan 11, 2016
@tfoote
Copy link
Member Author

tfoote commented Jan 11, 2016

I'm not sure I completely agree, but we can do that. reopened

@tfoote
Copy link
Member Author

tfoote commented Jan 13, 2016

Merging this to let container leakage be apparent and the hoped fix resolve it: ros-infrastructure/ros_buildfarm#124

tfoote added a commit that referenced this pull request Jan 13, 2016
All jobs should be run with --rm now which makes this not necessary.
@tfoote tfoote merged commit e3bc486 into master Jan 13, 2016
@tfoote tfoote deleted the remote_container_cleanup branch January 13, 2016 01:02
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.

None yet

2 participants