Skip to content
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

Bug 4613/dev/prevent duplicate cf execd #331

Conversation

ncharles
Copy link
Member

No description provided.

NB_CF_EXECD_RUNNING=`echo "${CF_EXECD_RUNNING}" | grep -v ^$ | wc -l`
if [ ${NB_CF_EXECD_RUNNING} -gt 1 ]; then
echo -n "WARNING: Too many instance of CFEngine cf-execd processes running. Killing them..."
echo "${CF_EXECD_RUNNING}" | awk 'BEGIN { OFS=" "} {print $2 }' | xargs kill -9 || true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What! What's with the "kill -9"? That's very brutal, I don't see any call for that...

I don't know why you have used this because no comments explain your intent. I see two options here:

  1. You have a good reason to "use the force". Please add a comment in the script explaining it.
  2. You don't have a reason. Then use a plain kill instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When several cf-execd are running, simple kill don't kill them, only -9 can stop them)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid you misread me: the comment should appear in the script, not here! This needs to be understandable and maintanable by the future us :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, GitHub is being weird. I see that you already did this by re-reading the full diff, but the diff above still displays the old code, and isn't marked as "outdated" like usual.

So this looks good, thanks.

@ncharles
Copy link
Member Author

ncharles commented Jun 2, 2014

PR has been updated

jooooooon added a commit that referenced this pull request Jun 2, 2014
…cf-execd

Bug 4613/dev/prevent duplicate cf execd
@jooooooon jooooooon merged commit ab394bd into Normation:branches/rudder/2.6 Jun 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants