The merge 6eed6e5795 broke the build because ab27e12ceb ("Implement
GetStat for cpuset cgroup.") dropped the errors import which was used by
c85cd2b325 ("libct/cg/fs/cpuset: don't parse mountinfo") and the CI
wasn't retriggered.
Fix this by just importing "github.com/pkg/errors" again.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
It was already explained why we ignore the error, so let's ignore this
deliberately.
This fixes
> name.go:22:7: Error return value of `join` is not checked (errcheck)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Rewrite getPageUsageByNUMA
1. Be less strict to unknown contents, i.e. skip it. This makes the
function more future-proof. Before this commit, if a line like
"a=b" is encountered, the function returns an error, which is
propagated all the way up to and returned by (CgroupManager).GetStats.
2. Be more strict to contents it recognizes, i.e. return an error.
In case the first field in the line is recognized (e.g. "total=123",
the rest of the line should be in format "N<id>=<value> ...".
3. Optimize. Before this commit, addNUMAStatsByType was called for every
item in the line, which is excessive and might even be slow in case
there are many NUMA nodes. It is enough to look up the field once.
4. Remove a bunch of global numaNode* and numaStat* constants. Those
were used by only one function, and it does not make sense to have
them defined globally. Some were moved to the function, some were
eliminated entirely.
5. Improve readability and added code comments.
Finally, add some test cases for good and bad contents.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
strings.SplitN not always return N fields if not staify, sometimes
cgroup interface add some custom fields make parse memory.numa_stat
fails, it will case panic
Signed-off-by: acetang <aceapril@126.com>
Here, we always create a parent directory, so using MkdirAll
is redundant. Use Mkdir instead.
One difference between MkdirAll and Mkdir is the former ignores
EEXIST, and since we sometimes try to create a directory that
already exists, we need to explicitly ignore that.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In cgroup v1, the parent of a cgroup mount is on tmpfs
(or maybe any other fs but definitely not cgroupfs).
Use this to traverse up the tree until we reach non-cgroupfs.
This should work in any setups (nested container, non-standard
cgroup mounts) so issue [1] won't happen.
Theoretically, the only problematic case is when someone mounts
cpuset cgroupfs into a directory on another cgroupfs. Let's
assume people don't do that -- if they do, they will get other
error (e.g. inability to read cpuset.cpus file).
[1] https://github.com/opencontainers/runc/issues/1367
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Drop the custom mountinfo parser, reuse the cgroups.GetCgroupMounts()
to get the cgroup root. Faster, and much less code.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Move the Device-related types to libcontainer/devices, so that
the package can be used in isolation. Aliases have been created
in libcontainer/configs for backward compatibility.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move the functionality of opening a cgroup file into a separate
function, OpenFile, which, similar to ReadFile and WriteFile,
use separate dir and file arguments.
Change ReadFile and WriteFile to rely on OpenFile, and use lower-level
read and write instead of ones from ioutil.
It changes the semantics of WriteFile a bit -- it no longer uses
O_CREAT flag. This is good for real cgroup as there is no need to try
creating the files in there, but can potentially break WriteFile users
-- previously, EPERM error was returned for non-existing files, and
now it's ENOENT.
This also breaks the fs/fs2 unit tests since they write to pseudo-cgroup
files inside a test directory (not to a real cgroup fs), and now
fscommon.WriteFile do not create or truncate files, so we have to add a
variable that is set by the unit tests.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
While at it,
- change some functions to not be methods of CpusetCgroup as
they don't use any members;
- simplify isEmpty.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
fscommon.WriteFile is added specifically to work with cgroup files,
and the error it returns does not need to be wrapped.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The names of these templates overlapped with some local
variables, which made reading the code somewhat confusing.
Changing them to a const, given that these were not updated anywere.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When paths are set, we only need to place the PID into proper
cgroups, and we do know all the paths already.
Both fs/d.path() and systemd/v1/getSubsystemPath() parse
/proc/self/mountinfo, and the only reason they are used
here is to check whether the subsystem is available.
Use a much simpler/faster check instead.
Frankly, I am not sure why the check is needed at all. Maybe it should
be dropped.
Also, for fs driver, since d is no longer used in this code path,
move its initialization to after it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
All the test cases are doing the same checks, only input differs,
so we can unify those using a test data table.
While at it:
- use t.Fatalf where it makes sense (no further checks are possible);
- remove the "XXX" comments as we won't get rid of cgroup Name/Parent.
PS I tried using t.Parallel() as well but it did not result in any
noticeable speedup, so I dropped it for simplicity.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
...by checking the default path first.
Quick benchmark shows it's about 5x faster on an idle system, and the
gain should be much more on a system doing mounts etc.
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>
Instead of iterating over m.paths, iterate over subsystems and look up
the path for each. This is faster since a map lookup is faster than
iterating over the names in Get. A quick benchmark shows that the new
way is 2.5x faster than the old one.
Note though that this is not done to make things faster, as savings are
negligible, but to make things simpler by removing some code.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Half of controllers' GetStats just return nil, and most of the others
ignore ENOENT on files, so it will be cheaper to not check that the
path exists in the main GetStats method, offloading that to the
controllers.
Drop PathExists check from GetStats, add it to those controllers'
GetStats where it was missing.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
RemovePaths() deletes elements from the paths map for paths that has
been successfully removed.
Although, it does not empty the map itself (which is needed that AFAIK
Go garbage collector does not shrink the map), but all its callers do.
Move this operation from callers to RemovePaths.
No functional change, except the old map should be garbage collected now.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The result of cgroupv1.FindCgroupMountpoint() call (which is relatively
expensive) is only used in case raw.innerPath is absolute, so it only
makes sense to call it in that case.
This drastically reduces the number of calls to FindCgroupMountpoint
during container start (from 116 to 62 in my setup).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In here, defer looks like an overkill, since the code is very simple and
we already have an error path.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Iterating over the list of subsystems and comparing their names to get an
instance of fs.cgroupFreezer is useless and a waste of time, since it is
a shallow type (i.e. does not have any data/state) and we can create an
instance in place.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The kubelet uses libct/cgroups code to set up cgroups. It creates a
parent cgroup (kubepods) to put the containers into.
The problem (for cgroupv2 that uses eBPF for device configuration) is
the hard requirement to have devices cgroup configured results in
leaking an eBPF program upon every kubelet restart. program. If kubelet
is restarted 64+ times, the cgroup can't be configured anymore.
Work around this by adding a SkipDevices flag to Resources.
A check was added so that if SkipDevices is set, such a "container"
can't be started (to make sure it is only used for non-containers).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This function is cgroupv1-specific, is only used once, and its name
is very close to the name of another function, FindCgroupMountpoint.
Inline it into the (only) caller.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes a few cases of accessing m.paths map directly without holding
the mutex lock.
Fixes: 9087f2e82
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 714c91e9f7, method GetPaths() should only be used
for saving container state. For other uses, we have a new method,
Path(), which is cleaner.
Fix GetPaths() usage introduced by recent commits 859a780d6f and 9087f2e82.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin