From 092bf5ef45ff8fb9b841db1ac76ae1a5d6c83969 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Sun, 15 May 2022 01:01:23 +0900 Subject: [PATCH] Refactor service user/group management This change refactors how the ceilometer service user and group are managed. - The ceilometer service user and group are created by packages. 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 | 42 +++++++++++++------ manifests/init.pp | 14 ------- .../notes/manage_user-a18dad4c2ad95daf.yaml | 6 +++ spec/classes/ceilometer_agent_polling_spec.rb | 14 ++++++- spec/classes/ceilometer_init_spec.rb | 18 -------- 5 files changed, 48 insertions(+), 46 deletions(-) create mode 100644 releasenotes/notes/manage_user-a18dad4c2ad95daf.yaml diff --git a/manifests/agent/polling.pp b/manifests/agent/polling.pp index 9360d5fa..842e8cb3 100644 --- a/manifests/agent/polling.pp +++ b/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] + $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 can access libvirt to gather some metrics. + if $::ceilometer::params::libvirt_group { + $ceilometer_groups = ['nova', 'libvirt'] + } else { + $ceilometer_groups = ['nova'] } - Package <| title == 'libvirt' |> -> User['ceilometer'] - } else { - User['ceilometer'] { - groups => ['nova'] + + user { 'ceilometer': + ensure => present, + name => 'ceilometer', + gid => 'ceilometer' + groups => ['nova', $::ceilometer::params::libvirt_group], + require => Anchor['ceilometer::install::end'], + before => Anchor['ceilometer::service::begin]', } + + 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; diff --git a/manifests/init.pp b/manifests/init.pp index 89892eb3..2b143376 100644 --- a/manifests/init.pp +++ b/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, diff --git a/releasenotes/notes/manage_user-a18dad4c2ad95daf.yaml b/releasenotes/notes/manage_user-a18dad4c2ad95daf.yaml new file mode 100644 index 00000000..a18ed43c --- /dev/null +++ b/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. diff --git a/spec/classes/ceilometer_agent_polling_spec.rb b/spec/classes/ceilometer_agent_polling_spec.rb index 32d450d7..eee09f51 100644 --- a/spec/classes/ceilometer_agent_polling_spec.rb +++ b/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' + :ceilometer_groups => ['nova'], } end end diff --git a/spec/classes/ceilometer_init_spec.rb b/spec/classes/ceilometer_init_spec.rb index ec560d38..1ba0958d 100644 --- a/spec/classes/ceilometer_init_spec.rb +++ b/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',