From 0947974ff64cd758608b224669b19d58e8e63163 Mon Sep 17 00:00:00 2001
From: Michal Nasiadka <mnasiadka@gmail.com>
Date: Thu, 21 Nov 2019 08:25:57 +0000
Subject: [PATCH] Split out OVS/Linuxbridge agent configs from ml2_conf.ini

Change-Id: I799993728112a525e34cfbc4e786a10f0ed03be9
---
 ansible/roles/neutron/defaults/main.yml       | 29 ++++++--
 .../neutron/tasks/config-neutron-fake.yml     | 10 +--
 ansible/roles/neutron/tasks/config.yml        | 66 ++++++++++++++-----
 .../templates/linuxbridge_agent.ini.j2        | 15 +++++
 .../roles/neutron/templates/ml2_conf.ini.j2   | 63 +-----------------
 .../neutron-linuxbridge-agent.json.j2         |  6 +-
 .../neutron-openvswitch-agent-xenapi.json.j2  |  6 +-
 .../neutron-openvswitch-agent.json.j2         |  6 +-
 .../templates/openvswitch_agent.ini.j2        | 25 +++++++
 ...ini.j2 => openvswitch_agent_xenapi.ini.j2} |  1 -
 doc/source/reference/networking/neutron.rst   |  2 +-
 ...ovs_lb_agents_config-96f1f90e7d678209.yaml | 14 ++++
 12 files changed, 146 insertions(+), 97 deletions(-)
 create mode 100644 ansible/roles/neutron/templates/linuxbridge_agent.ini.j2
 create mode 100644 ansible/roles/neutron/templates/openvswitch_agent.ini.j2
 rename ansible/roles/neutron/templates/{ml2_conf_xenapi.ini.j2 => openvswitch_agent_xenapi.ini.j2} (98%)
 create mode 100644 releasenotes/notes/split_ovs_lb_agents_config-96f1f90e7d678209.yaml

diff --git a/ansible/roles/neutron/defaults/main.yml b/ansible/roles/neutron/defaults/main.yml
index a924b6d35..3ad700873 100644
--- a/ansible/roles/neutron/defaults/main.yml
+++ b/ansible/roles/neutron/defaults/main.yml
@@ -349,20 +349,37 @@ openstack_neutron_auth: "{{ openstack_auth }}"
 
 neutron_l3_agent_host_rp_filter_mode: 0
 
+####################
+# Mechanism drivers
+####################
+mechanism_drivers:
+  - name: "linuxbridge"
+    enabled: "{{ neutron_plugin_agent == 'linuxbridge' }}"
+  - name: "openvswitch"
+    enabled: "{{ neutron_plugin_agent == 'openvswitch' }}"
+  - name: "hyperv"
+    enabled: "{{ enable_hyperv | bool }}"
+  - name: "baremetal"
+    enabled: "{{ enable_ironic_neutron_agent | bool }}"
+  - name: "l2population"
+    enabled: "{{ not enable_hyperv | bool }}"
+
+neutron_mechanism_drivers: "{{ mechanism_drivers | selectattr('enabled', 'equalto', true) | list }}"
+
 ####################
 # Extension drivers
 ####################
 extension_drivers:
   - name: "qos"
-    enabled: "{{ enable_neutron_qos | bool }}"
+    enabled: "{{ enable_neutron_qos | bool and not enable_hyperv | bool }}"
   - name: "port_security"
     enabled: true
   - name: "dns"
-    enabled: "{{ enable_designate | bool }}"
+    enabled: "{{ enable_designate | bool and not enable_hyperv | bool }}"
   - name: "sfc"
-    enabled: "{{ enable_neutron_sfc | bool }}"
+    enabled: "{{ enable_neutron_sfc | bool and not enable_hyperv | bool }}"
 
-neutron_extension_drivers: "{{ extension_drivers|selectattr('enabled', 'equalto', true)|list }}"
+neutron_extension_drivers: "{{ extension_drivers | selectattr('enabled', 'equalto', true) | list }}"
 
 ####################
 # Neutron upgrade
