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

test: fix test_pidfile #13646

Merged
1 commit merged into from Mar 17, 2017

Conversation

Projects
None yet
2 participants
@mslovy
Contributor

mslovy commented Feb 25, 2017

kill_daemons cat the pidfile, then kill the pid in it.
it does not work in this case since pid in osd.0.pid is wrong

@mslovy

This comment has been minimized.

Contributor

mslovy commented Feb 25, 2017

@dachary , fix the false positive for pidfile test

@liewegas liewegas requested a review from Feb 26, 2017

@@ -71,7 +71,7 @@ function TEST_pidfile() {
# another daemon
cp $dir/osd.0.pid $dir/osd.0.pid.old # so that kill_daemon finds the pid

This comment has been minimized.

@ghost

ghost Feb 26, 2017

Good catch ! I think it is better to rename osd.0.old.pid so that kill_daemons finds it. And also in other places where the script expects that the file is found by kill_daemons.

This comment has been minimized.

@mslovy

mslovy Feb 27, 2017

Contributor

@dachary
So I think we need to put osd.0.pid.old in another directory, such as $old/osd.0.pid,and
kill_daemons $old TERM osd.0 || return 1 , right?

This comment has been minimized.

@ghost

ghost Feb 27, 2017

We need kill_daemons to kill the daemons (reason why it must find the old pid) and observe that it does not remove $dir/osd.0.pid (because it either is a different path/inode or because its content has changed).

yaoning
test: fix test_pidfile
kill_daemons cat the pidfile, then kill the pid in it.
it does not work in this case since pid in osd.0.pid is wrong

Signed-off-by: yaoning <yaoning@unitedstack.com>
@mslovy

This comment has been minimized.

Contributor

mslovy commented Mar 17, 2017

@dachary move it to $dir/osd.0.pid.old, and use kill daemons again. Is that OK?

@ghost

This comment has been minimized.

ghost commented Mar 17, 2017

jenkins test this please (log of failed test is gone)

@ghost

ghost approved these changes Mar 17, 2017

@ghost ghost merged commit 0214e06 into ceph:master Mar 17, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment