]> review.fuel-infra Code Review - puppet-modules/puppet-ceilometer.git/commitdiff
Refactor service user/group management
authorTakashi Kajinami <tkajinam@redhat.com>
Sat, 14 May 2022 16:01:23 +0000 (01:01 +0900)
committerTakashi Kajinami <tkajinam@redhat.com>
Mon, 16 May 2022 02:57:13 +0000 (11:57 +0900)
This change refactors how the ceilometer service user and group are
managed.

- The ceilometer service user and group are created by the common
  package. While the user resource should still be declared to manage
  its group membership, we don't need the group resource.

- Introduces the configuration knob to disable user/group management.
  This would be useful in the case where all service users are
  declared externally.

Change-Id: Iaabe5b02f0ebd782debd0f3ca41e2fdafbf9c80f

manifests/agent/polling.pp
manifests/init.pp
releasenotes/notes/manage_user-a18dad4c2ad95daf.yaml [new file with mode: 0644]
spec/classes/ceilometer_agent_polling_spec.rb
spec/classes/ceilometer_init_spec.rb

index 9360d5fa05441d27b2a98d98610dd33e14c029b5..35d75ac520cc699e41be447e3fbe5023319a9566 100644 (file)
 #   (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;
index 89892eb36eb7ccb33f8bfe0b5e6db2c30d8645a0..2b143376dee1b0df31b3b9c492ff3ab838a5822c 100644 (file)
@@ -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,
diff --git a/releasenotes/notes/manage_user-a18dad4c2ad95daf.yaml b/releasenotes/notes/manage_user-a18dad4c2ad95daf.yaml
new file mode 100644 (file)
index 0000000..a18ed43
--- /dev/null
@@ -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.
index 32d450d7b4be88ac8a558648c1c6ec3397fdac8c..634814bdd4ef2230dddfc142ea130335c0fbd368 100644 (file)
@@ -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
index ec560d384404b19ff22bc0dae06b371938765c95..1ba0958d466ae1b878417e1b2af3ce3054818f82 100644 (file)
@@ -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',