Commit Graph

459 Commits

Author SHA1 Message Date
Shiming Zhang
fcf3b275fc Add lock for ListPids
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
2021-04-15 13:55:03 +08:00
Maksym Pavlenko
0ad8c0a169 Decouple shim start from task creation
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
2021-04-11 18:51:27 -07:00
Sebastiaan van Stijn
7bb73da6b9
runtime/v2/shim: remove unused SetScore() and remove sys.OOMScoreMaxKillable
The shim.SetScore() utility was no longer used since 7dfc605fc6.

Checking for uses outside of this repository, I found only one external use of
this in gVisor; a9441aea27/pkg/shim/service.go (L262-L264)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-04-07 19:16:58 +02:00
Sebastiaan van Stijn
91e7d21ee8
sys: add AdjustOOMScore() utility
Handle the limits in this function so that consumers don't have to
perform the boundary checks.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-04-07 19:16:56 +02:00
Sebastiaan van Stijn
708299ca40
Move RunningInUserNS() to its own package
This allows using the utility without bringing whole of "sys" with it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-03-23 11:29:53 +01:00
Maksym Pavlenko
56f17a0856
Merge pull request #5148 from wzshiming/fix/defer-cleanup
runtime/v2: Fix defer cleanup for TaskManager.Create
2021-03-20 13:24:42 -07:00
Shiming Zhang
30e1e66e5c runtime/v2: Fix defer cleanup
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
2021-03-20 18:40:36 +08:00
Michael Crosby
e0c94bb269
Merge pull request #4708 from kzys/enable-criu
Re-enable CRIU tests by not using overlayfs snapshotter
2021-03-19 14:23:05 -04:00
Shiming Zhang
fe787efa2b Fix error log when kill shim
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
2021-03-19 19:03:47 +08:00
Wei Fu
9fdc96c095 runtime/v2: add comment for checkCopyShimLogError
After #4906, containerd opens fifo in read/write mode in linux platform
The original comment doesn't correct and is removed by #5174.

```
// original comment

// When using a multi-container shim, the fifo of the 2nd to Nth
// container will not be opened when the ctx is done. This will
// cause an ErrReadClosed that can be ignored.
```

However, we should add comment for checkCopyShimLogError to mention why
we call checkCopyShimLogError. The checkCopyShimLogError, it is to prevent
the flood of expected error messages after task die and the expected
errors depend on platform.

Signed-off-by: Wei Fu <fuweid89@gmail.com>
2021-03-18 13:02:28 +08:00
Phil Estes
a0cc9b432d
Merge pull request #5195 from fuweid/fix-5173
runtime/v2/runc: fix leaking socket path
2021-03-17 09:33:41 -04:00
Kazuyoshi Kato
b520428b5a Fix CRIU
- process.Init#io could be nil
- Make sure CreateTaskRequest#Options is not empty before unmarshaling

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
2021-03-16 16:46:45 -07:00
Iceber Gu
5e484c9613
runtime/v2/runc: fix the defer cleanup of the NewContainer
Signed-off-by: Iceber Gu <wei.cai-nat@daocloud.io>
2021-03-16 11:41:17 +08:00
Phil Estes
a1138182d5
Merge pull request #5180 from dmcgowan/lint-enforce-comments
Fix exported comments enforcer in CI
2021-03-15 10:50:06 -04:00
Wei Fu
d895118c7c runtime/v2/runc: fix leaking socket path
When runC shimv2 starts, the StartShim interface will re-exec itself as
long-running process, which will read the `address` during initializing.

```happycase
Process

containerd-shim-runc-v1/v2 start             containerd-shim-runc-v1/v2

	initializing socket

	reexec containerd-shim-runc-v1/v2

	write address into file

						initializing

							read address

	write back to containerd daemon

						serving

						...

						remove address in Shutdown call
```

However, there is no synchronization after reexec. Then the data race is
like:

```leaking-case
Process

containerd-shim-runc-v1/v2 start             containerd-shim-runc-v1/v2

	initializing socket

	reexec containerd-shim-runc-v1/v2

						initializing

							read address

	write address into file

	write back to containerd daemon

						serving

						...

						fail to remove address
						because of empty address
```

The `address` should be writen into file first before reexec.

And if shutdown the whole service before cleanup temporary
resource (like socket file), the Shutdown caller will receive `ttrpc: closed`
sometime, which depends on go runtime scheduler. Then it also causes leaking
socket files.

Since the shimV2-Delete binary API must be called to cleanup shim temporary
resource and shimV2-runC-v1 doesn't support grouping multi containers in one,
it is safe to remove the socket file in the binary call for shimV2-runC-v1.
But for the shimV2-runC-v2 shim, we still cleanup socket in Shutdown.
Hopefully we can find a way to cleanup socket in shimV2-Delete binary
call.

Fix: #5173

Signed-off-by: Wei Fu <fuweid89@gmail.com>
2021-03-15 18:32:00 +08:00
Wei Fu
eabd9b98b6 runtime: ignore file-already-closed error if dead shim
fix: #5130

