run: Add cgroup-parent flag #1782
Conversation
dcantah
force-pushed
the
cgroup-parent
branch
2 times, most recently
from
5f8137e
to
2d0f05b
Compare
dcantah
force-pushed
the
cgroup-parent
branch
from
2d0f05b
to
0a2b5f8
Compare
| } | ||
|
|
||
| if cpus > 0.0 || memStr != "" || memSwap != "" || pidsLimit > 0 { | ||
| logrus.Warn("cgroup manager is set to \"none\", discarding resource limit requests. " + | ||
| logrus.Warn(`cgroup manager is set to "none", discarding resource limit requests. ` + |
There was a problem hiding this comment.
Any reason for this change?
| if cgroupManager == "none" { | ||
| if !rootlessutil.IsRootless() { | ||
| return nil, errors.New("cgroup-manager \"none\" is only supported for rootless") | ||
| return nil, errors.New(`cgroup-manager "none" is only supported for rootless`) |
There was a problem hiding this comment.
Any reason for this change?
There was a problem hiding this comment.
Feel like raw string literals fit nicer than manually escaping quotes and other things. Also a habit from windows land where you'd commonly have to escape backslashes, so raw string literals came in handy there.
| base := testutil.NewBase(t) | ||
| info := base.Info() | ||
| containerName := testutil.Identifier(t) | ||
| defer base.Cmd("rm", "-f", containerName).Run() |
There was a problem hiding this comment.
Should be AssertOK; since bugs were found in the past of containers leaked out.
There was a problem hiding this comment.
Are you referring to the deferred cleanup? I was just following the standard set in most of the other tests in this file, but seems it's actually the lesser used of the two when taking into account all of the other tests; only run_cgroup_linux_test and run_mount_linux_test just defer Run and call it a day. I'll open up a PR to swap the cgroup tests to using AssertOK instead (and use AssertOK here)
Adds a test case and the functionality for cgroup-parent to match docker runs support. Signed-off-by: Danny Canter <danny@dcantah.dev>
Adds a test case and the functionality for cgroup-parent to match docker runs support.