Fix Bad formed prometheus.service issue #306 #307
Conversation
|
@dubcl can you take a look at the failing tests? Can you add a test for your usecase? |
| @@ -7,7 +7,7 @@ After=basic.target network.target | |||
| User=<%= scope.lookupvar('prometheus::server::user') %> | |||
| Group=<%= scope.lookupvar('prometheus::server::group') %> | |||
| ExecStart=<%= scope.lookupvar('prometheus::server::bin_dir') %>/prometheus \ | |||
| <%= @daemon_flags.join(" \\\n ") %> \ | |||
| <%= @daemon_flags.join("\n ") %> | |||
| <%= scope.lookupvar('prometheus::server::extra_options') %> | |||
There was a problem hiding this comment.
If extra_options is used (ie not an empty string), this will be broken?
There was a problem hiding this comment.
Yes.
I'm using this fix:
- $daemon_flags = $daemon_flags_basic + "--web.external-url=${prometheus::server::external_url}"
+ $daemon_flags = "--web.external-url=${prometheus::server::external_url} \\" + $daemon_flags_basicThere was a problem hiding this comment.
Hi, yes, the fix proposal by @matejzero works
There was a problem hiding this comment.
Looking at it again, I think this still won't fix every usecase. Right not, it works if we are using external_url, but it will fail if we use extra_options, since there is no trailing \ at the end of daemon_flags.
In the pre-PR code, it failed if extra_options were empty string, since there was a trailing \ at the end.
I have a fix, should I open another PR or should I post the solution here?
There was a problem hiding this comment.
Will do. I did some work yesterday, but it looks like it's gonna take some time to cover all init providers, not only systemd.
There was a problem hiding this comment.
How about merging extra_options and daemon_flags in the puppet code?
|
Hi, |
Our init scripts sometimes contain a trailing `\` after the last argument. Some might lead to wrong parsing. Fixes voxpupuli#306 Replaces voxpupuli#307
Our init scripts sometimes contain a trailing `\` after the last argument. Some might lead to wrong parsing. Fixes voxpupuli#306 Replaces voxpupuli#307
Pull Request (PR) description
Hi, this fix the "prometheus.systemd.erb" and "config.pp" to generate a correct "prometheus.systemd" file and avoid "prometheus: error: unexpected ExecReload=/bin/kill"
This Pull Request (PR) fixes the following issues
Fixes #306