@@ -395,7 +412,7 @@ service_plugins:
   - name: "port_forwarding"
     enabled: "{{ enable_neutron_port_forwarding | bool }}"
 
-neutron_service_plugins: "{{ service_plugins|selectattr('enabled', 'equalto', true)|list }}"
+neutron_service_plugins: "{{ service_plugins | selectattr('enabled', 'equalto', true) | list }}"
 
 ####################
 # Notification
@@ -486,7 +503,7 @@ infoblox_wapi_max_results: "-50000"
 ######################
 notification_drivers: []
 
-neutron_notification_drivers: "{{ notification_drivers|selectattr('enabled', 'equalto', true)|list }}"
+neutron_notification_drivers: "{{ notification_drivers | selectattr('enabled', 'equalto', true) | list }}"
 
 ####################
 # Kolla
diff --git a/ansible/roles/neutron/tasks/config-neutron-fake.yml b/ansible/roles/neutron/tasks/config-neutron-fake.yml
index 08f82c705..d39299016 100644
--- a/ansible/roles/neutron/tasks/config-neutron-fake.yml
+++ b/ansible/roles/neutron/tasks/config-neutron-fake.yml
@@ -42,16 +42,16 @@
   notify:
     - Restart fake neutron-openvswitch-agent container
 
-- name: Copying over ml2_conf.ini
+- name: Copying over openvswitch_agent.ini
   become: true
   vars:
     service_name: "{{ item }}"
   merge_configs:
     sources:
-      - "{{ role_path }}/templates/ml2_conf.ini.j2"
-      - "{{ node_custom_config }}/neutron/ml2_conf.ini"
-      - "{{ node_custom_config }}/neutron/{{ inventory_hostname }}/neutron.conf"
-    dest: "{{ node_config_directory }}/neutron-openvswitch-agent-fake-{{ item }}/ml2_conf.ini"
+      - "{{ role_path }}/templates/openvswitch_agent.ini.j2"
+      - "{{ node_custom_config }}/neutron/openvswitch_agent.ini"
+      - "{{ node_custom_config }}/neutron/{{ inventory_hostname }}/openvswitch_agent.ini"
+    dest: "{{ node_config_directory }}/neutron-openvswitch-agent-fake-{{ item }}/openvswitch_agent.ini"
     mode: "0660"
   with_sequence: start=1 end={{ num_nova_fake_per_node }}
   when:
diff --git a/ansible/roles/neutron/tasks/config.yml b/ansible/roles/neutron/tasks/config.yml
index f967f8bda..93837ce21 100644
--- a/ansible/roles/neutron/tasks/config.yml
+++ b/ansible/roles/neutron/tasks/config.yml
@@ -105,8 +105,6 @@
   vars:
     service_name: "{{ item.key }}"
     services_need_ml2_conf_ini:
-      - "neutron-linuxbridge-agent"
-      - "neutron-openvswitch-agent"
       - "neutron-infoblox-ipam-agent"
       - "neutron-server"
   merge_configs:
@@ -124,29 +122,67 @@
   notify:
     - "Restart {{ item.key }} container"
 
-- name: Copying over ml2_conf.ini for XenAPI
+- name: Copying over linuxbridge_agent.ini
   become: true
   vars:
-    service_name: "{{ item.key }}"
-    services_need_ml2_conf_ini:
-      - "neutron-openvswitch-agent-xenapi"
+    service_name: "neutron-linuxbridge-agent"
+  merge_configs:
+    sources:
+      - "{{ role_path }}/templates/linuxbridge_agent.ini.j2"
+      - "{{ node_custom_config }}/neutron/linuxbridge_agent.ini"
+      - "{{ node_custom_config }}/neutron/{{ inventory_hostname }}/linuxbridge_agent.ini"
+      # TODO(mnasiadka): Remove in V - left to not break existing deployments
+      - "{{ node_custom_config }}/neutron/ml2_conf.ini"
+      - "{{ node_custom_config }}/neutron/{{ inventory_hostname }}/ml2_conf.ini"
+    dest: "{{ node_config_directory }}/{{ service_name }}/linuxbridge_agent.ini"
+    mode: "0660"
+  when:
+    - neutron_services[service_name].enabled | bool
+    - neutron_services[service_name].host_in_groups | bool
+  notify:
+    - "Restart {{ service_name }} container"
+
+- name: Copying over openvswitch_agent.ini
+  become: true
+  vars:
+    service_name: "neutron-openvswitch-agent"
+  merge_configs:
+    sources:
+      - "{{ role_path }}/templates/openvswitch_agent.ini.j2"
+      - "{{ node_custom_config }}/neutron/openvswitch_agent.ini"
+      - "{{ node_custom_config }}/neutron/{{ inventory_hostname }}/openvswitch_agent.ini"
+      # TODO(mnasiadka): Remove in V - left to not break existing deployments
+      - "{{ node_custom_config }}/neutron/ml2_conf.ini"
+      - "{{ node_custom_config }}/neutron/{{ inventory_hostname }}/ml2_conf.ini"
+    dest: "{{ node_config_directory }}/{{ service_name }}/openvswitch_agent.ini"
+    mode: "0660"
+  when:
+    - neutron_services[service_name].enabled | bool
+    - neutron_services[service_name].host_in_groups | bool
+  notify:
+    - "Restart {{ service_name }} container"
+
+- name: Copying over openvswitch_agent.ini for XenAPI
+  become: true
+  vars:
+    service_name: "neutron-openvswitch-agent-xenapi"
     os_xenapi_variables: "{{ lookup('file', xenapi_facts_root + '/' + inventory_hostname + '/' + xenapi_facts_file) | from_json }}"
   merge_configs:
     sources:
-      - "{{ role_path }}/templates/ml2_conf.ini.j2"
-      - "{{ role_path }}/templates/ml2_conf_xenapi.ini.j2"
+      - "{{ role_path }}/templates/openvswitch_agent_xenapi.ini.j2"
+      - "{{ node_custom_config }}/neutron/openvswitch_agent_xenapi.ini"
+      - "{{ node_custom_config }}/neutron/{{ inventory_hostname }}/openvswitch_agent_xenapi.ini"
+      - "{{ node_custom_config }}/neutron/{{ service_name }}/openvswitch_agent_xenapi.ini"
+      # TODO(mnasiadka): Remove in V - left to not break existing deployments
       - "{{ node_custom_config }}/neutron/ml2_conf.ini"
       - "{{ node_custom_config }}/neutron/{{ inventory_hostname }}/ml2_conf.ini"
-      - "{{ node_custom_config }}/neutron/{{ service_name }}/ml2_conf.ini"
-    dest: "{{ node_config_directory }}/{{ service_name }}/ml2_conf.ini"
+    dest: "{{ node_config_directory }}/{{ service_name }}/openvswitch_agent.ini"
     mode: "0660"
   when:
-    - item.key in services_need_ml2_conf_ini
-    - item.value.enabled | bool
-    - item.value.host_in_groups | bool
-  with_dict: "{{ neutron_services }}"
+    - neutron_services[service_name].enabled | bool
+    - neutron_services[service_name].host_in_groups | bool
   notify:
-    - "Restart {{ item.key }} container"
+    - "Restart {{ service_name }} container"
 
 - name: Copying over sriov_agent.ini
   vars:
diff --git a/ansible/roles/neutron/templates/linuxbridge_agent.ini.j2 b/ansible/roles/neutron/templates/linuxbridge_agent.ini.j2
new file mode 100644
index 000000000..b95203736
--- /dev/null
+++ b/ansible/roles/neutron/templates/linuxbridge_agent.ini.j2
@@ -0,0 +1,15 @@
+[agent]
+{% if neutron_agent_extensions %}
+extensions = {{ neutron_agent_extensions|map(attribute='name')|join(',') }}
+{% endif %}
+
+[linux_bridge]
+physical_interface_mappings = physnet1:{{ neutron_external_interface }}
+
+[security_group]
+firewall_driver = neutron.agent.linux.iptables_firewall.IptablesFirewallDriver
+
+[vxlan]
+l2_population = true
+local_ip = {{ tunnel_interface_address }}
+arp_responder = true
diff --git a/ansible/roles/neutron/templates/ml2_conf.ini.j2 b/ansible/roles/neutron/templates/ml2_conf.ini.j2
index fbed49fbe..2ea2195c3 100644
--- a/ansible/roles/neutron/templates/ml2_conf.ini.j2
+++ b/ansible/roles/neutron/templates/ml2_conf.ini.j2
@@ -3,25 +3,14 @@
 # Changing type_drivers after bootstrap can lead to database inconsistencies
 type_drivers = {{ neutron_type_drivers }}
 tenant_network_types = {{ neutron_tenant_network_types }}
-
 {% if tunnel_address_family == 'ipv6' %}
 overlay_ip_version = 6
 {% endif %}
-
-{% if neutron_plugin_agent == "openvswitch" %}
-{% if enable_hyperv | bool %}
-mechanism_drivers = openvswitch,hyperv
-{% else %}
-mechanism_drivers = openvswitch,{% if enable_ironic_neutron_agent | bool %}baremetal,{% endif %}l2population
-{% endif %}
-{% elif neutron_plugin_agent == "linuxbridge" %}
-mechanism_drivers = linuxbridge,l2population
+{% if neutron_mechanism_drivers %}
+mechanism_drivers = {{ neutron_mechanism_drivers | map(attribute='name') | join(',') }}
 {% endif %}
-
 {% if neutron_extension_drivers %}
-extension_drivers = {{ neutron_extension_drivers|map(attribute='name')|join(',') }}
-{% elif enable_hyperv | bool %}
-extension_drivers = port_security
+extension_drivers = {{ neutron_extension_drivers | map(attribute='name') | join(',') }}
 {% endif %}
 
 [ml2_type_vlan]
@@ -40,49 +29,3 @@ flat_networks = {% for bridge in neutron_bridge_name.split(',') %}physnet{{ loop
 
 [ml2_type_vxlan]
 vni_ranges = 1:1000
-
-[securitygroup]
-{% if neutron_plugin_agent == "openvswitch" %}
-firewall_driver = neutron.agent.linux.iptables_firewall.OVSHybridIptablesFirewallDriver
-{% elif neutron_plugin_agent == "linuxbridge" %}
-firewall_driver = neutron.agent.linux.iptables_firewall.IptablesFirewallDriver
-{% endif %}
-
-{% if neutron_plugin_agent == "openvswitch" %}
-[agent]
-tunnel_types = vxlan
-{% if nova_compute_virt_type != 'xenapi' %}
-l2_population = true
-{% endif %}
-arp_responder = true
-
-{% if enable_neutron_dvr | bool %}
-enable_distributed_routing = True
-{% endif %}
-
-{% if neutron_agent_extensions %}
-extensions = {{ neutron_agent_extensions|map(attribute='name')|join(',') }}
-{% endif %}
-
-[ovs]
-{% if inventory_hostname in groups["network"] or (inventory_hostname in groups["compute"] and computes_need_external_bridge | bool ) %}
-bridge_mappings = {% for bridge in neutron_bridge_name.split(',') %}physnet{{ loop.index0 + 1 }}:{{ bridge }}{% if not loop.last %},{% endif %}{% endfor %}
-{% endif %}
-
-{# NOTE: newline above is required for correct config generation. Do not remove. #}
-datapath_type = {{ ovs_datapath }}
-ovsdb_connection = tcp:127.0.0.1:{{ ovsdb_port }}
-{% if enable_nova_fake | bool %}
-integration_bridge = br-int-{{ item }}
-{% endif %}
-{% elif neutron_plugin_agent == "linuxbridge" %}
-[linux_bridge]
-physical_interface_mappings = physnet1:{{ neutron_external_interface }}
-
-[vxlan]
-l2_population = true
-{% endif %}
-
-{% if inventory_hostname in groups["network"] or inventory_hostname in groups["compute"] %}
-local_ip = {{ tunnel_interface_address }}
-{% endif %}
diff --git a/ansible/roles/neutron/templates/neutron-linuxbridge-agent.json.j2 b/ansible/roles/neutron/templates/neutron-linuxbridge-agent.json.j2
index 759a72256..cf82cc602 100644
--- a/ansible/roles/neutron/templates/neutron-linuxbridge-agent.json.j2
+++ b/ansible/roles/neutron/templates/neutron-linuxbridge-agent.json.j2
@@ -1,5 +1,5 @@
 {
-    "command": "neutron-linuxbridge-agent --config-file /etc/neutron/neutron.conf --config-file /etc/neutron/plugins/ml2/ml2_conf.ini",
+    "command": "neutron-linuxbridge-agent --config-file /etc/neutron/neutron.conf --config-file /etc/neutron/plugins/ml2/linuxbridge_agent.ini",
     "config_files": [
         {
             "source": "{{ container_config_directory }}/neutron.conf",
@@ -8,8 +8,8 @@
             "perm": "0600"
         },
         {
-            "source": "{{ container_config_directory }}/ml2_conf.ini",
-            "dest": "/etc/neutron/plugins/ml2/ml2_conf.ini",
+            "source": "{{ container_config_directory }}/linuxbridge_agent.ini",
+            "dest": "/etc/neutron/plugins/ml2/linuxbridge_agent.ini",
             "owner": "neutron",
             "perm": "0600"
         },
diff --git a/ansible/roles/neutron/templates/neutron-openvswitch-agent-xenapi.json.j2 b/ansible/roles/neutron/templates/neutron-openvswitch-agent-xenapi.json.j2
index e5dfd784c..83be24eb7 100644
--- a/ansible/roles/neutron/templates/neutron-openvswitch-agent-xenapi.json.j2
+++ b/ansible/roles/neutron/templates/neutron-openvswitch-agent-xenapi.json.j2
@@ -1,5 +1,5 @@
 {
-    "command": "neutron-openvswitch-agent --config-file /etc/neutron/neutron.conf --config-file /etc/neutron/plugins/ml2/ml2_conf.ini",
+    "command": "neutron-openvswitch-agent --config-file /etc/neutron/neutron.conf --config-file /etc/neutron/plugins/ml2/openvswitch_agent.ini",
     "config_files": [
         {
             "source": "{{ container_config_directory }}/neutron.conf",
@@ -8,8 +8,8 @@
             "perm": "0600"
         },
         {
-            "source": "{{ container_config_directory }}/ml2_conf.ini",
-            "dest": "/etc/neutron/plugins/ml2/ml2_conf.ini",
+            "source": "{{ container_config_directory }}/openvswitch_agent.ini",
+            "dest": "/etc/neutron/plugins/ml2/openvswitch_agent.ini",
             "owner": "neutron",
             "perm": "0600"
         },
diff --git a/ansible/roles/neutron/templates/neutron-openvswitch-agent.json.j2 b/ansible/roles/neutron/templates/neutron-openvswitch-agent.json.j2
index 8048c7690..b59632bc6 100644
--- a/ansible/roles/neutron/templates/neutron-openvswitch-agent.json.j2
+++ b/ansible/roles/neutron/templates/neutron-openvswitch-agent.json.j2
@@ -1,5 +1,5 @@
 {
-    "command": "neutron-openvswitch-agent --config-file /etc/neutron/neutron.conf --config-file /etc/neutron/plugins/ml2/ml2_conf.ini",
+    "command": "neutron-openvswitch-agent --config-file /etc/neutron/neutron.conf --config-file /etc/neutron/plugins/ml2/openvswitch_agent.ini",
     "config_files": [
         {
             "source": "{{ container_config_directory }}/neutron.conf",
@@ -8,8 +8,8 @@
             "perm": "0600"
         },
         {
-            "source": "{{ container_config_directory }}/ml2_conf.ini",
-            "dest": "/etc/neutron/plugins/ml2/ml2_conf.ini",
+            "source": "{{ container_config_directory }}/openvswitch_agent.ini",
+            "dest": "/etc/neutron/plugins/ml2/openvswitch_agent.ini",
             "owner": "neutron",
             "perm": "0600"
         },
diff --git a/ansible/roles/neutron/templates/openvswitch_agent.ini.j2 b/ansible/roles/neutron/templates/openvswitch_agent.ini.j2
new file mode 100644
index 000000000..23b6dc885
--- /dev/null
+++ b/ansible/roles/neutron/templates/openvswitch_agent.ini.j2
@@ -0,0 +1,25 @@
+#jinja2: trim_blocks: False
+[agent]
+tunnel_types = vxlan
+l2_population = true
+arp_responder = true
+{% if enable_neutron_dvr | bool %}
+enable_distributed_routing = True
+{% endif %}
+{% if neutron_agent_extensions %}
+extensions = {{ neutron_agent_extensions|map(attribute='name')|join(',') }}
+{% endif %}
+
+[securitygroup]
+firewall_driver = neutron.agent.linux.iptables_firewall.OVSHybridIptablesFirewallDriver
+
+[ovs]
+{% if inventory_hostname in groups["network"] or (inventory_hostname in groups["compute"] and computes_need_external_bridge | bool ) %}
+bridge_mappings = {% for bridge in neutron_bridge_name.split(',') %}physnet{{ loop.index0 + 1 }}:{{ bridge }}{% if not loop.last %},{% endif %}{% endfor %}
+{% endif %}
+datapath_type = {{ ovs_datapath }}
+ovsdb_connection = tcp:127.0.0.1:{{ ovsdb_port }}
+local_ip = {{ tunnel_interface_address }}
+{% if enable_nova_fake | bool %}
+integration_bridge = br-int-{{ item }}
+{% endif %}
diff --git a/ansible/roles/neutron/templates/ml2_conf_xenapi.ini.j2 b/ansible/roles/neutron/templates/openvswitch_agent_xenapi.ini.j2
similarity index 98%
rename from ansible/roles/neutron/templates/ml2_conf_xenapi.ini.j2
rename to ansible/roles/neutron/templates/openvswitch_agent_xenapi.ini.j2
index 51e822bdb..9c19e3fe6 100644
--- a/ansible/roles/neutron/templates/ml2_conf_xenapi.ini.j2
+++ b/ansible/roles/neutron/templates/openvswitch_agent_xenapi.ini.j2
@@ -1,4 +1,3 @@
-# ml2_conf.ini
 [DEFAULT]
 # Use service_name as the log file name for neutron-openvswitch-agent-xenapi,
 # so that it will use a different log file from neutron-openvswitch-agent.
diff --git a/doc/source/reference/networking/neutron.rst b/doc/source/reference/networking/neutron.rst
index ac78c2a1d..20af193ec 100644
--- a/doc/source/reference/networking/neutron.rst
+++ b/doc/source/reference/networking/neutron.rst
@@ -50,7 +50,7 @@ When using Open vSwitch on a compatible kernel (4.3+ upstream, consult the
 documentation of your distribution for support details), you can switch
 to using the native OVS firewall driver by employing a configuration override
 (see :ref:`service-config`). You can set it in
-``/etc/kolla/config/neutron/ml2_conf.ini``:
+``/etc/kolla/config/neutron/openvswitch_agent.ini``:
 
 .. code-block:: ini
 
diff --git a/releasenotes/notes/split_ovs_lb_agents_config-96f1f90e7d678209.yaml b/releasenotes/notes/split_ovs_lb_agents_config-96f1f90e7d678209.yaml
new file mode 100644
index 000000000..3bc5fa0a8
--- /dev/null
+++ b/releasenotes/notes/split_ovs_lb_agents_config-96f1f90e7d678209.yaml
@@ -0,0 +1,14 @@
+---
+upgrade:
+  - |
+    Neutron Linux bridge and Open vSwitch Agents config has been split out into
+    linuxbridge_agent.ini and openvswitch_agent.ini respectively.
+    Please move your custom service config from ml2_conf.ini into those files.
+deprecations:
+  - |
+    Customizing Neutron Linux bridge and Open vSwitch Agents config via ml2_conf.ini
+    is deprecated. The config has been split out for these agents into
+    linuxbridge_agent.ini and openvswitch_agent.ini respectively.
+    In this release (Ussuri) custom service config ml2_conf.ini overrides will still
+    be used when merging configs - but that functionality will be removed
+    in V release.
-- 
GitLab