[WIP] validation: add test for NSProcInPath#613
Closed
alban wants to merge 2 commits intoopencontainers:masterfrom
Closed
[WIP] validation: add test for NSProcInPath#613alban wants to merge 2 commits intoopencontainers:masterfrom
alban wants to merge 2 commits intoopencontainers:masterfrom
Conversation
I initially tried to add the checks in the container process 'runtimetest' by adding annotations prefixed with "runtimetest/". But that proved impractical with TAP outputs because I wanted to have several tests for each namespace. This patch now validates the namespaces outside the container with util.RuntimeOutsideValidate(). Signed-off-by: Alban Crequy <alban@kinvolk.io>
alban
force-pushed
the
alban/NSProcInPath
branch
from
f6ef8c1 to
4a9bf2c
Compare
alban
commented
Apr 4, 2018
| } | ||
| defer func() { | ||
| if cmd.Process != nil { | ||
| cmd.Process.Kill() |
Contributor
Author
There was a problem hiding this comment.
The sleep processes are surviving the death of the unshare parent unfortunately.
I tried cmd.Process.Signal(syscall.SIGTERM) but it does not help because:
- unshare does not catch the signal so it dies without forwarding the SIGTERM to its children
- processes in a different pid namespaces would not receive the signal anyway because of signal rules in pid namespaces (see man 7 pid_namespaces)
We need to deal with additional namespace strings, in case of mount & network namespaces, because `MapStrToNamespace()` does not recognize input strings like `mnt` or `net`. Found by @alban.
|
I created a new version of PR #628, based on this PR #613. For the record, I'll go through each TODO item.
|
Contributor
Author
|
Superseded by #628. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I initially tried to add the checks in the container process
'runtimetest' by adding annotations prefixed with "runtimetest/". But
that proved impractical with TAP outputs because I wanted to have
several tests for each namespace.
This patch now validates the namespaces outside the container with
util.RuntimeOutsideValidate().
TODO/notes:
Signed-off-by: Alban Crequy alban@kinvolk.io