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.

Sign up for GitHub

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

[pillar] Revamp pillar file structure & allow arbitrary keys in datadog.yaml #35

Merged
merged 3 commits into from
Oct 1, 2019
Merged

[pillar] Revamp pillar file structure & allow arbitrary keys in datadog.yaml #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be3339c
[pillar] Revamp pillar file structure & allow arbitrary keys in datad…
KSerrania Sep 9, 2019
2102ff4
Make Agent 5 work too
KSerrania Sep 30, 2019
822680c
Fix conf typo
KSerrania Sep 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 23 additions & 49 deletions datadog/config.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,61 +1,35 @@
{% from "datadog/map.jinja" import datadog_settings with context %}
{% set config_file_path = '%s/%s'|format(datadog_settings.config_folder, datadog_settings.config_file) -%}
{% set example_file_path = '%s.example'|format(config_file_path) -%}
{% from "datadog/map.jinja" import datadog_config, datadog_install_settings, datadog_checks, latest_agent_version, parsed_version with context %}
{% set config_file_path = '%s/%s'|format(datadog_install_settings.config_folder, datadog_install_settings.config_file) -%}

datadog-example:
file.copy:
{%- if not latest_agent_version and parsed_version[1] == '5' %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be parsed_version[0]?

Copy link
Contributor Author

@KSerrania KSerrania Oct 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parsed_version[0] contains the full match of the regex defined here: https://github.com/DataDog/datadog-formula/pull/35/files#diff-541c639658b352ec59d46a0868aa02d2R35
parsed_version[1] contains the first capture group, which is the major version number

datadog_conf_installed:
file.managed:
- name: {{ config_file_path }}
- source: {{ example_file_path }}
# file.copy will not overwrite a named file, so we only need to check if the example config file exists
- onlyif: test -f {{ example_file_path }}
- source: salt://datadog/files/datadog.conf.jinja
- user: dd-agent
- group: dd-agent
- mode: 600
- template: jinja
- require:
- pkg: datadog-pkg

{% if datadog_settings.api_key is defined %}
datadog-conf-api-key:
file.replace:
- name: {{ config_file_path }}
- pattern: "api_key:(.*)"
- repl: "api_key: {{ datadog_settings.api_key }}"
- count: 1
- onlyif: test -f {{ config_file_path }}
- watch:
- pkg: datadog-pkg
{% endif %}

datadog-conf-site:
file.replace:
- name: {{ config_file_path }}
- pattern: "(.*)site:(.*)"
{% if datadog_settings.site is defined %}
- repl: "site: {{ datadog_settings.site }}"
{% else %}
- repl: "# site: datadoghq.com"
{% endif %}
- count: 1
- onlyif: test -f {{ config_file_path }}
- watch:
- pkg: datadog-pkg

datadog-conf-python-version:
file.replace:
{%- else %}
datadog_yaml_installed:
file.managed:
- name: {{ config_file_path }}
- pattern: "(.*)python_version:(.*)"
{% if datadog_settings.python_version is defined %}
- repl: "python_version: {{ datadog_settings.python_version }}"
{% else %}
- repl: "# python_version: 2"
{% endif %}
- count: 1
- onlyif: test -f {{ config_file_path }}
- watch:
- source: salt://datadog/files/datadog.yaml.jinja
- user: dd-agent
- group: dd-agent
- mode: 600
- template: jinja
- require:
- pkg: datadog-pkg
{%- endif %}

{% if datadog_settings.checks is defined %}
{% for check_name in datadog_settings.checks %}
{% if datadog_checks is defined %}
{% for check_name in datadog_checks %}
datadog_{{ check_name }}_yaml_installed:
file.managed:
- name: {{ datadog_settings.checks_confd }}/{{ check_name }}.yaml
- name: {{ datadog_install_settings.checks_confd }}/{{ check_name }}.yaml
- source: salt://datadog/files/conf.yaml.jinja
- user: dd-agent
- group: root
Expand Down
6 changes: 4 additions & 2 deletions datadog/files/conf.yaml.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{% if pillar.datadog.checks[check_name].init_config is not defined -%}
{% from "datadog/map.jinja" import datadog_checks with context -%}

{% if datadog_checks[check_name].init_config is not defined -%}
init_config:
{% endif -%}

{{ pillar.datadog.checks[check_name] | yaml(False) }}
{{ datadog_checks[check_name] | yaml(False) }}
10 changes: 10 additions & 0 deletions datadog/files/datadog.conf.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% from "datadog/map.jinja" import datadog_config with context -%}

[Main]
Copy link
Contributor

@albertvaka albertvaka Oct 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How was agent 5 working before without this file? :o

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We took the example file that the package provides (or the datadog.conf file directly if it exists on the host already), and replaced the fields we wanted to change using regexes. But this is error-prone, we relied on fields being present on the config file (which may not be the case if it was modified by the user)

dd_url: https://app.datadoghq.com

{% if datadog_config.api_key is not defined -%}
api_key:
{% else -%}
api_key: {{ datadog_config.api_key }}
{% endif -%}
9 changes: 9 additions & 0 deletions datadog/files/datadog.yaml.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{% from "datadog/map.jinja" import datadog_config with context -%}

{% if datadog_config.api_key is not defined -%}
api_key:
{% endif -%}

{% if datadog_config | length -%}
{{ datadog_config | yaml(False) }}
{% endif -%}
2 changes: 1 addition & 1 deletion datadog/init.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
include:
- datadog.config
- datadog.install
- datadog.config
- datadog.service
10 changes: 5 additions & 5 deletions datadog/install.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{% from "datadog/map.jinja" import datadog_settings, latest_agent_version, parsed_version with context %}
{% from "datadog/map.jinja" import datadog_install_settings, latest_agent_version, parsed_version with context %}

{%- if grains['os_family'].lower() == 'debian' %}
datadog-apt-https:
Expand Down Expand Up @@ -49,15 +49,15 @@ datadog-repo:

datadog-pkg:
pkg.installed:
- name: {{ datadog_settings.pkg_name }}
- name: {{ datadog_install_settings.pkg_name }}
{%- if latest_agent_version %}
- version: 'latest'
{%- elif grains['os_family'].lower() == 'debian' %}
- version: 1:{{ datadog_settings.agent_version }}-1
- version: 1:{{ datadog_install_settings.agent_version }}-1
{%- elif grains['os_family'].lower() == 'redhat' %}
- version: {{ datadog_settings.agent_version }}-1
- version: {{ datadog_install_settings.agent_version }}-1
{%- endif %}
- ignore_epoch: True
- refresh: True
- require:
- pkgrepo: datadog-repo
- pkgrepo: datadog-repo
44 changes: 26 additions & 18 deletions datadog/map.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,48 +7,56 @@

{% set default_settings = {
'datadog': {
'pkg_name': 'datadog-agent',
'service_name': 'datadog-agent',
'api_key': 'aaaaaaaabbbbbbbbccccccccdddddddd',
'agent_version': 'latest',
'config': {},
'checks': {},
'install_settings': {
'agent_version': 'latest',
'pkg_name': 'datadog-agent',
'service_name': 'datadog-agent',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense for pkg_name and service_name to be overridable by the user? I would remove them from here and just hardcode them (otherwise, we should document them?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were here when I started maintaining the repo, but I agree that they should be hardcoded. But I didn't want to mix the cleanup of the repo with this change ; I'll do the cleanup in another PR

},

}
}%}

{# Merge os_family_map into the default settings #}
{% do default_settings.datadog.update(os_family_map) %}
{% do default_settings.datadog.update(os_family_map) %}

{# Merge in datadog pillar #}
{% set datadog_settings = salt['pillar.get']('datadog', default=default_settings.datadog, merge=True) %}
{% set datadog = salt['pillar.get']('datadog', default=default_settings.datadog, merge=True) %}
{% set datadog_config = datadog['config'] %}
{% set datadog_checks = datadog['checks'] %}
{% set datadog_install_settings = datadog['install_settings'] %}

{# Determine if we're looking for the latest package or a specific version #}
{%- if datadog_settings.agent_version == 'latest' %}
{%- if datadog_install_settings.agent_version == 'latest' %}
{%- set latest_agent_version = true %}
{%- else %}
{%- set latest_agent_version = false %}
{%- set parsed_version = datadog_settings.agent_version | regex_match('(([0-9]+)\.[0-9]+\.[0-9]+)(?:~(rc|beta)\.([0-9]+-[0-9]+))?') %}
{%- set parsed_version = datadog_install_settings.agent_version | regex_match('(([0-9]+)\.[0-9]+\.[0-9]+)(?:~(rc|beta)\.([0-9]+-[0-9]+))?') %}
{%- endif %}


{# Determine defaults depending on specified version #}
{% if 'config_folder' not in datadog_settings %}
{% if 'config_folder' not in datadog_install_settings %}
{%- if latest_agent_version or parsed_version[1] == '6' %}
{% do datadog_settings.update({'config_folder': '/etc/datadog-agent'}) %}
{% do datadog_install_settings.update({'config_folder': '/etc/datadog-agent'}) %}
{%- else %}
{% do datadog_settings.update({'config_folder': '/etc/dd-agent'}) %}
{% do datadog_install_settings.update({'config_folder': '/etc/dd-agent'}) %}
{%- endif %}
{% endif %}

{% if 'config_file' not in datadog_settings %}
{% if 'config_file' not in datadog_install_settings %}
{%- if latest_agent_version or parsed_version[1] == '6' %}
{% do datadog_settings.update({'config_file': 'datadog.yaml'}) %}
{% do datadog_install_settings.update({'config_file': 'datadog.yaml'}) %}
{%- else %}
{% do datadog_settings.update({'config_file': 'datadog.conf'}) %}
{% do datadog_install_settings.update({'config_file': 'datadog.conf'}) %}
{%- endif %}
{% endif %}

{% if 'checks_confd' not in datadog_settings %}
{% if 'checks_confd' not in datadog_install_settings %}
Copy link
Contributor

@albertvaka albertvaka Sep 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For config_folder and config_file I think the same as above: I don't see why the user would need to override them.

For checks_confd, this field overlaps with confd_path in datadog_config (which is read by the agent), so I wonder if we should merge them (otherwise, the user needs to change both)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: I'll get rid of config_folder and config_file in a separate cleanup PR (they should not be used: if you use them, SaltStack will put the datadog.yaml file there, and the Agent will never find it, failing Agent starts).

For checks_confd, I have another planned change (after this PR is done) to make it use confd_path

{%- if latest_agent_version or parsed_version[1] == '6' %}
{% do datadog_settings.update({'checks_confd': '/etc/datadog-agent/conf.d'}) %}
{% do datadog_install_settings.update({'checks_confd': '/etc/datadog-agent/conf.d'}) %}
{%- else %}
{% do datadog_settings.update({'checks_confd': '/etc/dd-agent/conf.d'}) %}
{% do datadog_install_settings.update({'checks_confd': '/etc/dd-agent/conf.d'}) %}
{%- endif %}
{% endif %}
{% endif %}
12 changes: 6 additions & 6 deletions datadog/service.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{% from "datadog/map.jinja" import datadog_settings with context %}
{% set config_file_path = '%s/%s'|format(datadog_settings.config_folder, datadog_settings.config_file) -%}
{% from "datadog/map.jinja" import datadog_install_settings, datadog_checks with context %}
{% set config_file_path = '%s/%s'|format(datadog_install_settings.config_folder, datadog_install_settings.config_file) -%}

datadog-agent-service:
service.running:
- name: {{ datadog_settings.service_name }}
- name: {{ datadog_install_settings.service_name }}
- enable: True
- watch:
- pkg: {{ datadog_settings.pkg_name }}
- pkg: {{ datadog_install_settings.pkg_name }}
- file: {{ config_file_path }}
{%- if datadog_settings.checks is defined %}
- file: {{ datadog_settings.checks_confd }}/*
{%- if datadog_checks | length %}
- file: {{ datadog_install_settings.checks_confd }}/*
{% endif %}
6 changes: 3 additions & 3 deletions datadog/uninstall.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{% from "datadog/map.jinja" import datadog_settings with context %}
{% from "datadog/map.jinja" import datadog_install_settings with context %}

datadog-uninstall:
service.dead:
- name: {{ datadog_settings.service_name }}
- name: {{ datadog_install_settings.service_name }}
- enable: False
pkg.removed:
- pkgs:
- {{ datadog_settings.pkg_name }}
- {{ datadog_install_settings.pkg_name }}
- require:
- service: datadog-uninstall
11 changes: 8 additions & 3 deletions pillar.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
datadog:
api_key: aaaaaaaabbbbbbbbccccccccdddddddd
site: datadoghq.com
python_version: 2
config:
api_key: aaaaaaaabbbbbbbbccccccccdddddddd
site: datadoghq.com
python_version: 2

checks:
process:
init_config:
Expand All @@ -14,3 +16,6 @@ datadog:
- host: 127.0.0.1
name: sshd
port: 22

install_settings:
agent_version: latest
11 changes: 8 additions & 3 deletions test/pillar/datadog.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
datadog:
api_key: aaaaaaaabbbbbbbbccccccccdddddddd
site: datadoghq.com
python_version: 2
config:
api_key: aaaaaaaabbbbbbbbccccccccdddddddd
site: datadoghq.com
python_version: 2

checks:
directory:
instances:
- directory: "/srv/pillar"
name: "pillars"

install_settings:
agent_version: latest