Go 1.18 and up now provides a strings.Cut() which is better suited for
splitting key/value pairs (and similar constructs), and performs better:
```go
func BenchmarkSplit(b *testing.B) {
b.ReportAllocs()
data := []string{"12hello=world", "12hello=", "12=hello", "12hello"}
for i := 0; i < b.N; i++ {
for _, s := range data {
_ = strings.SplitN(s, "=", 2)[0]
}
}
}
func BenchmarkCut(b *testing.B) {
b.ReportAllocs()
data := []string{"12hello=world", "12hello=", "12=hello", "12hello"}
for i := 0; i < b.N; i++ {
for _, s := range data {
_, _, _ = strings.Cut(s, "=")
}
}
}
```
BenchmarkSplit
BenchmarkSplit-10 8244206 128.0 ns/op 128 B/op 4 allocs/op
BenchmarkCut
BenchmarkCut-10 54411998 21.80 ns/op 0 B/op 0 allocs/op
While looking at occurrences of `strings.Split()`, I also updated some for alternatives,
or added some constraints; for cases where an specific number of items is expected, I used `strings.SplitN()`
with a suitable limit. This prevents (theoretical) unlimited splits.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This updates the test to:
- Use correctly formatted values for RequiredPlugins and DisabledPlugins (values
are expected to have a `io.containerd.` prefix). While not needed for the test
to pass (no validation is performed), it's good to have these values in the
correct format (in case we want to add validation at this stage).
- Set a `Version` for both (as version 1 / no version was deprecated)
The `Version` field in this test was used to verify the "integer override"
behavior; setting "Version: 2" for both would no longer cover that case. As there
are only 2 integer fields in the config (Version and OOMScore) and OOMScore was
already used in the test, I added separate test-cases for that.
Looking at the test, we should consider what we want the behaviour to be if the
override file does not specify a version (implicitly: version 1), or if the version
is different from the original one; do we want mergeConfig() to produce an error
when merging a v2 config with a v1 config (or v3 with v2)?
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
GOGC=75 golangci-lint run
services/server/server.go:320:27: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
return trapClosedConnErr(http.Serve(l, m))
^
services/server/server.go:340:27: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
return trapClosedConnErr(http.Serve(l, m))
^
cmd/containerd-stress/main.go:238:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
if err := http.ListenAndServe(c.Metrics, metrics.Handler()); err != nil {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In Go 1.16 `net.ErrClosed` was exported, removing the need to check the
exact text of "use of closed network connection". The stdlib's net listeners
are all setup for this to be a reality, but on Windows containerd uses the
the go-winio projects named pipe implementation as the listener for services.
Before version 0.6.0 this project returned a different error named
`ErrPipeListenerClosed` for using a closed pipe, where this error was just
an `errors.New` with the same text as `net.ErrClosed`, so checking against
`net.ErrClosed` wasn't possible.
Starting in 0.6.0 go-winio has that error assigned to `net.ErrClosed` directly
so this *should* be alright to finally change.
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
A bbolt database has a freelist to track all pages that are available
for allocation. However writing the list takes some time and reading
the list sometimes panics.
This commit sets NoFreelistSync true to skipping the freelist entirely,
following what etcd does.
https://github.com/etcd-io/etcd/blob/v3.5.2/server/mvcc/backend/config_linux.go#L31Fixes#4838.
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
All occurrences only passed a PID, so we can use this utility to make
the code more symmetrical with their cgroups v2 counterparts.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The directory created by `T.TempDir` is automatically removed when the
test and all its subtests complete.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
When running containerd as a service it may be hard to
override the TMP location of the process. This is especially
true on Windows when running containerd in SCM. This change
allows you to set the 'temp' location in the config.toml when
the service starts up that overrides its TEMP/TMP/TMPDIR usage.
This is helpful on Linux as well but it primarily solves the
performance issue on Windows when running containerd across
volumes. IE: If you configure your data/root paths on a volume
other than the SystemDrive the snapshotter does a temporary unpack
on the SystemDrive and then has to copy contents of that data
to the snapshot folder on the destination volume. By alinging the
tmp with the destination it is a simple move operation instead of
a copy operation.
Signed-off-by: Justin Terry <jlterry@amazon.com>
Before this change we only ever had the grpc incoming medata set so when
we make a request to a shim or a grpc plugin the namespace is not sent
over the RPC.
I need this for github.com/cpuguy83/systemdshim, and I am sure there are
other use-cases where this would be needed.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
In linux 5.14 and hopefully some backports, core scheduling allows processes to
be co scheduled within the same domain on SMT enabled systems.
The containerd impl sets the core sched domain when launching a shim. This
allows a clean way for each shim(container/pod) to be in its own domain and any
additional containers, (v2 pods) be be launched with the same domain as well as
any exec'd process added to the container.
kernel docs: https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/core-scheduling.html
Signed-off-by: Michael Crosby <michael@thepasture.io>
The io/ioutil package has been deprecated as of Go 1.16, see
https://golang.org/doc/go1.16#ioutil. This commit replaces the existing
io/ioutil functions with their new definitions in io and os packages.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Refactor shim v2 to load and register plugins.
Update init shim interface to not require task service implementation on
returned service, but register as plugin if it is.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Remove build tags which are already implied by the name of the file.
Ensures build tags are used consistently
Signed-off-by: Derek McGowan <derek@mcg.dev>
Add basic intiialization of opentelemetry including minimum support to
be able to read open telemetry config from config.toml and initialize
exporter. Tracer is initialized and ready to be be used for creating
spans, sub spans etc. With no opentelemetry configuration enabled in
config file, this patch is a no-op.
Basic config stub to be added to use opentelemetry is to add following
in config.toml. We use otlp exporter with default port 4317.
[otel]
exporter_name = "otlp"
exporter_endpoint = "0.0.0.1:4317"
otel-collector binary needs to run listening at the same port.
Signed-off-by: Alakesh Haloi <alakeshh@amazon.com>
According to the doc about `config.toml` of containerd:
```
If no version number is specified inside the config file then it is assumed to
be a version 1 config and parsed as such.
```
However, it's not true recently.
This will break the backward-compatibility in some environment.
This commit fixes this issue.
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Add support for an 'env' field to the StreamProcessor configuration
and append the environment variables found there to the os.Environ()
array.
The env field takes environment variables in the form of key=value.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
* only shim v2 runc v2 ("io.containerd.runc.v2") is supported
* only PID metrics is implemented. Others should be implemented in separate PRs.
* lots of code duplication in v1 metrics and v2 metrics. Dedupe should be separate PR.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
