Skip to content

Don't unmount cgroup2 when restarting#26610

Merged
espadolini merged 2 commits intomasterfrom
espadolini/cgroup-restart
May 22, 2023
Merged

Don't unmount cgroup2 when restarting#26610
espadolini merged 2 commits intomasterfrom
espadolini/cgroup-restart

Conversation

@espadolini
Copy link
Copy Markdown
Contributor

This PR makes it so that we don't unmount the cgroup2 filesystem if Teleport is restarting (either in-process or via SIGHUP into another process), since the new Teleport might've already checked that the mount point is in place, and will be responsible for cleaning it up in the future.

Fixes #26273.

@github-actions github-actions Bot requested review from lxea and probakowski May 19, 2023 15:10
@github-actions github-actions Bot added bpf Used to bugs with bpf and enhanced session recording. size/sm labels May 19, 2023
@espadolini
Copy link
Copy Markdown
Contributor Author

Waiting for a customer to confirm that the dev build with this change fixes their problem.

Copy link
Copy Markdown
Contributor

@jakule jakule left a comment

Choose a reason for hiding this comment

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

@espadolini Why will we leave the cgroup instead of re-creating it if it doesn't exist?

@espadolini
Copy link
Copy Markdown
Contributor Author

I don't see how that could be done sanely, since we don't really know when the old instance of teleport will get around to unmounting the hierarchy; even the very silly option of attempting to mount the file system on every cgroup interaction is racy like that.

I suspect that the actual solution is to simply never unmount the filesystem, to be honest.

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from probakowski May 22, 2023 19:41
@espadolini espadolini enabled auto-merge May 22, 2023 19:41
@espadolini espadolini added this pull request to the merge queue May 22, 2023
Merged via the queue into master with commit d2288d0 May 22, 2023
@espadolini espadolini deleted the espadolini/cgroup-restart branch May 22, 2023 19:59
@public-teleport-github-review-bot
Copy link
Copy Markdown

@espadolini See the table below for backport results.

Branch Result
branch/v11 Failed
branch/v12 Create PR
branch/v13 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bpf Used to bugs with bpf and enhanced session recording. size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSH sessions sometimes fail when enhanced recording is enabled

5 participants