Skip to content

Conversation

@gfidente
Copy link

@gfidente gfidente commented Apr 4, 2013

this will also ensure restart will work on systems using systemd

this will also ensure restart will work on systems using systemd
@lzap
Copy link
Contributor

lzap commented Jul 19, 2013

+1 :-D

@hunner
Copy link
Contributor

hunner commented Jul 30, 2013

Hmm, interesting. Will this work on systems not using systemd as well? Also, could you please rebase this against master? Thanks!

@hunner
Copy link
Contributor

hunner commented Jul 30, 2013

Related to #8

@lzap
Copy link
Contributor

lzap commented Jul 31, 2013

This will work everywhere for sure, since we are sending a signal to the process directly to reload. The only issue I can see is the PATH variable (maybe we want to provide /usr/bin/pkill).

@gfidente
Copy link
Author

gfidente commented Aug 1, 2013

This will work regardless of the start script manager so I've rebased the changes but decided not to use the absolute path for pkill ; we should be safe to go by trusting the default PATH variable definition and also preserve some level of fexibility in sub-standard environments; if you think that is not the case, please comment

@goneri
Copy link

goneri commented Jan 28, 2014

Hi,

Thank you for the bug report @giulivo. I faced the same issue I resolved with a similar pkill call:

exec{ 'reload_xinetd':
      command => '/usr/bin/pkill -F /var/run/xinetd.pid --signal HUP',
      refreshonly => true,
  }

This sounds a bit less dangerous to me even if this mean we have to identify where the PID is stored.

@apenney
Copy link

apenney commented Apr 24, 2014

Do you think you can change this to the pkill @goneri posted?

@gfidente
Copy link
Author

@apenney and @goneri sure I could. LSB suggests the pid file should be searched in "/var/run/$daemon.pid" so assuming it will be there seems safe. Regarding the path to "pkill" instead, is there a reason to prefer full path rather than just "pkill" ?

@goneri
Copy link

goneri commented Apr 27, 2014

Hi @giulivo and @apenney,

Thank you for taking care of this annoying issue.

If you do pkill foo, pkill will search for every process name matching “foo” and send them a sigterm. You can reduce the risk with the --exact parameter. With it pkill will only look for process named just “foo”. But you still can get two different processes called “foo”. The use of the pid file is a safe you to identify the correct process. That's actually what the init scripts do in general.

@gfidente
Copy link
Author

@goneri I wasn't asking about passing the path of the pid file, but about using the full path of the pkill executable. Is there a reasnon for using "/usr/bin/pkill" rather than "pkill" ?

@goneri
Copy link

goneri commented Apr 27, 2014

Oh sorry. Well since /usr/bin is probably in the $PATH everywhere, I'm not sure that necessary. It's just an habit.

@gfidente
Copy link
Author

@apenney I rebased and noticed that this is now managed by defining a different restart command for each distro in manifests/params.pp

Do you really want to replace that with a more generic pkill?

@hunner
Copy link
Contributor

hunner commented Jun 12, 2014

I think this PR is actually no longer needed, since the params.pp parameterizes the restart command and hasrestart attribute.

@hunner hunner closed this Jun 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants