openstack/puppet-nova
Code Issues Proposed changes

Fix VNC console in Debian

Browse Source 
In Debian, SPICE is the default. Unfortunately, puppet-openstack fails to
set [spice]/enabled = false when VNC is selected, and therefore, both VNC
and SPICE ends up being enabled in nova.conf.

Also, Debian has a unique package nova-consoleproxy handling SPICE, VNC
and the XenVNC console, with /etc/default/nova-consoleproxy being used to
select what daemon to start. As puppet-openstack doesn't set it before
starting the VNC console service, it stays with spicehtml5 as default
value, and therefore, nova-novncproxy cannot start.

This patch fixes both issues.

Change-Id: Ia40805f27e8833fa01576432ae792e1becedd729
This commit is contained in:
Thomas Goirand
2018-04-25 21:29:54 +02:00
parent 7b0c2cf947
commit 26fbc01f53
9 changed files with 160 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
manifests/compute.pp
View File

@@ -24,6 +24,11 @@
# (optional) Whether to use a VNC proxy
# Defaults to true
#
# [*spice_enabled*]
# (optional) Whether to use a SPICE html5 proxy
# Mutually exclusive with vnc_enabled.
# Defaults to false.
#
# [*vncserver_proxyclient_address*]
# (optional) The IP address of the server running the VNC proxy client
# Defaults to '127.0.0.1'
@@ -164,6 +169,7 @@ class nova::compute (
$manage_service = true,
$ensure_package = 'present',
$vnc_enabled = true,
$spice_enabled = false,
$vncserver_proxyclient_address = '127.0.0.1',
$vncproxy_host = false,
$vncproxy_protocol = 'http',
@@ -210,6 +216,10 @@ class nova::compute (
$keymgr_backend_real = $keymgr_backend
}
if ($vnc_enabled and $spice_enabled) {
fail('vnc_enabled and spice_enabled is mutually exclusive')
}
# cryptsetup is required when Barbican is encrypting volumes
if $keymgr_backend_real =~ /barbican/ {
ensure_packages('cryptsetup', {
@@ -262,8 +272,10 @@ class nova::compute (
'vnc/keymap': ensure => absent;
}
}
nova_config {
'vnc/enabled': value => $vnc_enabled;
'vnc/enabled': value => $vnc_enabled;
'spice/enabled': value => $spice_enabled;
}
if $neutron_enabled != true and $install_bridge_utils {

1
manifests/compute/spice.pp
View File

@@ -58,7 +58,6 @@ class nova::compute::spice(
}
nova_config {
'spice/enabled': value => true;
'spice/agent_enabled': value => $agent_enabled;
'spice/server_listen': value => $server_listen;
'spice/server_proxyclient_address': value => $server_proxyclient_address;

25
manifests/spicehtml5proxy.pp
View File

@@ -39,13 +39,35 @@ class nova::spicehtml5proxy(
include ::nova::deps
include ::nova::params
# Nodes running spicehtml5proxy do *not* need (and in fact, don't care)
# about [spice]/enable to be set. This setting is for compute nodes,
# where we must select VNC or SPICE so that it can be passed on to
# libvirt which passes it as parameter when starting VMs with KVM.
# Therefore, this setting is set within compute.pp only.
nova_config {
'spice/enabled': value => $enabled;
'spice/agent_enabled': value => $enabled;
'spice/html5proxy_host': value => $host;
'spice/html5proxy_port': value => $port;
}
# The Debian package needs some scheduling:
# 1/ Install the packagin
# 2/ Fix /etc/default/nova-consoleproxy
# 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 $enabled {
file_line { '/etc/default/nova-consoleproxy:NOVA_CONSOLE_PROXY_TYPE':
path => '/etc/default/nova-consoleproxy',
match => '^NOVA_CONSOLE_PROXY_TYPE=(.*)$',
line => 'NOVA_CONSOLE_PROXY_TYPE=spicehtml5',
tag => 'nova-consoleproxy',
require => Anchor['nova::config::begin'],
notify => Anchor['nova::config::end'],
}
}
}
nova::generic_service { 'spicehtml5proxy':
enabled => $enabled,
manage_service => $manage_service,
@@ -54,4 +76,3 @@ class nova::spicehtml5proxy(
ensure_package => $ensure_package,
}
}

23
manifests/vncproxy.pp
View File

@@ -103,12 +103,35 @@ class nova::vncproxy(
$auth_schemes = 'none'
}
# Nodes running novncproxy do *not* need (and in fact, don't care)
# about [vnc]/enable to be set. This setting is for compute nodes,
# where we must select VNC or SPICE so that it can be passed on to
# libvirt which passes it as parameter when starting VMs with KVM.
# Therefore, this setting is set within compute.pp only.
nova_config {
'vnc/novncproxy_host': value => $host;
'vnc/novncproxy_port': value => $port;
'vnc/auth_schemes': value => $auth_schemes;
}
# The Debian package needs some scheduling:
# 1/ Install the packagin
# 2/ Fix /etc/default/nova-consoleproxy
# 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 $enabled {
file_line { '/etc/default/nova-consoleproxy:NOVA_CONSOLE_PROXY_TYPE':
path => '/etc/default/nova-consoleproxy',
match => '^NOVA_CONSOLE_PROXY_TYPE=(.*)$',
line => 'NOVA_CONSOLE_PROXY_TYPE=novnc',
tag => 'nova-consoleproxy',
require => Anchor['nova::config::begin'],
notify => Anchor['nova::config::end'],
}
}
}
nova::generic_service { 'vncproxy':
enabled => $enabled,
manage_service => $manage_service,

13
releasenotes/notes/spice-enabled-78f1bf8f333928aa.yaml Normal file
View File

@@ -0,0 +1,13 @@
---
features:
- |
Puppet-openstack now manages both [vnc]/enabled and [spice]/enabled. This
was needed because Debian has [spice]/enabled set to True by default, and
one cannot have both enabled in a compute node. Therefore, it was mandatory
to have [spice]/enabled set to Flase if we're using VNC.
upgrade:
- |
Users of puppet-openstack will have to set ::nova::compute::spice_enabled to
True to make a meaningful decisioin to use Spice and not use VNC, and not
rely on ::nova::spicehtml5proxy or ::nova::compute::spice to set it as
enabled by default.

22
spec/classes/nova_compute_spec.rb
View File

@@ -122,6 +122,10 @@ describe 'nova::compute' do
)
end
it 'should have spice disabled' do
is_expected.to contain_nova_config('spice/enabled').with_value(false)
end
it { is_expected.to contain_nova_config('DEFAULT/heal_instance_info_cache_interval').with_value('120') }
it { is_expected.to contain_nova_config('DEFAULT/force_raw_images').with(:value => false) }
@@ -221,9 +225,10 @@ describe 'nova::compute' do
end
context 'with vnc_enabled set to false' do
context 'with vnc_enabled set to false and spice_enabled set to true' do
let :params do
{ :vnc_enabled => false }
{ :vnc_enabled => false,
:spice_enabled => true, }
end
it 'disables vnc in nova.conf' do
@@ -232,6 +237,19 @@ describe 'nova::compute' do
is_expected.to contain_nova_config('vnc/keymap').with_ensure('absent')
is_expected.to_not contain_nova_config('vnc/novncproxy_base_url')
end
it 'enables spice' do
is_expected.to contain_nova_config('spice/enabled').with_value(true)
end
end
context 'with vnc_enabled and spice_enabled set to true' do
let :params do
{ :vnc_enabled => true,
:spice_enabled => true, }
end
it { is_expected.to raise_error Puppet::Error, /vnc_enabled and spice_enabled is mutually exclusive/ }
end
context 'with force_config_drive parameter set to true' do

1
spec/classes/nova_compute_spice_spec.rb
View File

@@ -1,7 +1,6 @@
require 'spec_helper'
describe 'nova::compute::spice' do
it { is_expected.to contain_nova_config('spice/enabled').with_value('true')}
it { is_expected.to contain_nova_config('spice/agent_enabled').with_value('true')}
it { is_expected.to contain_nova_config('spice/server_proxyclient_address').with_value('127.0.0.1')}
it { is_expected.to_not contain_nova_config('spice/html5proxy_base_url')}

34
spec/classes/nova_spicehtml5proxy_spec.rb
View File

@@ -6,22 +6,36 @@ describe 'nova::spicehtml5proxy' do
'include nova'
end
shared_examples 'nova-spicehtml5proxy' do
shared_examples 'nova-spicehtml5proxy debian package' do
let :params do
{ :enabled => true }
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=(.*)$',
:line => 'NOVA_CONSOLE_PROXY_TYPE=spicehtml5',
:tag => 'nova-consoleproxy',
:require => 'Anchor[nova::config::begin]',
:notify => 'Anchor[nova::config::end]',
)}
end
shared_examples 'nova-spicehtml5proxy' do
it 'configures nova.conf' do
is_expected.to contain_nova_config('spice/html5proxy_host').with(:value => '0.0.0.0')
is_expected.to contain_nova_config('spice/html5proxy_port').with(:value => '6082')
end
it { is_expected.to contain_package('nova-spicehtml5proxy').with(
:name => platform_params[:spicehtml5proxy_package_name],
:ensure => 'present'
:ensure => 'present',
:name => platform_params[:spicehtml5proxy_package_name]
) }
it { is_expected.to contain_service('nova-spicehtml5proxy').with(
:ensure => 'running',
:name => platform_params[:spicehtml5proxy_service_name],
:hasstatus => 'true',
:ensure => 'running'
:hasstatus => true
)}
context 'with manage_service as false' do
@@ -30,6 +44,7 @@ describe 'nova::spicehtml5proxy' do
:manage_service => false
}
end
it { is_expected.to contain_service('nova-spicehtml5proxy').without_ensure }
end
@@ -39,7 +54,8 @@ describe 'nova::spicehtml5proxy' do
end
it { is_expected.to contain_package('nova-spicehtml5proxy').with(
:ensure => params[:ensure_package]
:ensure => params[:ensure_package],
:name => platform_params[:spicehtml5proxy_package_name],
)}
end
end
@@ -49,8 +65,8 @@ describe 'nova::spicehtml5proxy' do
@default_facts.merge({
:osfamily => 'Debian',
:operatingsystem => 'Ubuntu',
:os_package_type => 'ubuntu'
})
:os_package_type => 'ubuntu',
})
end
let :platform_params do
@@ -76,6 +92,7 @@ describe 'nova::spicehtml5proxy' do
:spicehtml5proxy_service_name => 'nova-spicehtml5proxy' }
end
it_configures 'nova-spicehtml5proxy debian package'
it_configures 'nova-spicehtml5proxy'
end
@@ -93,6 +110,7 @@ describe 'nova::spicehtml5proxy' do
:spicehtml5proxy_service_name => 'nova-spicehtml5proxy' }
end
it_configures 'nova-spicehtml5proxy debian package'
it_configures 'nova-spicehtml5proxy'
end

