-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Check pid file #1147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check pid file #1147
Conversation
faacdf7
to
9723638
Compare
…ight information Signed-off-by: Wang Long <long.wanglong@huawei.com>
9723638
to
2c74f86
Compare
@@ -51,7 +51,7 @@ function teardown() { | |||
|
|||
run cat pid.txt | |||
[ "$status" -eq 0 ] | |||
[[ ${lines[0]} =~ [0-9]+ ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why delete the digit test here.... but not below.
@@ -42,6 +42,7 @@ function teardown() { | |||
run cat pid.txt | |||
[ "$status" -eq 0 ] | |||
[[ ${lines[0]} =~ [0-9]+ ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to de 8000 scribe this comment to others. Learn more.
Because this check is negative (is the pid not the main container pid) while the other checks are positive (is the pid the main container pid). So we need to retain the "is this actually a number" check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx missed the !=
LGTM
At #1132 (review), It is necessary to make sure that pid-file contains the right information.
This patch employ
jq
andrunc state
command to achieve it.Signed-off-by: Wang Long long.wanglong@huawei.com