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

feat: bunch of improvements on blackbox_exporer #224

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions roles/blackbox_exporter/defaults/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ blackbox_exporter_configuration_modules:
# validate_answer_rrs:
# fail_if_matches_regexp: [test]

# Where to put the blackbox_exporter.yml main configuration file
blackbox_exporter_config_dir: /etc
# Where to put the blackbox.yml main configuration file
blackbox_exporter_config_dir: /etc/blackbox_exporter

blackbox_exporter_binary_install_dir: "/usr/local/bin"
blackbox_exporter_binary_name: "blackbox_exporter"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want to do this.

Copy link
Author

Choose a reason for hiding this comment

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

could you expand on this? what exactly do we want to not do here, and why?

Copy link
Member

Choose a reason for hiding this comment

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

I don't see any need/benefit with having the binary name customizable, we should keep the options at minimal and avoid having options just for options sake.

Copy link
Author

Choose a reason for hiding this comment

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

this is not options option's sake: i have modified this option for it to be compatible with the debian package....

Copy link
Member

Choose a reason for hiding this comment

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

Ah apologies. At the moment we do not support other installation methods than the binary install method that's currently in the roles.
When and if we expand that into rpm/deb/etc. packages we would definitely want the roles to install those packages, then variables such as these would most likely be populated internally in the role and not user configurable.
With that being said, such a feature would be a major change and I would first like to take care of some streamlining/deduplicating of tasks in the roles we currently have. (#64 )

Copy link
Author

Choose a reason for hiding this comment

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

ah. well, i guess one thing i'm saying is you're almost there and you just need to use the right binary name here. i've deployed prom on a debian 12 host using this playbook and it kind of worked after the bunch of patches i had to roll here...


blackbox_exporter_service_name: blackbox_exporter
4 changes: 2 additions & 2 deletions roles/blackbox_exporter/handlers/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
become: true
ansible.builtin.systemd:
daemon_reload: true
name: blackbox_exporter
name: "{{ blackbox_exporter_service_name }}"
state: restarted

- name: Reload blackbox_exporter
listen: "reload blackbox_exporter"
become: true
ansible.builtin.systemd:
name: blackbox_exporter
name: "{{ blackbox_exporter_service_name }}"
state: reloaded
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@


@pytest.mark.parametrize("files", [
"/etc/blackbox_exporter.yml",
"/etc/prometheus/blackbox.yml",
"/etc/systemd/system/blackbox_exporter.service",
"/usr/local/bin/blackbox_exporter"
])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@


@pytest.mark.parametrize("files", [
"/etc/blackbox_exporter.yml",
"/etc/prometheus/blackbox.yml",
"/etc/systemd/system/blackbox_exporter.service",
"/usr/local/bin/blackbox_exporter"
])
Expand Down
4 changes: 2 additions & 2 deletions roles/blackbox_exporter/tasks/configure.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
- name: Create systemd service unit
ansible.builtin.template:
src: blackbox_exporter.service.j2
dest: /etc/systemd/system/blackbox_exporter.service
dest: "/etc/systemd/system/{{ blackbox_exporter_service_name }}.service"
owner: root
group: root
mode: '0644'
Expand All @@ -12,7 +12,7 @@
- name: Configure blackbox exporter
ansible.builtin.template:
src: blackbox_exporter.yml.j2
dest: "{{ blackbox_exporter_config_dir }}/blackbox_exporter.yml"
dest: "{{ blackbox_exporter_config_dir }}/blackbox.yml"
owner: root
group: "{{ blackbox_exporter_group }}"
mode: '0644'
Expand Down
6 changes: 3 additions & 3 deletions roles/blackbox_exporter/tasks/install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
- name: Propagate blackbox_exporter binaries
ansible.builtin.copy:
src: "/tmp/blackbox_exporter-{{ blackbox_exporter_version }}.linux-{{ go_arch }}/blackbox_exporter"
dest: "{{ blackbox_exporter_binary_install_dir }}/blackbox_exporter"
dest: "{{ blackbox_exporter_binary_install_dir }}/{{ blackbox_exporter_binary_name }}"
mode: 0755
owner: root
group: root
Expand All @@ -57,7 +57,7 @@
- name: Propagate locally distributed blackbox_exporter binary
ansible.builtin.copy:
src: "{{ blackbox_exporter_binary_local_dir }}/blackbox_exporter"
dest: "{{ blackbox_exporter_binary_install_dir }}/blackbox_exporter"
dest: "{{ blackbox_exporter_binary_install_dir }}/{{ blackbox_exporter_binary_name }}"
mode: '0755'
owner: root
group: root
Expand All @@ -78,7 +78,7 @@

- name: Ensure blackbox exporter binary has cap_net_raw capability
community.general.capabilities:
path: '/usr/local/bin/blackbox_exporter'
path: '{{ blackbox_exporter_binary_install_dir }}/{{ blackbox_exporter_binary_name }}'
capability: cap_net_raw+ep
state: present
when: not ansible_check_mode
Expand Down
2 changes: 1 addition & 1 deletion roles/blackbox_exporter/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
become: true
ansible.builtin.systemd:
daemon_reload: true
name: blackbox_exporter
name: "{{ blackbox_exporter_service_name }}"
state: started
enabled: true
tags:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ User={{ blackbox_exporter_user }}
Group={{ blackbox_exporter_group }}
PermissionsStartOnly=true
ExecReload=/bin/kill -HUP $MAINPID
ExecStart=/usr/local/bin/blackbox_exporter \
--config.file={{ blackbox_exporter_config_dir }}/blackbox_exporter.yml \
ExecStart={{ blackbox_exporter_binary_install_dir }}/{{ blackbox_exporter_binary_name }} \
--config.file={{ blackbox_exporter_config_dir }}/blackbox.yml \
{% for flag, flag_value in blackbox_exporter_cli_flags.items() -%}
--{{ flag }}={{ flag_value }} \
{% endfor -%}
Expand Down
Loading