Consolidate service_providers into neutron server

The configuration of vpnaas and lbaas services created
additional files that were never included in the service
startup. The lbaas service config provider was updated
to put service providers directly into neutron.conf
but now if we update vpnaas to do the same we will fail
being idempotent since both providers will overwrite
each other. This change moves neutron_config provider from
ini_setting to openstackconfig to handle the multi-value
of service_provider and adds a variable to neutron::server
to manage service providers.

Closes-Bug: 1578912
Change-Id: I69b7635984fe74038db2025b89f638def5029849

lib/puppet/provider/neutron_config/openstackconfig.rb

@@ -1,6 +1,6 @@
 Puppet::Type.type(:neutron_config).provide(
-  :ini_setting,
-  :parent => Puppet::Type.type(:openstack_config).provider(:ini_setting)
+  :openstackconfig,
+  :parent => Puppet::Type.type(:openstack_config).provider(:ruby)
 ) do
 
   def self.file_path

lib/puppet/type/neutron_config.rb

@@ -7,14 +7,25 @@ Puppet::Type.newtype(:neutron_config) do
     newvalues(/\S+\/\S+/)
   end
 
-  newproperty(:value) do
+  newproperty(:value, :array_matching => :all) do
     desc 'The value of the setting to be defined.'
+    def insync?(is)
+      return true if @should.empty?
+      return false unless is.is_a? Array
+      return false unless is.length == @should.length
+      # we don't care about the order of items in array, hence
+      # it is necessary to override insync
+      return (
+        is & @should == is or
+        is & @should.map(&:to_s) == is
+      )
+    end
+
     munge do |value|
       value = value.to_s.strip
       value.capitalize! if value =~ /^(true|false)$/i
       value
     end
-    newvalues(/^[\S ]*$/)
 
     def is_to_s( currentvalue )
       if resource.secret?

manifests/server.pp

@@ -244,6 +244,22 @@
 #   RedHat platforms won't take care of this parameter
 #   true/false
 #   Defaults to false
+#
+# [*service_providers*]
+#   (optional) (Array) Configures the service providers for neutron server.
+#   This needs to be set for lbaas, vpnaas, and fwaas.
+#   Defaults to $::os_service_default
+#
+#   Example:
+#
+#   class { 'neutron::server':
+#     service_providers => [
+#        'LOADBALANCERV2:Octavia:neutron_lbaas.drivers.octavia.driver.OctaviaDriver:default',
+#        'LOADBALANCER:Haproxy:neutron_lbaas.services.loadbalancer.drivers.haproxy.plugin_driver.HaproxyOnHostPluginDriver',
+#        'VPN:openswan:neutron_vpnaas.services.vpn.service_drivers.ipsec.IPsecVPNDriver:default'
+#     ]
+#   }
+#
 # === Deprecated Parameters
 #
 # [*identity_uri*]
