Configure OVS mechanism agent configs in its config file

Configurations for the OVS agent should go in its config file:
/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini
but as packaging is broken and different in Ubuntu and Redhat
we have to use new neutron_agent_ovs provider to configure
ovs agent correctly.

Therefore, we should not remove the agent config file and
replace it with a symlink to plugin.ini or ml2_conf.ini.

Those config files are meant to be used by the core plugin itself
in neutron-server, not the agents themselves.

- Remove symlink creation from ovs_neutron_plugin.ini to plugin.ini
- Use neutron_agent_ovs to configure OVS mechanism agent configs

Change-Id: I53d9b923784587e8a2a934f004a3b054c716daaa
Signed-off-by: Gael Chamoulaud <gchamoul@redhat.com>

lib/puppet/provider/neutron_agent_ovs/ini_setting.rb

@@ -0,0 +1,26 @@
+Puppet::Type.type(:neutron_agent_ovs).provide(
+  :ini_setting,
+  :parent => Puppet::Type.type(:ini_setting).provider(:ruby)
+) do
+
+  def section
+    resource[:name].split('/', 2).first
+  end
+
+  def setting
+    resource[:name].split('/', 2).last
+  end
+
+  def separator
+    '='
+  end
+
+  def file_path
+    if Facter['operatingsystem'].value == 'Ubuntu'
+      '/etc/neutron/plugins/ml2/ml2_conf.ini'
+    else
+      '/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini'
+    end
+  end
+
+end

lib/puppet/type/neutron_agent_ovs.rb

@@ -0,0 +1,18 @@
+Puppet::Type.newtype(:neutron_agent_ovs) do
+
+  ensurable
+
+  newparam(:name, :namevar => true) do
+    desc 'Section/setting name to manage from ovs agent config.'
+    newvalues(/\S+\/\S+/)
+  end
+
+  newproperty(:value) do
+    desc 'The value of the setting to be defined.'
+    munge do |value|
+      value = value.to_s.strip
+      value.capitalize! if value =~ /^(true|false)$/i
+      value
+    end
+  end
+end

manifests/agents/ml2/ovs.pp

@@ -117,7 +117,8 @@ class neutron::agents::ml2::ovs (
     fail('L2 population must be enabled when DVR is enabled')
   }
 
-  Neutron_plugin_ml2<||> ~> Service['neutron-ovs-agent-service']
+  Package['neutron-ovs-agent'] -> Neutron_agent_ovs<||>
+  Neutron_agent_ovs<||> ~> Service['neutron-ovs-agent-service']
 
   if ($bridge_mappings != []) {
     # bridge_mappings are used to describe external networks that are
@@ -135,7 +136,7 @@ class neutron::agents::ml2::ovs (
     # Set config for bridges that we're going to create
     # The OVS neutron plugin will talk in terms of the networks in the bridge_mappings
     $br_map_str = join($bridge_mappings, ',')
-    neutron_plugin_ml2 {
+    neutron_agent_ovs {
       'ovs/bridge_mappings': value => $br_map_str;
     }
     neutron::plugins::ovs::bridge{ $bridge_mappings:
@@ -146,7 +147,7 @@ class neutron::agents::ml2::ovs (
     }
   }
 
-  neutron_plugin_ml2 {
+  neutron_agent_ovs {
     'agent/polling_interval':           value => $polling_interval;
     'agent/l2_population':              value => $l2_population;
     'agent/arp_responder':              value => $arp_responder;
@@ -154,12 +155,10 @@ class neutron::agents::ml2::ovs (
     'ovs/integration_bridge':           value => $integration_bridge;
   }
 
-  if ($firewall_driver) {
+  if $firewall_driver {
-    neutron_plugin_ml2 { 'securitygroup/firewall_driver':
+    neutron_agent_ovs { 'securitygroup/firewall_driver': value => $firewall_driver }
-      value => $firewall_driver
-    }
   } else {
-    neutron_plugin_ml2 { 'securitygroup/firewall_driver': ensure => absent }
+    neutron_agent_ovs { 'securitygroup/firewall_driver': ensure => absent }
   }
 
   vs_bridge { $integration_bridge:
@@ -172,25 +171,25 @@ class neutron::agents::ml2::ovs (
       ensure => present,
       before => Service['neutron-ovs-agent-service'],
     }
-    neutron_plugin_ml2 {
+    neutron_agent_ovs {
       'ovs/enable_tunneling': value => true;
       'ovs/tunnel_bridge':    value => $tunnel_bridge;
       'ovs/local_ip':         value => $local_ip;
     }
 
     if size($tunnel_types) > 0 {
-      neutron_plugin_ml2 {
+      neutron_agent_ovs {
         'agent/tunnel_types': value => join($tunnel_types, ',');
       }
     }
     if 'vxlan' in $tunnel_types {
       validate_vxlan_udp_port($vxlan_udp_port)
-      neutron_plugin_ml2 {
+      neutron_agent_ovs {
         'agent/vxlan_udp_port': value => $vxlan_udp_port;
       }
     }
   } else {
-    neutron_plugin_ml2 {
+    neutron_agent_ovs {
       'ovs/enable_tunneling': value  => false;
       'ovs/tunnel_bridge':    ensure => absent;
       'ovs/local_ip':         ensure => absent;
@@ -199,7 +198,6 @@ class neutron::agents::ml2::ovs (
 
 
   if $::neutron::params::ovs_agent_package {
-    Package['neutron-ovs-agent'] -> Neutron_plugin_ml2<||>
     package { 'neutron-ovs-agent':
       ensure => $package_ensure,
       name   => $::neutron::params::ovs_agent_package,
@@ -209,22 +207,12 @@ class neutron::agents::ml2::ovs (
     # Some platforms (RedHat) do not provide a separate
     # neutron plugin ovs agent package. The configuration file for
     # the ovs agent is provided by the neutron ovs plugin package.
-    Package['neutron-ovs-agent'] -> Neutron_plugin_ml2<||>
-    Package['neutron-ovs-agent'] -> Service['ovs-cleanup-service']
-
     if ! defined(Package['neutron-ovs-agent']) {
       package { 'neutron-ovs-agent':
         ensure => $package_ensure,
         name   => $::neutron::params::ovs_server_package,
         tag    => 'openstack',
-      } ->
+      }
-      # https://bugzilla.redhat.com/show_bug.cgi?id=1087647
-      # Causes init script for agent to load the old ovs file
-      # instead of the ml2 config file.
-      file { '/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini':
-        ensure => link,
-        target => '/etc/neutron/plugins/ml2/ml2_conf.ini'
-      } ~> Service<| title == 'neutron-ovs-agent-service' |>
     }
   }
 

spec/classes/neutron_agents_ml2_ovs_spec.rb

@@ -34,15 +34,15 @@ describe 'neutron::agents::ml2::ovs' do
     it { is_expected.to contain_class('neutron::params') }
 
     it 'configures ovs_neutron_plugin.ini' do
-      is_expected.to contain_neutron_plugin_ml2('agent/polling_interval').with_value(p[:polling_interval])
+      is_expected.to contain_neutron_agent_ovs('agent/polling_interval').with_value(p[:polling_interval])
-      is_expected.to contain_neutron_plugin_ml2('agent/l2_population').with_value(p[:l2_population])
+      is_expected.to contain_neutron_agent_ovs('agent/l2_population').with_value(p[:l2_population])
-      is_expected.to contain_neutron_plugin_ml2('agent/arp_responder').with_value(p[:arp_responder])
+      is_expected.to contain_neutron_agent_ovs('agent/arp_responder').with_value(p[:arp_responder])
-      is_expected.to contain_neutron_plugin_ml2('ovs/integration_bridge').with_value(p[:integration_bridge])
+      is_expected.to contain_neutron_agent_ovs('ovs/integration_bridge').with_value(p[:integration_bridge])
-      is_expected.to contain_neutron_plugin_ml2('securitygroup/firewall_driver').\
+      is_expected.to contain_neutron_agent_ovs('securitygroup/firewall_driver').\
         with_value(p[:firewall_driver])
-      is_expected.to contain_neutron_plugin_ml2('ovs/enable_tunneling').with_value(false)
+      is_expected.to contain_neutron_agent_ovs('ovs/enable_tunneling').with_value(false)
-      is_expected.to contain_neutron_plugin_ml2('ovs/tunnel_bridge').with_ensure('absent')
+      is_expected.to contain_neutron_agent_ovs('ovs/tunnel_bridge').with_ensure('absent')
-      is_expected.to contain_neutron_plugin_ml2('ovs/local_ip').with_ensure('absent')
+      is_expected.to contain_neutron_agent_ovs('ovs/local_ip').with_ensure('absent')
     end
 
     it 'configures vs_bridge' do
@@ -60,7 +60,7 @@ describe 'neutron::agents::ml2::ovs' do
           :ensure => p[:package_ensure],
           :tag    => 'openstack'
         )
-        is_expected.to contain_package('neutron-ovs-agent').with_before(/Neutron_plugin_ml2\[.+\]/)
+        is_expected.to contain_package('neutron-ovs-agent').with_before(/Neutron_agent_ovs\[.+\]/)
       else
       end
     end
@@ -79,7 +79,7 @@ describe 'neutron::agents::ml2::ovs' do
         params.merge!(:firewall_driver => false)
       end
       it 'should configure firewall driver' do
-        is_expected.to contain_neutron_plugin_ml2('securitygroup/firewall_driver').with_ensure('absent')
+        is_expected.to contain_neutron_agent_ovs('securitygroup/firewall_driver').with_ensure('absent')
       end
     end
 
@@ -88,7 +88,7 @@ describe 'neutron::agents::ml2::ovs' do
         params.merge!(:arp_responder => true)
       end
       it 'should enable ARP responder' do
-        is_expected.to contain_neutron_plugin_ml2('agent/arp_responder').with_value(true)
+        is_expected.to contain_neutron_agent_ovs('agent/arp_responder').with_value(true)
       end
     end
 
@@ -98,7 +98,7 @@ describe 'neutron::agents::ml2::ovs' do
                       :l2_population              => true )
       end
       it 'should enable DVR' do
-        is_expected.to contain_neutron_plugin_ml2('agent/enable_distributed_routing').with_value(true)
+        is_expected.to contain_neutron_agent_ovs('agent/enable_distributed_routing').with_value(true)
       end
     end
 
@@ -108,7 +108,7 @@ describe 'neutron::agents::ml2::ovs' do
       end
 
       it 'configures bridge mappings' do
-        is_expected.to contain_neutron_plugin_ml2('ovs/bridge_mappings')
+        is_expected.to contain_neutron_agent_ovs('ovs/bridge_mappings')
       end
 
       it 'should configure bridge mappings' do
@@ -137,9 +137,9 @@ describe 'neutron::agents::ml2::ovs' do
           params.merge!(:enable_tunneling => true, :local_ip => '127.0.0.1' )
         end
         it 'should configure ovs for tunneling' do
-          is_expected.to contain_neutron_plugin_ml2('ovs/enable_tunneling').with_value(true)
+          is_expected.to contain_neutron_agent_ovs('ovs/enable_tunneling').with_value(true)
-          is_expected.to contain_neutron_plugin_ml2('ovs/tunnel_bridge').with_value(default_params[:tunnel_bridge])
+          is_expected.to contain_neutron_agent_ovs('ovs/tunnel_bridge').with_value(default_params[:tunnel_bridge])
-          is_expected.to contain_neutron_plugin_ml2('ovs/local_ip').with_value('127.0.0.1')
+          is_expected.to contain_neutron_agent_ovs('ovs/local_ip').with_value('127.0.0.1')
           is_expected.to contain_vs_bridge(default_params[:tunnel_bridge]).with(
             :ensure  => 'present',
             :before => 'Service[neutron-ovs-agent-service]'
@@ -156,8 +156,8 @@ describe 'neutron::agents::ml2::ovs' do
         end
 
         it 'should perform vxlan network configuration' do
-          is_expected.to contain_neutron_plugin_ml2('agent/tunnel_types').with_value(params[:tunnel_types])
+          is_expected.to contain_neutron_agent_ovs('agent/tunnel_types').with_value(params[:tunnel_types])
-          is_expected.to contain_neutron_plugin_ml2('agent/vxlan_udp_port').with_value(params[:vxlan_udp_port])
+          is_expected.to contain_neutron_agent_ovs('agent/vxlan_udp_port').with_value(params[:vxlan_udp_port])
         end
       end
 
@@ -204,12 +204,5 @@ describe 'neutron::agents::ml2::ovs' do
       )
       is_expected.to contain_package('neutron-ovs-agent').with_before(/Service\[ovs-cleanup-service\]/)
     end
-
-    it 'links from ovs config to plugin config' do
-      is_expected.to contain_file('/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini').with(
-        :ensure => 'link',
-        :target => '/etc/neutron/plugins/ml2/ml2_conf.ini'
-      )
-    end
   end
 end

spec/unit/provider/neutron_agent_ovs/ini_setting_spec.rb

@@ -0,0 +1,45 @@
+$LOAD_PATH.push(
+  File.join(
+    File.dirname(__FILE__),
+    '..',
+    '..',
+    '..',
+    'fixtures',
+    'modules',
+    'inifile',
+    'lib')
+)
+
+require 'spec_helper'
+
+provider_class = Puppet::Type.type(:neutron_agent_ovs).provider(:ini_setting)
+describe provider_class do
+  let(:resource ) do
+    Puppet::Type::Neutron_agent_ovs.new({
+      :name => 'DEFAULT/foo',
+      :value => 'bar',
+    })
+  end
+
+  let (:provider) { resource.provider }
+
+  [ 'RedHat', 'Debian' ].each do |os|
+    context "on #{os} with default setting" do
+      it 'it should fall back to default and use ovs_neutron_plugin.ini' do
+        Facter.fact(:operatingsystem).stubs(:value).returns("#{os}")
+        expect(provider.section).to eq('DEFAULT')
+        expect(provider.setting).to eq('foo')
+        expect(provider.file_path).to eq('/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini')
+      end
+    end
+  end
+
+  context 'on Ubuntu with default setting' do
+    it 'it should fall back to default and use ml2_conf.ini' do
+      Facter.fact(:operatingsystem).stubs(:value).returns('Ubuntu')
+      expect(provider.section).to eq('DEFAULT')
+      expect(provider.setting).to eq('foo')
+      expect(provider.file_path).to eq('/etc/neutron/plugins/ml2/ml2_conf.ini')
+    end
+  end
+end