From 3cff64247465270158bf5093ccec4d375e08f842 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 1 Feb 2017 15:41:21 +0100 Subject: [PATCH] Support new driver configuration options Allows enabling hardware types in addition to classic drivers. Also allows providing enabled and default implementations for all hardware interfaces (not only network one). Required for TripleO blueprint ironic-driver-composition. Change-Id: Iaec6c52fcee66dc1bba315dcd6facd902b59d7f5 Related-Bug: #1524745 --- manifests/conductor.pp | 10 +- manifests/drivers/hardware_interface.pp | 49 +++++++ manifests/drivers/interfaces.pp | 138 ++++++++++++++++-- .../driver-interfaces-189be236ebc05337.yaml | 6 + spec/classes/ironic_conductor_spec.rb | 4 + .../classes/ironic_drivers_interfaces_spec.rb | 35 ++++- .../ironic_drivers_hardware_interface_spec.rb | 64 ++++++++ 7 files changed, 285 insertions(+), 21 deletions(-) create mode 100644 manifests/drivers/hardware_interface.pp create mode 100644 releasenotes/notes/driver-interfaces-189be236ebc05337.yaml create mode 100644 spec/defines/ironic_drivers_hardware_interface_spec.rb diff --git a/manifests/conductor.pp b/manifests/conductor.pp index 223f743d..a0f48ed6 100644 --- a/manifests/conductor.pp +++ b/manifests/conductor.pp @@ -31,6 +31,10 @@ # (optional) Array of drivers to load during service initialization. # Defaults to ['pxe_ipmitool']. # +# [*enabled_hardware_types*] +# (optional) Array of hardware types to load during service initialization. +# Defaults to ['ipmi']. +# # [*max_time_interval*] # (optional) Maximum time, in seconds, since the last check-in of a conductor. # Should be an interger value @@ -125,6 +129,7 @@ class ironic::conductor ( $package_ensure = 'present', $enabled = true, $enabled_drivers = ['pxe_ipmitool'], + $enabled_hardware_types = ['ipmi'], $max_time_interval = '120', $force_power_state_during_sync = true, $http_url = $::os_service_default, @@ -170,6 +175,7 @@ moved to ironic::glance manifest") } validate_array($enabled_drivers_real) + validate_array($enabled_hardware_types) # NOTE(dtantsur): all in-tree drivers are IPA-based, so it won't hurt # including its manifest (which only contains configuration options) @@ -177,7 +183,8 @@ moved to ironic::glance manifest") # On Ubuntu, ipmitool dependency is missing and ironic-conductor fails to start. # https://bugs.launchpad.net/cloud-archive/+bug/1572800 - if member($enabled_drivers_real, 'pxe_ipmitool') and $::osfamily == 'Debian' { + if (member($enabled_drivers_real, 'pxe_ipmitool') or + member($enabled_hardware_types, 'ipmi')) and $::osfamily == 'Debian' { ensure_packages('ipmitool', { ensure => $package_ensure, @@ -215,6 +222,7 @@ moved to ironic::glance manifest") # Configure ironic.conf ironic_config { 'DEFAULT/enabled_drivers': value => join($enabled_drivers_real, ','); + 'DEFAULT/enabled_hardware_types': value => join($enabled_hardware_types, ','); 'conductor/max_time_interval': value => $max_time_interval; 'conductor/force_power_state_during_sync': value => $force_power_state_during_sync; 'conductor/automated_clean': value => $automated_clean; diff --git a/manifests/drivers/hardware_interface.pp b/manifests/drivers/hardware_interface.pp new file mode 100644 index 00000000..bee2169b --- /dev/null +++ b/manifests/drivers/hardware_interface.pp @@ -0,0 +1,49 @@ +# +# Copyright (C) 2016 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +# Internal define for hardware interfaces declaration +# +# === Parameters +# +# [*interface_type*] +# Interface type name (e.g. 'boot'). +# Defaults to namevar. +# +# [*enabled_list*] +# List of enabled implementations. +# Defaults to $::os_service_default +# +# [*default*] +# The default implementation to use when none is requested by a user. +# Defaults to $::os_service_default +# +define ironic::drivers::hardware_interface ( + $interface_type = $title, + $enabled_list = $::os_service_default, + $default = $::os_service_default, +) { + if !is_service_default($enabled_list) and !empty($enabled_list){ + validate_array($enabled_list) + $enabled_list_real = join($enabled_list, ',') + } else { + $enabled_list_real = $::os_service_default + } + + ironic_config { + "DEFAULT/enabled_${interface_type}_interfaces": value => $enabled_list_real; + "DEFAULT/default_${interface_type}_interface": value => $default; + } +} + diff --git a/manifests/drivers/interfaces.pp b/manifests/drivers/interfaces.pp index ba2c7f6f..56d7c17b 100644 --- a/manifests/drivers/interfaces.pp +++ b/manifests/drivers/interfaces.pp @@ -14,25 +14,137 @@ # # === Parameters # -# [*enabled_network_interfaces*] -# (optional) Specify the list of network drivers to load during +# [*default_boot_interface*] +# (optional) Default boot interface to be used for nodes that do not have +# boot_interface field set. +# Defaults to $::os_service_default +# +# [*default_console_interface*] +# (optional) Default console interface to be used for nodes that do not have +# console_interface field set. +# Defaults to $::os_service_default +# +# [*default_deploy_interface*] +# (optional) Default deploy interface to be used for nodes that do not have +# deploy_interface field set. +# Defaults to $::os_service_default +# +# [*default_inspect_interface*] +# (optional) Default inspect interface to be used for nodes that do not have +# inspect_interface field set. +# Defaults to $::os_service_default +# +# [*default_management_interface*] +# (optional) Default management interface to be used for nodes that do not have +# management_interface field set. +# Defaults to $::os_service_default +# +# [*default_network_interface*] +# (optional) Default network interface to be used for nodes that do not have +# network_interface field set. +# Defaults to $::os_service_default +# +# [*default_power_interface*] +# (optional) Default power interface to be used for nodes that do not have +# power_interface field set. +# Defaults to $::os_service_default +# +# [*default_raid_interface*] +# (optional) Default raid interface to be used for nodes that do not have +# raid_interface field set. +# Defaults to $::os_service_default +# +# [*default_vendor_interface*] +# (optional) Default vendor interface to be used for nodes that do not have +# vendor_interface field set. +# Defaults to $::os_service_default +# +# [*enabled_boot_interfaces*] +# (optional) Specify the list of boot interfaces to load during +# service initialization. +# Defaults to $::os_service_default +# +# [*enabled_console_interfaces*] +# (optional) Specify the list of console interfaces to load during +# service initialization. +# Defaults to $::os_service_default +# +# [*enabled_deploy_interfaces*] +# (optional) Specify the list of deploy interfaces to load during +# service initialization. +# Defaults to $::os_service_default +# +# [*enabled_inspect_interfaces*] +# (optional) Specify the list of inspect interfaces to load during +# service initialization. +# Defaults to $::os_service_default +# +# [*enabled_management_interfaces*] +# (optional) Specify the list of management interfaces to load during +# service initialization. +# Defaults to $::os_service_default +# +# [*enabled_network_interfaces*] +# (optional) Specify the list of network interfaces to load during +# service initialization. +# Defaults to $::os_service_default +# +# [*enabled_power_interfaces*] +# (optional) Specify the list of power interfaces to load during +# service initialization. +# Defaults to $::os_service_default +# +# [*enabled_raid_interfaces*] +# (optional) Specify the list of raid interfaces to load during +# service initialization. +# Defaults to $::os_service_default +# +# [*enabled_vendor_interfaces*] +# (optional) Specify the list of vendor interfaces to load during # service initialization. # Defaults to $::os_service_default # - class ironic::drivers::interfaces ( - $enabled_network_interfaces = $::os_service_default, + $default_boot_interface = $::os_service_default, + $default_console_interface = $::os_service_default, + $default_deploy_interface = $::os_service_default, + $default_inspect_interface = $::os_service_default, + $default_management_interface = $::os_service_default, + $default_network_interface = $::os_service_default, + $default_power_interface = $::os_service_default, + $default_raid_interface = $::os_service_default, + $default_vendor_interface = $::os_service_default, + $enabled_boot_interfaces = $::os_service_default, + $enabled_console_interfaces = $::os_service_default, + $enabled_deploy_interfaces = $::os_service_default, + $enabled_inspect_interfaces = $::os_service_default, + $enabled_management_interfaces = $::os_service_default, + $enabled_network_interfaces = $::os_service_default, + $enabled_power_interfaces = $::os_service_default, + $enabled_raid_interfaces = $::os_service_default, + $enabled_vendor_interfaces = $::os_service_default, ) { - if !is_service_default($enabled_network_interfaces) and !empty($enabled_network_interfaces){ - validate_array($enabled_network_interfaces) - $enabled_network_interfaces_real = join($enabled_network_interfaces, ',') - } else { - $enabled_network_interfaces_real = $::os_service_default - } - - ironic_config { - 'DEFAULT/enabled_network_interfaces': value => $enabled_network_interfaces_real; + $interfaces = { + 'boot' => { 'enabled_list' => $enabled_boot_interfaces, + 'default' => $default_boot_interface }, + 'console' => { 'enabled_list' => $enabled_console_interfaces, + 'default' => $default_console_interface }, + 'deploy' => { 'enabled_list' => $enabled_deploy_interfaces, + 'default' => $default_deploy_interface }, + 'inspect' => { 'enabled_list' => $enabled_inspect_interfaces, + 'default' => $default_inspect_interface }, + 'management' => { 'enabled_list' => $enabled_management_interfaces, + 'default' => $default_management_interface }, + 'network' => { 'enabled_list' => $enabled_network_interfaces, + 'default' => $default_network_interface }, + 'power' => { 'enabled_list' => $enabled_power_interfaces, + 'default' => $default_power_interface }, + 'raid' => { 'enabled_list' => $enabled_raid_interfaces, + 'default' => $default_raid_interface }, + 'vendor' => { 'enabled_list' => $enabled_vendor_interfaces, + 'default' => $default_vendor_interface }, } + create_resources(ironic::drivers::hardware_interface, $interfaces) } diff --git a/releasenotes/notes/driver-interfaces-189be236ebc05337.yaml b/releasenotes/notes/driver-interfaces-189be236ebc05337.yaml new file mode 100644 index 00000000..26899d31 --- /dev/null +++ b/releasenotes/notes/driver-interfaces-189be236ebc05337.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Add options for configuring enabled hardware types and all currently + supported hardware interfaces. See the driver composition spec for details: + http://specs.openstack.org/openstack/ironic-specs/specs/approved/driver-composition-reform.html diff --git a/spec/classes/ironic_conductor_spec.rb b/spec/classes/ironic_conductor_spec.rb index 269f7467..251dac94 100644 --- a/spec/classes/ironic_conductor_spec.rb +++ b/spec/classes/ironic_conductor_spec.rb @@ -26,6 +26,7 @@ describe 'ironic::conductor' do { :package_ensure => 'present', :enabled => true, :enabled_drivers => ['pxe_ipmitool'], + :enabled_hardware_types => ['ipmi'], :max_time_interval => '120', :force_power_state_during_sync => true } end @@ -63,6 +64,7 @@ describe 'ironic::conductor' do it 'configures ironic.conf' do is_expected.to contain_ironic_config('DEFAULT/enabled_drivers').with_value('pxe_ipmitool') + is_expected.to contain_ironic_config('DEFAULT/enabled_hardware_types').with_value('ipmi') is_expected.to contain_ironic_config('conductor/max_time_interval').with_value(p[:max_time_interval]) is_expected.to contain_ironic_config('conductor/force_power_state_during_sync').with_value(p[:force_power_state_during_sync]) is_expected.to contain_ironic_config('conductor/automated_clean').with(:value => '') @@ -81,6 +83,7 @@ describe 'ironic::conductor' do before :each do params.merge!( :enabled_drivers => ['pxe_ssh', 'agent_ssh'], + :enabled_hardware_types => ['ipmi', 'irmc'], :max_time_interval => '50', :force_power_state_during_sync => false, :automated_clean => false, @@ -97,6 +100,7 @@ describe 'ironic::conductor' do end it 'should replace default parameter with new value' do is_expected.to contain_ironic_config('DEFAULT/enabled_drivers').with_value('pxe_ssh,agent_ssh') + is_expected.to contain_ironic_config('DEFAULT/enabled_hardware_types').with_value('ipmi,irmc') is_expected.to contain_ironic_config('conductor/max_time_interval').with_value(p[:max_time_interval]) is_expected.to contain_ironic_config('conductor/force_power_state_during_sync').with_value(p[:force_power_state_during_sync]) is_expected.to contain_ironic_config('conductor/automated_clean').with_value(p[:automated_clean]) diff --git a/spec/classes/ironic_drivers_interfaces_spec.rb b/spec/classes/ironic_drivers_interfaces_spec.rb index dd633893..f00ddb35 100644 --- a/spec/classes/ironic_drivers_interfaces_spec.rb +++ b/spec/classes/ironic_drivers_interfaces_spec.rb @@ -17,18 +17,42 @@ require 'spec_helper' describe 'ironic::drivers::interfaces' do - shared_examples_for 'ironic deploy interfaces' do + shared_examples_for 'ironic hardware interfaces' do context 'with default parameters' do + it { is_expected.to contain_ironic_config('DEFAULT/enabled_boot_interfaces').with_value('') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_console_interfaces').with_value('') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_deploy_interfaces').with_value('') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_inspect_interfaces').with_value('') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_management_interfaces').with_value('') } it { is_expected.to contain_ironic_config('DEFAULT/enabled_network_interfaces').with_value('') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_power_interfaces').with_value('') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_raid_interfaces').with_value('') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_vendor_interfaces').with_value('') } end context 'when overriding parameters' do let :params do - { :enabled_network_interfaces => ['flat','neutron'] } + { :enabled_boot_interfaces => ['pxe'], + :enabled_console_interfaces => ['socat', 'shellinabox'], + :enabled_deploy_interfaces => ['iscsi'], + :enabled_inspect_interfaces => ['inspector'], + :enabled_management_interfaces => ['ipmitool', 'irmc'], + :enabled_network_interfaces => ['flat','neutron'], + :enabled_power_interfaces => ['irmc', 'ipmitool'], + :enabled_raid_interfaces => ['agent', 'no-raid'], + :enabled_vendor_interfaces => ['no-vendor'] } end + it { is_expected.to contain_ironic_config('DEFAULT/enabled_boot_interfaces').with_value('pxe') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_console_interfaces').with_value('socat,shellinabox') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_deploy_interfaces').with_value('iscsi') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_inspect_interfaces').with_value('inspector') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_management_interfaces').with_value('ipmitool,irmc') } it { is_expected.to contain_ironic_config('DEFAULT/enabled_network_interfaces').with_value('flat,neutron') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_power_interfaces').with_value('irmc,ipmitool') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_raid_interfaces').with_value('agent,no-raid') } + it { is_expected.to contain_ironic_config('DEFAULT/enabled_vendor_interfaces').with_value('no-vendor') } end end @@ -38,12 +62,9 @@ describe 'ironic::drivers::interfaces' do }).each do |os,facts| context "on #{os}" do let (:facts) do - facts.merge(OSDefaults.get_facts({ - :processorcount => 8, - :concat_basedir => '/var/lib/puppet/concat' - })) + facts.merge(OSDefaults.get_facts()) end - it_configures 'ironic deploy interfaces' + it_configures 'ironic hardware interfaces' end end diff --git a/spec/defines/ironic_drivers_hardware_interface_spec.rb b/spec/defines/ironic_drivers_hardware_interface_spec.rb new file mode 100644 index 00000000..19f9b0ff --- /dev/null +++ b/spec/defines/ironic_drivers_hardware_interface_spec.rb @@ -0,0 +1,64 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# +# Unit tests for ironic::drivers::hardware_interface +# + +require 'spec_helper' + +describe 'ironic::drivers::hardware_interface' do + + let :params do + {} + end + + let (:title) { 'foo' } + + shared_examples_for 'ironic hardware interface' do + let :p do + params + end + + it 'configures ironic.conf' do + is_expected.to contain_ironic_config('DEFAULT/enabled_foo_interfaces').with_value('') + is_expected.to contain_ironic_config('DEFAULT/default_foo_interface').with_value('') + end + + context 'when overriding parameters' do + before :each do + params.merge!( + :enabled_list => ['one', 'two'], + :default => 'two', + ) + end + + it 'should replace default parameter with new value' do + is_expected.to contain_ironic_config('DEFAULT/enabled_foo_interfaces').with_value('one,two') + is_expected.to contain_ironic_config('DEFAULT/default_foo_interface').with_value(p[:default]) + end + end + + end + + on_supported_os({ + :supported_os => OSDefaults.get_supported_os + }).each do |os,facts| + context "on #{os}" do + let (:facts) do + facts.merge!(OSDefaults.get_facts()) + end + + it_behaves_like 'ironic hardware interface' + end + end + +end