@@ -327,6 +343,7 @@ class neutron::server (
   $ensure_vpnaas_package            = false,
   $ensure_fwaas_package             = false,
   $vpnaas_agent_package             = false,
+  $service_providers                = $::os_service_default,
   # DEPRECATED PARAMETERS
   $log_dir                          = undef,
   $log_file                         = undef,
@@ -356,6 +373,10 @@ class neutron::server (
     validate_re($dhcp_load_type, ['^networks$', '^subnets$', '^ports$'], 'Must pass either networks, subnets, or ports as values for dhcp_load_type')
   }
 
+  if !is_service_default($service_providers) {
+    validate_array($service_providers)
+  }
+
   if $ensure_fwaas_package {
     if ($::osfamily == 'Debian') {
       # Debian platforms
@@ -433,6 +454,7 @@ class neutron::server (
     'DEFAULT/network_scheduler_driver':         value => $network_scheduler_driver;
     'DEFAULT/dhcp_load_type':                   value => $dhcp_load_type;
     'DEFAULT/default_availability_zones':       value => join(any2array($default_availability_zones), ',');
+    'service_providers/service_provider':       value => $service_providers;
   }
 
   if $state_path {

manifests/services/lbaas.pp

@@ -15,6 +15,9 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 #
+# == DEPRECATED
+# This class has been deprecated in favor of using the parameter in
+# neutron::server::service_provider
 #
 # == Class: neutron::services::lbaas
 #
@@ -26,6 +29,8 @@
 #   (optional) Whether to install the lbaas driver package
 #   Defaults to 'present'
 #
+# === Deprecated Parameters
+#
 # [*service_providers*]
 #   (optional) Array of allowed service types or '<SERVICE DEFAULT>'.
 #   Note: The default upstream value is empty.
@@ -35,8 +40,6 @@
 #   Must be in form <service_type>:<name>:<driver>[:default].
 #   Defaults to $::os_service_default
 #
-# === Deprecated Parameters
-#
 # [*package_ensure*]
 #   (optional) Deprecated. Used to install the lbaas v2 agent. This was moved into
 #   neutron::agents::lbaas as the lbaas services handles scheduling of new load balancers
@@ -57,6 +60,10 @@ class neutron::services::lbaas (
       tag    => ['openstack', 'neutron-package']
     })
   }
+  if !is_service_default($service_providers) {
+    warning('service_providers in neutron::services::lbaas is deprecated in newton release, please use service provider in neutron::server class')
+  }
+
   if $package_ensure {
     warning('Package ensure is deprecated. The neutron::agents::lbaas class should be used to install the agent')
     # agent package contains both agent and service resources

manifests/services/vpnaas.pp

@@ -15,6 +15,9 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 #
+# == DEPRECATED
+# This class has been deprecated in favor of using the parameter in
+# neutron::server::service_provider
 #
 # == Class: neutron::services::vpnaas
 #
@@ -41,6 +44,9 @@ class neutron::services::vpnaas (
 ) {
 
   include ::neutron::params
+  if !is_service_default($service_providers) {
+    warning('service_providers in neutron::services::vpnaas is deprecated in newton release, please use service provider in neutron::server class')
+  }
 
   # agent package contains both agent and service resources
   ensure_resource( 'package', 'neutron-vpnaas-agent', {

releasenotes/notes/consolidate_service_provider_configuration-3ce48947a396ad25.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - service_providers are configured through neutron::server class.
+issues:
+  - VPNaaS service_provider was being configured in a file not being loaded up
+    by neutron server. It has been consolidated with lbaas service configuration
+    into neutron.conf.

spec/acceptance/basic_neutron_spec.rb

@@ -52,6 +52,9 @@ describe 'basic neutron' do
         auth_password       => 'a_big_secret',
         identity_uri        => 'http://127.0.0.1:35357/',
         sync_db             => true,
+        service_providers   => [
+          'LOADBALANCER:Haproxy:neutron_lbaas.services.loadbalancer.drivers.haproxy.plugin_driver.HaproxyOnHostPluginDriver:default',
+        ],
       }
       class { '::neutron::client': }
       class { '::neutron::quota': }
@@ -72,9 +75,6 @@ describe 'basic neutron' do
         mechanism_drivers    => ['openvswitch', 'sriovnicswitch']
       }
       class { '::neutron::agents::ml2::sriov': }
-      class { '::neutron::services::lbaas':
-        service_providers => 'LOADBALANCER:Haproxy:neutron_lbaas.services.loadbalancer.drivers.haproxy.plugin_driver.HaproxyOnHostPluginDriver:default',
-      }
       class { '::neutron::services::lbaas::haproxy': }
       class { '::neutron::services::lbaas::octavia': }
       EOS

spec/acceptance/neutron_config_spec.rb

@@ -525,28 +525,6 @@ describe 'basic neutron_config resource' do
         ensure_absent_val => 'toto',
       }
 
-      neutron_lbaas_service_config { 'DEFAULT/thisshouldexist' :
-        value => 'foo',
-      }
-
-      neutron_lbaas_service_config { 'DEFAULT/thisshouldexist2' :
-        value             => '<SERVICE DEFAULT>',
-        ensure_absent_val => 'toto',
-      }
-
-      neutron_lbaas_service_config { 'DEFAULT/thisshouldexist3' :
-        value => ['value1', 'value2'],
-      }
-
-      neutron_lbaas_service_config { 'DEFAULT/thisshouldnotexist' :
-        value => '<SERVICE DEFAULT>',
-      }
-
-      neutron_lbaas_service_config { 'DEFAULT/thisshouldnotexist2' :
-        value             => 'toto',
-        ensure_absent_val => 'toto',
-      }
-
       EOS
 
 
@@ -590,11 +568,5 @@ describe 'basic neutron_config resource' do
         end
       end
     end
-
-    describe file('/etc/neutron/neutron.conf') do
-      it { is_expected.to contain('thisshouldexist3=value1') }
-      it { is_expected.to contain('thisshouldexist3=value2') }
-    end
-
   end
 end

spec/classes/neutron_server_spec.rb

@@ -212,6 +212,20 @@ describe 'neutron::server' do
       it_raises 'a Puppet::Error', /Must pass either networks, subnets, or ports as values for dhcp_load_type/
     end
 
+    context 'with multiple service providers' do
+      before :each do
+        params.merge!(
+          { :service_providers => ['provider1', 'provider2'] }
+        )
+      end
+
+      it 'configures neutron.conf' do
+        is_expected.to contain_neutron_config(
+          'service_providers/service_provider'
+        ).with_value(['provider1', 'provider2'])
+      end
+    end
+
     context 'with availability zone hints set' do
       before :each do
         params.merge!(:dhcp_load_type             => 'networks',

spec/unit/type/neutron_config_spec.rb

@@ -31,12 +31,12 @@ describe 'Puppet::Type.type(:neutron_config)' do
 
   it 'should accept a valid value' do
     @neutron_config[:value] = 'bar'
-    expect(@neutron_config[:value]).to eq('bar')
+    expect(@neutron_config[:value]).to eq(['bar'])
   end
 
   it 'should not accept a value with whitespace' do
     @neutron_config[:value] = 'b ar'
-    expect(@neutron_config[:value]).to eq('b ar')
+    expect(@neutron_config[:value]).to eq(['b ar'])
   end
 
   it 'should accept valid ensure values' do