This updates the platforms package to be an alias for the new platforms module.
This helps transitioning consumers to the new module, and makes sure that
containerd v2 and v1 use the same definitions.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The reference/docker package was a fork of github.com/distribution/distribution,
which could not easily be used as a direct dependency, as it brought many other
dependencies with it.
The "reference' package has now moved to a separate repository, which means
we can replace the local fork, and use the upstream implementation again.
The new module was extracted from the distribution repository at commit:
b9b19409cf
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 4923470902)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Change in 989f1ec54f assumed a single
place (pkg/cri/sbserver) where this had to be fixed.
Commit in main branch for reference:
(cherry picked from commit c7ea06a69b)
Signed-off-by: Maksim An <maksiman@microsoft.com>
Commit 3c8469a782 removed uses of the api
types.Platform type from public interfaces, instead using the type from
the OCI image spec.
For convenience, it also introduced an alias in the platforms package.
While this alias allows packages that already import containerd's
platforms package (now a separate module), it may also cause confusion
(it's not clear that it's an alias for the OCI type), and for packages
that do not depend on containerd's platforms package / module may now
be resulting in an extra dependency.
Let's remove the use of this alias, and instead use the OCI type directly.
Equivalent of 446e63579c in main.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Import the log module directly, instead of using the aliases that are
provided in the log package. This helps find code-paths that still depend
on the old location, and that haven't migrated yet to the new module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We should set sandbox CreatedAt first time when we create sandbox struct,
and then set sandbox CreatedAt second time after container started.
Before this commit, we just set sandbox CreatedAt after container
started, but if network create failed, the sandbox time is the
default time, which is 269 years ago, so we need to set sandbox
CreatedAt at first, even if an error occurred before start container.
Signed-off-by: zzzzzzzzzy9 <zhang.yu58@zte.com.cn>
(cherry picked from commit b16d467404)
There was no merge conflict, but the commit was amended to cover
sbserver.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Currently it didn't take into account that certain snapshots can explicitly
have their root directories placed at a different location. This changes
it to use the RootPath method of the snapshotter if it implements it.
Without this change, cadvisor is not able to get filesystem usage
information, which prevents the kubelet from doing image garbage
collection and enforcing ephemeral storage limits.
Signed-off-by: Danny Canter <danny@dcantah.dev>
(cherry picked from commit 6aeec45)
Signed-off-by: Kern Walster <walster@amazon.com>
This includes migrating from cdi.GetRegistry() to cdi.Configure() and
using top-level cdi Refresh and InjectDevices functions as applicable.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Fixes#10013. It seems we can end up in a spot where the sandbox store still
has a listing for a pod, whereas containerds underlying store has removed it.
It might be better to shield the caller (k8s) from these transient errors.
Signed-off-by: Danny Canter <danny@dcantah.dev>
(cherry picked from commit 2474a99c30)
Signed-off-by: Danny Canter <danny@dcantah.dev>
When the pods are transitioning there are several
cases where containers might not be in valid state.
There were several cases where the stats where
failing hard but we should just continue on as
they are transient and will be picked up again
when kubelet queries for the stats again.
Signed-off-by: James Sturtevant <jstur@microsoft.com>
Signed-off-by: Mark Rossetti <marosset@microsoft.com>
The cherry-pick has been amended to duplicate the tests in cri/sbserver and
cri/server.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
(cherry picked from commit 06ed897b22)
We added support for userns but we weren't showing it in the
podSandboxStatus.
Let's just show the whole nsOpts, so we don't forget in the future
either if something else inside there changes.
Please note that this will expose the content of nsOpts.TargetId that we
weren't exposing before. But that seemed like a bug to me.
The cherry-pick has been amended to do the change in cri/sbserver and
cri/server.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
(cherry picked from commit 6c356a5c75)
It's to ensure the data integrity during unexpected power failure.
Background:
Since release 1.3, in Linux system, containerD unpacks and writes files into
overlayfs snapshot directly. It doesn’t involve any mount-umount operations
so that the performance of pulling image has been improved.
As we know, the umount syscall for overlayfs will force kernel to flush
all the dirty pages into disk. Without umount syscall, the files’ data relies
on kernel’s writeback threads or filesystem's commit setting (for
instance, ext4 filesystem).
The files in committed snapshot can be loss after unexpected power failure.
However, the snapshot has been committed and the metadata also has been
fsynced. There is data inconsistency between snapshot metadata and files
in that snapshot.
We, containerd, received several issues about data loss after unexpected
power failure.
* https://github.com/containerd/containerd/issues/5854
* https://github.com/containerd/containerd/issues/3369#issuecomment-1787334907
Solution:
* Option 1: SyncFs after unpack
Linux platform provides [syncfs][syncfs] syscall to synchronize just the
filesystem containing a given file.
* Option 2: Fsync directories recursively and fsync on regular file
The fsync doesn't support symlink/block device/char device files. We
need to use fsync the parent directory to ensure that entry is
persisted.
However, based on [xfstest-dev][xfstest-dev], there is no case to ensure
fsync-on-parent can persist the special file's metadata, for example,
uid/gid, access mode.
Checkout [generic/690][generic/690]: Syncing parent dir can persist
symlink. But for f2fs, it needs special mount option. And it doesn't say
that uid/gid can be persisted. All the details are behind the
implemetation.
> NOTE: All the related test cases has `_flakey_drop_and_remount` in
[xfstest-dev].
Based on discussion about [Documenting the crash-recovery guarantees of Linux file systems][kernel-crash-recovery-data-integrity],
we can't rely on Fsync-on-parent.
* Option 1 is winner
This patch is using option 1.
There is test result based on [test-tool][test-tool].
All the networking traffic created by pull is local.
* Image: docker.io/library/golang:1.19.4 (992 MiB)
* Current: 5.446738579s
* WIOS=21081, WBytes=1329741824, RIOS=79, RBytes=1197056
* Option 1: 6.239686088s
* WIOS=34804, WBytes=1454845952, RIOS=79, RBytes=1197056
* Option 2: 1m30.510934813s
* WIOS=42143, WBytes=1471397888, RIOS=82, RBytes=1209344
* Image: docker.io/tensorflow/tensorflow:latest (1.78 GiB, ~32590 Inodes)
* Current: 8.852718042s
* WIOS=39417, WBytes=2412818432, RIOS=2673, RBytes=335987712
* Option 1: 9.683387174s
* WIOS=42767, WBytes=2431750144, RIOS=89, RBytes=1238016
* Option 2: 1m54.302103719s
* WIOS=54403, WBytes=2460528640, RIOS=1709, RBytes=208237568
The Option 1 will increase `wios`. So, the `image_pull_with_sync_fs` is
option in CRI plugin.
[syncfs]: <https://man7.org/linux/man-pages/man2/syncfs.2.html>
[xfstest-dev]: <https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git>
[generic/690]: <https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/690?h=v2023.11.19>
[kernel-crash-recovery-data-integrity]: <https://lore.kernel.org/linux-fsdevel/1552418820-18102-1-git-send-email-jaya@cs.utexas.edu/>
[test-tool]: <a17fb2010d/contrib/syncfs/containerd/main_test.go (L51)>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
(cherry picked from commit 23278c81fb)
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Close connection if no more data. It's to fix false alert filed by image
pull progress.
```
dst = OpenWriter (--> Content Store)
src = Fetch
Open (--> Registry)
Mark it as active request
Copy(dst, src) (--> Keep updating total received bytes)
^
| (Active Request > 0, but total received bytes won't be updated)
v
defer src.Close()
content.Commit(dst)
```
Before migrating to transfer service, CRI plugin doesn't limit global
concurrent downloads for ImagePulls. Each ImagePull requests have 3 concurrent
goroutines to download blob and 1 goroutine to unpack blob. Like ext4
filesystem [1][1], the fsync from content.Commit may sync unrelated dirty pages
into disk. The host is running under IO pressure, and then the content.Commit
will take long time and block other goroutines. If httpreadseeker
doesn't close the connection after io.EOF, this connection will be
considered as active. The pull progress reporter reports there is no
bytes transfered and cancels the ImagePull.
The original 1-minute timeout[2][2] is from kubelet settting. Since CRI-plugin
can't limit the total concurrent downloads, this patch is to update 1-minute
to 5-minutes to prevent from unexpected cancel.
[1]: https://lwn.net/Articles/842385/
[2]: https://github.com/kubernetes/kubernetes/blob/release-1.23/pkg/kubelet/config/flags.go#L45-L48
Signed-off-by: Wei Fu <fuweid89@gmail.com>
The new active request is filed and there is no bytes read yet when the
progress reporter just wakes up. If the timeout / 2 is less than the
minPullProgressReportInternal, it's easy to file false alert.
We should remove the minPullProgressReportInternal limit.
Fixes: #8024
Signed-off-by: Wei Fu <fuweid89@gmail.com>
`NewCRIService()` may easily fail and its error has to be ignored
unless the CRI plugin is in the `required_plugins` list.
Now this has to be called before `RegisterReadiness()`, as
PR 9153 "Require plugins to succeed after registering readiness"
was merged on 2023-09-29.
Fix issue 9163: `[Regression in main (2023-09-29)]: containerd-rootless.sh doesn't start up`
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
(cherry picked from commit 5365f4b29e)
Signed-off-by: Derek McGowan <derek@mcg.dev>
