From eafc9088718239b6b9b4a8a579c4588dfb7d10af 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 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 | 40 ++++++++++++++----- manifests/init.pp | 14 ------- .../notes/manage_user-a18dad4c2ad95daf.yaml | 6 +++ spec/classes/ceilometer_agent_polling_spec.rb | 16 ++++++-- 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..35d75ac5 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] + 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; 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..634814bd 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' + :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',