From adcecaffaf5e532490b85096859f8adf136e1c62 Mon Sep 17 00:00:00 2001 From: Slavi Pantaleev Date: Mon, 19 Apr 2021 18:27:11 +0300 Subject: [PATCH] Fix connectivity between prometheus and prometheus-node-exporter Expected to have regressed after https://github.com/spantaleev/matrix-docker-ansible-deploy/pull/1008 This patch comes with its own downsides (as described in the comments for matrix_prometheus_node_exporter_container_http_host_bind_port), but at least there's: - no security issue - metrics remain readable from matrix-prometheus (even if the network metrics are inaccurate) A better patch is certainly welcome. --- group_vars/matrix_servers | 1 + .../defaults/main.yml | 12 ++++++++++++ .../matrix-prometheus-node-exporter.service.j2 | 10 ++++------ .../matrix-prometheus/tasks/setup_install.yml | 18 ------------------ 4 files changed, 17 insertions(+), 24 deletions(-) diff --git a/group_vars/matrix_servers b/group_vars/matrix_servers index 441cc1625..1f9d59262 100755 --- a/group_vars/matrix_servers +++ b/group_vars/matrix_servers @@ -1701,6 +1701,7 @@ matrix_prometheus_scraper_synapse_targets: ['matrix-synapse:{{ matrix_synapse_me matrix_prometheus_scraper_synapse_rules_synapse_tag: "{{ matrix_synapse_docker_image_tag }}" matrix_prometheus_scraper_node_enabled: "{{ matrix_prometheus_node_exporter_enabled }}" +matrix_prometheus_scraper_node_targets: "{{ ['matrix-prometheus-node-exporter:9100'] if matrix_prometheus_node_exporter_enabled else [] }}" ###################################################################### # diff --git a/roles/matrix-prometheus-node-exporter/defaults/main.yml b/roles/matrix-prometheus-node-exporter/defaults/main.yml index 90f809f12..492d48b19 100644 --- a/roles/matrix-prometheus-node-exporter/defaults/main.yml +++ b/roles/matrix-prometheus-node-exporter/defaults/main.yml @@ -19,4 +19,16 @@ matrix_prometheus_node_exporter_systemd_wanted_services_list: [] # Controls whether the matrix-prometheus container exposes its HTTP port (tcp/9100 in the container). # # Takes an ":" value (e.g. "127.0.0.1:9100"), or empty string to not expose. +# +# Official recommendations are to run this container with `--net=host`, +# but we don't do that, since it: +# - likely exposes the metrics web server way too publicly (before applying https://github.com/spantaleev/matrix-docker-ansible-deploy/pull/1008) +# - or listens on a loopback interface only (--net=host and 127.0.0.1:9100), which is not reachable from another container (like `matrix-prometheus`) +# +# Using `--net=host` and binding to Docker's `matrix` bridge network may be a solution to both, +# but that's trickier to accomplish and won't necessarily work (hasn't been tested). +# +# Not using `--net=host` means that our network statistic reports are likely broken (inaccurate), +# because node-exporter can't see all interfaces, etc. +# For now, we'll live with that, until someone develops a better solution. matrix_prometheus_node_exporter_container_http_host_bind_port: '' diff --git a/roles/matrix-prometheus-node-exporter/templates/systemd/matrix-prometheus-node-exporter.service.j2 b/roles/matrix-prometheus-node-exporter/templates/systemd/matrix-prometheus-node-exporter.service.j2 index 5a72d37a7..210a0d97a 100644 --- a/roles/matrix-prometheus-node-exporter/templates/systemd/matrix-prometheus-node-exporter.service.j2 +++ b/roles/matrix-prometheus-node-exporter/templates/systemd/matrix-prometheus-node-exporter.service.j2 @@ -25,15 +25,13 @@ ExecStart={{ matrix_host_command_docker }} run --rm --name matrix-prometheus-nod {% for arg in matrix_prometheus_node_exporter_container_extra_arguments %} {{ arg }} \ {% endfor %} - --net=host \ + --network={{ matrix_docker_network }} \ + {% if matrix_prometheus_node_exporter_container_http_host_bind_port %} + -p {{ matrix_prometheus_node_exporter_container_http_host_bind_port }}:9100 \ + {% endif %} --pid=host \ --mount type=bind,src=/,dst=/host,ro,bind-propagation=rslave \ {{ matrix_prometheus_node_exporter_docker_image }} \ - {% if matrix_prometheus_node_exporter_container_http_host_bind_port %} - --web.listen-address={{ matrix_prometheus_node_exporter_container_http_host_bind_port }} \ - {% else %} - --web.listen-address=localhost:9100 \ - {% endif %} --path.rootfs=/host ExecStop=-{{ matrix_host_command_sh }} -c '{{ matrix_host_command_docker }} kill matrix-prometheus-node-exporter 2>/dev/null' diff --git a/roles/matrix-prometheus/tasks/setup_install.yml b/roles/matrix-prometheus/tasks/setup_install.yml index df2fb687e..15a692797 100644 --- a/roles/matrix-prometheus/tasks/setup_install.yml +++ b/roles/matrix-prometheus/tasks/setup_install.yml @@ -19,24 +19,6 @@ - "{{ matrix_prometheus_config_path }}" - "{{ matrix_prometheus_data_path }}" -- block: - # Well, this actually creates the network if it doesn't exist, but.. - # The network should have been created by `matrix-base` already. - # We don't rely on that other call and its result, because it runs - # on `--tags=setup-all`, but will get skipped during `--tags=setup-prometheus`. - - name: Fetch Matrix Docker network details - docker_network: - name: "{{ matrix_docker_network }}" - driver: bridge - register: matrix_docker_network_info - - # The `matrix_docker_network_info.ansible_facts.docker_network` workaround is for Ansible < 2.8. - # See: https://github.com/spantaleev/matrix-docker-ansible-deploy/issues/907 - - set_fact: - matrix_prometheus_scraper_node_targets: ["{{ (matrix_docker_network_info.network|default(matrix_docker_network_info.ansible_facts.docker_network)).IPAM.Config[0].Gateway }}:9100"] - when: "matrix_prometheus_scraper_node_enabled|bool and matrix_prometheus_scraper_node_targets|length == 0" - - - name: Download synapse-v2.rules get_url: url: "{{ matrix_prometheus_scraper_synapse_rules_download_url }}"