The 10-containerd-net.conflist file generated from the conf_template
should be written atomically so that partial writes are not visible to
CNI plugins. Use the new consistentfile package to ensure this on
Unix-like platforms such as Linux, FreeBSD, and Darwin.
Fixes https://github.com/containerd/containerd/issues/8607
Signed-off-by: Samuel Karp <samuelkarp@google.com>
Certain files may need to be written atomically so that partial writes
are not visible to other processes. On Unix-like platforms such as
Linux, FreeBSD, and Darwin, this is accomplished by writing a temporary
file, syncing, and renaming over the destination file name. On Windows,
the same operations are performed, but Windows does not guarantee that a
rename operation is atomic.
Partial/inconsistent reads can occur due to:
1. A process attempting to read the file while containerd is writing it
(both in the case of a new file with a short/incomplete write or in
the case of an existing, updated file where new bytes may be written
at the beginning but old bytes may still be present after).
2. Concurrent goroutines in containerd leading to multiple active
writers of the same file.
The above mechanism explicitly protects against (1) as all writes are to
a file with a temporary name.
There is no explicit protection against multiple, concurrent goroutines
attempting to write the same file. However, atomically writing the file
should mean only one writer will "win" and a consistent file will be
visible.
Signed-off-by: Samuel Karp <samuelkarp@google.com>
The initial PR had a check for nil metrics but after some refactoring in the PR the test case that was suppose cover HPC was missing a scenario where the metric was not nil but didn't contain any metrics. This fixes that case and adds a testcase to cover it.
Signed-off-by: James Sturtevant <jstur@microsoft.com>
This change adds support for CDI devices to the ctr --device flag.
If a fully-qualified CDI device name is specified, this is injected
into the OCI specification before creating the container.
Note that the CDI specifications and the devices that they represent
are local and mirror the behaviour of linux devices in the ctr command.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Several bits of code unmarshal image config JSON into an `ocispec.Image`, and then immediately create an `ocispec.Platform` out of it, but then discard the original image *and* miss several potential platform fields (most notably, `variant`).
Because `ocispec.Platform` is a strict subset of `ocispec.Image`, most of these can be updated to simply unmarshal the image config directly to `ocispec.Platform` instead, which allows these additional fields to be picked up appropriately.
We can use `tianon/raspbian` as a concrete reproducer to demonstrate.
Before:
```console
$ ctr content fetch docker.io/tianon/raspbian:bullseye-slim
...
$ ctr image ls
REF TYPE DIGEST SIZE PLATFORMS LABELS
docker.io/tianon/raspbian:bullseye-slim application/vnd.docker.distribution.manifest.v2+json sha256:66e96f8af40691b335acc54e5f69711584ef7f926597b339e7d12ab90cc394ce 28.6 MiB linux/arm/v7 -
```
(Note that the `PLATFORMS` column lists `linux/arm/v7` -- the image itself is actually `linux/arm/v6`, but one of these bits of code leads to only `linux/arm` being extracted from the image config, which `platforms.Normalize` then updates to an explicit `v7`.)
After:
```console
$ ctr image ls
REF TYPE DIGEST SIZE PLATFORMS LABELS
docker.io/tianon/raspbian:bullseye-slim application/vnd.docker.distribution.manifest.v2+json sha256:66e96f8af40691b335acc54e5f69711584ef7f926597b339e7d12ab90cc394ce 28.6 MiB linux/arm/v6 -
```
Signed-off-by: Tianon Gravi <admwiggin@gmail.com>
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Windows systems are capable of running both Windows Containers and Linux
containers. For windows containers we need to sanitize the volume path
and skip non-C volumes from the copy existing contents code path. Linux
containers running on Windows and Linux must not have the path sanitized
in any way.
Supplying the targetOS of the container allows us to proprely decide
when to activate that code path.
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Images may be created with a VOLUME stanza pointed to drive letters that
are not C:. Currently, an image that has such VOLUMEs defined, will
cause containerd to error out when starting a container.
This change skips copying existing contents to volumes that are not C:.
as an image can only hold files that are destined for the C: drive of a
container.
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
To further some ongoing work in containerd to make as much code as possible
able to be used on any platform (to handle runtimes that can virtualize/emulate
a variety of different OSes), this change makes stats able to be handled on
any of the supported stat types (just linux and windows). To accomplish this,
we use the platform the sandbox returns from its `Platform` rpc to decide
what format the containers in a given sandbox are returning metrics in, then
we can typecast/marshal accordingly.
Signed-off-by: Danny Canter <danny@dcantah.dev>
The oci.WithUser option was being applied in container_create_linux.go
instead of the cross plat buildLinuxSpec method. There's been recent
work to try and make every spec option that can be applied on any platform
able to do so, and this falls under that. However, WithUser on linux platforms
relies on the containers SnapshotKey being filled out, which means the spec
option needs to be applied during container creation.
To make this a little more generic, I've created a new platformSpecOpts
method that handles any spec opts that rely on runtime state (rootfs mounted
for example) for some platforms, or just platform options that we still don't
have workarounds for to be able to specify them for other platforms
(apparmor, seccomp etc.) by internally calling the already existing
containerSpecOpts method.
Signed-off-by: Danny Canter <danny@dcantah.dev>
There was a couple uses of Readdir/ReadDir here where the only thing the return
value was used for was the Name of the entry. This is exactly what Readdirnames
returns, so we can avoid the overhead of making/returning a bunch of interfaces
and calling lstat everytime in the case of Readdir(-1).
https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/os/dir_unix.go;l=114-137
Signed-off-by: Danny Canter <danny@dcantah.dev>
This pointer to an issue never got updated after the CRI plugin was
absorbed into the main containerd repo as an in-tree plugin.
Signed-off-by: Samuel Karp <samuelkarp@google.com>
If containerd does not see a container but criservice's
container store does, then we should try to recover from
this error state by removing the container from criservice's
container store as well.
Signed-off-by: Kirtana Ashok <Kirtana.Ashok@microsoft.com>
Using array to build sub-tests is to avoid random pick. The shuffle
thing should be handled by go-test framework. And we should capture
range var before runing sub-test.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Using array to build sub-tests is to avoid random pick. The shuffle
thing should be handled by go-test framework. And we should capture
range var before runing sub-test.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
We need support in containerd and the OCI runtime to use idmap mounts.
Let's just throw an error for now if the kubelet requests some mounts
with mappings.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Currently if you're using the shim-mode sandbox server support, if your
shim that's hosting the Sandbox API dies for any reason that wasn't
intentional (segfault, oom etc.) PodSandboxStatus is kind of wedged.
We can use the fact that if we didn't go through the usual k8s flow
of Stop->Remove and we still have an entry in our sandbox store,
us not having a shim mapping anymore means this was likely unintentional.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Using symlinks for bind mounts means we are not protecting an RO-mounted
layer against modification. Windows doesn't currently appear to offer a
better approach though, as we cannot create arbitrary empty WCOW scratch
layers at this time.
For windows-layer mounts, Unmount does not have access to the mounts
used to create it. So we store the relevant data in an Alternate Data
Stream on the mountpoint in order to be able to Unmount later.
Based on approach in https://github.com/containerd/containerd/pull/2366,
with sign-offs recorded as 'Based-on-work-by' trailers below.
This also partially-reverts some changes made in #6034 as they are not
needed with this mounting implmentation, which no longer needs to be
handled specially by the caller compared to non-Windows mounts.
Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Based-on-work-by: Michael Crosby <crosbymichael@gmail.com>
Based-on-work-by: Darren Stahl <darst@microsoft.com>
This error message currently does not provide useful information, because the `src` value that is interleaved will have been overridden by the call to `osi.ResolveSymbolicLink`. This stores the original `src` before the `osi.ResolveSymbolicLink` call so the error message can be useful.
Signed-off-by: June Rhodes <504826+hach-que@users.noreply.github.com>
In the CRI server initialization a syncgroup is setup that adds to the
counter for every cni config found/registered. This functions on platforms
where CNI is supported/theres an assumption that there will always be
the loopback config. However, on platforms like Darwin where there's generally
nothing registered the Wait() on the syncgroup returns immediately and the
channel used to return any Network config sync errors is closed. This channel
is one of three that's used to monitor if we should Close the CRI service in
containerd, so it's not great if this happens.
Signed-off-by: Danny Canter <danny@dcantah.dev>
These deprecations were mentioned in `pkg/cri/config/config.go`
but not mentioned in `RELEASES.md`.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
This commit adds supports for the ArgsEscaped
value for the image got from the dockerfile.
It is used to evaluate and process the image
entrypoint/cmd and container entrypoint/cmd
options got from the podspec.
Signed-off-by: Kirtana Ashok <Kirtana.Ashok@microsoft.com>
1. it's easy to check wrong input if using drain_exec_sync_io_timeout in error
2. avoid to use full error message, as part of error generated by go
stdlib would be changed in the future
3. delete the extra empty line
Signed-off-by: Wei Fu <fuweid89@gmail.com>
By default, the child processes spawned by exec process will inherit standard
io file descriptors. The shim server creates a pipe as data channel. Both exec
process and its children write data into the write end of the pipe. And the
shim server will read data from the pipe. If the write end is still open, the
shim server will continue to wait for data from pipe.
So, if the exec command is like `bash -c "sleep 365d &"`, the exec process is
bash and quit after create `sleep 365d`. But the `sleep 365d` will hold the
write end of the pipe for a year! It doesn't make senses that CRI plugin
should wait for it.
For this case, we should use timeout to drain exec process's io instead of
waiting for it.
Fixes: #7802
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Update NRI plugin configuration to match that of NRI. Remove
option for the eliminated NRI configuration file. Add option
to disable connections from externally launched plugins. Add
options to override default plugin registration and request
timeouts.
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
The snapshotter annotation definitions and related functions have been
public in the new packge snapshotter
Also remove a test for container image layer's annotation.
Signed-off-by: Changwei Ge <gechangwei@bytedance.com>
From golangci-lint:
> SA1019: rand.Read has been deprecated since Go 1.20 because it
>shouldn't be used: For almost all use cases, crypto/rand.Read is more
>appropriate. (staticcheck)
> SA1019: rand.Seed has been deprecated since Go 1.20 and an alternative
>has been available since Go 1.0: Programs that call Seed and then expect
>a specific sequence of results from the global random source (using
>functions such as Int) can be broken when a dependency changes how
>much it consumes from the global random source. To avoid such breakages,
>programs that need a specific result sequence should use
>NewRand(NewSource(seed)) to obtain a random generator that other
>packages cannot access. (staticcheck)
See also:
- https://pkg.go.dev/math/rand@go1.20#Read
- https://pkg.go.dev/math/rand@go1.20#Seed
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
There is a new CNI capability argument, cgroupPath, where runtimes can
pass cgroup paths to CNI plugins.
Implement that.
Signed-off-by: Casey Callendrello <cdc@isovalent.com>
All of the CRI sandbox and container specs all get assigned
almost the exact same default annotations (sandboxID, name, metadata,
container type etc.) so lets make a helper to return the right set for
a sandbox or regular workload container.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Split out the criService-agnostic bits of nri-api* from
pkg/cri/server to pkg/cri/nri to allow sharing a single
implementation betwen the server and sbserver versions.
Rework the interfaces to not require access to package
internals.
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
In https://github.com/containerd/containerd/pull/7764 it was made
so that generic runtime options in the containerd toml config file
would get passed to shims regardless of if containerd knew of the
type beforehand and could supply the struct. However, this was only
added for the sandbox server fork here and not the regular ol' CRI
server. This change just mirrors the parts that need to be plopped in
pkg/cri/server
Signed-off-by: Danny Canter <danny@dcantah.dev>
Before this patch, both the RdtEnabled and BlockIOEnabled are provided
by services/tasks pkg. Since the services/tasks can be pkg plugin which
can be initialized multiple times or concurrently. It will fire data-race
issue as there is no mutex to protect `enable`.
This patch is aimed to provide wrapper pkgs to use intel/{blockio,rdt}
safely.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
For determinism of human-readable string representation.
e.g., "2023-01-10T12:34:56Z" vs "2023-01-10T21:34:56+09:00"
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
/etc/cni has to be readable for non-root users (0755), because /etc/cni/tuning/allowlist.conf is used for rootless mode too.
This file was introduced in CNI plugins 1.2.0 (containernetworking/plugins PR 693), and its path is hard-coded.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
- Add Target to mount.Mount.
- Add UnmountMounts to unmount a list of mounts in reverse order.
- Add UnmountRecursive to unmount deepest mount first for a given target, using
moby/sys/mountinfo.
Signed-off-by: Edgar Lee <edgarhinshunlee@gmail.com>
Some of this code was originally added in b7b1200dd3,
which likely meant to initialize the slice with a length to reduce allocations,
however, instead of initializing with a zero-length and a capacity, it
initialized the slice with a fixed length, which was corrected in commit
0c63c42f81.
This patch initializes the slice with a zero-length and expected capacity.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Provides a couple helper functions that provide a background context for
running cleanup jobs while preserving the original context values.
The new contexts will not inherit the errors or cancellations.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Comments in initPlatform for Windows states that the options were
Linux specific. Additionally properly wrap an error after trying
to setup CDI on Linux.
Signed-off-by: Danny Canter <danny@dcantah.dev>
The sandbox and container both have the userns config. Lets make sure
they are the same, therefore consistent.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Currently we require that c.containerSpec() does not return an error
if test.err is not set.
However, if the require fails (i.e. it indeed returned an error) the
rest of the code is executed anyways. The rest of the code assumes it
did not return an error (so code assumes spec is not nil). This fails
miserably if it indeed returned an error, as spec is nil and go crashes
while running the unit tests.
Let's require it is not an error, so code does not continue to execute
if that fails and go doesn't crash.
In the test.err case is not harmful the bug of using assert, but let's
switch it to require too as that is what we really want.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
A domainname field was recently added to the OCI spec. Prior to this
folks would need to set this with a sysctl, but now runtimes should be
able to setdomainname(2). There's an open change to runc at the moment
to add support for this so I've just left testing as a couple spec
validations in CRI until that's in and usable.
Signed-off-by: Danny Canter <danny@dcantah.dev>
This patch requests the OCI runtime to create a userns when the CRI
message includes such request.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
This allows user namespace support to progress, either by allowing
snapshotters to deal with ownership, or falling back to containerd doing
a recursive chown.
In the future, when snapshotters implement idmap mounts, they should
report the "remap-ids" capability.
Co-authored-by: Rodrigo Campos <rodrigoca@microsoft.com>
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Signed-off-by: David Leadbeater <dgl@dgl.cx>
As explained in the comments, this patch lets the OCI runtime create the
netns when userns are in use. This is needed because the netns needs to
be owned by the userns (otherwise can't modify the IP, etc.).
Before this patch, we are creating the netns and then starting the pod
sandbox asking to join this netns. This can't never work with userns, as
the userns needs to be created first for the netns ownership to be
correct.
One option would be to also create the userns in containerd, then create
the netns. But this is painful (needs tricks with the go runtime,
special care to write the mapping, etc.).
So, we just let the OCI runtime create the userns and netns, that
creates them with the proper ownership.
As requested by Mike Brown, the current code when userns is not used is
left unchanged. We can unify the cases (with and without userns) in a
future release.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Due to when we were updating the pod sandboxes underlying container
object, the pointer to the sandbox would have the right info, but
the on-disk representation of the data was behind. This would cause
the data returned from loading any sandboxes after a restart to have
no CNI result or IP information for the pod.
This change does an additional update to the on-disk container info
right after we invoke the CNI plugin so the metadata for the CNI result
and other networking information is properly flushed to disk.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Skip automatic `if swapLimit == 0 { s.Linux.Resources.Memory.Swap = &limit }` when the swap controller is missing.
(default on Ubuntu 20.04)
Fix issue 7828 (regression in PR 7783 "cri: make swapping disabled with memory limit")
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
While we need to support CRI v1alpha2, the implementation doesn't have
to be tied to gogo/protobuf.
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
We do a ton of host networking checks around the CRI plugin, all mainly
doing the same thing of checking the different quirks on various platforms
(for windows are we a HostProcess pod, for linux is namespace mode the
right thing, darwin doesn't have CNI support etc.) which could all be
bundled up into a small helper that can be re-used.
Signed-off-by: Danny Canter <danny@dcantah.dev>
OCI runtime spec defines memory.swap as 'limit of memory+Swap usage'
so setting them to equal should disable the swap. Also, this change
should make containerd behaviour same as other runtimes e.g
'cri-dockerd/dockershim' and won't be impacted when user turn on
'NodeSwap' (https://github.com/kubernetes/enhancements/issues/2400) feature.
Signed-off-by: Qasim Sarfraz <qasimsarfraz@microsoft.com>
Adds a service capable of streaming Any objects bi-directionally.
This can be used by services to send data, received data, or to
initiate requests from server to client.
Signed-off-by: Derek McGowan <derek@mcg.dev>
In the CRI streaming server, a goroutine (`handleResizeEvents`) is launched
to handle terminal resize events if a TTY is asked for with an exec; this
is the sender of terminal resize events. Another goroutine is launched
shortly after successful process startup to actually do something with
these events, however the issue arises if the exec process fails to start
for any reason that would have `process.Start` return non-nil. The receiver
goroutine never gets launched so the sender is stuck blocked on a channel send
infinitely.
This could be used in a malicious manner by repeatedly launching execs
with a command that doesn't exist in the image, as a single goroutine
will get leaked on every invocation which will slowly grow containerd's
memory usage.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Remove direct invocation of old v0.1.0 NRI plugins. They
can be enabled using the revised NRI API and the v0.1.0
adapter plugin.
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Implement the adaptation interface required by the NRI
service plugin to handle CRI sandboxes and containers.
Hook the NRI service plugin into CRI request processing.
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Add a common NRI 'service' plugin. It takes care of relaying
requests and respones to and from NRI (external NRI plugins)
and the high-level containerd namespace-independent logic of
applying NRI container adjustments and updates to actual CRI
and other containers.
The namespace-dependent details of the necessary container
manipulation operations are to be implemented by namespace-
specific adaptations. This NRI plugin defines the API which
such adaptations need to implement.
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
All pause container object references must be removed
from sbserver. This is an implementation detail of
podsandbox package.
Added TODOs for remaining work.
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
In golang when copy a slice, if the slice is initialized with a
desired length, then appending to it will cause the size double.
Signed-off-by: bin liu <liubin0329@gmail.com>
Add a new config as sandbox controller mod, which can be either
"podsandbox" or "shim". If empty, set it to default "podsandbox"
when CRI plugin inits.
Signed-off-by: Zhang Tianyang <burning9699@gmail.com>
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>
Signed-off-by: Swagat Bora <sbora@amazon.com>
Add spans around image unpack operations
Use image.ref to denote image name and image.id for the image config digest
Add top-level spand and record errors in the CRI instrumentation service
Old TODO stating that pkg/cri/opts's `WithWindowsNetworkNamespace`
should be moved to the main containerd pkg was out of date as thats
already been done (well, to the /oci package). This just removes it
and swaps all uses of `WithWindowsNetworkNamespace` to the oci
packages impl.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Remove nolint-comments that weren't hit by linters, and remove the "structcheck"
and "varcheck" linters, as they have been deprecated:
WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [linters context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- fix "nolint" comments to be in the correct format (`//nolint:<linters>[,<linter>`
no leading space, required colon (`:`) and linters.
- remove "nolint" comments for errcheck, which is disabled in our config.
- remove "nolint" comments that were no longer needed (nolintlint).
- where known, add a comment describing why a "nolint" was applied.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This `//nolint` was added in f5c7ac9272
to suppress warnings about the `NameToCertificate` function being deprecated:
// Deprecated: NameToCertificate only allows associating a single certificate
// with a given name. Leave that field nil to let the library select the first
// compatible chain from Certificates.
Looking at that, it was deprecated in Go 1.14 through
eb93c684d4
(https://go-review.googlesource.com/c/go/+/205059), which describes:
crypto/tls: select only compatible chains from Certificates
Now that we have a full implementation of the logic to check certificate
compatibility, we can let applications just list multiple chains in
Certificates (for example, an RSA and an ECDSA one) and choose the most
appropriate automatically.
NameToCertificate only maps each name to one chain, so simply deprecate
it, and while at it simplify its implementation by not stripping
trailing dots from the SNI (which is specified not to have any, see RFC
6066, Section 3) and by not supporting multi-level wildcards, which are
not a thing in the WebPKI (and in crypto/x509).
We should at least have a comment describing why we are ignoring this, but preferably
review whether we should still use it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Currently CDI registry is reconfigured on every
WithCDI call, which is a relatively heavy operation.
This happens because cdi.GetRegistry(cdi.WithSpecDirs(cdiSpecDirs...))
unconditionally reconfigures the registry (clears fs notify watch,
sets up new watch, rescans directories).
Moving configuration to the criService.initPlatform should result
in performing registry configuration only once on the service start.
Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
As WithCDI is CRI-only API it makes sense to move it
out of oci module.
This move can also fix possible issues with this API when
CRI plugin is disabled.
Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
This makes diff archives to be reproducible.
The value is expected to be passed from CLI applications via the $SOUCE_DATE_EPOCH env var.
See https://reproducible-builds.org/docs/source-date-epoch/
for the $SOURCE_DATE_EPOCH specification.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Rework sandbox monitoring, we should rely on Controller.Wait instead of
CRIService.StartSandboxExitMonitor
Signed-off-by: WangLei <wllenyj@linux.alibaba.com>
pkg/cri/sbserver/cri_fuzzer.go and pkg/cri/server/cri_fuzzer.go were
mostly the same.
This commit merges them together and move the unified fuzzer to
contrib/fuzz again to sort out dependencies. pkg/cri/ shouldn't consume
cmd/.
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
As part of the effort of getting hypervisor isolated windows container
support working for the CRI entrypoint here, add the runhcs-wcow-hypervisor
handler for the default config. This sets the correct SandboxIsolation
value that the Windows shim uses to differentiate process vs. hypervisor
isolation. This change additionally sets the wcow-process runtime to
passthrough io.microsoft.container* annotations and the hypervisor runtime
to accept io.microsoft.virtualmachine* annotations.
Note that for K8s users this runtime handler will need to be configured by
creating the corresponding RuntimeClass resources on the cluster as it's
not the default runtime.
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
It is follow-up of #7254. This commit will increase ReadHeaderTimeout
from 3s to 30m, which prevent from unexpected timeout when the node is
running with high-load. 30 Minutes is longer enough to get close to
before what #7254 changes.
And ideally, we should allow user to configure the streaming server if
the users want this feature.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
`ioutil` has been deprecated by golang. All the code in `ioutil` just
forwards functionality to code in either the `io` or `os` packages.
See https://github.com/golang/go/pull/51961 for more info.
Signed-off-by: Jeff Widman <jeff@jeffwidman.com>
Failpoint is used to control the fail during API call when testing, especially
the API is complicated like CRI-RunPodSandbox. It can help us to test
the unexpected behavior without mock. The control design is based on freebsd
fail(9), but simpler.
REF: https://www.freebsd.org/cgi/man.cgi?query=fail&sektion=9&apropos=0&manpath=FreeBSD%2B10.0-RELEASE
Signed-off-by: Wei Fu <fuweid89@gmail.com>
All of the CRI store related packages all use the standard errdefs
errors now for if a key doesn't or already exists (ErrAlreadyExists,
ErrNotFound), but the comments for the methods still referenced
some unused package specific error definitions. This change just
updates the comments to reflect what errors are actually returned
and adds comments for some previously undocumented exported functions.
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
All containers except the pause container, mount `/dev/shm" with flags
`nosuid,nodev,noexec`. So change mount options for pause container to
keep consistence.
This also helps to solve issues of failing to mount `/dev/shm` when
pod/container level user namespace is enabled.
Fixes: #6911
Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
Signed-off-by: Lei Wang <wllenyj@linux.alibaba.com>
This's an optimization to get rid of redundant `/dev/shm" mounts for pause container.
In `oci.defaultMounts`, there is a default `/dev/shm` mount which is redundant for
pause container.
Fixes: #6911
Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
Signed-off-by: Lei Wang <wllenyj@linux.alibaba.com>
The TestPodAnnotationPassthroughContainerSpec test and the
TestContainerAnnotationPassthroughContainerSpec test both depend on a
platform-specific implementation of criService.containerSpec, which is
unimplemented on FreeBSD.
The TestSandboxContainerSpec depends on a platform-specific
implementation oc criService.sandboxContainerSpec, which is
unimplemented on FreeBSD.
Signed-off-by: Samuel Karp <me@samuelkarp.com>
This change does a couple things to remove some cruft/unused functionality
in the Windows snapshotter, as well as add a way to specify the rootfs
size in bytes for a Windows container via a new field added in the CRI api in
k8s 1.24. Setting the rootfs/scratch volume size was assumed to be working
prior to this but turns out not to be the case.
Previously I'd added a change to pass any annotations in the containerd
snapshot form (containerd.io/snapshot/*) as labels for the containers
rootfs snapshot. This was added as a means for a client to be able to provide
containerd.io/snapshot/io.microsoft.container.storage.rootfs.size-gb as an
annotation and have that be translated to a label and ultimately set the
size for the scratch volume in Windows. However, this actually only worked if
interfacing with the CRI api directly (crictl) as Kubernetes itself will
fail to validate annotations that if split by "/" end up with > 2 parts,
which the snapshot labels will (containerd.io / snapshot / foobarbaz).
With this in mind, passing the annotations and filtering to
containerd.io/snapshot/* is moot, so I've removed this code in favor of
a new `snapshotterOpts()` function that will return platform specific
snapshotter options if ones exist. Now on Windows we can just check if
RootfsSizeInBytes is set on the WindowsContainerResources struct and
then return a snapshotter option that sets the right label.
So all in all this change:
- Gets rid of code to pass CRI annotations as labels down to snapshotters.
- Gets rid of the functionality to create a 1GB sized scratch disk if
the client provided a size < 20GB. This code is not used currently and
has a few logical shortcomings as it won't be able to create the disk
if a container is already running and using the same base layer. WCIFS
(driver that handles the unioning of windows container layers together)
holds open handles to some files that we need to delete to create the
1GB scratch disk is the underlying problem.
- Deprecates the containerd.io/snapshot/io.microsoft.container.storage.rootfs.size-gb
label in favor of a new containerd.io/snapshot/windows/rootfs.sizebytes label.
The previous label/annotation wasn't being used by us, and from a cursory
github search wasn't being used by anyone else either. Now that there is a CRI
field to specify the size, this should just be a field that users can set
on their pod specs and don't need to concern themselves with what it eventually
gets translated to, but non-CRI clients can still use the new label/deprecated
label as usual.
- Add test to cri integration suite to validate expanding the rootfs size.
Signed-off-by: Daniel Canter <dcanter@microsoft.com>
