From fdea19a305c0926f4be4c0c5f29e57bdb5a9ee85 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Fri, 17 Jan 2020 17:00:21 +0000
Subject: [PATCH] Separate per-service host configuration tasks

Currently there are a few services that perform host configuration
tasks. This is done in config.yml. This means that these changes are
performed during 'kolla-ansible genconfig', when we might expect not to
be making any changes to the remote system.

This change separates out these host configuration tasks into a
config-host.yml file, which is included directly from deploy.yml.

One change in behaviour is that this prevents these tasks from running
during an upgrade or genconfig. This is probably what we want, but we
should be careful when any of these host configuration tasks are
changed, to ensure they are applied during an upgrade if necessary.

Change-Id: I001defc75d1f1e6caa9b1e11246abc6ce17c775b
Closes-Bug: #1860161
---
 .../roles/elasticsearch/tasks/config-host.yml | 12 ++++++++
 ansible/roles/elasticsearch/tasks/config.yml  |  9 ------
 ansible/roles/elasticsearch/tasks/deploy.yml  |  2 ++
 ansible/roles/haproxy/tasks/config-host.yml   | 20 +++++++++++++
 ansible/roles/haproxy/tasks/config.yml        | 17 -----------
 ansible/roles/haproxy/tasks/deploy.yml        |  2 ++
 ansible/roles/ironic/tasks/config-host.yml    |  8 +++++
 ansible/roles/ironic/tasks/config.yml         |  7 -----
 ansible/roles/ironic/tasks/deploy.yml         |  2 ++
 ansible/roles/iscsi/tasks/config-host.yml     | 10 +++++++
 ansible/roles/iscsi/tasks/config.yml          | 10 -------
 ansible/roles/iscsi/tasks/deploy.yml          |  2 ++
 .../roles/multipathd/tasks/config-host.yml    |  7 +++++
 ansible/roles/multipathd/tasks/config.yml     |  7 -----
 ansible/roles/multipathd/tasks/deploy.yml     |  2 ++
 ansible/roles/neutron/tasks/config-host.yml   | 30 +++++++++++++++++++
 ansible/roles/neutron/tasks/config.yml        | 27 -----------------
 ansible/roles/neutron/tasks/deploy.yml        |  2 ++
 ansible/roles/nova-cell/tasks/config-host.yml | 15 ++++++++++
 ansible/roles/nova-cell/tasks/config.yml      | 12 --------
 ansible/roles/nova-cell/tasks/deploy.yml      |  2 ++
 .../roles/openvswitch/tasks/config-host.yml   |  7 +++++
 ansible/roles/openvswitch/tasks/config.yml    |  7 -----
 ansible/roles/openvswitch/tasks/deploy.yml    |  2 ++
 ...-config-in-genconfig-7321f0dcfc9d728d.yaml |  7 +++++
 25 files changed, 132 insertions(+), 96 deletions(-)
 create mode 100644 ansible/roles/elasticsearch/tasks/config-host.yml
 create mode 100644 ansible/roles/haproxy/tasks/config-host.yml
 create mode 100644 ansible/roles/ironic/tasks/config-host.yml
 create mode 100644 ansible/roles/iscsi/tasks/config-host.yml
 create mode 100644 ansible/roles/multipathd/tasks/config-host.yml
 create mode 100644 ansible/roles/neutron/tasks/config-host.yml
 create mode 100644 ansible/roles/nova-cell/tasks/config-host.yml
 create mode 100644 ansible/roles/openvswitch/tasks/config-host.yml
 create mode 100644 releasenotes/notes/no-host-config-in-genconfig-7321f0dcfc9d728d.yaml

diff --git a/ansible/roles/elasticsearch/tasks/config-host.yml b/ansible/roles/elasticsearch/tasks/config-host.yml
new file mode 100644
index 000000000..638721a43
--- /dev/null
+++ b/ansible/roles/elasticsearch/tasks/config-host.yml
@@ -0,0 +1,12 @@
+---
+- name: Setting sysctl values
+  become: true
+  sysctl:
+    name: "{{ item.name }}"
+    value: "{{ item.value }}"
+    sysctl_set: yes
+  with_items:
+    - { name: "vm.max_map_count", value: 262144}
+  when:
+    - set_sysctl | bool
+    - inventory_hostname in groups['elasticsearch']
diff --git a/ansible/roles/elasticsearch/tasks/config.yml b/ansible/roles/elasticsearch/tasks/config.yml
index 341073283..8ba746cfb 100644
--- a/ansible/roles/elasticsearch/tasks/config.yml
+++ b/ansible/roles/elasticsearch/tasks/config.yml
@@ -1,13 +1,4 @@
 ---
-- name: Setting sysctl values
-  become: true
-  sysctl: name={{ item.name }} value={{ item.value }} sysctl_set=yes
-  with_items:
-    - { name: "vm.max_map_count", value: 262144}
-  when:
-    - set_sysctl | bool
-    - inventory_hostname in groups['elasticsearch']
-
 - name: Ensuring config directories exist
   file:
     path: "{{ node_config_directory }}/{{ item.key }}"
diff --git a/ansible/roles/elasticsearch/tasks/deploy.yml b/ansible/roles/elasticsearch/tasks/deploy.yml
index 375dcad19..476832489 100644
--- a/ansible/roles/elasticsearch/tasks/deploy.yml
+++ b/ansible/roles/elasticsearch/tasks/deploy.yml
@@ -1,4 +1,6 @@
 ---
+- include_tasks: config-host.yml
+
 - include_tasks: config.yml
 
 - name: Flush handlers
diff --git a/ansible/roles/haproxy/tasks/config-host.yml b/ansible/roles/haproxy/tasks/config-host.yml
new file mode 100644
index 000000000..eb5bb4910
--- /dev/null
+++ b/ansible/roles/haproxy/tasks/config-host.yml
@@ -0,0 +1,20 @@
+---
+- name: Setting sysctl values
+  sysctl:
+    name: "{{ item.name }}"
+    value: "{{ item.value }}"
+    sysctl_set: yes
+  become: true
+  with_items:
+    - { name: "net.ipv4.ip_nonlocal_bind", value: 1}
+    - { name: "net.ipv6.ip_nonlocal_bind", value: 1}
+    - { name: "net.unix.max_dgram_qlen", value: 128}
+  when:
+    - set_sysctl | bool
+
+- name: Load and persist keepalived module
+  import_role:
+    name: module-load
+  vars:
+    modules:
+      - {'name': ip_vs }
diff --git a/ansible/roles/haproxy/tasks/config.yml b/ansible/roles/haproxy/tasks/config.yml
index fd215a32d..99ea810fd 100644
--- a/ansible/roles/haproxy/tasks/config.yml
+++ b/ansible/roles/haproxy/tasks/config.yml
@@ -1,14 +1,4 @@
 ---
-- name: Setting sysctl values
-  sysctl: name={{ item.name }} value={{ item.value }} sysctl_set=yes
-  become: true
-  with_items:
-    - { name: "net.ipv4.ip_nonlocal_bind", value: 1}
-    - { name: "net.ipv6.ip_nonlocal_bind", value: 1}
-    - { name: "net.unix.max_dgram_qlen", value: 128}
-  when:
-    - set_sysctl | bool
-
 - name: Ensuring config directories exist
   file:
     path: "{{ node_config_directory }}/{{ item.key }}"
@@ -83,13 +73,6 @@
   notify:
     - Restart haproxy container
 
-- name: Load and persist keepalived module
-  import_role:
-    name: module-load
-  vars:
-    modules:
-      - {'name': ip_vs }
-
 - name: Copying over keepalived.conf
   vars:
     service: "{{ haproxy_services['keepalived'] }}"
diff --git a/ansible/roles/haproxy/tasks/deploy.yml b/ansible/roles/haproxy/tasks/deploy.yml
index 375dcad19..476832489 100644
--- a/ansible/roles/haproxy/tasks/deploy.yml
+++ b/ansible/roles/haproxy/tasks/deploy.yml
@@ -1,4 +1,6 @@
 ---
+- include_tasks: config-host.yml
+
 - include_tasks: config.yml
 
 - name: Flush handlers
diff --git a/ansible/roles/ironic/tasks/config-host.yml b/ansible/roles/ironic/tasks/config-host.yml
new file mode 100644
index 000000000..6181f7ccd
--- /dev/null
+++ b/ansible/roles/ironic/tasks/config-host.yml
@@ -0,0 +1,8 @@
+---
+- name: Load and persist iscsi_tcp module
+  import_role:
+    name: module-load
+  vars:
+    modules:
+      - {'name': iscsi_tcp}
+  when: inventory_hostname in groups['ironic-conductor']
diff --git a/ansible/roles/ironic/tasks/config.yml b/ansible/roles/ironic/tasks/config.yml
index bcbd4bf28..e359e4791 100644
--- a/ansible/roles/ironic/tasks/config.yml
+++ b/ansible/roles/ironic/tasks/config.yml
@@ -1,11 +1,4 @@
 ---
-- name: Load and persist iscsi_tcp module
-  import_role:
-    name: module-load
-  vars:
-    modules:
-      - {'name': iscsi_tcp}
-
 - name: Ensuring config directories exist
   file:
     path: "{{ node_config_directory }}/{{ item.key }}"
diff --git a/ansible/roles/ironic/tasks/deploy.yml b/ansible/roles/ironic/tasks/deploy.yml
index f4c0d8ca6..4dddfa9b7 100644
--- a/ansible/roles/ironic/tasks/deploy.yml
+++ b/ansible/roles/ironic/tasks/deploy.yml
@@ -4,6 +4,8 @@
         (inventory_hostname in groups['ironic-api'] or
         inventory_hostname in groups['ironic-inspector'])
 
+- include_tasks: config-host.yml
+
 - include_tasks: config.yml
   when: inventory_hostname in groups['ironic-api'] or
         inventory_hostname in groups['ironic-conductor'] or
diff --git a/ansible/roles/iscsi/tasks/config-host.yml b/ansible/roles/iscsi/tasks/config-host.yml
new file mode 100644
index 000000000..302c39f46
--- /dev/null
+++ b/ansible/roles/iscsi/tasks/config-host.yml
@@ -0,0 +1,10 @@
+---
+- name: Load and persist configfs module
+  import_role:
+    name: module-load
+  vars:
+    modules:
+      - name: configfs
+  when:
+    - inventory_hostname in groups[iscsi_services.iscsid.group]
+    - iscsi_services.iscsid.enabled | bool
diff --git a/ansible/roles/iscsi/tasks/config.yml b/ansible/roles/iscsi/tasks/config.yml
index 6bda5f701..e884678ec 100644
--- a/ansible/roles/iscsi/tasks/config.yml
+++ b/ansible/roles/iscsi/tasks/config.yml
@@ -1,14 +1,4 @@
 ---
-- name: Load and persist configfs module
-  import_role:
-    name: module-load
-  vars:
-    modules:
-      - name: configfs
-  when:
-    - inventory_hostname in groups[iscsi_services.iscsid.group]
-    - iscsi_services.iscsid.enabled | bool
-
 - name: Ensuring config directories exist
   file:
     path: "{{ node_config_directory }}/{{ item.key }}"
diff --git a/ansible/roles/iscsi/tasks/deploy.yml b/ansible/roles/iscsi/tasks/deploy.yml
index 375dcad19..476832489 100644
--- a/ansible/roles/iscsi/tasks/deploy.yml
+++ b/ansible/roles/iscsi/tasks/deploy.yml
@@ -1,4 +1,6 @@
 ---
+- include_tasks: config-host.yml
+
 - include_tasks: config.yml
 
 - name: Flush handlers
diff --git a/ansible/roles/multipathd/tasks/config-host.yml b/ansible/roles/multipathd/tasks/config-host.yml
new file mode 100644
index 000000000..7c646d7d0
--- /dev/null
+++ b/ansible/roles/multipathd/tasks/config-host.yml
@@ -0,0 +1,7 @@
+---
+- name: Load and persist dm-multipath module
+  import_role:
+    name: module-load
+  vars:
+    modules:
+      - {'name': dm-multipath}
diff --git a/ansible/roles/multipathd/tasks/config.yml b/ansible/roles/multipathd/tasks/config.yml
index b11ea424e..601cdbf48 100644
--- a/ansible/roles/multipathd/tasks/config.yml
+++ b/ansible/roles/multipathd/tasks/config.yml
@@ -1,11 +1,4 @@
 ---
-- name: Load and persist dm-multipath module
-  import_role:
-    name: module-load
-  vars:
-    modules:
-      - {'name': dm-multipath}
-
 - name: Ensuring config directories exist
   file:
     path: "{{ node_config_directory }}/{{ item.key }}"
diff --git a/ansible/roles/multipathd/tasks/deploy.yml b/ansible/roles/multipathd/tasks/deploy.yml
index 375dcad19..476832489 100644
--- a/ansible/roles/multipathd/tasks/deploy.yml
+++ b/ansible/roles/multipathd/tasks/deploy.yml
@@ -1,4 +1,6 @@
 ---
+- include_tasks: config-host.yml
+
 - include_tasks: config.yml
 
 - name: Flush handlers
diff --git a/ansible/roles/neutron/tasks/config-host.yml b/ansible/roles/neutron/tasks/config-host.yml
new file mode 100644
index 000000000..59db51add
--- /dev/null
+++ b/ansible/roles/neutron/tasks/config-host.yml
@@ -0,0 +1,30 @@
+---
+- name: Load and persist ip6_tables module
+  include_role:
+    name: module-load
+  vars:
+    modules:
+      - {'name': ip6_tables}
+  when: neutron_services | select_services_enabled_and_mapped_to_host | list | intersect([ "neutron-l3-agent", "neutron-linuxbridge-agent", "neutron-openvswitch-agent" ]) | list | length > 0
+
+- name: Setting sysctl values
+  become: true
+  vars:
+    neutron_l3_agent: "{{ neutron_services['neutron-l3-agent'] }}"
+  sysctl:
+    name: "{{ item.name }}"
+    value: "{{ item.value }}"
+    sysctl_set: yes
+  with_items:
+    - { name: "net.ipv4.ip_forward", value: 1}
+    - { name: "net.ipv4.conf.all.rp_filter", value: "{{ neutron_l3_agent_host_rp_filter_mode }}"}
+    - { name: "net.ipv4.conf.default.rp_filter", value: "{{ neutron_l3_agent_host_rp_filter_mode }}"}
+    - { name: "net.ipv4.neigh.default.gc_thresh1", value: "{{ neutron_l3_agent_host_ipv4_neigh_gc_thresh1 }}"}
+    - { name: "net.ipv4.neigh.default.gc_thresh2", value: "{{ neutron_l3_agent_host_ipv4_neigh_gc_thresh2 }}"}
+    - { name: "net.ipv4.neigh.default.gc_thresh3", value: "{{ neutron_l3_agent_host_ipv4_neigh_gc_thresh3 }}"}
+    - { name: "net.ipv6.neigh.default.gc_thresh1", value: "{{ neutron_l3_agent_host_ipv6_neigh_gc_thresh1 }}"}
+    - { name: "net.ipv6.neigh.default.gc_thresh2", value: "{{ neutron_l3_agent_host_ipv6_neigh_gc_thresh2 }}"}
+    - { name: "net.ipv6.neigh.default.gc_thresh3", value: "{{ neutron_l3_agent_host_ipv6_neigh_gc_thresh3 }}"}
+  when:
+    - set_sysctl | bool
+    - (neutron_l3_agent.enabled | bool and neutron_l3_agent.host_in_groups | bool)
diff --git a/ansible/roles/neutron/tasks/config.yml b/ansible/roles/neutron/tasks/config.yml
index 107daa416..bf8b3bcb4 100644
--- a/ansible/roles/neutron/tasks/config.yml
+++ b/ansible/roles/neutron/tasks/config.yml
@@ -1,31 +1,4 @@
 ---
