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

ntp.service fails over missing dependency #28

Open
sroegner opened this issue Apr 5, 2016 · 12 comments
Open

ntp.service fails over missing dependency #28

sroegner opened this issue Apr 5, 2016 · 12 comments
Labels

Comments

@sroegner
Copy link
Member

sroegner commented Apr 5, 2016

This is CentOS release 6.5 (Final), salt 2015.8.8.2 (Beryllium)

salt-call state.sls ntp.server

local:
----------
          ID: ntp
    Function: pkg.installed
      Result: True
     Comment: Package ntp is already installed
     Started: 15:30:51.197693
    Duration: 546.288 ms
     Changes:   
----------
          ID: ntp_running
    Function: service.running
        Name: ntpd
      Result: False
     Comment: The following requisites were not found:
                                 watch:
                                     file: /etc/ntp.conf
     Started: 
    Duration: 
     Changes:   
----------
          ID: ntpd
    Function: service.running
      Result: True
     Comment: The service ntpd is already running
     Started: 15:30:51.747220
    Duration: 119.102 ms
     Changes:   

Summary for local
------------
Succeeded: 2
Failed:    1
------------
@sroegner sroegner added the bug label Apr 5, 2016
@sroegner
Copy link
Member Author

sroegner commented Apr 5, 2016

So ntpd is checked in init.sls and in server.sls?

@gravyboat
Copy link
Contributor

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.

@sroegner
Copy link
Member Author

sroegner commented Apr 5, 2016

@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?

@gravyboat
Copy link
Contributor

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.

@tiadobatima
Copy link

Hi guys... We're seeing the same problem on ubuntu 14.04

@gravyboat
Copy link
Contributor

@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.

@gravyboat
Copy link
Contributor

@sroegner, @tiadobatima, can you guys please give this a shot: #29

@sroegner
Copy link
Member Author

sroegner commented Apr 6, 2016

@gravyboat am still getting the same error calling init.sls

local:
----------
          ID: ntp
    Function: pkg.installed
      Result: True
     Comment: Package ntp is already installed.
     Started: 14:50:34.437156
    Duration: 326.221 ms
     Changes:   
----------
          ID: ntp_running
    Function: service.running
        Name: ntpd
      Result: False
     Comment: The following requisites were not found:
                                 watch:
                                     file: ntp_conf
     Started: 
    Duration: 
     Changes:   

Summary
------------
Succeeded: 1
Failed:    1
------------

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)

@markwherry9
Copy link
Contributor

Hi All,
I added the watch on the service in init.sls because previously the formula was not reflecting changes to ntp configuration file and checking ntpq showed peers were always the default NOT those specified by the user. In fact, a manual service restart was needed to make ntpd peer to the correct servers.

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

@sroegner
Copy link
Member Author

sroegner commented Apr 6, 2016

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.

@markwherry9
Copy link
Contributor

Hi, that sounds fine to me @sroegner Just as long as there is a working watch function I'm happy.

sroegner added a commit to sroegner/ntp-formula that referenced this issue Apr 6, 2016
@sroegner
Copy link
Member Author

sroegner commented Apr 6, 2016

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?

aboe76 added a commit that referenced this issue Feb 15, 2019
fix for #28: move config file watch, remove service check from ntp state
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants