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

Improved rpm scripts for systemd #650

Merged
merged 1 commit into from
Aug 23, 2015
Merged

Improved rpm scripts for systemd #650

merged 1 commit into from
Aug 23, 2015

Conversation

michal-hamuda
Copy link

I have encountered a problem that a systemd appplication packaged to rpm does not restart after installing a new version. So, I made changes in the bash scripts included in the rpm - I made use of already made systemd macros

@jkramarz
Copy link

Most of these changes should be implemented in systemloader/systemd/loader-functions as they're valid only if systemd is being used. Leave uninstall/upgrade check ([ $1 -eq 1 ]) in place and move the others. Furthermore, services should not be explicitly enabled, but rather systemd's preset mechanism (http://freedesktop.org/wiki/Software/systemd/Preset/) should be used.

@muuki88 muuki88 added the rpm label Aug 12, 2015
@muuki88
Copy link
Contributor

muuki88 commented Aug 12, 2015

Thanks for the PR. @jkramarz is right about implementhing this inside the SystemD Systemloader Template.

I quickly read in the link you posted. The SystemD presets seem like a good thing for admins to control their servers. I fear for devs who don't know about that yet will wonder why the service doesn't start anymore. What would it look like if we used presets?

And TravisCI is acting weird. can't see why the build fails. But I have a clue ( the loader-functions are wrong in many tests)

@jkramarz
Copy link

@muuki88, I'd rather say that current state (enabling and starting by default) is unexpected, at last on RHEL, CentOS and Fedora. These systems won't start nor enable newly-installed service unless you've explicitly asked for it. This behaviour has been implemented in systemd presets by disable * default directive.

On the other side, it is perfectly correct on Debian derivatives - this part of Linux family has chosen a path of starting whatever you've installed (even if default configuration is incomplete or insecure) :-)

So, it's a mater of OS-wide or sbt-wide consitency of packages behaviour.

Back to the topic:
Including a file /usr/lib/systemd/system-preset/50-${{app_name}}.preset containing

enable ${{app_name}}.service

should do the job, according to systemd.preset man page (http://www.freedesktop.org/software/systemd/man/systemd.preset.html).

@michal-hamuda
Copy link
Author

Thanks for the comments! I've applied the changes suggested by @jkramarz - moved the calls to systemctl to the loader functions and added a presetService() function which is then called in postinst-template.

@muuki88
Copy link
Contributor

muuki88 commented Aug 13, 2015

LGTM. @jkramarz WDYT?

@Zarratustra could you squash your commits into one?

@michal-hamuda
Copy link
Author

Ok, squashed!

@explicite
Copy link

@muuki88 maybe some documentations changes are needed?

@jkramarz
Copy link

Nope. Version in this pull request:

  • breaks other launchers (uses undefined elsewhere function),
  • does not start a service at all,
  • changes previous behaviour of starting service everywhere (no preset included).

@muuki88
Copy link
Contributor

muuki88 commented Aug 14, 2015

@jkramarz the issue you counted would all be fixed with a preset that starts the service?

@Zarratustra this would mean an additional setting in the rpmSettings in the JavaServerAppPackaging plugin. It would look something like this

mappings in Rpm := {
   val app = (packageName in Rpm).value
   val presetMappings = (serverLoader in Rpm).value match {
     case Systemd => 
         val preset = ... // load with getResource, write tmp file in target
         Seq("/usr/lib/systemd/system-preset/50-${app}.preset" -> preset)
     case _ => Seq()
   }
  (mappings in Rpm).value ++ presetMappings
}

@michal-hamuda
Copy link
Author

@jkramarz you are right - I forgot that loader-functions have different implementations. Now I've fixed this - now all implementations of loader-fuctions consist of three functions:

  • startService()
  • restartService()
  • stopService()

I think it should be OK now. However, I'm not sure about the preset calls - why include a preset file that does the same thing as the previous script - enabling by default?

@muuki88
Copy link
Contributor

muuki88 commented Aug 23, 2015

@jkramarz any objections?

@jkramarz
Copy link

👍

muuki88 added a commit that referenced this pull request Aug 23, 2015
@muuki88 muuki88 merged commit d774209 into sbt:master Aug 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants