Add amtool configuration on alertmanager host #135
Conversation
github-actions
bot
added
area/jinja
|
@paulfantom I think this is done! Thoughts? |
defaults/main.yml
Outdated
| @@ -113,3 +113,8 @@ alertmanager_route: {} | |||
| # - match: | |||
| # owner: team-Y | |||
| # receiver: team-Y-pager | |||
|
|
|||
| amtool_config_dir: '/etc/amtool' | |||
There was a problem hiding this comment.
For consistency this should be prefixed with alertmanager_
There was a problem hiding this comment.
This should default to /etc/alertmanager/amtool/ as this is one of default locations for amtool config.
Or even better to {{ alertmanager_config_dir }}/amtool/
There was a problem hiding this comment.
@paulfantom I can prefix the vars.
As for the location. When I move to /etc/alertmanager/amtool then it doesn't find the configuration.
I used this:
https://github.com/prometheus/alertmanager#configuration
There was a problem hiding this comment.
I read old docs, sorry for that. /etc/amtool sounds good, but it still needs to be created and this is currently lacking in role :)
Does amtool_config_dir need to be configurable? I would like to prevent issues when people put some value and later find out that configuration is not picked up.
There was a problem hiding this comment.
DOH. 🤦 Sorry, I will add directory creation.
Does amtool_config_dir need to be configurable? I would like to prevent issues when people put some value and later find out that configuration is not picked up.
Probably not. I mean, you could role out per user in $HOME. But no idea if that is "needed". I can add a comment and a link to prometheus/alertmanager so people check that before configuring something else?
There was a problem hiding this comment.
I was thinking about either moving amtool_config_dir to vars/ or removing completely.
tasks/configure.yml
Outdated
| @@ -11,6 +11,16 @@ | |||
| notify: | |||
| - restart alertmanager | |||
|
|
|||
| - name: copy amtool config | |||
There was a problem hiding this comment.
I think this should be executed before copy alertmanager config task as amtool is already used there for config validation.
There was a problem hiding this comment.
I wonder if after implementing this, validation from https://github.com/cloudalchemy/ansible-alertmanager/blob/master/tasks/configure.yml#L10 should be fixed not to use --alertmanager.url=?
till
force-pushed
the
configure-amtool
branch
2 times, most recently
from
c9f928f
to
4f429be
Compare
tasks/configure.yml
Outdated
| template: | ||
| force: true | ||
| src: "{{ alertmanager_amtool_config_file }}" | ||
| dest: "{{ alertmanager_amtool_config_dir }}/config.yml" |
There was a problem hiding this comment.
| dest: "{{ alertmanager_amtool_config_dir }}/config.yml" | |
| dest: "{{ _alertmanager_amtool_config_dir }}/config.yml" |
There was a problem hiding this comment.
CI complains about:
fatal: [bionic]: FAILED! => {"msg": "The task includes an option with an undefined variable. The error was: 'alertmanager_amtool_config_dir' is undefined\n\nThe error appears to have been in '/home/travis/build/cloudalchemy/ansible-alertmanager/tasks/configure.yml': line 2, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n---\n- name: copy amtool config\n ^ here\n"}
Few issues with variable names.
Also could you add entries about new variables from defaults/main.yml to README.md?
I think when those are fixed we are ready to merge it :)
github-actions
bot
added
the
area/docs
|
Worst PR ever. 🤪 But I think it's done. |
paulfantom
changed the title
|
i try to configure my playbook for alertmanager to choose a cluster. i configure the services with |
I can rebase that on the other PR. :)