EugenMayer / docker-image-rsyncd

A rsyncd image used with docker-sync
3 stars 4 forks source link

Killall fails when PID is not running #9

Closed JeremylawrenceRf closed 5 years ago

JeremylawrenceRf commented 5 years ago

When the pidfile exists but the process doesn't the killall command fails causing entrypoint.sh to terminate early. Suggested modification: killall $PID > /dev/null 2>&1 || echo "Process was not running"

https://github.com/EugenMayer/docker-image-rsyncd/blob/3b36eae0d04cb92ad5f2eae1befdfbdcc7798681/entrypoint.sh#L81

EugenMayer commented 5 years ago

interested, but how is that of any use later?

JeremylawrenceRf commented 5 years ago

Since the next line is where the pidfile is deleted, when we exit after killall the file is never deleted. Next run we still enter the if statement block and end up in the same situation. Alternatively we could delete the pidfile first before running killall and have a failure, which may be desired behavior, but at least we wouldn't then enter a loop of failures.

EugenMayer commented 5 years ago

that makes a lot of sense, thank you for the clarification. just go forward with a PR, it is all yours

Thank you for the contribution!

JeremylawrenceRf commented 5 years ago

No problem! Thank you for responding so quickly. This project has been really useful for me. Thanks for all the work you've done on it!

JeremylawrenceRf commented 5 years ago

I'll submit the PR shortly.

JeremylawrenceRf commented 5 years ago

Unfortunately it looks like I don't have permission to publish a PR for this change.

EugenMayer commented 5 years ago

why should that be? there is nothing like that permission on guthub

JeremylawrenceRf commented 5 years ago

Sorry for the delay! Here is the PR: https://github.com/EugenMayer/docker-image-rsyncd/pull/10

JeremylawrenceRf commented 5 years ago

Sorry, do I need to push the image to docker hub or will you be doing that? Thanks!

EugenMayer commented 5 years ago

that will be my task yes :)

JeremylawrenceRf commented 5 years ago

Thanks!