-- name: Load and persist ip6_tables module
-  include_role:
-    name: module-load
-  vars:
-    modules:
-      - {'name': ip6_tables}
-  when: neutron_services | select_services_enabled_and_mapped_to_host | list | intersect([ "neutron-l3-agent", "neutron-linuxbridge-agent", "neutron-openvswitch-agent" ]) | list | length > 0
-
-- name: Setting sysctl values
-  become: true
-  vars:
-    neutron_l3_agent: "{{ neutron_services['neutron-l3-agent'] }}"
-  sysctl: name={{ item.name }} value={{ item.value }} sysctl_set=yes
-  with_items:
-    - { name: "net.ipv4.ip_forward", value: 1}
-    - { name: "net.ipv4.conf.all.rp_filter", value: "{{ neutron_l3_agent_host_rp_filter_mode }}"}
-    - { name: "net.ipv4.conf.default.rp_filter", value: "{{ neutron_l3_agent_host_rp_filter_mode }}"}
-    - { name: "net.ipv4.neigh.default.gc_thresh1", value: "{{ neutron_l3_agent_host_ipv4_neigh_gc_thresh1 }}"}
-    - { name: "net.ipv4.neigh.default.gc_thresh2", value: "{{ neutron_l3_agent_host_ipv4_neigh_gc_thresh2 }}"}
-    - { name: "net.ipv4.neigh.default.gc_thresh3", value: "{{ neutron_l3_agent_host_ipv4_neigh_gc_thresh3 }}"}
-    - { name: "net.ipv6.neigh.default.gc_thresh1", value: "{{ neutron_l3_agent_host_ipv6_neigh_gc_thresh1 }}"}
-    - { name: "net.ipv6.neigh.default.gc_thresh2", value: "{{ neutron_l3_agent_host_ipv6_neigh_gc_thresh2 }}"}
-    - { name: "net.ipv6.neigh.default.gc_thresh3", value: "{{ neutron_l3_agent_host_ipv6_neigh_gc_thresh3 }}"}
-  when:
-    - set_sysctl | bool
-    - (neutron_l3_agent.enabled | bool and neutron_l3_agent.host_in_groups | bool)
-
 - name: Ensuring config directories exist
   become: true
   file:
diff --git a/ansible/roles/neutron/tasks/deploy.yml b/ansible/roles/neutron/tasks/deploy.yml
index 6662124c7..86aa7c1cc 100644
--- a/ansible/roles/neutron/tasks/deploy.yml
+++ b/ansible/roles/neutron/tasks/deploy.yml
@@ -5,6 +5,8 @@
 - include_tasks: clone.yml
   when: neutron_dev_mode | bool
 
+- include_tasks: config-host.yml
+
 - include_tasks: config.yml
 
 - include_tasks: config-neutron-fake.yml
diff --git a/ansible/roles/nova-cell/tasks/config-host.yml b/ansible/roles/nova-cell/tasks/config-host.yml
new file mode 100644
index 000000000..2737d4ab5
--- /dev/null
+++ b/ansible/roles/nova-cell/tasks/config-host.yml
@@ -0,0 +1,15 @@
+---
+- name: Setting sysctl values
+  become: true
+  sysctl:
+    name: "{{ item.name }}"
+    value: "{{ item.value }}"
+    sysctl_set: yes
+  with_items:
+    - { name: "net.bridge.bridge-nf-call-iptables", value: 1}
+    - { name: "net.bridge.bridge-nf-call-ip6tables", value: 1}
+    - { name: "net.ipv4.conf.all.rp_filter", value: "{{ nova_compute_host_rp_filter_mode }}"}
+    - { name: "net.ipv4.conf.default.rp_filter", value: "{{ nova_compute_host_rp_filter_mode }}"}
+  when:
+    - set_sysctl | bool
+    - inventory_hostname in groups[nova_cell_compute_group]
diff --git a/ansible/roles/nova-cell/tasks/config.yml b/ansible/roles/nova-cell/tasks/config.yml
index 04c291fec..ca64b96aa 100644
--- a/ansible/roles/nova-cell/tasks/config.yml
+++ b/ansible/roles/nova-cell/tasks/config.yml
@@ -1,16 +1,4 @@
 ---
