-
Notifications
You must be signed in to change notification settings - Fork 29
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
Implement local prometheus #2
base: master
Are you sure you want to change the base?
Conversation
Initial implementation of local deployment.
Add collectd exporter.
Support for statement in alerts.
Alerts for
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but please let somebody from prometheus team review it.
I'm not sure if this kind of map.jinja
magic is desired.
prometheus/map.jinja
Outdated
{%- set version = pillar.prometheus.get('version', '1.6.2') %} | ||
|
||
{%- load_yaml as base_defaults %} | ||
{%- if pillar.docker is defined and pillar.docker.host is defined %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use pillar.get('docker', {}).get('host')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but this is still wrong, because you could have docker host without prometheus in them. I recommend using engine like gitlab:runner:engine = docker | local
or something like this
this PR looks very similar to https://github.com/richerve/prometheus-formula ? |
@michaelkuty can you please rebase? I will merge it then. |
|
||
collectd_exporter_bin_link: | ||
file.symlink: | ||
- name: /usr/bin/collectd_exporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use /usr/local/bin instead
- name: prometheus | ||
- shell: /bin/bash | ||
- system: true | ||
- home: /srv/prometheus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use /var/lib/prometheus instead
install: /opt | ||
version_path: /opt/prometheus-{{ version }}.linux-amd64 | ||
version: {{ version }}.linux-amd64 | ||
source: https://github.com/prometheus/prometheus/releases/download/v{{ version }}/prometheus-{{ version }}.linux-amd64.tar.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to use "standarized" metadata for selecting installation method:
source:
engine: tarball
url: https://...
hash: blahblah
defaulting for example to this to keep backward compatibility:
source:
engine: docker
in the similar manner as eg. for salt-formula-aptly, salt-formula-letsencrypt, etc.
I would like to upgrade to prometheus 2.0 instead reworking this PR |
I've updated https://github.com/richerve/prometheus-formula to prometheus 2.0, since the codebase is very similar it can be merged into this. |
really ? |
No description provided.