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

Apache2 reload doesn't seem to work #12880

Closed
stevage opened this issue May 19, 2014 · 10 comments
Closed

Apache2 reload doesn't seem to work #12880

stevage opened this issue May 19, 2014 · 10 comments
Assignees
Labels
Feature new functionality including changes to functionality and code refactors, etc. stale
Milestone

Comments

@stevage
Copy link
Contributor

stevage commented May 19, 2014

My state, running on Ubuntu Quantal:

apache2:
  service:
    - running
    - reload: True

generates this output:

          ID: apache2
    Function: service.running
      Result: True
     Comment: The service apache2 is already running
     Changes:   

(And fails to reload the server.)

It's not a question of the init script not supporting reload, as this works just fine:

reload_apache:
  cmd.run:
    - name: "service apache2 reload"
@basepi
Copy link
Contributor

basepi commented May 19, 2014

Thanks for the report. We'll investigate this.

@basepi basepi added this to the Approved milestone May 19, 2014
@audreyfeldroy
Copy link

I'm having a similar problem with my uwsgi service state, which is probably due to the same issue with service.running.

uwsgi:
  pkg.installed:
    - name: uwsgi
  service.running:
    - enable: True
    - require:
      - pkg: uwsgi

I am able to work around it for now by adding this to my dependency state:

  module:
    - run
    - name: service.restart
    - m_name: uwsgi

Of course, I'd rather use watch_in instead, but changes to the watched file aren't triggering a restart.

I'm on Ubuntu 14.04.

@terminalmage
Copy link
Contributor

The docs seem pretty clear on this, at least to me. reload is not listed as a valid argument for any of the states, and is only ever mentioned in the context of a watch statement:

By default if a service is triggered to refresh due to a watch statement the service is by default restarted. If the desired behaviour is to reload the service, then set the reload value to True:

redis:
  service:
    - running
    - enable: True
    - reload: True
    - watch:
      - pkg: redis

So, this does seem a bit more like a feature request than a bug. @stevage, what is the desired functionality? Is it to reload the service whenever the state is run, if reload==True? Help paint me a use case, so that I make sure I implement this in the way that would be most helpful to you.

@terminalmage
Copy link
Contributor

@audreyr, your issue seems to be a simple misconfiguration, and doesn't at all appear to be related as you are trying to do a service restart, not a reload. Try replacing require with watch, like so:

uwsgi:
  pkg.installed:
    - name: uwsgi
  service.running:
    - enable: True
    - watch:
      - pkg: uwsgi

A watch requisite acts just like a require, but with one added step: if the state module has a mod_watch function (such as this one in the service state), and the state which is being watched made changes to the minion, then that mod_watch function is also executed. For the service state, as noted earlier the default behavior is to restart the service, so this should take care of the issue.

@terminalmage terminalmage self-assigned this May 19, 2014
@stevage
Copy link
Contributor Author

stevage commented May 19, 2014

By default if a service is triggered to refresh due to a watch statement the service is by default restarted. If the desired behaviour is to reload the service, then set the reload value to True:

Yeah, interesting - when I read this before, I didn't see the implied link between the first and second sentence. I just read it as "If you want to reload the service, set reload=True".

This might be clearer (and more skim-friendly):

"To trigger a service restart in response to another change, use a watch statement. Set reload=True to reload rather than restart."

what is the desired functionality? Is it to reload the service whenever the state is run, if reload==True?

Yes. (I had assumed, incorrectly, that there was also a "restart: True" option.)

I don't know if there's a compelling use case, but basically I don't use watch statements much if the cost of re-running the state is low. I find they quickly add a lot of complexity and an additional thing to debug. The cost of reloading Apache for my server was low, so it didn't occur to me to use a watch statement.

@terminalmage
Copy link
Contributor

OK. To be honest, the amount of work done to check watch requisites is not that bad. And the purpose of the state.running state is to check if it is running and, if not, to start it, optionally enabling it to start at boot, so there's not often much of a call to restart/reload a service. The reason we have the reload functionality enabled for watch requisites is that a common use case is reloading a service when a config file changes. So a service.running state can watch for changes in a file.managed state and then reload the service.

Having said that though, I see no reason why we can't support a reload (and even restart) flag, so long as it is well-documented that the action will be taken EVERY time the state is executed.

Perhaps the docs are clearer to me because I am constantly buried in them. I'll try to make them more clear for those new to Salt (or just skimming 😄).

@ghost
Copy link

ghost commented Nov 18, 2015

Is there a way to force reloads even if the service is running, without a watch, using state and not the module? Or is using the module the right way to do this?

@basepi
Copy link
Contributor

basepi commented Nov 25, 2015

The module is the right way to do this. A forced reload is not very stateful, so it doesn't really fit into the paradigm of state modules. Note that you can use module.run to integrate it into a state run.

@ghost
Copy link

ghost commented Dec 2, 2015

@basepi, thank you!

@stale
Copy link

stale bot commented Feb 19, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Feb 19, 2018
@stale stale bot closed this as completed Feb 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. stale
Projects
None yet
Development

No branches or pull requests

4 participants