Signed-off-by: Wei Fu <fuweid89@gmail.com>
2021-03-15 12:18:26 +08:00
Derek McGowan
35eeb24a17
Fix exported comments enforcer in CI
Add comments where missing and fix incorrect comments

Signed-off-by: Derek McGowan <derek@mcg.dev>
2021-03-12 08:47:05 -08:00
Kevin Parsons
c9afc4250a Fix error checking when resolving shim binary path
Previously a typo was introduced that caused the wrong error to be
checked against when calling exec.LookPath. This had the effect that
containerd would never locate the shim binary if it was in the same
directory as containerd's binary, but not in PATH.

Signed-off-by: Kevin Parsons <kevpar@microsoft.com>
2021-03-08 16:24:19 -08:00
Maksym Pavlenko
134f7a7370
Merge pull request #5007 from fidencio/wip/allow-shimv2-to-also-be-loaded-from-an-arbitrary-path
v2, util: Take the full binary path when starting the shimv2 process
2021-03-01 14:52:27 -08:00
Derek McGowan
10bbd1a462
Merge pull request #5051 from wzshiming/fix/missing-close
Fix missing close
2021-02-26 14:59:43 -08:00
Derek McGowan
9884730e5c
Merge pull request #5069 from AkihiroSuda/restart-fast
restart: skip Sleep() for the first iteration of the reconcilation
2021-02-25 16:37:53 -08:00
Akihiro Suda
6ab6eaa790
restart: skip Sleep() for the first iteration of the reconcilation
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2021-02-25 13:30:38 +09:00
Akihiro Suda
b23dc1131e
restart: parallelize reconcile()
The only shared variable `m.client` is thread-safe, so we can safely
parallelize the loops.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2021-02-25 13:30:00 +09:00
Shiming Zhang
05ef2fe2fb Fix missing close
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
2021-02-18 13:21:42 +08:00
Fabiano Fidêncio
d80dbdae68 v2, util: Take the full binary path when starting the shimv2 process
The current code simply ignores the full binary path when starting the
shimv2 process, and instead fallbacks to a binary in the path, and this
is problematic (and confusing) for those using CRI-O, which has this
bits vendored.

The reason it's problematic with CRI-O is because the user can simply
set the full binary path and, instead of having that executed, CRI-O
will simply fail to create the container unless that binary is part of
the path, which may not be case in a few different scenarios (testing
being the most common one).

Fixes: #5006

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
2021-02-05 13:35:22 +01:00
IceberGu
b458583b76
runtime: fix shutdown runc v2 service
Signed-off-by: IceberGu <wei.cai-nat@daocloud.io>
2021-02-02 15:36:49 +08:00
Phil Estes
49c5c14879
Merge pull request #4906 from payall4u/bugfix/fix-open-shim-fifo
bugfix: change the flag of open log fifo to avoid containerd hang on syscall open
2021-02-01 09:01:38 -05:00
payall4u
957fa3379d change flag from RDONLY to RDWR and close the fifo correct
Signed-off-by: Zhiyu Li <payall4u@qq.com>
2021-01-31 19:00:42 +08:00
Aditi Sharma
1423e9199d Update gogo/protobuf to v1.3.2
bump version 1.3.2 for gogo/protobuf due to CVE-2021-3121 discovered
in gogo/protobuf version 1.3.1, CVE has been fixed in 1.3.2

Signed-off-by: Aditi Sharma <adi.sky17@gmail.com>
2021-01-28 12:57:50 +00:00
Maksim An
ddb5e1651a Enhance logging driver and ctr tasks to support windows
Signed-off-by: Maksim An <maksiman@microsoft.com>
2021-01-21 12:17:32 -08:00
Wei Fu
846cb963cc runtime/v2: should use defer ctx to cleanup
Signed-off-by: Wei Fu <fuweid89@gmail.com>
2021-01-11 23:22:38 +08:00
Maksym Pavlenko
c1b01eabc0 Add copyright header to proto files
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
2021-01-05 10:44:07 -08:00
Michael Crosby
dc207b654d
Merge pull request #4860 from masters-of-cats/pr-process-not-found-err
Return GRPC not found error instead of plain one
2020-12-21 10:25:11 -05:00
Georgi Sabev
7451dd1ed1 Return GRPC not found error instead of plain one
When the shim returns a plain error when a process does not exist,
the server is unable to recognise its GRPC status code and assumes
UnknownError. This is awkward for containerd client users as they are
unable to recognise the actual reason for the error.

When the shim returns a NotFound GRPC error, it is properly translated
by the server and clients receive a proper NotFound error instead of
Unknown

Please note that we (CF Garden) would like to have the eventual fix backported to 1.4 as well.

Co-authored-by: Danail Branekov <danailster@gmail.com>

Signed-off-by: Danail Branekov <danailster@gmail.com>
Signed-off-by: Georgi Sabev <georgethebeatle@gmail.com>
2020-12-18 15:33:48 +02:00
Phil Estes
070b698449
Merge pull request #4845 from skaegi/oom_score-max
Add bounds on max oom_score_adj value for AdjustOOMScore
2020-12-17 16:22:46 -05:00
Simon Kaegi
da2fd657ab Add bounds on max oom_score_adj value for AdjustOOMScore
oom_score_adj must be in the range -1000 to 1000. In AdjustOOMScore if containerd's score is already at the maximum value we should set that value for the shim instead of trying to set 1001 which is invalid.

Signed-off-by: Simon Kaegi <simon_kaegi@ca.ibm.com>
2020-12-14 15:09:24 -05:00
Akihiro Suda
0356d5d4b2
restart: allow passing existing log URI object
The new function `WithLogURI(uri *url.URL)` replaces `WithBinaryLogURI(binary string, args map[string]string)`
so as to allow passing an existring URI object.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2020-12-12 05:11:03 +09:00
Akihiro Suda
7126310a09
Merge pull request #4784 from fuweid/fix-4769
runtime: should not send duplicate task exit event
2020-12-02 15:26:57 +09:00
Wei Fu
faec5d4ffd runtime: should not send duplicate task exit event
If the shim has been killed and ttrpc connection has been
closed, the shimErr will not be nil. For this case, the event
subscriber, like moby/moby, might have received the exit or delete
events. Just in case, we should allow ttrpc-callback-on-close to
send the exit and delete events again. And the exit status will
depend on result of shimV2.Delete.

If not, the shim has been delivered the exit and delete events.
So we should remove the task record and prevent duplicate events from
ttrpc-callback-on-close.

Fix: #4769

Signed-off-by: Wei Fu <fuweid89@gmail.com>
2020-12-01 21:54:04 +08:00
Derek McGowan
4a4bb851f5
Merge pull request from GHSA-36xw-fx78-c5r4
Use path based unix socket for shims
2020-11-30 10:32:18 -08:00
Maksym Pavlenko
0d4734655f
Merge pull request #4647 from katiewasnothere/task_update_annotations_upstream
Add annotations to task update request api
2020-11-18 14:44:19 -08:00
Samuel Karp
126b35ca43
containerd-shim: use path-based unix socket
This allows filesystem-based ACLs for configuring access to the socket
of a shim.

Ported from Michael Crosby's similar patch for v2 shims.

Signed-off-by: Samuel Karp <skarp@amazon.com>
2020-11-11 11:47:47 -08:00
Michael Crosby
bd908acabd
Use path based unix socket for shims
This allows filesystem based ACLs for configuring access to the socket of a
shim.

Co-authored-by: Samuel Karp <skarp@amazon.com>
Signed-off-by: Samuel Karp <skarp@amazon.com>
Signed-off-by: Michael Crosby <michael@thepasture.io>
Signed-off-by: Michael Crosby <michael.crosby@apple.com>
2020-11-11 11:47:46 -08:00
Kathryn Baldauf
95ba6e9f75 Add annotations to task update request api
Signed-off-by: Kathryn Baldauf <kabaldau@microsoft.com>
2020-11-09 14:13:33 -08:00
Maksym Pavlenko
4da306e1e9 Fix panic in shim not logged
Fix #4274
Carry #4298

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
2020-10-26 09:05:47 -07:00
Giuseppe Capizzi
8eda32e107 Check if a process exists before returning it
Fixes #4632.

Signed-off-by: Giuseppe Capizzi <gcapizzi@pivotal.io>
Co-authored-by: Danail Branekov <danailster@gmail.com>
2020-10-22 16:50:14 +03:00
Akihiro Suda
915263f269
Merge pull request #4502 from akshat-kmr/master
Add logging binary support when terminal is true
2020-10-08 12:14:39 +09:00
Maksym Pavlenko
c59d1cd5b0 Fix linter issues
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
2020-10-07 15:42:01 -07:00
Phil Estes
68d97331be
Merge pull request #4538 from fuweid/update-shim-cleanup
runtime/v2: cleanup dead shim before delete bundle
2020-09-21 13:32:40 -04:00
Wei Fu
4b05d03903 runtime/v2: cleanup dead shim before delete bundle
The shim delete action needs bundle information to cleanup resources
created by shim. If the cleanup dead shim is called after delete bundle,
the part of resources maybe leaky.

The ttrpc client UserOnCloseWait() can make sure that resources are
cleanup before delete bundle, which synchronizes task deletion and
cleanup deadshim. It might slow down the task deletion, but it can make
sure that resources can be cleanup and avoid EBUSY umount case. For
example, the sandbox container like Kata/Firecracker might have mount
points over the rootfs. If containerd handles task deletion and cleanup
deadshim parallelly, the task deletion will meet EBUSY during umount and
fail to cleanup bundle, which makes case worse.

And also update cleanupAfterDeadshim, which makes sure that
cleanupAfterDeadshim must be called after shim disconnected. In some
case, shim fails to call runc-create for some reason, but the runc-create
already makes runc-init into ready state. If containerd doesn't call shim
deletion, the runc-init process will be leaky and hold the cgroup, which
makes pod terminating :(.

Signed-off-by: Wei Fu <fuweid89@gmail.com>
2020-09-20 11:24:31 +08:00