openstack
/
puppet-nova
Code Issues Proposed changes

Fix handling of nova pci MultiStrOpt params 

The changes in Ie27dbbc510c73c685b239a9be4af2700a0eb42f0 did not appear to fix
the handling of the nova pci_* params. The config being written to nova.conf
remains in the ListOpt format.

As support for MultiStrOpt was added to nova_config in
I6be7bb4cea1906bd98c513bd2d01153e4643e3ac we just need to pass an array of
strings to nova_config to set these MultiStrOpt params.

Nova expects the string values to be JSON encoded so that is what the parser
function now returns.  This function has also been renamed from
'check_array_of_hash' (which it never did) to 'to_array_of_json_strings'.

The acceptable input formats are a JSON array of objects or a puppet Array of
Hashes. Perviously a "JSON with single quotes" format was used but should now
be considered deprecated as it could corrupt data.

This also removes any pci_alias entries from nova.conf when the param is not
set.

Change-Id: Ida3ecab717bc3113ba23553c559263f35c49c46a
Closes-bug: #1696955
This commit is contained in:
Oliver Walsh 2017-08-09 15:38:49 +01:00
parent cd223bf4dc
commit 1cd349f893
7 changed files with 145 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
lib/puppet/parser/functions/check_array_of_hash.rb
View File

@ -1,27 +0,0 @@
require 'json'
def array_of_hash?(list)
return false unless !list.empty? && list.class == Array
list.each do |e|
return false unless e.class == Hash
end
true
end
module Puppet::Parser::Functions
newfunction(:check_array_of_hash, :arity =>1, :type => :rvalue, :doc => "Check
input String is a valid Array of Hash in JSON style") do |arg|
if arg[0].class == String
begin
list = JSON.load(arg[0].gsub("'","\""))
rescue JSON::ParserError
raise Puppet::ParseError, "Syntax error: #{arg[0]} is invalid"
else
return arg[0] if array_of_hash?(list)
end
else
raise Puppet::ParseError, "Syntax error: #{arg[0]} is not a String"
end
return ''
end
end

38
lib/puppet/parser/functions/to_array_of_json_strings.rb Normal file
View File

@ -0,0 +1,38 @@
require 'json'
def array_of_hash?(list)
return false unless list.class == Array
list.each do |e|
return false unless e.class == Hash
end
true
end
module Puppet::Parser::Functions
newfunction(:to_array_of_json_strings, :arity =>1, :type => :rvalue, :doc => "Convert
input array of hashes (optionally JSON encoded) to a puppet Array of JSON encoded Strings") do |arg|
list = arg[0]
if list.class == String
begin
begin
list = JSON.load(list)
rescue JSON::ParserError
# If parsing failed it could be a legacy format that uses single quotes.
# NB This will corrupt valid JSON data, e.g {"foo": "\'"} => {"foo": "\""}
list = JSON.load(list.gsub("'","\""))
Puppet.warning("#{arg[0]} is not valid JSON. Support for this format is deprecated and may be removed in future.")
end
rescue JSON::ParserError
raise Puppet::ParseError, "Syntax error: #{arg[0]} is not valid"
end
end
unless array_of_hash?(list)
raise Puppet::ParseError, "Syntax error: #{arg[0]} is not an Array or JSON encoded String"
end
rv = []
list.each do |e|
rv.push(e.to_json)
end
return rv
end
end

22
manifests/api.pp
View File

@ -75,10 +75,11 @@
# Defaults to undef
#
# [*pci_alias*]
# (optional) Pci passthrough for controller:
# Defaults to undef
# Example
# "[ {'vendor_id':'1234', 'product_id':'5678', 'name':'default'}, {...} ]"
# (optional) A list of pci alias hashes
# Defaults to $::os_service_default
# Example:
# [{"vendor_id" => "1234", "product_id" => "5678", "name" => "default"},
# {"vendor_id" => "1234", "product_id" => "6789", "name" => "other"}]
#
# [*ratelimits*]
# (optional) A string that is a semicolon-separated list of 5-tuples.
@ -291,7 +292,7 @@ class nova::api(
$sync_db_api = true,
$db_online_data_migrations = false,
$neutron_metadata_proxy_shared_secret = undef,
$pci_alias = undef,
$pci_alias = $::os_service_default,
$ratelimits = undef,
$ratelimits_factory =
'nova.api.openstack.compute.limits:RateLimitingMiddleware.factory',
@ -535,10 +536,13 @@ as a standalone service, or httpd for being run by a httpd server")
'filter:authtoken/auth_admin_prefix': ensure => absent;
}
if $pci_alias {
nova_config {
'pci/alias': value => join(any2array(check_array_of_hash($pci_alias)), ',');
}
if !is_service_default($pci_alias) and !empty($pci_alias) {
$pci_alias_real = to_array_of_json_strings($pci_alias)
} else {
$pci_alias_real = $::os_service_default
}
nova_config {
'pci/alias': value => $pci_alias_real;
}
if $validate {

6
manifests/compute.pp
View File

@ -87,8 +87,8 @@
# (optional) Pci passthrough list of hash.
# Defaults to $::os_service_default
# Example of format:
# "[ { 'vendor_id':'1234','product_id':'5678' },
# { 'vendor_id':'4321','product_id':'8765','physical_network':'default' } ] "
# [ { "vendor_id" => "1234","product_id" => "5678" },
# { "vendor_id" => "4321","product_id" => "8765", "physical_network" => "default" } ]
#
# [*config_drive_format*]
# (optional) Config drive format. One of iso9660 (default) or vfat
@ -186,7 +186,7 @@ class nova::compute (
# the value is computed in a function and it makes things more complex. Let's just check if
# a value is set or if it's empty.
if !is_service_default($pci_passthrough) and !empty($pci_passthrough) {
$pci_passthrough_real = check_array_of_hash($pci_passthrough)
$pci_passthrough_real = to_array_of_json_strings($pci_passthrough)
} else {
$pci_passthrough_real = $::os_service_default
}

11
releasenotes/notes/pci_params_format-bf62bf47585ad1cc.yaml Normal file
View File

@ -0,0 +1,11 @@
---
features:
- |
The nova::api::pci_alias and nova::compute::pci_passthrough params now
accept an array of hashes. A valid JSON encoded array of objects is also
acceptable.
deprecations:
- |
Invalid JSON was previously accepted for the nova::api::pci_alias and
nova::compute::pci_passthrough parameters. This format is now deprecated
and may not be accepted in future.

44
spec/classes/nova_api_spec.rb
View File

@ -79,6 +79,7 @@ describe 'nova::api' do
is_expected.to contain_nova_config('vendordata_dynamic_auth/project_name').with('value' => '<SERVICE DEFAULT>')
is_expected.to contain_nova_config('vendordata_dynamic_auth/user_domain_name').with('value' => '<SERVICE DEFAULT>')
is_expected.to contain_nova_config('vendordata_dynamic_auth/username').with('value' => '<SERVICE DEFAULT>')
is_expected.to contain_nova_config('pci/alias').with(:value => '<SERVICE DEFAULT>')
end
it 'unconfigures neutron_metadata proxy' do
@ -117,7 +118,6 @@ describe 'nova::api' do
:enable_network_quota => false,
:enable_instance_password => true,
:password_length => 12,
:pci_alias => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\",\"name\":\"graphic_card\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"name\":\"network_card\"}]",
:allow_resize_to_same_host => true,
:vendordata_dynamic_auth_auth_type => 'password',
:vendordata_dynamic_auth_auth_url => 'http://127.0.0.1:5000',
@ -182,14 +182,54 @@ describe 'nova::api' do
is_expected.to contain_nova_config('vendordata_dynamic_auth/user_domain_name').with('value' => 'Default')
is_expected.to contain_nova_config('vendordata_dynamic_auth/username').with('value' => 'user')
end
end
context 'with pci_alias array' do
before do
params.merge!({
:pci_alias => [{
"vendor_id" => "8086",
"product_id" => "0126",
"name" => "graphic_card"
},
{
"vendor_id" => "9096",
"product_id" => "1520",
"name" => "network_card"
}
]
})
end
it 'configures nova pci_alias entries' do
is_expected.to contain_nova_config('pci/alias').with(
'value' => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\",\"name\":\"graphic_card\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"name\":\"network_card\"}]"
'value' => ['{"vendor_id":"8086","product_id":"0126","name":"graphic_card"}','{"vendor_id":"9096","product_id":"1520","name":"network_card"}']
)
end
end
context 'with pci_alias JSON encoded string (deprecated)' do
before do
params.merge!({
:pci_alias => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\",\"name\":\"graphic_card\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"name\":\"network_card\"}]",
})
end
it 'configures nova pci_alias entries' do
is_expected.to contain_nova_config('pci/alias').with(
'value' => ['{"vendor_id":"8086","product_id":"0126","name":"graphic_card"}','{"vendor_id":"9096","product_id":"1520","name":"network_card"}']
)
end
end
context 'when pci_alias is empty' do
let :params do
{ :pci_alias => "" }
end
it 'clears pci_alias configuration' do
is_expected.to contain_nova_config('pci/alias').with(:value => '<SERVICE DEFAULT>')
end
end
context 'while validating the service with default command' do
before do
params.merge!({

45
spec/classes/nova_compute_spec.rb
View File

@ -34,6 +34,7 @@ describe 'nova::compute' do
it { is_expected.to contain_nova_config('barbican/barbican_api_version').with_value('<SERVICE DEFAULT>') }
it { is_expected.to contain_nova_config('barbican/auth_endpoint').with_value('<SERVICE DEFAULT>') }
it { is_expected.to contain_nova_config('DEFAULT/max_concurrent_live_migrations').with_value('<SERVICE DEFAULT>') }
it { is_expected.to contain_nova_config('pci/passthrough_whitelist').with(:value => '<SERVICE DEFAULT>') }
it { is_expected.to_not contain_package('cryptsetup').with( :ensure => 'present' )}
@ -69,7 +70,6 @@ describe 'nova::compute' do
:force_raw_images => false,
:reserved_host_memory => '0',
:heal_instance_info_cache_interval => '120',
:pci_passthrough => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"physical_network\":\"physnet1\"}]",
:config_drive_format => 'vfat',
:resize_confirm_window => '3',
:vcpu_pin_set => ['4-12','^8','15'],
@ -130,12 +130,6 @@ describe 'nova::compute' do
it { is_expected.to contain_nova_config('DEFAULT/resume_guests_state_on_host_boot').with_value(true) }
it 'configures nova pci_passthrough_whitelist entries' do
is_expected.to contain_nova_config('pci/passthrough_whitelist').with(
'value' => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"physical_network\":\"physnet1\"}]"
)
end
it 'configures nova config_drive_format to vfat' do
is_expected.to contain_nova_config('DEFAULT/config_drive_format').with_value('vfat')
is_expected.to_not contain_package('genisoimage').with(
@ -144,6 +138,43 @@ describe 'nova::compute' do
end
end
context 'with pci_passthrough array' do
let :params do
{
:pci_passthrough => [
{
"vendor_id" => "8086",
"product_id" => "0126"
},
{
"vendor_id" => "9096",
"product_id" => "1520",
"physical_network" => "physnet1"
}
]
}
end
it 'configures nova pci_passthrough_whitelist entries' do
is_expected.to contain_nova_config('pci/passthrough_whitelist').with(
'value' => ['{"vendor_id":"8086","product_id":"0126"}','{"vendor_id":"9096","product_id":"1520","physical_network":"physnet1"}']
)
end
end
context 'with pci_passthrough JSON encoded string (deprecated)' do
let :params do
{
:pci_passthrough => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"physical_network\":\"physnet1\"}]"
}
end
it 'configures nova pci_passthrough_whitelist entries' do
is_expected.to contain_nova_config('pci/passthrough_whitelist').with(
'value' => ['{"vendor_id":"8086","product_id":"0126"}','{"vendor_id":"9096","product_id":"1520","physical_network":"physnet1"}']
)
end
end
context 'when vcpu_pin_set and pci_passthrough are empty' do
let :params do