-
Notifications
You must be signed in to change notification settings - Fork 158
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
ntp.service fails over missing dependency #28
Comments
So ntpd is checked in init.sls and in server.sls? |
So this was added a couple weeks ago (5231581) and I merged it in via the associated PR. I'm not quite sure why the requisite is failing there because the file is set by default. |
@gravyboat It is failing because the condition for the config file is different from the service in init.sls right? But is there really a case to (re)start the service in both states? |
Let me tag @markwherry9 since he worked on this. There are actually two configs it looks like @sroegner, one that is ntp, and one that is ntpd, but both are supported in the default pillar already, so maybe I'm just missing it. I think the case was fair for a restart but maybe we overlooked something. Looking forward to your input @markwherry9. |
Hi guys... We're seeing the same problem on ubuntu 14.04 |
@sroegner Okay just double checked, yes it's because of the naming: https://github.com/saltstack-formulas/ntp-formula/blob/master/ntp/init.sls#L12 versus https://github.com/saltstack-formulas/ntp-formula/blob/master/ntp/server.sls#L25, I looked through the code just a few minutes ago and this looks to be due to some confusion because of the damn NG formula using ntp and ntpd. |
@sroegner, @tiadobatima, can you guys please give this a shot: #29 |
@gravyboat am still getting the same error calling init.sls
I think the problem are the two different conditions - {% if ntp_conf_src %} for the ntp_conf file and {% if ntp.ntp_conf -%} for the service (which i still don't think belongs into init.sls) |
Hi All, I didn't realise that server.sls already had a watch. However, the watch in server.sls looks wrong to me because it's trying to watch 'ntp_conf' wheras in init.sls it watches "file: {{ ntp.ntp_conf }}" which should work across OSs. I'm quite new to Saltstack so I don't know the best location for this service watch. However, I'm pretty sure one is needed and it should probably use something like file: {{ ntp.ntp_confi }}" Thanks - Mark |
Thanks @markwherry9 for joining in - there is no doubt that the watch on the config makes sense but if that is ok with you (and the way you use the formula) the watch should probably be on the one service definition in server.sls. Otherwise using server.sls (as it includes init.sls) will attempt to start the same ntpd twice everywhere. |
Hi, that sounds fine to me @sroegner Just as long as there is a working watch function I'm happy. |
… check from ntp state
I believe that my PR fixes the issue at hand on Redhat - i am certainly not sure about any regressions especially on other platforms. Maybe @tiadobatima could take a look at this on Ubuntu? |
fix for #28: move config file watch, remove service check from ntp state
This is CentOS release 6.5 (Final), salt 2015.8.8.2 (Beryllium)
salt-call state.sls ntp.server
The text was updated successfully, but these errors were encountered: