Skip to content

Commit

Permalink
Refine a bit dnsmasq role
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cjeanner authored and openshift-merge-bot[bot] committed May 15, 2024
1 parent 7570d14 commit 88f79f8
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 16 deletions.
4 changes: 4 additions & 0 deletions roles/dnsmasq/tasks/configure.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 22 additions & 14 deletions roles/dnsmasq/tasks/manage_host.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions roles/dnsmasq/tasks/manage_network.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions roles/dnsmasq/templates/network.conf.j2
Original file line number Diff line number Diff line change
@@ -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([]) %}
Expand Down

0 comments on commit 88f79f8

Please sign in to comment.