openstack
/
puppet-nova
Code Issues Proposed changes

Avoid usage of custom os_package_type fact 

Currently the os_package_type fact is used to distinguish Ubuntu and
Debian but it's redundant and built-in $::operatingsystem should be
enough. What is worse the custom fact is not injected while composing
facts data in unit tests and has never been tested properly.

Change-Id: Ie1d6006020970ade3e1597dc9f68f9a5503283b4
This commit is contained in:
Takashi Kajinami 2021-11-04 22:31:33 +09:00
parent 76c5e9b955
commit 4b553afc8c
8 changed files with 12 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
manifests/params.pp
View File

@ -95,8 +95,8 @@ class nova::params {
# debian specific nova config
$root_helper = 'sudo nova-rootwrap'
$lock_path = '/var/lock/nova'
case $::os_package_type {
'debian': {
case $::operatingsystem {
'Debian': {
$api_metadata_service_name = 'nova-api-metadata'
$spicehtml5proxy_package_name = 'nova-consoleproxy'
$spicehtml5proxy_service_name = 'nova-spicehtml5proxy'

2
manifests/spicehtml5proxy.pp
View File

@ -56,7 +56,7 @@ class nova::spicehtml5proxy(
# 3/ Start the service
# Other OS don't need this scheduling and can use
# the standard nova::generic_service
if $::os_package_type == 'debian' {
if $::operatingsystem == 'Debian' {
if $enabled {
file_line { '/etc/default/nova-consoleproxy:NOVA_CONSOLE_PROXY_TYPE':
path => '/etc/default/nova-consoleproxy',

2
manifests/vncproxy.pp
View File

@ -120,7 +120,7 @@ class nova::vncproxy(
# 3/ Start the service
# Other OS don't need this scheduling and can use
# the standard nova::generic_service
if $::os_package_type == 'debian' {
if $::operatingsystem == 'Debian' {
if $enabled {
file_line { '/etc/default/nova-consoleproxy:NOVA_CONSOLE_PROXY_TYPE':
path => '/etc/default/nova-consoleproxy',

4
spec/classes/nova_api_spec.rb
View File

@ -319,7 +319,7 @@ describe 'nova::api' do
shared_examples 'nova-api on Debian' do
context 'with default parameters' do
it { is_expected.to contain_service('nova-api- metadata').with(
it { is_expected.to contain_service('nova-api-metadata').with(
:name => 'nova-api-metadata',
:ensure => 'running',
:hasstatus => true,
@ -348,7 +348,7 @@ describe 'nova::api' do
end
end
it_behaves_like 'nova-api'
if facts[:os_package_type] == 'debian'
if facts[:operatingsystem] == 'Debian'
it_behaves_like 'nova-api on Debian'
end
end

3
spec/classes/nova_compute_libvirt_guests_spec.rb
View File

@ -74,9 +74,6 @@ describe 'nova::compute::libvirt_guests' do
case [:osfamily]
when 'RedHat'
let (:facts) do
facts.merge!(OSDefaults.get_facts({ :os_package_type => 'rpm' }))
end
it_behaves_like 'redhat-nova-compute-libvirt-guests'
end
end

2
spec/classes/nova_serial_proxy_spec.rb
View File

@ -59,7 +59,7 @@ describe 'nova::serialproxy' do
let (:platform_params) do
case facts[:osfamily]
when 'Debian'
if facts[:os_package_type] == 'debian'
if facts[:operatingsystem] == 'Debian'
{ :serialproxy_package_name => 'nova-consoleproxy',
:serialproxy_service_name => 'nova-serialproxy' }
else

12
spec/classes/nova_spicehtml5proxy_spec.rb
View File

@ -67,19 +67,13 @@ describe 'nova::spicehtml5proxy' do
}).each do |os,facts|
context "on #{os}" do
let (:facts) do
if facts[:operatingsystem] == 'Debian'
extra = { :os_package_type => 'debian' }
else
extra = {}
end
facts.merge!(OSDefaults.get_facts(extra))
facts.merge!(OSDefaults.get_facts())
end
let (:platform_params) do
case facts[:osfamily]
when 'Debian'
if facts[:os_package_type] == 'debian' then
if facts[:operatingsystem] == 'Debian' then
package_name = 'nova-consoleproxy'
service_name = 'nova-spicehtml5proxy'
else
@ -100,7 +94,7 @@ describe 'nova::spicehtml5proxy' do
it_behaves_like 'nova::spicehtml5proxy'
if facts[:os_package_type] == 'debian'
if facts[:operatingsystem] == 'Debian'
it_behaves_like 'nova::spicehtml5proxy on Debian'
end
end

8
spec/classes/nova_vnc_proxy_spec.rb
View File

@ -135,10 +135,6 @@ describe 'nova::vncproxy' do
'include nova'
end
before do
facts.merge!( :os_package_type => 'debian' )
end
it { is_expected.to contain_file_line('/etc/default/nova-consoleproxy:NOVA_CONSOLE_PROXY_TYPE').with(
:path => '/etc/default/nova-consoleproxy',
:match => '^NOVA_CONSOLE_PROXY_TYPE=(.*)$',
@ -160,7 +156,7 @@ describe 'nova::vncproxy' do
let (:platform_params) do
case facts[:osfamily]
when 'Debian'
if facts[:os_package_type] == 'debian'
if facts[:operatingsystem] == 'Debian'
{ :nova_vncproxy_package => 'nova-consoleproxy',
:nova_vncproxy_service => 'nova-novncproxy' }
else
@ -175,7 +171,7 @@ describe 'nova::vncproxy' do
it_behaves_like 'nova_vnc_proxy'
if facts[:os_package_type] == 'debian'
if facts[:operatingsystem] == 'Debian'
it_behaves_like 'nova_vnc_proxy debian package'
end