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

Zabbix services should restart if pkg changes #127

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

xenadmin
Copy link
Contributor

@xenadmin xenadmin commented Oct 9, 2019

I started playing around with forced version updates, and noticed that even if the packages install successfully, the service doesn't notice it. This PR could fix that.

Example:
/srv/pillar/zabbix/init.sls

# Overrides map.jinja
zabbix:
  lookup:
    version_repo: 4.0
    agent:
      version: 1:4.0.13-1+stretch
    proxy:
      version: 1:4.0.13-1+stretch

@myii myii requested a review from hatifnatt October 16, 2020 14:12
@xenadmin
Copy link
Contributor Author

@hatifnatt: @myii pinged me, if I'm available for lifting the formula to 5.0. That reminded me of this open PR and I wanted to ask you for your opinion? That would help me catch up with my fork, so I could come back for improving.

@myii
Copy link
Member

myii commented Oct 16, 2020

@hatifnatt I've been discussing some stuff about the formula with @xenadmin and we realised that this PR was still outstanding. Would you mind having a look and merging if all is OK? It's a relatively small change, so should be easy enough to review.

@myii
Copy link
Member

myii commented Oct 16, 2020

Whoops, both at the same time!

@hatifnatt
Copy link
Collaborator

It's a good change for sure, but module run used for restart of Zabbix agent for a reason see #77

I can suggest to use it to preform restart after package changes too, like this:

zabbix-agent:
  pkg.installed:
    - pkgs:
      {%- for name in zabbix.agent.pkgs %}
      - {{ name }}{% if zabbix.agent.version is defined and 'zabbix' in name %}: '{{ zabbix.agent.version }}'{% endif %}
      {%- endfor %}
{% if salt['grains.get']('os') != 'Windows' %}
    - require_in:
      - user: zabbix-formula_zabbix_user
      - group: zabbix-formula_zabbix_group
{% endif %}
    - watch_in:
      - module: zabbix-agent-restart
  service.running:
    - name: {{ zabbix.agent.service }}
    - enable: True
    - require:
      - pkg: zabbix-agent
      - file: zabbix-agent-logdir
{% if salt['grains.get']('os') != 'Windows' %}
      - file: zabbix-agent-piddir
{% endif %}

zabbix-agent-restart:
  module.wait:
    - name: service.restart
    - m_name: {{ zabbix.agent.service }}

Things to notice:

zabbix-agent:
  pkg.installed:
    ...
    - watch_in:
      - module: zabbix-agent-restart

instead of

zabbix-agent:
  service.running:
    ...
    - watch:
      - pkg: zabbix-agent

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

Successfully merging this pull request may close these issues.

3 participants