48
spec/classes/nova_vnc_proxy_spec.rb
View File

@@ -8,6 +8,10 @@ describe 'nova::vncproxy' do
'include nova'
end
let :params do
{ :enabled => true }
end
context 'with default parameters' do
it { is_expected.to contain_nova_config('vnc/novncproxy_host').with(:value => '0.0.0.0') }
@@ -22,7 +26,7 @@ describe 'nova::vncproxy' do
:name => platform_params[:nova_vncproxy_service],
:hasstatus => true,
:ensure => 'running'
)}
) }
describe 'with manage_service as false' do
let :params do
@@ -30,16 +34,20 @@ describe 'nova::vncproxy' do
:manage_service => false
}
end
it { is_expected.to contain_service('nova-vncproxy').without_ensure }
end
describe 'with package version' do
let :params do
{:ensure_package => '2012.1-2'}
{ :ensure_package => '2012.1-2' }
end
it { is_expected.to contain_package('nova-vncproxy').with(
'ensure' => '2012.1-2'
)}
:ensure => '2012.1-2',
:name => platform_params[:nova_vncproxy_package],
)
}
end
end
@@ -122,6 +130,25 @@ describe 'nova::vncproxy' do
end
shared_examples 'nova_vnc_proxy debian package' do
let :pre_condition 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=(.*)$',
:line => 'NOVA_CONSOLE_PROXY_TYPE=novnc',
:tag => 'nova-consoleproxy',
:require => 'Anchor[nova::config::begin]',
:notify => 'Anchor[nova::config::end]',
)}
end
on_supported_os({
:supported_os => OSDefaults.get_supported_os
}).each do |os,facts|
@@ -133,8 +160,13 @@ describe 'nova::vncproxy' do
let (:platform_params) do
case facts[:osfamily]
when 'Debian'
{ :nova_vncproxy_package => 'nova-novncproxy',
:nova_vncproxy_service => 'nova-novncproxy' }
if facts[:os_package_type] == 'debian'
{ :nova_vncproxy_package => 'nova-consoleproxy',
:nova_vncproxy_service => 'nova-novncproxy' }
else
{ :nova_vncproxy_package => 'nova-novncproxy',
:nova_vncproxy_service => 'nova-novncproxy' }
end
when 'RedHat'
{ :nova_vncproxy_package => 'openstack-nova-novncproxy',
:nova_vncproxy_service => 'openstack-nova-novncproxy' }
@@ -143,6 +175,10 @@ describe 'nova::vncproxy' do
it_behaves_like 'nova_vnc_proxy'
if facts[:os_package_type] == 'debian'
it_behaves_like 'nova_vnc_proxy debian package'
end
end
end