-- name: Setting sysctl values
-  become: true
-  sysctl: name={{ item.name }} value={{ item.value }} sysctl_set=yes
-  with_items:
-    - { name: "net.bridge.bridge-nf-call-iptables", value: 1}
-    - { name: "net.bridge.bridge-nf-call-ip6tables", value: 1}
-    - { name: "net.ipv4.conf.all.rp_filter", value: "{{ nova_compute_host_rp_filter_mode }}"}
-    - { name: "net.ipv4.conf.default.rp_filter", value: "{{ nova_compute_host_rp_filter_mode }}"}
-  when:
-    - set_sysctl | bool
-    - inventory_hostname in groups[nova_cell_compute_group]
-
 - name: Ensuring config directories exist
   become: true
   file:
diff --git a/ansible/roles/nova-cell/tasks/deploy.yml b/ansible/roles/nova-cell/tasks/deploy.yml
index 7e0a9cd22..37d05f276 100644
--- a/ansible/roles/nova-cell/tasks/deploy.yml
+++ b/ansible/roles/nova-cell/tasks/deploy.yml
@@ -7,6 +7,8 @@
 - include_tasks: clone.yml
   when: nova_dev_mode | bool
 
+- include_tasks: config-host.yml
+
 - include_tasks: config.yml
 
 - include_tasks: config-nova-fake.yml
diff --git a/ansible/roles/openvswitch/tasks/config-host.yml b/ansible/roles/openvswitch/tasks/config-host.yml
new file mode 100644
index 000000000..18dae6a07
--- /dev/null
+++ b/ansible/roles/openvswitch/tasks/config-host.yml
@@ -0,0 +1,7 @@
+---
+- name: Load and persist openvswitch module
+  import_role:
+    name: module-load
+  vars:
+    modules:
+      - {'name': openvswitch}
diff --git a/ansible/roles/openvswitch/tasks/config.yml b/ansible/roles/openvswitch/tasks/config.yml
index 3069071d1..e32c76ec4 100644
--- a/ansible/roles/openvswitch/tasks/config.yml
+++ b/ansible/roles/openvswitch/tasks/config.yml
@@ -1,11 +1,4 @@
 ---
-- name: Load and persist openvswitch module
-  import_role:
-    name: module-load
-  vars:
-    modules:
-      - {'name': openvswitch}
-
 - name: Ensuring config directories exist
   become: true
   file:
diff --git a/ansible/roles/openvswitch/tasks/deploy.yml b/ansible/roles/openvswitch/tasks/deploy.yml
index 110210a16..60c9a9902 100644
--- a/ansible/roles/openvswitch/tasks/deploy.yml
+++ b/ansible/roles/openvswitch/tasks/deploy.yml
@@ -1,4 +1,6 @@
 ---
+- include_tasks: config-host.yml
+
 - include_tasks: config.yml
 
 - name: Flush Handlers
diff --git a/releasenotes/notes/no-host-config-in-genconfig-7321f0dcfc9d728d.yaml b/releasenotes/notes/no-host-config-in-genconfig-7321f0dcfc9d728d.yaml
new file mode 100644
index 000000000..9a7214d14
--- /dev/null
+++ b/releasenotes/notes/no-host-config-in-genconfig-7321f0dcfc9d728d.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes an issue where host configuration tasks (``sysctl``, loading kernel
+    modules) could be performed during the ``kolla-ansible genconfig`` command.
+    See `bug 1860161 <https://bugs.launchpad.net/kolla-ansible/+bug/1860161>`__
+    for details.
-- 
GitLab