openstack/puppet-tempest
Code Issues Proposed changes

Allow arbitrary section name for *_id_setter

Browse Source 
... so that we can set the parameters in different sections.
(eg. [database] db_flavor_ref instead of [compute] db_flavor_ref )

Implementation to inject parameters to tempset.conf is replaced by
the tempest_config type and the ini_setting provider, so that we can
use the consistent implementation to modify tempest.conf .

This change also fixes "flapping" of database/db_flavor_ref. Now
the parameter is removed by the initial tempest_config resource then
set by the tempest_glance_id_setter, when db_flavor_name is used.

Closes-Bug: #1978848
Change-Id: Ife2c563213b9b4118d3925192a2ff45253bcee2b
This commit is contained in:
Takashi Kajinami 2022-03-07 16:24:23 +09:00
parent 7b20e48cec
commit affbcb0cc6
10 changed files with 119 additions and 195 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
lib/puppet/provider/tempest_flavor_id_setter/openstack.rb
View File

@ -7,22 +7,29 @@ Puppet::Type.type(:tempest_flavor_id_setter).provide(
@credentials = Puppet::Provider::Openstack::CredentialsV3.new
def exists?
lines.find do |line|
should_line.chomp == line.chomp
end
end
def file_path
resource[:tempest_conf_path]
end
def exists?
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.exists?
end
def create
handle_create_with_match
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :value => get_flavor_id, :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.create
end
def destroy
handle_create_with_match
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.destroy
end
def get_flavor_id
@ -40,49 +47,4 @@ Puppet::Type.type(:tempest_flavor_id_setter).provide(
end
@flavor_id
end
def should_line
"#{resource[:name]} = #{get_flavor_id}"
end
def match
/^\s*#{resource[:name]}\s*=\s*/
end
def handle_create_with_match()
regex = match
match_count = lines.select { |l| regex.match(l) }.count
file = lines
case match_count
when 1
File.open(file_path, 'w') do |fh|
lines.each do |l|
fh.puts(regex.match(l) ? "#{should_line}" : l)
end
end
when 0
block_pos = lines.find_index { |l| /^\[compute\]/ =~ l }
if block_pos.nil?
file += ["[compute]\n", "#{should_line}\n"]
else
file.insert(block_pos+1, "#{should_line}\n")
end
File.write(file_path, file.join)
else # cannot be negative.
raise Puppet::Error, "More than one line in file \
'#{file_path}' matches pattern '#{regex}'"
end
end
private
def lines
# If this type is ever used with very large files, we should
# write this in a different way, using a temp
# file; for now assuming that this type is only used on
# small-ish config files that can fit into memory without
# too much trouble.
@lines ||= File.readlines(file_path)
end
end

68
lib/puppet/provider/tempest_glance_id_setter/openstack.rb
View File

@ -7,22 +7,29 @@ Puppet::Type.type(:tempest_glance_id_setter).provide(
@credentials = Puppet::Provider::Openstack::CredentialsV3.new
def exists?
lines.find do |line|
should_line.chomp == line.chomp
end
end
def file_path
resource[:tempest_conf_path]
end
def exists?
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.exists?
end
def create
handle_create_with_match
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :value => get_image_id, :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.create
end
def destroy
handle_create_with_match
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.destroy
end
def get_image_id
@ -40,49 +47,4 @@ Puppet::Type.type(:tempest_glance_id_setter).provide(
end
@image_id
end
def should_line
"#{resource[:name]} = #{get_image_id}"
end
def match
/^\s*#{resource[:name]}\s*=\s*/
end
def handle_create_with_match()
regex = match
match_count = lines.select { |l| regex.match(l) }.count
file = lines
case match_count
when 1
File.open(file_path, 'w') do |fh|
lines.each do |l|
fh.puts(regex.match(l) ? "#{should_line}" : l)
end
end
when 0
block_pos = lines.find_index { |l| /^\[compute\]/ =~ l }
if block_pos.nil?
file += ["[compute]\n", "#{should_line}\n"]
else
file.insert(block_pos+1, "#{should_line}\n")
end
File.write(file_path, file.join)
else # cannot be negative.
raise Puppet::Error, "More than one line in file \
'#{file_path}' matches pattern '#{regex}'"
end
end
private
def lines
# If this type is ever used with very large files, we should
# write this in a different way, using a temp
# file; for now assuming that this type is only used on
# small-ish config files that can fit into memory without
# too much trouble.
@lines ||= File.readlines(file_path)
end
end

67
lib/puppet/provider/tempest_neutron_net_id_setter/openstack.rb
View File

@ -7,22 +7,29 @@ Puppet::Type.type(:tempest_neutron_net_id_setter).provide(
@credentials = Puppet::Provider::Openstack::CredentialsV3.new
def exists?
lines.find do |line|
should_line.chomp == line.chomp
end
end
def file_path
resource[:tempest_conf_path]
end
def exists?
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.exists?
end
def create
handle_create_with_match
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :value => get_network_id, :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.create
end
def destroy
handle_create_with_match
conf = Puppet::Type::Tempest_config
.new(:title => resource[:name], :path => file_path)
entry = Puppet::Type.type(:tempest_config).provider(:ini_setting).new(conf)
entry.destroy
end
def get_network_id
@ -40,48 +47,4 @@ Puppet::Type.type(:tempest_neutron_net_id_setter).provide(
end
@network_id
end
def should_line
"#{resource[:name]} = #{get_network_id}"
end
def match
/^\s*#{resource[:name]}\s*=\s*/
end
def handle_create_with_match()
regex = match
match_count = lines.select { |l| regex.match(l) }.count
file = lines
case match_count
when 1
File.open(file_path, 'w') do |fh|
lines.each do |l|
fh.puts(regex.match(l) ? should_line : l)
end
end
when 0
block_pos = lines.find_index { |l| /^\[network\]/ =~ l }
if block_pos.nil?
file += ["[network]\n", "#{should_line}\n"]
else
file.insert(block_pos+1, "#{should_line}\n")
end
File.write(file_path, file.join)
else # cannot be negative.
raise Puppet::Error, "More than one line in file \
'#{file_path}' matches pattern '#{regex}'"
end
end
private
def lines
# If this type is ever used with very large files, we should
# write this in a different way, using a temp
# file; for now assuming that this type is only used on
# small-ish config files that can fit into memory without
# too much trouble.
@lines ||= File.readlines(file_path)
end
end

10
lib/puppet/type/tempest_flavor_id_setter.rb
View File

@ -1,6 +1,6 @@
Puppet::Type.newtype(:tempest_flavor_id_setter) do
#
# tempest_flavor_id_setter { 'flavor_id':
# tempest_flavor_id_setter { 'compute/flavor_id':
# tempest_conf_path => '/var/lib/tempest/etc/tempest.conf',
# flavor_name => $name,
# }
@ -10,6 +10,14 @@ Puppet::Type.newtype(:tempest_flavor_id_setter) do
newparam(:name, :namevar => true) do
desc 'name of the setting to update'
munge do |value|
if value.include? '/'
value
else
# This is to keep the backword compatibility
"compute/#{value}"
end
end
end
newparam(:tempest_conf_path) do

10
lib/puppet/type/tempest_glance_id_setter.rb
View File

@ -1,6 +1,6 @@
Puppet::Type.newtype(:tempest_glance_id_setter) do
#
# tempest_glance_id_setter { 'image_id':
# tempest_glance_id_setter { 'compute/image_id':
# tempest_conf_path => '/var/lib/tempest/etc/tempest.conf',
# image_name => $name,
# }
@ -10,6 +10,14 @@ Puppet::Type.newtype(:tempest_glance_id_setter) do
newparam(:name, :namevar => true) do
desc 'name of the setting to update'
munge do |value|
if value.include? '/'
value
else
# This is to keep the backword compatibility
"compute/#{value}"
end
end
end
newparam(:tempest_conf_path) do

8
lib/puppet/type/tempest_neutron_net_id_setter.rb
View File

@ -4,6 +4,14 @@ Puppet::Type.newtype(:tempest_neutron_net_id_setter) do
newparam(:name, :namevar => true) do
desc 'The name of the setting to update.'
munge do |value|
if value.include? '/'
value
else
# This is to keep the backword compatibility
"network/#{value}"
end
end
end
newparam(:tempest_conf_path) do

44
manifests/init.pp
View File

@ -218,7 +218,7 @@
# Defaults to $::os_service_default
# [*db_flavor_ref*]
# Valid primary flavor to use in Trove tests.
# Defaults to $::os_service_default
# Defaults to undef
# [*db_flavor_name*]
# Defaults to undef
# [*baremetal_driver*]
@ -334,7 +334,7 @@ class tempest(
# Sahara config
$sahara_plugins = undef,
# Trove config
$db_flavor_ref = $::os_service_default,
$db_flavor_ref = undef,
$db_flavor_name = undef,
# Service configuration
$cinder_available = true,
@ -770,38 +770,38 @@ class tempest(
}
if ! $flavor_ref and $flavor_name {
tempest_flavor_id_setter { 'flavor_ref':
tempest_flavor_id_setter { 'compute/flavor_ref':
ensure => present,
tempest_conf_path => $tempest_conf,
flavor_name => $flavor_name,
}
Tempest_config<||> -> Tempest_flavor_id_setter['flavor_ref']
Keystone_user_role<||> -> Tempest_flavor_id_setter['flavor_ref']
Tempest_config<||> -> Tempest_flavor_id_setter['compute/flavor_ref']
Keystone_user_role<||> -> Tempest_flavor_id_setter['compute/flavor_ref']
} elsif ($flavor_name and $flavor_ref) {
fail('flavor_ref and flavor_name are both set: please set only one of them')
}
if ! $flavor_ref_alt and $flavor_name_alt {
tempest_flavor_id_setter { 'flavor_ref_alt':
tempest_flavor_id_setter { 'compute/flavor_ref_alt':
ensure => present,
tempest_conf_path => $tempest_conf,
flavor_name => $flavor_name_alt,
}
Tempest_config<||> -> Tempest_flavor_id_setter['flavor_ref_alt']
Keystone_user_role<||> -> Tempest_flavor_id_setter['flavor_ref_alt']
Tempest_config<||> -> Tempest_flavor_id_setter['compute/flavor_ref_alt']
Keystone_user_role<||> -> Tempest_flavor_id_setter['compute/flavor_ref_alt']
} elsif ($flavor_name_alt and $flavor_ref_alt) {
fail('flavor_ref_alt and flavor_name_alt are both set: please set only one of them')
}
if is_service_default($db_flavor_ref) and $db_flavor_name {
tempest_flavor_id_setter { 'db_flavor_ref':
if ! $db_flavor_ref and $db_flavor_name {
tempest_flavor_id_setter { 'database/db_flavor_ref':
ensure => present,
tempest_conf_path => $tempest_conf,
flavor_name => $db_flavor_name,
}
Tempest_config<||> -> Tempest_flavor_id_setter['db_flavor_ref']
Keystone_user_role<||> -> Tempest_flavor_id_setter['db_flavor_ref']
} elsif ($db_flavor_name and ! is_service_default($db_flavor_ref)) {
Tempest_config<||> -> Tempest_flavor_id_setter['database/db_flavor_ref']
Keystone_user_role<||> -> Tempest_flavor_id_setter['database/db_flavor_ref']
} elsif ($db_flavor_name and $db_flavor_ref) {
fail('db_flavor_ref and db_flavor_name are both set: please set only one of them')
}
@ -809,24 +809,24 @@ class tempest(
if ! $image_ref and $image_name {
# If the image id was not provided, look it up via the image name
# and set the value in the conf file.
tempest_glance_id_setter { 'image_ref':
tempest_glance_id_setter { 'compute/image_ref':
ensure => present,
tempest_conf_path => $tempest_conf,
image_name => $image_name,
}
Tempest_config<||> -> Tempest_glance_id_setter['image_ref']
Keystone_user_role<||> -> Tempest_glance_id_setter['image_ref']
Tempest_config<||> -> Tempest_glance_id_setter['compute/image_ref']
Keystone_user_role<||> -> Tempest_glance_id_setter['compute/image_ref']
} elsif ($image_name and $image_ref) or (! $image_name and ! $image_ref) {
fail('A value for either image_name or image_ref must be provided.')
}
if ! $image_ref_alt and $image_name_alt {
tempest_glance_id_setter { 'image_ref_alt':
tempest_glance_id_setter { 'compute/image_ref_alt':
ensure => present,
tempest_conf_path => $tempest_conf,
image_name => $image_name_alt,
}
Tempest_config<||> -> Tempest_glance_id_setter['image_ref_alt']
Keystone_user_role<||> -> Tempest_glance_id_setter['image_ref_alt']
Tempest_config<||> -> Tempest_glance_id_setter['compute/image_ref_alt']
Keystone_user_role<||> -> Tempest_glance_id_setter['compute/image_ref_alt']
} elsif ($image_name_alt and $image_ref_alt) or (! $image_name_alt and ! $image_ref_alt) {
fail('A value for either image_name_alt or image_ref_alt must \
be provided.')
@ -835,13 +835,13 @@ be provided.')
if $neutron_available and $configure_networks {
if ! $public_network_id and $public_network_name {
tempest_neutron_net_id_setter { 'public_network_id':
tempest_neutron_net_id_setter { 'network/public_network_id':
ensure => present,
tempest_conf_path => $tempest_conf,
network_name => $public_network_name,
}
Tempest_config<||> -> Tempest_neutron_net_id_setter['public_network_id']
Keystone_user_role<||> -> Tempest_neutron_net_id_setter['public_network_id']
Tempest_config<||> -> Tempest_neutron_net_id_setter['network/public_network_id']
Keystone_user_role<||> -> Tempest_neutron_net_id_setter['network/public_network_id']
} elsif ($public_network_name and $public_network_id) or (! $public_network_name and ! $public_network_id) {
fail('A value for either public_network_id or public_network_name \
must be provided.')

11
releasenotes/notes/bug-1978848-c1f78da88d406f98.yaml Normal file
View File

@ -0,0 +1,11 @@
---
features:
- |
The id setter resources type now accepts a resource name in
``<section>/<parameter name>`` format, to use arbitrary section name.
fixes:
- |
Fixed the wrong section(``[DEFAULT]`` instead of ``[database]``) used by
automated resolution of image id value using the ``db_flavor_name``
parameter.

18
spec/classes/tempest_init_spec.rb
View File

@ -42,7 +42,7 @@ describe 'tempest' do
end
it 'uses a resource to configure image_ref from image_name' do
is_expected.to contain_tempest_glance_id_setter('image_ref').with_image_name('cirros')
is_expected.to contain_tempest_glance_id_setter('compute/image_ref').with_image_name('cirros')
end
end
@ -77,7 +77,7 @@ describe 'tempest' do
end
it 'uses a resource to configure public_network_id from public_network_name' do
is_expected.to contain_tempest_neutron_net_id_setter('public_network_id').with_network_name('public')
is_expected.to contain_tempest_neutron_net_id_setter('network/public_network_id').with_network_name('public')
end
end
@ -242,7 +242,7 @@ describe 'tempest' do
is_expected.to contain_tempest_config('network/public_router_id').with(:value => '')
is_expected.to contain_tempest_config('dashboard/login_url').with(:value => nil)
is_expected.to contain_tempest_config('dashboard/dashboard_url').with(:value => nil)
is_expected.to contain_tempest_config('database/db_flavor_ref').with_value('<SERVICE DEFAULT>')
is_expected.to contain_tempest_config('database/db_flavor_ref').with(:value => nil)
is_expected.to contain_tempest_config('service_available/cinder').with(:value => true)
is_expected.to contain_tempest_config('volume-feature-enabled/backup').with(:value => false)
is_expected.to contain_tempest_config('service_available/glance').with(:value => true)
@ -299,13 +299,13 @@ describe 'tempest' do
end
it 'set glance id' do
is_expected.to contain_tempest_glance_id_setter('image_ref').with(
is_expected.to contain_tempest_glance_id_setter('compute/image_ref').with(
:ensure => 'present',
:tempest_conf_path => '/var/lib/tempest/etc/tempest.conf',
:image_name => 'image name',
)
is_expected.to contain_tempest_glance_id_setter('image_ref_alt').with(
is_expected.to contain_tempest_glance_id_setter('compute/image_ref_alt').with(
:ensure => 'present',
:tempest_conf_path => '/var/lib/tempest/etc/tempest.conf',
:image_name => 'image name alt',
@ -313,7 +313,7 @@ describe 'tempest' do
end
it 'neutron net id' do
is_expected.to contain_tempest_neutron_net_id_setter('public_network_id').with(
is_expected.to contain_tempest_neutron_net_id_setter('network/public_network_id').with(
:ensure => 'present',
:tempest_conf_path => '/var/lib/tempest/etc/tempest.conf',
:network_name => 'network name',
@ -393,17 +393,17 @@ describe 'tempest' do
end
it "sets flavor id using setter" do
is_expected.to contain_tempest_flavor_id_setter('flavor_ref').with(
is_expected.to contain_tempest_flavor_id_setter('compute/flavor_ref').with(
:ensure => 'present',
:tempest_conf_path => '/var/lib/tempest/etc/tempest.conf',
:flavor_name => 'm1.tiny',
)
is_expected.to contain_tempest_flavor_id_setter('flavor_ref_alt').with(
is_expected.to contain_tempest_flavor_id_setter('compute/flavor_ref_alt').with(
:ensure => 'present',
:tempest_conf_path => '/var/lib/tempest/etc/tempest.conf',
:flavor_name => 'm1.nano',
)
is_expected.to contain_tempest_flavor_id_setter('db_flavor_ref').with(
is_expected.to contain_tempest_flavor_id_setter('database/db_flavor_ref').with(
:ensure => 'present',
:tempest_conf_path => '/var/lib/tempest/etc/tempest.conf',
:flavor_name => 'm1.micro',

10
spec/unit/puppet/provider/ruby_spec.rb
View File

@ -62,11 +62,11 @@ EOS
validate_file(<<-EOS
# This is a comment
[compute]
image_ref = abcdef
; This is also a comment
foo=foovalue
bar = barvalue
master = true
image_ref=abcdef
[network]
foo= foovalue2
[blah]
@ -90,8 +90,8 @@ foo=foovalue
bar = barvalue
master = true
[network]
public_network_id = abcdef
foo= foovalue2
public_network_id=abcdef
[blah]
EOS
@ -115,8 +115,9 @@ EOS
validate_file(<<-EOS
# This is a comment
[compute]
image_ref = abcdef
image_ref=abcdef
EOS
)
end
@ -131,8 +132,9 @@ EOS
validate_file(<<-EOS
# This is a comment
[network]
public_network_id = abcdef
public_network_id=abcdef
EOS
)
end