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

Handle Restart/RestartSec systemd service file options #627

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

Conversation

eLvErDe
Copy link

@eLvErDe eLvErDe commented May 11, 2016

Hello,

Here is a quick improvement to be able to use auto-restart feature provided by systemd service files.

Best regards, Adam.

@tylerjl
Copy link
Contributor

tylerjl commented May 20, 2016

This could definitely be useful for people on systemd-based distributions.

Is there a particular reason the two parameters are stored in the init_defaults hash? They may be more easily usable as a top-level parameter on instance and they won't do anything as shell variables in the init defaults file AFAIK.

@eLvErDe
Copy link
Author

eLvErDe commented May 21, 2016

Hello Tyler,

No specific reason I just saw it was done that way so I mimiced the existing.
I'm not a Puppet expert, just tried to solve my issue and though it could be useful to everyone!

Regards, Adam.

@tylerjl
Copy link
Contributor

tylerjl commented May 24, 2016

Thanks @eLvErDe. I think this only needs a few changes changes to merge it in:

  • Make systemd_restart and systemd_restart_sec top-level parameters, which can make them more easily validated in puppet and so they can given their own pieces of documentation for users.
  • In the template, use the erb -%> notation to avoid interpolating empty lines into the systemd unit template. For example, this
<% if @systemd_restart %>Restart=<%= @systemd_restart %><% end %>

Becomes this:

<% if @systemd_restart -%>Restart=<%= @systemd_restart %><% end -%>

@eLvErDe
Copy link
Author

eLvErDe commented May 24, 2016

Hello,

Thanks for the feedback. When you say top level you mean parameter of the elasticsearch class directly?

Regards, Adam

Le 24 mai 2016 22:01:06 GMT+02:00, Tyler Langlois [email protected] a écrit :

Thanks @eLvErDe. I think this only needs a few changes changes to merge
it in:

  • Make systemd_restart and systemd_restart_sec top-level
    parameters, which can make them more easily validated in puppet and so
    they can given their own pieces of documentation for users.
  • In the template, use the erb -%> notation to avoid interpolating
    empty lines into the systemd unit template. For example, this
<% if @systemd_restart %>Restart=<%= @systemd_restart %><% end %>

Becomes this:

<% if @systemd_restart -%>Restart=<%= @systemd_restart %><% end -%>

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#627 (comment)

Envoyé de mon appareil Android avec K-9 Mail. Veuillez excuser ma brièveté.

@tylerjl
Copy link
Contributor

tylerjl commented May 24, 2016

Correct; placing them in init.pp means every instance on the host will get the same settings but we can add per-instance parameter overrides later if needed.

init.pp would also be a good place to validate that the values assigned to the parameters are valid for systemd (i.e. check that Restart= is on-failure, on-success, etc., and RestartSec= is an integer).

@eLvErDe
Copy link
Author

eLvErDe commented May 24, 2016

Got it, I'll try to find time to do this tomorrow. Should I fail() if parameters are invalid?

Le 24 mai 2016 23:20:32 GMT+02:00, Tyler Langlois [email protected] a écrit :

Correct; placing them in init.pp means every instance on the host
will get the same settings but we can add per-instance parameter
overrides later if needed.

init.pp would also be a good place to validate that the values
assigned to the parameters are valid for systemd (i.e. check that
Restart= is on-failure, on-success, etc., and RestartSec= is an
integer).


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#627 (comment)

Envoyé de mon appareil Android avec K-9 Mail. Veuillez excuser ma brièveté.

@tylerjl
Copy link
Contributor

tylerjl commented May 24, 2016

Yep! I'd just follow the way the other parameters are validated (like status for example).

@fatmcgav
Copy link
Contributor

fatmcgav commented Feb 4, 2020

Hi there,

Firstly, thank you for taking the time to contribute, and apologies for the radio silence from Elastic on this PR.

I'm going to be working on this module over the next few weeks, with an ultimate aim of updating the module to support the elasticsearch 7.x and simplifying the module to make it easier to use.

I'll be reviewing all the open issues and PRs over the next few days, and will merge or provide feedback where appropriate.

So please hang in there whilst we give this module some TLC.

Thanks again.

@elasticmachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@vox-pupuli-tasks
Copy link

Dear @eLvErDe, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

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

Successfully merging this pull request may close these issues.

4 participants