add test cases for exec command#1133
Conversation
|
|
||
| wait_for_container 15 1 test_busybox | ||
|
|
||
| runc exec --cwd /bin test_busybox ls -la |
There was a problem hiding this comment.
Why not make this test use /bin/pwd instead -- so we can actually check that --cwd does what it says?
There was a problem hiding this comment.
ok, good idea.
update done.
|
|
||
| wait_for_container 15 1 test_busybox | ||
|
|
||
| runc exec --user 1000:1000 test_busybox id |
There was a problem hiding this comment.
I kinda want to also add a test for --user username:groupname. Busybox contains a passwd file:
root:x:0:0:root:/root:/bin/ash
bin:x:1:1:bin:/bin:/sbin/nologin
daemon:x:2:2:daemon:/sbin:/sbin/nologin
adm:x:3:4:adm:/var/adm:/sbin/nologin
lp:x:4:7:lp:/var/spool/lpd:/sbin/nologin
sync:x:5:0:sync:/sbin:/bin/sync
shutdown:x:6:0:shutdown:/sbin:/sbin/shutdown
halt:x:7:0:halt:/sbin:/sbin/halt
mail:x:8:12:mail:/var/spool/mail:/sbin/nologin
news:x:9:13:news:/usr/lib/news:/sbin/nologin
uucp:x:10:14:uucp:/var/spool/uucppublic:/sbin/nologin
operator:x:11:0:operator:/root:/bin/sh
man:x:13:15:man:/usr/man:/sbin/nologin
postmaster:x:14:12:postmaster:/var/spool/mail:/sbin/nologin
cron:x:16:16:cron:/var/spool/cron:/sbin/nologin
ftp:x:21:21::/var/lib/ftp:/sbin/nologin
sshd:x:22:22:sshd:/dev/null:/sbin/nologin
at:x:25:25:at:/var/spool/cron/atjobs:/sbin/nologin
squid:x:31:31:Squid:/var/cache/squid:/sbin/nologin
xfs:x:33:33:X Font Server:/etc/X11/fs:/sbin/nologin
games:x:35:35:games:/usr/games:/sbin/nologin
postgres:x:70:70::/var/lib/postgresql:/bin/sh
nut:x:84:84:nut:/var/state/nut:/sbin/nologin
cyrus:x:85:12::/usr/cyrus:/sbin/nologin
vpopmail:x:89:89::/var/vpopmail:/sbin/nologin
ntp:x:123:123:NTP:/var/empty:/sbin/nologin
smmsp:x:209:209:smmsp:/var/spool/mqueue:/sbin/nologin
guest:x:405:100:guest:/dev/null:/sbin/nologin
nobody:x:65534:65534:nobody:/:/sbin/nologin
There was a problem hiding this comment.
but the current implementation of runc exec command only support --user value, -u value UID(format:<uid>[:gid])
There was a problem hiding this comment.
That's right, I completely forgot that libcontainer/user isn't actually used by us "properly".
|
In future, if you're going to create a series of "add test case" patch sets or something like that, feel free to group them in a single PR. Please take a look at my nits. |
datawolf
force-pushed
the
add-integration-test-for-exec
branch
from
5be97af to
72b1f8a
Compare
@cyphar , got it, thanks. |
| runc exec --cwd /bin test_busybox pwd | ||
| [ "$status" -eq 0 ] | ||
|
|
||
| [[ ${output} == *"/bin"* ]] |
There was a problem hiding this comment.
My main concern with this is that
% [[ "a" == *"a"* ]]; echo $?
0
% [[ "ab" == *"a"* ]]; echo $?
0
Can't we do something like:
[[ $(echo "${output}" | tr -d ' \n') == "/bin" ]]
Though there's probably a simpler way of doing it than that.
There was a problem hiding this comment.
Actually you can just do [[ ${line[0]} == "/bin" ]]
There was a problem hiding this comment.
thanks for your review.
but [[ $(echo "${output}" | tr -d ' \n') == "/bin" ]] and [[ ${lines[0]} == "/bin" ]] do not works. the same as [[ ${output} == "/bin" ]].
and I write a simple test bats scripts, it test ok
#!/usr/bin/env bats
@test "pwd test" {
run runc exec --cwd /bin test_busybox pwd
[[ ${output} == "/bin" ]]
}
So I don't know why they all do not work in tests/integration/*.bats?
There was a problem hiding this comment.
@datawolf [[ $(echo "${output}" | tr -d '\r') == "/bin" ]] will work.
There was a problem hiding this comment.
thanks @hqhq . why the output has a '\r' at the end? If we have no way to delete this character. A simpler way of doing it may be:
[[ ${output} == $(echo -e "/bin\r") ]]
Update patch done.
There was a problem hiding this comment.
It also happens with other commands! What!
% sudo ./runc exec test echo lol | cat -v
lol^M
And it happens with the container cmd set to echo lol!
% sudo ./runc run test | cat -v
lol^M
.. wat.
There was a problem hiding this comment.
If we have no way to delete this character. A simpler way of doing it may be:
I prefer using `tr -d '\r'. Or just using the variable regex replacement used in the linked bats issue.
There was a problem hiding this comment.
I've opened #1145 to track this. I have no clue what's going on, but I have a suspicion it's related to creating ptys...
datawolf
force-pushed
the
add-integration-test-for-exec
branch
4 times, most recently
from
c22a5ec to
6b2c2b7
Compare
|
My testing shows that #1146 fixes the issue with |
|
OK, after #1146 merged, I will update this pr. |
datawolf
force-pushed
the
add-integration-test-for-exec
branch
3 times, most recently
from
557339f to
48bed6f
Compare
datawolf
force-pushed
the
add-integration-test-for-exec
branch
from
48bed6f to
9af2668
Compare
|
As the #1146 has been merged. I update this pr and rebased. |
|
I don't see the updates, you are still |
This patch add test `--cwd`, `--env`, `--user` option for exec command. Signed-off-by: Wang Long <long.wanglong@huawei.com>
datawolf
force-pushed
the
add-integration-test-for-exec
branch
from
9af2668 to
d5525cc
Compare
|
@hqhq, Sorry for that. I just forgot to |
|
LGTM |
1 similar comment
|
LGTM |
4 participants
This patch add test
--cwd,--env,--useroption for exec command.Signed-off-by: Wang Long long.wanglong@huawei.com