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

Ability to independently override different systemloader templates #937

Closed
ANorwell opened this issue Feb 3, 2017 · 7 comments
Closed

Comments

@ANorwell
Copy link
Contributor

ANorwell commented Feb 3, 2017

sbt-native-packager provides the ability to include a single override template in (e.g. for systemd) templates/systemloader/systemd. Additionally, there are multiple default templates provided, which are used as fallbacks, e.g. for systemd loader_functions and start_template.

However, it seems that providing the override template will cause it to override all templates. It doesn't seem like there's a way to override start_template without overriding loader_functions as well, with the same file. This rarely makes sense.

To give a concrete example, for systemd I want to modify the unit definition beyond what's in start_template. So, I provide templates/systemloader/systemd with my modified unit template. But, this results in this template also being used for loader_functions and being inserted into the start script, which fails to run.

To me, the solution here is to allow users to provide loader_functions and start_template files, not a single systemd file. This would be somewhat of a breaking change, though.

@ANorwell
Copy link
Contributor Author

ANorwell commented Feb 3, 2017

The functionality to override partial templates seems to already exist, via linuxScriptReplacements. However, it remains true that at least for systemd, the behaviour when templates/systemloader/systemd is overridden does not seem to make any sense.

This comment isn't accurate. linuxScriptReplacements defines additional templating replacements, like ${{app_name}}.

@ANorwell
Copy link
Contributor Author

ANorwell commented Feb 3, 2017

I've found a very hacky workaround that allows me to insert arbitrary entries into the sytemd unit, as follows:

linuxScriptReplacements += "SuccessExitStatus" -> "\nLimitNOFILE=16384"

Obviously this is a complete hack, but in some ways it's better than redefining the entire unit file template. I wonder if it would make sense for the unit file template to have something like ${{additionalSystemdOptions}}, that would allow for this.

@muuki88
Copy link
Contributor

muuki88 commented Feb 4, 2017

Thanks @ANorwell for the detailed explanation and concrete use cases.

From your good research and the one example you are giving, I assume you use native-packager 1.1.x or lower. With 1.2.0-M8 the systemd plugin is extracted in its own autoplugin and you can set the SuccessExitStatus via an sbt setting.

enablePlugin(SystemdPlugin)
systemdSuccessExitStatus := Seq("0", "1")

If you like to have additional settings, you can submit a pull request. The hacky workaround is actually how things work in native-packager. The linuxScriptReplacements are a bunch of string-replacements. It's not very elegant, but straightforward and easy to debug.

However, it seems that providing the override template will cause it to override all templates

If this is the case, then we have a bug that must be fixed. The linuxStartScriptTemplate setting is responsible for the start template. It looks for a file in a specific place or you override the setting.

If that doesn't help, can you provide a minimal example to reproduce this error?

@ANorwell
Copy link
Contributor Author

ANorwell commented Feb 6, 2017

I've created a repo with a minimal reproduction. The readme for that repo describes in detail how to reproduce. I also verified that the PR I opened related to this fixes that issue (although as that PR mentions, it's a breaking change and may not be the right approach).

The hacky workaround is actually how things work in native-packager. The linuxScriptReplacements are a bunch of string-replacements.

I want to explain this more. My original goal when encountering this issue was to create a systemd unit file that increases the max number of file descriptors. The way you would do this would be to add the line LimitNOFILE=16384 to the systemd unit description. However, there's no good way to do this. While I can control e.g SuccessExitStatus via the template, there is no way to insert arbitrary lines such as to modify the max number of files.

My first attempt around this was to redefine the entire template, using the approach in the repo.

My second attempt around this was use the SuccessExitStatus template as a way to sneak in the file limit line by defining the template with a newline and then the line I wanted:

linuxScriptReplacements += "SuccessExitStatus" -> "\nLimitNOFILE=16384"

To me this is quite hacky and probably not a use that you were intending.

I think that possibly an easy way to allow for arbitrary lines such as max number of files to be added to the unit handle would be to have a catch-all template, such as ${{extra_systemd_unit_entries}}:

//systemd start-template
[Service]
Type=simple
...
ExecStartPre=/bin/chmod 755 /run/${{app_name}}
PermissionsStartOnly=true
${{extra_systemd_unit_entries}}

@muuki88
Copy link
Contributor

muuki88 commented Feb 6, 2017

Thanks a lot @ANorwell for your PR and fixing this bug. I already commented on the PR.

I think that possibly an easy way to allow for arbitrary lines such as max number of files to be added to the unit handle would be to have a catch-all template, such as ${{extra_systemd_unit_entries}}

This would be possible ( like the bashScriptExtras ). We could add that as three simple settings in the SystemdPlugin.

  • systemdExtraUnitEntries: TaskKey[Seq[String]]
  • systemdExtraServiceEntries: TaskKey[Seq[String]]
  • systemdExtraInstallEntries: TaskKey[Seq[String]]

However I'm not sure if this is scalable approach as we spam the user with a lot of settings. Native-packager also provides a lot of helpers to do the same task. E.g. we could also add an autoImported helper if the SystemdPlugin is enabled.

linuxScriptReplacements ++= systemdAddUnitEntries(
  "SuccessExitStatus" -> "0, 1, 130",
  "OtherSetting" -> "Value"
)

WDYT?

@ANorwell
Copy link
Contributor Author

ANorwell commented Feb 7, 2017

WDYT?

This sounds great! Much more useable than what I was suggesting.

@muuki88
Copy link
Contributor

muuki88 commented Apr 4, 2018

Implemented in #938

@muuki88 muuki88 closed this as completed Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants