Deprecate service_provider

Overriding service_provider was an hack and should not exist in any
module.
Puppet is by itself able to find which Service provider to use.
If you want to override it for any reason, please use a Puppet resource
collector, using keystone-service resource tag.

This patch deprecates the service_provider parameter and drop its usage,
so puppet-keystone can easily work on more systems, (ie: Ubuntu Xenial
with Systemd).

Change-Id: I661319aa83676880a83f3ecfc00e9a803524c7cf

manifests/init.pp

@@ -390,13 +390,6 @@
 #   (optional) The url to validate keystone against
 #   Defaults to undef
 #
-# [*service_provider*]
-#   (optional) Provider, that can be used for keystone service.
-#   Default value defined in keystone::params for given operation system.
-#   If you use Pacemaker or another Cluster Resource Manager, you can make
-#   custom service provider for changing start/stop/status behavior of service,
-#   and set it here.
-#
 # [*service_name*]
 #   (optional) Name of the service that will be providing the
 #   server functionality of keystone.  For example, the default
@@ -522,6 +515,11 @@
 #   (optional) Specify the keystone system group to be used with keystone-manage.
 #   Defaults to 'keystone'
 #
+# DEPRECATED PARAMETERS
+#
+# [*service_provider*]
+#   (optional) DEPRECATED. Provider, that can be used for keystone service.
+#
 # == Dependencies
 #  None
 #
@@ -632,7 +630,6 @@ class keystone(
   $validate_auth_url                    = false,
   $validate_cacert                      = undef,
   $paste_config                         = $::os_service_default,
-  $service_provider                     = $::keystone::params::service_provider,
   $service_name                         = $::keystone::params::service_name,
   $max_token_size                       = $::os_service_default,
   $sync_db                              = true,
@@ -654,11 +651,16 @@ class keystone(
   # DEPRECATED PARAMETERS
   $admin_workers                        = max($::processorcount, 2),
   $public_workers                       = max($::processorcount, 2),
+  $service_provider                     = undef,
 ) inherits keystone::params {
 
   include ::keystone::deps
   include ::keystone::logging
 
+  if $service_provider {
+    warning('service_provider is deprecated, does nothing and will be removed in a future release, use a Puppet resource collector if you want to modify the service provider.')
+  }
+
   if ! $catalog_driver {
     validate_re($catalog_type, 'template|sql')
   }
@@ -892,7 +894,6 @@ class keystone(
         enable         => $enabled,
         hasstatus      => true,
         hasrestart     => true,
-        provider       => $service_provider,
         validate       => true,
         admin_endpoint => $v_auth_url,
         admin_token    => $admin_token,
@@ -906,7 +907,6 @@ class keystone(
         enable       => $enabled,
         hasstatus    => true,
         hasrestart   => true,
-        provider     => $service_provider,
         validate     => false,
       }
     }
@@ -917,7 +917,6 @@ class keystone(
       ensure       => 'stopped',
       service_name => $::keystone::params::service_name,
       enable       => false,
-      provider     => $service_provider,
       validate     => false,
     }
     $service_name_real = $::apache::params::service_name

manifests/params.pp

@@ -13,23 +13,17 @@ class keystone::params {
       $keystone_wsgi_script_source  = '/usr/share/keystone/wsgi.py'
       $python_memcache_package_name = 'python-memcache'
       $mellon_package_name          = 'libapache2-mod-auth-mellon'
-      case $::operatingsystem {
-        'Debian': {
-          $service_provider            = undef
-        }
-        default: {
-          $service_provider            = 'upstart'
-        }
-      }
     }
     'RedHat': {
       $package_name                 = 'openstack-keystone'
       $service_name                 = 'openstack-keystone'
       $keystone_wsgi_script_path    = '/var/www/cgi-bin/keystone'
       $python_memcache_package_name = 'python-memcached'
-      $service_provider             = undef
       $keystone_wsgi_script_source  = '/usr/share/keystone/keystone.wsgi'
       $mellon_package_name          = 'mod_auth_mellon'
     }
+    default: {
+      fail("Unsupported osfamily ${::osfamily}")
+    }
   }
 }

manifests/service.pp

@@ -28,10 +28,6 @@
 #   (optional) Whether the keystone service has restart
 #   Defaults to true
 #
-# [*provider*]
-#   (optional) Provider for keystone service
-#   Defaults to $::keystone::params::service_provider
-#
 # [*validate*]
 #   (optional) Whether to validate the service is working after any service refreshes
 #   Defaults to false
@@ -69,7 +65,6 @@ class keystone::service(
   $enable         = true,
   $hasstatus      = true,
   $hasrestart     = true,
-  $provider       = $::keystone::params::service_provider,
   $validate       = false,
   $admin_token    = undef,
   $admin_endpoint = 'http://localhost:35357/v2.0',
@@ -88,7 +83,6 @@ class keystone::service(
     enable     => $enable,
     hasstatus  => $hasstatus,
     hasrestart => $hasrestart,
-    provider   => $provider,
     tag        => 'keystone-service',
   }
 

releasenotes/notes/service_provider_deprecation-50421064c823f3ee.yaml

@@ -0,0 +1,6 @@
+---
+deprecations:
+  - service_provider parameter is deprecated, does nothing and will be removed in a future release.
+    The parameter has no effect. The Service provider will be found by Puppet itself.
+    If you really need to override this value, please use a Puppet resource collector,
+    using keystone-service resource tag.

spec/classes/keystone_client_spec.rb

@@ -2,6 +2,10 @@ require 'spec_helper'
 
 describe 'keystone::client' do
 
+  let :facts do
+    @default_facts.merge({ :osfamily => 'Debian' })
+  end
+
   describe "with default parameters" do
     it { is_expected.to contain_package('python-keystoneclient').with(
         'ensure' => 'present',

spec/classes/keystone_service_spec.rb

@@ -2,6 +2,10 @@ require 'spec_helper'
 
 describe 'keystone::service' do
 
+  let :facts do
+    @default_facts.merge({ :osfamily => 'Debian' })
+  end
+
   describe "with default parameters" do
     it { is_expected.to contain_service('keystone').with(
       :ensure     => nil,

spec/classes/keystone_spec.rb

@@ -833,38 +833,6 @@ describe 'keystone' do
     it { is_expected.to contain_keystone_config('catalog/template_file').with_value('/some/template_file') }
   end
 
-  describe 'setting service_provider' do
-    let :facts do
-      @default_facts.merge(global_facts.merge({
-        :osfamily               => 'RedHat',
-        :operatingsystemrelease => '6.0'
-      }))
-    end
-
-    describe 'with default service_provider' do
-      let :params do
-        { 'admin_token'    => 'service_token' }
-      end
-
-      it { is_expected.to contain_service('keystone').with(
-        :provider => nil
-      )}
-    end
-
-    describe 'with overrided service_provider' do
-      let :params do
-        {
-          'admin_token'      => 'service_token',
-          'service_provider' => 'pacemaker'
-        }
-      end
-
-      it { is_expected.to contain_service('keystone').with(
-        :provider => 'pacemaker'
-      )}
-    end
-  end
-
   describe 'when using fernet tokens' do
     describe 'when enabling fernet_setup' do
       let :params do