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>
Replace ioutil.TempDir (mostly) with t.TempDir, which require no
explicit cleanup.
While at it, fix incorrect usage of os.ModePerm in libcontainer/intelrdt
test. This is supposed to be a mask, not mode bits.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This one is tough as errorlint insists on using errors.Is, and the
latter is known to not work for Go 1.13 which we still support.
So, add a nolint annotation to suppress the warning, and a TODO to
address it later.
For intelrdt, we can do the same, but it is easier to reuse the very
same function from fscommon (note we can't use fscommon for other stuff
as it expects cgroupfs).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Introduce ParseError type as a way to unify error messages related to
file parsing. Use it from GetCgroup* functions.
2. Do not discard the error from strconv.Parse{Int,Uint} -- it contains
the value being parsed, and the details about the error.
2. As the error above already contains the value, drop it from format.
[v2: use path.Join in Error]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The error returned from strconv.ParseUint is already pretty descriptive,
something like:
strconv.ParseUint: parsing "000d": invalid syntax
So, there is no need to add more context to it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is a better place as cgroups itself is using these.
Should help with moving more stuff common in between fs and fs2 to
fscommon.
Looks big, but this is just moving the code around:
fscommon/{fscommon,open}.go -> cgroups/file.go
fscommon/fscommon_test.go -> cgroups/file_test.go
and fixes for TestMode moved to a different package.
There's no functional change.
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>
Commit 88e8350de2, among the other things, replaced filepath.Join with
securejoin.SecureJoin for both reads and writes to cgroupfs.
Commits e76ac1c054 and 31f0f5b7e0 switched more code to use
fscommon.ReadFile (and thus securejoin). Commit 0228226e6d introduced
fscommon.OpenFile (which uses securejoin as the fallback if openat2(2)
is not available, which is the case for older kernels), and commit
c95e69007c switched most of cgroup/fs[2] code to use it.
As a result, fs.GetStats() method became noticeable slower, mostly due
to securejoin calling os.Lstat and filepath.Clean.
Using securejoin as a security measure for cgroupfs files is
not well justified, as cgroupfs do not contain symlinks, and none of the
code using it have uncleaned paths. In particular, fs/fs2/systemd
managers do check and sanitize their paths.
This commit modifies the code to not use securejoin. Instead, it checks
that the opened file is indeed on cgroupfs.
Using BenchmarkGetStats on a CentOS 8 VM, I see the following
improvement:
Before:
> BenchmarkGetStats-8 8376 625135 ns/op
After:
> BenchmarkGetStats-8 12226 485015 ns/op
An intermediate version, with no fstatfs to check fstype:
> BenchmarkGetStats-8 13162 452281 ns/op
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case openat2() is not available, it does not make sense to calculate
relpath (and check if path has /sys/fs/cgroup prefix).
Reverse the order of checks to not do that in case openat2 is not
available.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Generalize the libct/getValueFromCgroup() as fscommon.GetValueByKey(),
and document it.
No changes other than using fscommon.ParseUint to convert the value.
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 case we get ENOSYS from openat2(2), this is expected, so log that
we're falling back to using securejoin as debug.
Otherwise, log it as a warning (as the error is unexpected, but we're
still good to go).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case openat2 is available, it will be used to guarantee
that we're not accessing anything other than cgroupfs[2] files.
In cases when openat2 is not available, or when cgroup has a
non-standard prefix (not "/sys/fs/cgroup", which might theoretically
be the case on some very old installs and/or very custom systems),
fall back to using securejoin + os.Open like we did before.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
This removes package dependency on cgroup, as following commits
make cgroup use fscommon, which would result in dependency cycle.
The code to find out memory cgroup root is not really needed,
as 99% of test envrionments will have it at /sys/fs/cgroup/memory.
If not, that means we're either on cgroupv2 or on some very custom
system, so just skip the test.
The code that checks if we're on cgroupv2 is replaced by the check
of the particular v1 control file.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Use GetCgroupParamString as the initial part of both functions are
the same and we can reuse it. This also gives us whatever security
measures GetCgroupParamString has (see previous commit).
2. Fix the error wrapping to not add the value, as it is already a part
of the error returned by ParseUint.
3. Improve docstring.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Use own ReadFile wrapper instead of ioutils.ReadFile.
This makes it use the security measures of ReadFile.
2. Improve doc.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2. Fix wrapping the error to not have the value as it's already
part of the error returned from ParseUint.
3. Fix/improve doc.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As the underlying error message from iotuils.WriteFile already contains
file name, there's no need to put it, otherwise we end up with something
like:
failed to write "val" to "/sys/fs/cgroup/.../file": open /sys/fs/cgroup/.../file: permission denied
With this patch, the error will be
failed to write "val": open /sys/fs/cgroup/.../file: permission denied
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Using errors.Unwrap() is not the best thing to do, since it returns
nil in case of an error which was not wrapped. More to say,
errors package provides more elegant ways to check for underlying
errors, such as errors.As() and errors.Is().
This reverts commit f8e138855d, reversing
changes made to 6ca9d8e6da.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Tested that the EINTR is still being detected:
> $ go1.14 test -c # 1.14 is needed for EINTR to happen
> $ sudo ./fscommon.test
> INFO[0000] interrupted while writing 1063068 to /sys/fs/cgroup/memory/test-eint-89293785/memory.limit_in_bytes
> PASS
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Golang 1.14 introduces asynchronous preemption which results into
applications getting frequent EINTR (syscall interrupted) errors when
invoking slow syscalls, e.g. when writing to cgroup files.
As writing to cgroups is idempotent, it is safe to retry writing to the
file whenever the write syscall is interrupted.
Signed-off-by: Mario Nitchev <marionitchev@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>
Kir Kolyshkin