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

Create PID directory on init start commands #75

Merged
merged 7 commits into from Jun 23, 2017

Conversation

Projects
None yet
3 participants
@therobot
Copy link
Contributor

commented Jun 21, 2017

This PR is related to #74 and its upstream bug on PowerDNS init scripts.

PowerDNS refuses to start if the PID directory does not exist before, this may present problems in a few situations, being the most important after a server reboot.

This PR solves the problem by creating a pid directory on start command. It fixes the problem in both RHEL and Debian platforms.

Additionaly it improves the way the default recursor installed by the official package is disabled on init systems, by replacing [:stop, :disable] command for just :disable and leaving to the underlying system to do what's necessary for that process. This allows a cleaner syntax on the custom resource and less errors on debian (which was not stopping/disabling) the default recursor process.

therobot added some commits Jun 21, 2017

Disabling the default recursor service
It stops/disables the service on Debian platforms

It disables the service on RHEL, no need to stop
the recursor in this case because its not launched 
in the first place. 

This way we remove only_if ward on the pidfile
which was preventing chef to stop/disable the 
recursor on Ubuntu. 

It’s also a cleaner solution that leaves to the
underlying os to do what’s required

@therobot therobot requested review from martinisoft and onlyhavecans Jun 21, 2017

@martinisoft
Copy link
Contributor

left a comment

Minor syntax thing here, but I also think we should remove the /var/run file resource in the pdns_recursor_service resource since that is pushed to the init script.

I'd also consider expanding the one-liner in the init scripts for checking PIDDIR for readability sake.

# or fall back to the default /var/run if not specified there.
PIDDIR=$(awk -F= '/^socket-dir=/ {print $2}' $CONFIG_FILE)
if [ -z "$PIDDIR" ]; then PIDDIR=/var/run; fi
# The binary "pdns_recursor" handles its own pidfile according the following

This comment has been minimized.

Copy link
@martinisoft

martinisoft Jun 21, 2017

Contributor

Why is this indented?

This comment has been minimized.

Copy link
@therobot

therobot Jun 22, 2017

Author Contributor

It was not meant, fixed.

@therobot

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2017

@martinisoft that oneliner came straight from their own pdns recursor code. This is why I think it's better not to expand it.

therobot added some commits Jun 22, 2017

Fixing tests
This resource has been removed from the source
Revert "Fixing tests"
This reverts commit 207e057.
@therobot

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2017

@martinisoft removing the /var/run/ resource made chef break for systemd so I reverted those changes. This is ready for merge.

@martinisoft

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2017

removing the /var/run/ resource made chef break for systemd so I reverted those changes. This is ready for merge.

Ah ok, approved then 👍

@therobot therobot merged commit b62e019 into master Jun 23, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@therobot therobot deleted the create_pid_dir_on_init branch Jun 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.