1. Dismantle and remove struct cgroupData. It contained three unrelated
entities (cgroup paths, pid, and resources), and made the code
harder to read. Most importantly, though, it is not needed.
Now, subsystems' Apply methods take path, resources, and pid.
To a reviewer -- the core of the changes is in fs.go and paths.go,
the rest of it is adapting to the new signatures and related test
changes.
2. Dismantle and remove struct cgroupTestUtil. This is a followup
to the previous item -- since cgroupData is gone, there is nothing
to hold in cgroupTestUtil. The change itself is very small (see
util_test.go), but this patch is big because of it -- mostly
because we had to replace helper.cgroup.Resources with
&config.Resources{}.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Only some libcontainer packages can be built on non-linux platforms
(not that it make sense, but at least go build succeeds). Let's call
these "good" packages.
For all other packages (i.e. ones that fail to build with GOOS other
than linux), it does not make sense to have linux build tag (as they
are broken already, and thus are not and can not be used on anything
other than Linux).
Remove linux build tag for all non-"good" packages.
This was mostly done by the following script, with just a few manual
fixes on top.
function list_good_pkgs() {
for pkg in $(find . -type d -print); do
GOOS=freebsd go build $pkg 2>/dev/null \
&& GOOS=solaris go build $pkg 2>/dev/null \
&& echo $pkg
done | sed -e 's|^./||' | tr '\n' '|' | sed -e 's/|$//'
}
function remove_tag() {
sed -i -e '\|^// +build linux$|d' $1
go fmt $1
}
SKIP="^("$(list_good_pkgs)")"
for f in $(git ls-files . | grep .go$); do
if echo $f | grep -qE "$SKIP"; then
echo skip $f
continue
fi
echo proc $f
remove_tag $f
done
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As reported in issue 3084, sometimes setting CPU quota period fails
when a new period is lower and a parent cgroup has CPU quota limit set.
This happens as in cgroup v1 the quota and the period can not be set
together (this is fixed in v2), and since the period is being set first,
new_limit = old_quota/new_period may be higher than the parent cgroup
limit.
The fix is to retry setting the period after the quota, to cover all
possible scenarios.
Add a test case to cover a regression caused by an earlier version of
this patch (ignoring a failure of setting invalid period when quota is
not set).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This builds on top of recently introduced fscommon.ParseError.
Errors returned from parsers (mostly ones used by GetStats()) are all
different, and many are incomplete. For example, in many cases errors
from strconv.ParseUint are returned as is, meaning there is no context
telling which file we were reading. Similarly, errors from
fscommon.ParseKeyValue should be wrapped to add more context.
Same is true for scanner.Err().
One special case that repeats a few times is "malformed line: xxx".
Add and use a helper for that to simplify things.
OTOH, errors from fscommon.GetCgroup* do have enough context and there
is no need to wrap them.
Fix all the above.
While at it, add a missing scanner.Err() check.
[v2: use parseError not ParseError]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
gofumpt (mvdan.cc/gofumpt) is a fork of gofmt with stricter rules.
Brought to you by
git ls-files \*.go | grep -v ^vendor/ | xargs gofumpt -s -w
Looking at the diff, all these changes make sense.
Also, replace gofmt with gofumpt in golangci.yml.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
A cgroup manager's Set method sets cgroup resources, but historically
it was accepting configs.Cgroups.
Refactor it to accept resources only. This is an improvement from the
API point of view, as the method can not change cgroup configuration
(such as path to the cgroup etc), it can only set (modify) its
resources/limits.
This also lays the foundation for complicated resource updates, as now
Set has two sets of resources -- the one that was previously specified
during cgroup manager creation (or the previous Set), and the one passed
in the argument, so it could deduce the difference between these. This
is a long term goal though.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. This is the only function in the package with Get prefix
that does not read a file (but parses a string). Rename
accordingly, and convert the callers.
GetCgroupParamKeyValue -> ParseKeyValue
2. Use strings.Split rather than strings.Fields. Split by a space
is 2x faster, plus we can limit the splitting. The downside is
we have to strip a newline in one of the callers.
3. Improve the doc and the code flow.
4. Fix a test case with invalid data (spaces at BOL).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In manager.Apply() method, a path to each subsystem is obtained by
calling d.path(sys.Name()), and the sys.Apply() is called that does
the same call to d.path() again.
d.path() is an expensive call, so rather than to call it twice, let's
reuse the result.
This results the number of times we parse mountinfo during container
start from 62 to 34 on my setup.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 4e65e0e90a added a check for cpu shares. Apparently, the
kernel allows to set a value higher than max or lower than min without
an error, but the value read back is always within the limits.
The check (which was later moved out to a separate CheckCpushares()
function) is always performed after setting the cpu shares, so let's
move it to the very place where it is set.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
split fs2 package from fs, as mixing up fs and fs2 is very likely to result in
unmaintainable code.
Inspired by containerd/cgroups#109
Fix#2157
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
cgroupData.join method using `WriteCgroupProc` to place the pid into
the proc file, it can avoid attach any pid to the cgroup if -1 is
specified as a pid.
so, replace `writeFile` with `WriteCgroupProc` like `cpuset.go`'s
ApplyDir method.
Signed-off-by: Wang Long <long.wanglong@huawei.com>
before trying to move the process into the cgroup.
This is required if runc itself is running in SCHED_RR mode, as it is not
possible to add a process in SCHED_RR mode to a cgroup which hasn't been
assigned any RT bandwidth. And RT bandwidth is not inherited, each new
cgroup starts with 0 b/w.
Signed-off-by: Ben Gray <ben.r.gray@gmail.com>
Apply and Set are two separate operations, and it doesn't make sense to
group the two together (especially considering that the bootstrap
process is added to the cgroup as well). The only exception to this is
the memory cgroup, which requires the configuration to be set before
processes can join.
One of the weird cases to deal with is systemd. Systemd sets some of the
cgroup configuration options, but not all of them. Because memory is a
special case, we need to explicitly set memory in the systemd Apply().
Otherwise, the rest can be safely re-applied in .Set() as usual.
Signed-off-by: Aleksa Sarai <asarai@suse.com>
Apply and Set are two separate operations, and it doesn't make sense to
group the two together (especially considering that the bootstrap
process is added to the cgroup as well). The only exception to this is
the memory cgroup, which requires the configuration to be set before
processes can join.
Signed-off-by: Aleksa Sarai <asarai@suse.com>
This allows us to distinguish cases where a container
needs to just join the paths or also additionally
set cgroups settings. This will help in implementing
cgroupsPath support in the spec.
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
The former cgroup entry is confusing, separate it to parent
and name.
Rename entry `c` to `config`.
Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
'parent' function is confusing with parent cgroup, it's actually
parent path, so rename it to parentPath.
The name 'data' is too common to be identified, rename it to cgroupData
which is exactly what it is.
Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
wineway