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

[Bug] parent-cgroup does not work as expected #4287

Closed
MrMaxxan opened this issue Nov 29, 2023 · 1 comment · Fixed by #4309
Closed

[Bug] parent-cgroup does not work as expected #4287

MrMaxxan opened this issue Nov 29, 2023 · 1 comment · Fixed by #4309
Assignees

Comments

@MrMaxxan
Copy link

Describe the bug

When starting a jailer with the parent-cgroup specified but no cgroup flags specified, then the rules in the parent cgroup folder is ignored.

To Reproduce

jailer --id myid --parent-cgroup firecracker --cgroup-version 2 --exec-file /usr/local/bin/firecracker-v1.5.0 --uid 1000 --gid 1000 --chroot-base-dir /tmp

but if I run with a dummy cgroup parameter it works:

jailer --id myid --parent-cgroup firecracker --cgroup-version 2 --cgroup=memory.max=max --exec-file /usr/local/bin/firecracker-v1.5.0 --uid 1000 --gid 1000 --chroot-base-dir /tmp

Expected behaviour

If I have a cgroup folder set up with all my settings it should be possible to use that without using any dummy parameters

Environment

Firecracker v1.5.0, kernel 5.10, Ubuntu 22.04

Additional context

More info in this slack thread

@roypat
Copy link
Contributor

roypat commented Dec 4, 2023

Hi @MrMaxxan,
Thank you for the bug report. We will look into this issue as soon as possible

pb8o added a commit to pb8o/firecracker that referenced this issue Dec 4, 2023
Document the issue in firecracker-microvm#4287 so customers are aware of it.

We will continue working on providing a resolution for it.

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
@pb8o pb8o mentioned this issue Dec 4, 2023
9 tasks
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 4, 2023
Document the issue in firecracker-microvm#4287 so customers are aware of it.

We will continue working on providing a resolution for it.

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 4, 2023
Document the issue in firecracker-microvm#4287 so customers are aware of it.

We will continue working on providing a resolution for it.

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 4, 2023
Document the issue in firecracker-microvm#4287 so customers are aware of it.

We will continue working on providing a resolution for it.

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit that referenced this issue Dec 5, 2023
Document the issue in #4287 so customers are aware of it.

We will continue working on providing a resolution for it.

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 5, 2023
Document the issue in firecracker-microvm#4287 so customers are aware of it.

We will continue working on providing a resolution for it.

(cherry picked from commit 0b498b2)

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 5, 2023
Document the issue in firecracker-microvm#4287 so customers are aware of it.

We will continue working on providing a resolution for it.

(cherry picked from commit 0b498b2)

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 5, 2023
Document the issue in firecracker-microvm#4287 so customers are aware of it.

We will continue working on providing a resolution for it.

(cherry picked from commit 0b498b2)

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 6, 2023
Document the issue in firecracker-microvm#4287 so customers are aware of it.

We will continue working on providing a resolution for it.

(cherry picked from commit 0b498b2)

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit that referenced this issue Dec 6, 2023
Document the issue in #4287 so customers are aware of it.

We will continue working on providing a resolution for it.

(cherry picked from commit 0b498b2)

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 11, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer do anything.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: firecracker-microvm#4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 11, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer do anything.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: firecracker-microvm#4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 11, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer do anything.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: firecracker-microvm#4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 11, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer do anything.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: firecracker-microvm#4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 11, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer doesn't do anything, though a user specifying it could reasonably
expect it to move the process under that cgroup.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: firecracker-microvm#4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 12, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer doesn't do anything, though a user specifying it could reasonably
expect it to move the process under that cgroup.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: firecracker-microvm#4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 13, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer doesn't do anything, though a user specifying it could reasonably
expect it to move the process under that cgroup.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: firecracker-microvm#4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit to pb8o/firecracker that referenced this issue Dec 14, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer doesn't do anything, though a user specifying it could reasonably
expect it to move the process under that cgroup.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: firecracker-microvm#4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
pb8o added a commit that referenced this issue Dec 14, 2023
If we specify `--parent-cgroup` without any `--cgroup` option, the
jailer doesn't do anything, though a user specifying it could reasonably
expect it to move the process under that cgroup.

We could just error in that situation, and expect something else to
launch the jailer process in a cgroup, but we risk breaking anybody that
is already using it that way.

Instead, we move the process to the `--parent-cgroup` since it's the
most intuitive, although the specified `--parent-cgroup` is not a parent
in that case.

Link: #4287

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
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 a pull request may close this issue.

3 participants