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

Removed --now from dokku installation #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jordikroon
Copy link

On debian, there is no --now option for systemctl. Thus it makes dokku unable to remove the plugin. Not sure if this is a requirement on some linux distro's, but if it's not then it's better not to add this option.

Fixes:
systemctl: unrecognized option '--now'

On debian, there is no `--now` option for systemctl. Thus it makes dokku unable to remove the plugin. Not sure if this is a requirement on some linux distro's, but if it's not then it's better not to add this option.

Fixes:
systemctl: unrecognized option '--now'
@mbreit
Copy link
Owner

mbreit commented Apr 5, 2017

First of all, thanks for your contribution. First pull request, yeah. ;) And sorry for the late reply.

Without the --now option, systemd disables the service but keeps it running. --now also stops the running process. If we remove this option, we should call systemctl stop dokku-monit instead.

What systemd version are you running? 215 from Debian Jessie? According to the systemd NEWS file, --now was added in 220 for both the enable and disable command.
The install trigger uses systemd enable --now, so you should have gotten an error during install as well. Did you install the unit yourself?

So to support Debian Jessie (systemd < 220) we need to:

  • Add systemctl stop to the uninstall trigger
  • Remove --now from install
  • Add systemctl start to install

We also should add systemctl is-enabled dokku-monit.service to the uninstall hook, so it runs successfully if the unit is not installed. That might be the case if the monit package is not installed when the plugins install trigger runs. That would also allow you to disable the unit yourself and then uninstall the plugin.

What do you think? Do you want to make these changes? If not, I can do it, but then you have to wait a few days until I have time to setup a debian vm for testing. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants