openstack
/
puppet-cinder
Code Issues Proposed changes

lvm: Ensure target_ip_address is purged 

... instead of leaving the option unmanaged.

This also removes the reference in default definitions from the defined
resource type to the params class, which does not work if the params
class is not explicitly included in advance.

Unit tests coverage is also improved to ensure both default parameters
and customized parameters work.

Change-Id: I0b8199a27377a7d035f882586979a678b061e5da
This commit is contained in:
Takashi Kajinami 2023-10-07 11:40:24 +09:00
parent 90ac98f5de
commit e4bac84648
2 changed files with 105 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
manifests/backend/iscsi.pp
View File

@ -4,11 +4,11 @@
# === Parameters:
#
# [*target_ip_address*]
# (optional) The IP address that the iSCSI daemon is listening on.
# Defaults to undef.
# (Optional) The IP address that the iSCSI daemon is listening on.
# Defaults to $facts['os_service_default'].
#
# [*volume_backend_name*]
# (optional) Allows for the volume_backend_name to be separate of $name.
# (Optional) Allows for the volume_backend_name to be separate of $name.
# Defaults to: $name
#
# [*backend_availability_zone*]
@ -31,7 +31,7 @@
#
# [*target_helper*]
# (Optional) iSCSI target user-land tool to use.
# Defaults to '$::cinder::params::target_helper'.
# Defaults to $::cinder::params::target_helper.
#
# [*target_protocol*]
# (Optional) Protocol to use as iSCSI driver
@ -44,19 +44,19 @@
# Defaults to false.
#
# [*extra_options*]
# (optional) Hash of extra options to pass to the backend stanza
# (Optional) Hash of extra options to pass to the backend stanza
# Defaults to: {}
# Example :
# { 'iscsi_backend/param1' => { 'value' => value1 } }
#
define cinder::backend::iscsi (
$target_ip_address = undef,
$target_ip_address = $facts['os_service_default'],
$volume_backend_name = $name,
$backend_availability_zone = $facts['os_service_default'],
$volume_driver = 'cinder.volume.drivers.lvm.LVMVolumeDriver',
$volume_group = $facts['os_service_default'],
$volumes_dir = '/var/lib/cinder/volumes',
$target_helper = $::cinder::params::target_helper,
$target_helper = undef,
$target_protocol = $facts['os_service_default'],
Boolean $manage_volume_type = false,
Hash $extra_options = {},
@ -65,6 +65,11 @@ define cinder::backend::iscsi (
include cinder::deps
include cinder::params
$target_helper_real = $target_helper ? {
undef => $::cinder::params::target_helper,
default => $target_helper,
}
# NOTE(mnaser): Cinder requires /usr/sbin/thin_check to create volumes which
# does not get installed with Cinder (see LP#1615134).
if $facts['os']['family'] == 'Debian' {
@ -79,7 +84,7 @@ define cinder::backend::iscsi (
"${name}/backend_availability_zone": value => $backend_availability_zone;
"${name}/volume_driver": value => $volume_driver;
"${name}/target_ip_address": value => $target_ip_address;
"${name}/target_helper": value => $target_helper;
"${name}/target_helper": value => $target_helper_real;
"${name}/volume_group": value => $volume_group;
"${name}/volumes_dir": value => $volumes_dir;
"${name}/target_protocol": value => $target_protocol;
@ -94,7 +99,7 @@ define cinder::backend::iscsi (
create_resources('cinder_config', $extra_options)
case $target_helper {
case $target_helper_real {
'tgtadm': {
ensure_packages('tgt', {
'ensure' => present,
@ -133,7 +138,7 @@ define cinder::backend::iscsi (
}
default: {
fail("Unsupported target helper: ${target_helper}.")
fail("Unsupported target helper: ${target_helper_real}.")
}
}

135
spec/defines/cinder_backend_iscsi_spec.rb
View File

@ -3,70 +3,92 @@ require 'spec_helper'
describe 'cinder::backend::iscsi' do
let(:title) {'hippo'}
let :req_params do
{
:target_ip_address => '127.0.0.2',
:target_helper => 'tgtadm',
}
end
let :params do
req_params
end
let :iser_params do
{
:target_protocol => 'iser'
}
end
let :volumes_dir_params do
{
:volumes_dir => '/etc/cinder/volumes'
}
end
shared_examples 'cinder::backend::iscsi' do
context 'with default params' do
it {
is_expected.to contain_cinder_config('hippo/volume_backend_name').with_value('hippo')
is_expected.to contain_cinder_config('hippo/backend_availability_zone').with_value('<SERVICE DEFAULT>')
is_expected.to contain_cinder_config('hippo/volume_driver').with_value('cinder.volume.drivers.lvm.LVMVolumeDriver')
is_expected.to contain_cinder_config('hippo/target_ip_address').with_value('127.0.0.2')
is_expected.to contain_cinder_config('hippo/target_helper').with_value('tgtadm')
is_expected.to contain_cinder_config('hippo/target_ip_address').with_value('<SERVICE DEFAULT>')
is_expected.to contain_cinder_config('hippo/target_helper').with_value(platform_params[:target_helper])
is_expected.to contain_cinder_config('hippo/volume_group').with_value('<SERVICE DEFAULT>')
is_expected.to contain_cinder_config('hippo/volumes_dir').with_value('/var/lib/cinder/volumes')
is_expected.to contain_cinder_config('hippo/target_protocol').with_value('<SERVICE DEFAULT>')
}
end
context 'with parameters' do
let :params do
{
:backend_availability_zone => 'nova',
:target_ip_address => '192.0.2.1',
:volume_group => 'volumegroup',
:volumes_dir => '/etc/cinder/volumes',
:target_protocol => 'iser'
}
end
it {
is_expected.to contain_cinder_config('hippo/backend_availability_zone').with_value('nova')
is_expected.to contain_cinder_config('hippo/target_ip_address').with_value('192.0.2.1')
is_expected.to contain_cinder_config('hippo/volume_group').with_value('volumegroup')
is_expected.to contain_cinder_config('hippo/volumes_dir').with_value('/etc/cinder/volumes')
is_expected.to contain_cinder_config('hippo/target_protocol').with_value('iser')
}
end
context 'with tatadm' do
let :params do
{
:target_helper => 'tgtadm',
}
end
it {
is_expected.to contain_cinder_config('hippo/target_helper').with_value('tgtadm')
}
it { is_expected.to contain_package('tgt').with(
:ensure => 'installed',
:name => platform_params[:tgt_package_name],
:tag => 'cinder-support-package',
) }
it { is_expected.to contain_file_line('cinder include /var/lib/cinder/volumes').with(
:line => 'include /var/lib/cinder/volumes/*',
:path => '/etc/tgt/targets.conf'
)}
it { is_expected.to contain_service('tgtd').with(
:ensure => 'running',
:enable => true,
:name => platform_params[:tgt_service_name],
:tag => 'cinder-support-service',
) }
end
context 'with iser protocol' do
before :each do
params.merge!(iser_params)
context 'with lioadm' do
let :params do
{
:target_helper => 'lioadm',
}
end
it { is_expected.to contain_cinder_config('hippo/target_protocol').with_value('iser') }
end
context 'with non-default volumes_dir' do
before :each do
params.merge!(volumes_dir_params)
end
it { is_expected.to contain_cinder_config('hippo/volumes_dir').with_value('/etc/cinder/volumes') }
it { is_expected.to contain_file_line('cinder include /etc/cinder/volumes').with(
:line => 'include /etc/cinder/volumes/*',
:path => '/etc/tgt/targets.conf'
)}
it {
is_expected.to contain_cinder_config('hippo/target_helper').with_value('lioadm')
}
it { is_expected.to contain_package('targetcli').with(
:ensure => 'installed',
:name => platform_params[:lio_package_name],
:tag => 'cinder-support-package',
) }
it { is_expected.to contain_service('target').with(
:ensure => 'running',
:enable => true,
:tag => 'cinder-support-service',
) }
end
context 'iscsi backend with cinder type' do
before :each do
params.merge!( :manage_volume_type => true )
let :params do
{
:manage_volume_type => true
}
end
it { is_expected.to contain_cinder_type('hippo').with(
@ -76,8 +98,12 @@ describe 'cinder::backend::iscsi' do
end
context 'iscsi backend with additional configuration' do
before :each do
params.merge!( :extra_options => {'hippo/param1' => {'value' => 'value1'}} )
let :params do
{
:extra_options => {
'hippo/param1' => {'value' => 'value1'},
}
}
end
it { is_expected.to contain_cinder_config('hippo/param1').with(
@ -94,6 +120,25 @@ describe 'cinder::backend::iscsi' do
facts.merge!(OSDefaults.get_facts())
end
let :platform_params do
case facts[:os]['family']
when 'Debian'
{
:target_helper => 'tgtadm',
:tgt_package_name => 'tgt',
:tgt_service_name => 'tgt',
:lio_package_name => 'targetcli',
}
when 'RedHat'
{
:target_helper => 'lioadm',
:tgt_package_name => 'scsi-target-utils',
:tgt_service_name => 'tgtd',
:lio_package_name => 'targetcli',
}
end
end
it_behaves_like 'cinder::backend::iscsi'
end
end