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

runner tempdir patch #582

Closed
wants to merge 2 commits into from
Closed

runner tempdir patch #582

wants to merge 2 commits into from

Conversation

dacbd
Copy link
Contributor

@dacbd dacbd commented May 17, 2022

I think the real fix is probably to run the GHA client in a separate slice enforcing resource limits

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented May 17, 2022

Launching the GitHub Actions runner in a separate slice is the ersatz version of using containers. 🙃 Still, sounds like a good workaround to have a bit more control over runners without triggering the container detection logic from #146 (comment) and actions/runner#367 (comment).

Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

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

Silly question: is the default mktemp path being mounted as tmpfs on the images we use? 🤔

@dacbd
Copy link
Contributor Author

dacbd commented May 17, 2022

I confess to not completely replicating the original failure brought by @JacksonMaxfield to confirm, but

Silly question: is the default mktemp path being mounted as tmpfs on the images we use? 🤔

No, as I have seen it survive reboots, however, I believe there is a reaping process that hits the dir. Or there was an existing process already in memory that deleted enough files from disk when it got SIGTERM from rebooting my test.

@dacbd
Copy link
Contributor Author

dacbd commented May 17, 2022

@0x2b3bfa0 looks like we should just do this: #62 ?

@0x2b3bfa0
Copy link
Member

@0x2b3bfa0 looks like we should just do this: #62?

Not until Docker releases the fix? 🤔

@dacbd
Copy link
Contributor Author

dacbd commented May 17, 2022

sorry I just saw that the link PR has been merged so I assumed it's been released?

@0x2b3bfa0
Copy link
Member

As per the relevant milestone, it has not been released yet

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented May 17, 2022

No, as I have seen it survive reboots, however, I believe there is a reaping process that hits the dir. Or there was an existing process already in memory that deleted enough files from disk when it got SIGTERM from rebooting my test.

I guess I don't have enough context. 😅 Looks clear as to me.

@0x2b3bfa0
Copy link
Member

If /tmp is not mounted as tmpfs, what does this pull request fix? 🤔

@dacbd dacbd closed this May 18, 2022
@dacbd dacbd deleted the runner-tempdir-patch branch May 18, 2022 01:36
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.

No space left on device creates hung EC2 instance
2 participants