changed template for /etc/init/{{app_name}} This seems to fix issue #357...

[flowma]

... for Ubuntu 14.04.

[lightbend-cla-validator]

Hi @flowma,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://typesafe.com/contribute/cla

[flowma]

Dupe of #357. (autogenerated issue from pr)

[muuki88]

does the -setuid not work on Ubuntu 14.04?

[flowma]

It works, but then the pre-start script fails because it is also executes as the ${{daemon_user}}.

[muuki88]

thanks for the quick response :) One question, see above

[flowma]

Build fails with "script doesn't contain start on [networking] header". True enough. 14.04's Upstart won't start this service, if app_name.conf contains this line.

Anyway it's probably not safe to merge this, as is might break Ubuntu 12.04 support. Pls, feel free to incorporate these changes when you start to adjust for 14.04 in #348 or elsewhere.

[muuki88]

You can remove this line. This can be changed via the settings requiredStartFacilities and requiredStopFacilities.
The build will then pass.

[muuki88]

I did some research and it looks like your approach is the only way:

• Upstart Job cannot create dir in var/run
• Upstart creating PID

I also found some posts referencing Ubuntu 12.04 LTS. Take a look at my last comment and then I'll test this on Ubuntu 12.04 and merge this.

[muuki88]

The logic works on my Ubuntu 12.04 LTS.

[flowma]

Removed the line from the template and the Assert in the test. Pls excuse the rebasing. Seems to work in 12.04 and 14.04.

[muuki88]

Sorry for asking again (and thanks for your patience). You removed these conditions as they won't work when stop_facilities is empty, right? (just guessing). Is there are way to add just "nothing" ? Or aren't they needed at all?

[muuki88]

Rebase is actually fine. One commits are a lot easier to merge/maintain between branches.

[flowma]

You're right: it doesn't start with start on started "" . I tried variations of network-services and network-interface, but no combination would make the service start, except removing the line altogether and just keep the run-level line. Also, I this seems to be the usual approach for application level services. One would have to dig a little deeper to figure out the purpose of the networking line.

[flowma]

@kardapoltsev You had improved upstart support in #262. What is your take on this?

[kardapoltsev]

Upstart is not my best friend). I'll have access to my Ubuntu 14 a bit later today and could try to dig into this issue. But I think we should try to keep ability for controlling startup order.

[kardapoltsev]

May be better defaults will be start on local-filesystems. Could you try it on your machine?

[muuki88]

The bigger problem is that, if start_facilities is an empty string, we don't want it do appear in the upstart.conf. My suggestion would be to change JavaServerApplication in something like this

case Upstart =>
 val requiredStartFacilitiesReplacement = requiredStartFacilities match {
     case null | "" => ""
     case facilities => "start on " + facilities
 }
 // same for stop facilities

 Seq("start_runlevels" -> startRunlevels,
          "stop_runlevels" -> stopRunlevels,
          "start_facilities" -> requiredStartFacilitiesReplacement ,
          "stop_facilities" -> requiredStopFacilitiesReplacement)

the script would then look like this

# When to start the service
start on runlevel ${{start_runlevels}}
${{start_facilities}}

# When to stop the service
stop on runlevel ${{stop_runlevels}}
${{stop_facilities}}

WDYT?

[kardapoltsev]

Why not use Option instead of null? It's look a bit more "scala friendly" (:
Otherwise looks good.

[muuki88]

I agree with you @kardapoltsev , unfortunately we would have to change the requiredStartFacilities from SettingKey[String] to SettingKey[Option[String]]. Thus we couldn't release this in the 0.7.x stream (what I would like to do).

Would you mind providing a second pr which fixes this after this one is merged?

[kardapoltsev]

I'll do.

[muuki88]

I'll merge this for now. Thanks @flowma for your work :)

@kardapoltsev , I'll rebase #348 and include this fix and the settings change. We need to make a release, soon because of a bouncy-castle update.