Merge "Refactor service user/group management"

manifests/agent/polling.pp

@@ -16,6 +16,12 @@
 #   (Optional) ensure state for package.
 #   Defaults to 'present'
 #
+# [*manage_user*]
+#   (Optional) Should the system user should be managed. When this flag is
+#   true then the class ensures the ceilometer user belongs to nova/libvirt
+#   group.
+#   Defaults to true.
+#
 # [*central_namespace*]
 #   (Optional) Use central namespace for polling agent.
 #   Defaults to true.
@@ -77,6 +83,7 @@ class ceilometer::agent::polling (
   $manage_service            = true,
   $enabled                   = true,
   $package_ensure            = 'present',
+  $manage_user               = true,
   $central_namespace         = true,
   $compute_namespace         = true,
   $ipmi_namespace            = true,
@@ -107,22 +114,33 @@ class ceilometer::agent::polling (
   }
 
   if $compute_namespace {
-    if $::ceilometer::params::libvirt_group {
-      User['ceilometer'] {
-        groups => ['nova', $::ceilometer::params::libvirt_group]
+    if $manage_user {
+      # The ceilometer user created by the ceilometer-common package does not
+      # belong to nova/libvirt group. That group membership is required so that
+      # the ceilometer user can access libvirt to gather some metrics.
+      $ceilometer_groups = delete_undef_values([
+        'nova',
+        $::ceilometer::params::libvirt_group
+      ])
+
+      user { 'ceilometer':
+        ensure  => present,
+        name    => 'ceilometer',
+        gid     => 'ceilometer',
+        groups  => $ceilometer_groups,
+        require => Anchor['ceilometer::install::end'],
+        before  => Anchor['ceilometer::service::begin'],
       }
-      Package <| title == 'libvirt' |> -> User['ceilometer']
-    } else {
-      User['ceilometer'] {
-        groups => ['nova']
+
+      if $::ceilometer::params::libvirt_group {
+        Package <| title == 'libvirt' |> -> User['ceilometer']
       }
+      Package <| title == 'nova-common' |> -> User['ceilometer']
+
+      User['ceilometer'] -> Anchor['ceilometer::service::begin']
     }
 
     $compute_namespace_name = 'compute'
-
-    Package <| title == 'ceilometer-common' |> -> User['ceilometer']
-    Package <| title == 'nova-common' |> -> Package['ceilometer-common']
-
     ceilometer_config {
       'compute/instance_discovery_method': value => $instance_discovery_method;
       'compute/resource_update_interval':  value => $resource_update_interval;

manifests/init.pp

@@ -419,20 +419,6 @@ class ceilometer(
   $snmpd_readonly_username_real = pick($snmpd_readonly_username, $::os_service_default)
   $snmpd_readonly_user_password_real = pick($snmpd_readonly_user_password, $::os_service_default)
 
-  group { 'ceilometer':
-    ensure  => present,
-    name    => 'ceilometer',
-    require => Anchor['ceilometer::install::end'],
-  }
-
-  user { 'ceilometer':
-    ensure  => present,
-    name    => 'ceilometer',
-    gid     => 'ceilometer',
-    system  => true,
-    require => Anchor['ceilometer::install::end'],
-  }
-
   package { 'ceilometer-common':
     ensure => $package_ensure,
     name   => $::ceilometer::params::common_package_name,

releasenotes/notes/manage_user-a18dad4c2ad95daf.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The new ``ceilometer::agents::polling::manage_user`` parameter has been
+    added. When this parameter is set to ``false``, the class does not ensure
+    the ``ceilometer`` system user and it's group membership.

spec/classes/ceilometer_agent_polling_spec.rb

@@ -25,8 +25,16 @@ describe 'ceilometer::agent::polling' do
         end
       }
 
+      it { should contain_user('ceilometer').with(
+        :ensure  => 'present',
+        :name    => 'ceilometer',
+        :gid     => 'ceilometer',
+        :groups  => platform_params[:ceilometer_groups],
+        :require => 'Anchor[ceilometer::install::end]',
+      ) }
+
       it { should contain_package('nova-common').with(
-       :before => /Package\[ceilometer-common\]/
+       :before => /User\[ceilometer\]/
       )}
 
       it {
@@ -285,12 +293,14 @@ sources:
             {
               :agent_package_name => 'ceilometer-polling',
               :agent_service_name => 'ceilometer-polling',
-              :libvirt_group      => 'libvirt'
+              :libvirt_group      => 'libvirt',
+              :ceilometer_groups  => ['nova', 'libvirt'],
             }
         when 'RedHat'
             {
               :agent_package_name => 'openstack-ceilometer-polling',
-              :agent_service_name => 'openstack-ceilometer-polling'
+              :agent_service_name => 'openstack-ceilometer-polling',
+              :ceilometer_groups  => ['nova'],
             }
         end
       end

spec/classes/ceilometer_init_spec.rb

@@ -59,24 +59,6 @@ describe 'ceilometer' do
 
     it { is_expected.to contain_class('ceilometer::params') }
 
-    it 'configures ceilometer group' do
-      is_expected.to contain_group('ceilometer').with(
-        :ensure  => 'present',
-        :name    => 'ceilometer',
-        :require => 'Anchor[ceilometer::install::end]'
-      )
-    end
-
-    it 'configures ceilometer user' do
-      is_expected.to contain_user('ceilometer').with(
-        :ensure  => 'present',
-        :name    => 'ceilometer',
-        :gid     => 'ceilometer',
-        :system  => true,
-        :require => 'Anchor[ceilometer::install::end]'
-      )
-    end
-
     it 'installs ceilometer common package' do
       is_expected.to contain_package('ceilometer-common').with(
         :ensure => 'present',