From 88f79f8324905d03bdc64f6f1cb9a4507ed512b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Jeanneret?= Date: Wed, 15 May 2024 07:50:40 +0200 Subject: [PATCH] Refine a bit dnsmasq role An assertion block was missing a condition, leading to issue when we wanted to just remove a network from the configuration. In the network configuration template, the test to know more about ipv4 and ipv6 ranges was a bit too loose, leading to issues under certain conditions. We faced the same issue in the host management, leading to potentially wrong DHCP configuration. In order to ease debugging, we now generate a fact, display its content, and use that content to inject the host data in the configuration. During a re-run, the service is already stopped/disabled, and the unit file is removed. Ansible will then fail, since systemd can't find the service to stop. --- roles/dnsmasq/tasks/configure.yml | 4 +++ roles/dnsmasq/tasks/manage_host.yml | 36 +++++++++++++++---------- roles/dnsmasq/tasks/manage_network.yml | 2 ++ roles/dnsmasq/templates/network.conf.j2 | 4 +-- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/roles/dnsmasq/tasks/configure.yml b/roles/dnsmasq/tasks/configure.yml index c4047786cb..8a4b7a6956 100644 --- a/roles/dnsmasq/tasks/configure.yml +++ b/roles/dnsmasq/tasks/configure.yml @@ -74,10 +74,14 @@ become: true when: - _act == 'cleanup' + register: _dnsmasq_stop ansible.builtin.systemd_service: name: cifmw-dnsmasq.service state: stopped enabled: false + failed_when: + - _dnsmasq_stop.msg is defined and + _dnsmasq_stop.msg is not match('Could not find the requested service cifmw-dnsmasq.service') - name: Remove unit file become: true diff --git a/roles/dnsmasq/tasks/manage_host.yml b/roles/dnsmasq/tasks/manage_host.yml index 64e8132d66..94e15a4deb 100644 --- a/roles/dnsmasq/tasks/manage_host.yml +++ b/roles/dnsmasq/tasks/manage_host.yml @@ -39,28 +39,36 @@ - _network_exists.stat.exists quiet: true msg: >- - You must create the network first, calling + You must create {{ cifmw_dnsmasq_host_network }} network first, calling dnsmasq/manage_network.yml task. -- name: Manage host entry - become: true - notify: Restart dnsmasq - ansible.builtin.blockinfile: - block: | - {% if cifmw_dnsmasq_host_ipv4 is defined -%} - {% set data = [cifmw_dnsmasq_host_mac] -%} - {% set _ = data.append(cifmw_dnsmasq_host_ipv4) -%} +- name: Compute entry + ansible.builtin.set_fact: + _host_entry: | + {% if cifmw_dnsmasq_host_ipv4 is defined and cifmw_dnsmasq_host_ipv4 | length > 0 -%} + {% set data = [cifmw_dnsmasq_host_mac] -%} + {% set _ = data.append(cifmw_dnsmasq_host_ipv4) -%} {% set _ = data.append(cifmw_dnsmasq_host_name) if - cifmw_dnsmasq_host_name is defined -%} + cifmw_dnsmasq_host_name is defined and cifmw_dnsmasq_host_name | length > 0 -%} dhcp-host={{ data | join(',') }} {% endif -%} - {% if cifmw_dnsmasq_host_ipv6 is defined -%} - {% set data = [cifmw_dnsmasq_host_mac] -%} - {% set _ = data.append('['~cifmw_dnsmasq_host_ipv6~']') -%} + {% if cifmw_dnsmasq_host_ipv6 is defined and cifmw_dnsmasq_host_ipv6 | length > 0 -%} + {% set data = [cifmw_dnsmasq_host_mac] -%} + {% set _ = data.append('['~cifmw_dnsmasq_host_ipv6~']') -%} {% set _ = data.append(cifmw_dnsmasq_host_name) if - cifmw_dnsmasq_host_name is defined -%} + cifmw_dnsmasq_host_name is defined and cifmw_dnsmasq_host_name | length > 0 -%} dhcp-host={{ data | join(',') }} {% endif -%} + +- name: Debug _host_entry + ansible.builtin.debug: + var: _host_entry + +- name: Manage host entry + become: true + notify: Restart dnsmasq + ansible.builtin.blockinfile: + block: "{{ _host_entry }}" dest: "{{ cifmw_dnsmasq_basedir }}/{{ cifmw_dnsmasq_host_network }}-hosts.conf" create: true mode: "0644" diff --git a/roles/dnsmasq/tasks/manage_network.yml b/roles/dnsmasq/tasks/manage_network.yml index b16d29b1c3..40392d432c 100644 --- a/roles/dnsmasq/tasks/manage_network.yml +++ b/roles/dnsmasq/tasks/manage_network.yml @@ -22,6 +22,8 @@ - cifmw_dnsmasq_network_state in ['present', 'absent'] - name: Assert mandatory parameters for new network + when: + - cifmw_dnsmasq_network_state == 'present' ansible.builtin.assert: that: - cifmw_dnsmasq_network_definition is defined diff --git a/roles/dnsmasq/templates/network.conf.j2 b/roles/dnsmasq/templates/network.conf.j2 index c279b25189..ac22167838 100644 --- a/roles/dnsmasq/templates/network.conf.j2 +++ b/roles/dnsmasq/templates/network.conf.j2 @@ -1,9 +1,9 @@ # Managed by ci-framework/dnsmasq {% for range in cifmw_dnsmasq_network_definition['ranges'] %} -{% if range.start_v4 is defined %} +{% if range.start_v4 is defined and range.start_v4 | length > 0 %} dhcp-range=set:{{ range.label }},{{ range.start_v4 }},static,{{ (range.start_v4 + "/" + range.prefix_length_v4 | default(24) | string) | ansible.utils.ipaddr('netmask') }},{{ range.ttl | default('1h') }} {% endif %} -{% if range.start_v6 is defined %} +{% if range.start_v6 is defined and range.start_v6 | length > 0 %} dhcp-range=set:{{ range.label }},{{ range.start_v6 }},static,{{ range.prefix_length_v6 | default('64') }},{{ range.ttl | default('1h') }} {% endif %} {% for option in range['options'] | default([]) %}