Commit Graph

466 Commits

Author SHA1 Message Date
Shiming Zhang
7966a6652a Cleanup code
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
2021-04-19 16:59:45 +08:00
Maksym Pavlenko
69a30ad581
Merge pull request #5378 from Iceber/check-flag
runtime/shim: check the namespace flag first
2021-04-18 09:10:46 -07:00
Phil Estes
1e5cb4edcb
Merge pull request #5368 from mxpv/runtime_cleanup
Runtime cleanup
2021-04-16 14:50:15 -04:00
Iceber Gu
34780d67ad runtime/shim: check the namespace flag first
Signed-off-by: Iceber Gu <wei.cai-nat@daocloud.io>
2021-04-16 17:32:21 +08:00
Samuel Karp
b431fe4fc0
freebsd: don't run shim delete in deleted dir
fork/exec can fail and log a warning like this in containerd's log:
failed to clean up after shim disconnected error=": fork/exec /usr/local/bin/containerd-shim-[my-shim]: no such file or directory" id=test namespace=default

Passing the bundle path on the command line allows the shim delete
command to run successfully.

Signed-off-by: Samuel Karp <me@samuelkarp.com>
2021-04-15 18:09:29 -07:00
Maksym Pavlenko
b83d04f910 Add variable names to runtime's interface definitions
To ease code readability

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
2021-04-15 12:03:13 -07:00
Maksym Pavlenko
993b863993 Add shim start opts
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
2021-04-15 11:55:24 -07:00
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