From 77138476e03534da6de378f8e9576be84e370c75 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Wed, 16 Feb 2022 18:50:07 +0900 Subject: [PATCH] nova_flavor: Add the new project_name property The nova_flavor resource has been providing the project property which accepts both project name and id. However this implementation results in broken idempotency with project name used. This change introduces a separate project_name property, so that users can use project name with proper idempotency. Closes-Bug: #1790795 Change-Id: Idee4af6931b8cf4a21d88f4cd38fe83468ec8efa --- lib/puppet/provider/nova_flavor/openstack.rb | 80 +++++++++---- lib/puppet/type/nova_flavor.rb | 26 ++++- ..._flavor-project_name-fbab75b57ad9d4b3.yaml | 6 + spec/acceptance/nova_wsgi_apache_spec.rb | 28 +++-- .../provider/nova_flavor/openstack_spec.rb | 107 ++++++++++++++++-- 5 files changed, 208 insertions(+), 39 deletions(-) create mode 100644 releasenotes/notes/nova_flavor-project_name-fbab75b57ad9d4b3.yaml diff --git a/lib/puppet/provider/nova_flavor/openstack.rb b/lib/puppet/provider/nova_flavor/openstack.rb index 055e369c2..1e590128a 100644 --- a/lib/puppet/provider/nova_flavor/openstack.rb +++ b/lib/puppet/provider/nova_flavor/openstack.rb @@ -32,11 +32,26 @@ Puppet::Type.type(:nova_flavor).provide( prop_opts << props_to_s(@resource[:properties]) self.class.request('flavor', 'set', prop_opts) end + if @resource[:project] and @resource[:project] != '' proj_opts = [@resource[:name]] proj_opts << '--project' << @resource[:project] self.class.request('flavor', 'set', proj_opts) + + project = self.class.request('project', 'show', @resource[:project]) + @property_hash[:project_name] = project[:name] + @property_hash[:project] = project[:id] + + elsif @resource[:project_name] and @resource[:project_name] != '' + proj_opts = [@resource[:name]] + proj_opts << '--project' << @resource[:project_name] + self.class.request('flavor', 'set', proj_opts) + + project = self.class.request('project', 'show', @resource[:project_name]) + @property_hash[:project_name] = project[:name] + @property_hash[:project] = project[:id] end + @property_hash[:ensure] = :present end @@ -73,6 +88,10 @@ Puppet::Type.type(:nova_flavor).provide( @project_flush[:project] = value end + def project_name=(value) + @project_flush[:project_name] = value + end + def self.instances request('flavor', 'list', ['--long', '--all']).collect do |attrs| project = request('flavor', 'show', [attrs[:id], '-c', 'access_project_ids']) @@ -89,20 +108,31 @@ Puppet::Type.type(:nova_flavor).provide( project_value = access_project_ids end project_value = project_value.gsub('\'', '') + + if project_value != '' + project = request('project', 'show', project_value) + project_id = project[:id] + project_name = project[:name] + else + project_id = '' + project_name = '' + end + properties = Hash[attrs[:properties].scan(/(\S+)='([^']*)'/)] rescue nil new( - :ensure => :present, - :name => attrs[:name], - :id => attrs[:id], - :ram => attrs[:ram], - :disk => attrs[:disk], - :ephemeral => attrs[:ephemeral], - :vcpus => attrs[:vcpus], - :is_public => attrs[:is_public].downcase.chomp == 'true'? true : false, - :swap => attrs[:swap], - :rxtx_factor => attrs[:rxtx_factor], - :properties => properties, - :project => project_value + :ensure => :present, + :name => attrs[:name], + :id => attrs[:id], + :ram => attrs[:ram], + :disk => attrs[:disk], + :ephemeral => attrs[:ephemeral], + :vcpus => attrs[:vcpus], + :is_public => attrs[:is_public].downcase.chomp == 'true'? true : false, + :swap => attrs[:swap], + :rxtx_factor => attrs[:rxtx_factor], + :properties => properties, + :project => project_id, + :project_name => project_name, ) end end @@ -124,14 +154,26 @@ Puppet::Type.type(:nova_flavor).provide( self.class.request('flavor', 'set', opts) @property_flush.clear end + unless @project_flush.empty? - opts = [@resource[:name]] - unless @project_flush[:project] == '' - opts << '--project' << @project_flush[:project] - self.class.request('flavor', 'set', opts) - else - opts << '--project' << @property_hash[:project] - self.class.request('flavor', 'unset', opts) + if @project_flush[:project] + if @property_hash[:project] and @property_hash[:project] != '' + opts = [@resource[:name], '--project', @property_hash[:project]] + self.class.request('flavor', 'unset', opts) + end + if @project_flush[:project] != '' + opts = [@resource[:name], '--project', @project_flush[:project]] + self.class.request('flavor', 'set', opts) + end + elsif @project_flush[:project_name] + if @property_hash[:project_name] and @property_hash[:project_name] != '' + opts = [@resource[:name], '--project', @property_hash[:project_name]] + self.class.request('flavor', 'unset', opts) + end + if @project_flush[:project_name] != '' + opts = [@resource[:name], '--project', @project_flush[:project_name]] + self.class.request('flavor', 'set', opts) + end end @project_flush.clear end diff --git a/lib/puppet/type/nova_flavor.rb b/lib/puppet/type/nova_flavor.rb index f99c48d96..0adae68f3 100644 --- a/lib/puppet/type/nova_flavor.rb +++ b/lib/puppet/type/nova_flavor.rb @@ -44,9 +44,15 @@ # Optional # # [*project*] -# Set flavor access to project (name or ID). +# Set flavor access to project (ID). # If you set this option, take care to set is_public to false. # Optional +# +# [*project_name*] +# Set flavor access to project (name). +# If you set this option, take care to set is_public to false. +# Optional +# require 'puppet' Puppet::Type.newtype(:nova_flavor) do @@ -60,6 +66,10 @@ Puppet::Type.newtype(:nova_flavor) do ['nova::service::end'] end + autorequire(:keystone_tenant) do + [self[:project_name]] if (self[:project_name] and self[:project_name] != '') + end + newparam(:name, :namevar => true) do desc 'Name for the flavor' validate do |value| @@ -116,8 +126,11 @@ Puppet::Type.newtype(:nova_flavor) do end newproperty(:project) do - desc 'Set flavor access to project (name or ID).' - defaultto('') + desc 'Set flavor access to project (ID).' + end + + newproperty(:project_name) do + desc 'Set flavor access to project (Name).' end newproperty(:properties) do @@ -144,6 +157,13 @@ Puppet::Type.newtype(:nova_flavor) do unless self[:name] raise(ArgumentError, 'Name must be set') end + + if self[:project] && self[:project_name] + raise(Puppet::Error, <<-EOT +Please provide a value for only one of project_name and project. +EOT + ) + end end end diff --git a/releasenotes/notes/nova_flavor-project_name-fbab75b57ad9d4b3.yaml b/releasenotes/notes/nova_flavor-project_name-fbab75b57ad9d4b3.yaml new file mode 100644 index 000000000..8ad971862 --- /dev/null +++ b/releasenotes/notes/nova_flavor-project_name-fbab75b57ad9d4b3.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + The ``nova_flavor`` resource now supports the ``project_name`` property. + This property should be used instead of the ``project`` property when + project name is used instead of project id. diff --git a/spec/acceptance/nova_wsgi_apache_spec.rb b/spec/acceptance/nova_wsgi_apache_spec.rb index 515c6ba58..24b0b509f 100644 --- a/spec/acceptance/nova_wsgi_apache_spec.rb +++ b/spec/acceptance/nova_wsgi_apache_spec.rb @@ -23,14 +23,23 @@ describe 'basic nova' do require => Class['nova::api'], } - nova_flavor { 'test_flavor': - ensure => present, - name => 'test_flavor', - id => '9999', - ram => '512', - disk => '1', - vcpus => '1', - require => [ Class['nova::api'], Class['nova::keystone::auth'] ], + nova_flavor { 'public_flavor': + ensure => present, + name => 'public_flavor', + id => '42', + ram => '512', + disk => '1', + vcpus => '1', + } + nova_flavor { 'private_flavor': + ensure => present, + name => 'private_flavor', + id => '43', + ram => '512', + disk => '1', + vcpus => '1', + is_public => 'False', + project_name => 'services' } EOS @@ -72,7 +81,8 @@ describe 'basic nova' do describe 'nova flavor' do it 'should create new flavor' do command('openstack --os-identity-api-version 3 --os-username nova --os-password a_big_secret --os-tenant-name services --os-user-domain-name Default --os-project-domain-name Default --os-auth-url http://127.0.0.1:5000/v3 flavor list') do |r| - expect(r.stdout).to match(/test_flavor/) + expect(r.stdout).to match(/public_flavor/) + expect(r.stdout).to match(/private_flavor/) end end end diff --git a/spec/unit/provider/nova_flavor/openstack_spec.rb b/spec/unit/provider/nova_flavor/openstack_spec.rb index 0daa0bee5..0b95d58a4 100644 --- a/spec/unit/provider/nova_flavor/openstack_spec.rb +++ b/spec/unit/provider/nova_flavor/openstack_spec.rb @@ -27,13 +27,11 @@ describe provider_class do end describe '#create' do - it 'creates flavor' do - provider.class.stubs(:openstack) - .with('flavor', 'list', ['--long', '--all']) - .returns('"ID", "Name", "RAM", "Disk", "Ephemeral", "VCPUs", "Is Public", "Swap", "RXTX Factor", "Properties"') - provider.class.stubs(:openstack) + context 'with defaults' do + it 'creates flavor' do + provider.class.expects(:openstack) .with('flavor', 'create', '--format', 'shell', ['example', '--public', '--id', '1', '--ram', '512', '--disk', '1', '--vcpus', '1']) - .returns('os-flv-disabled:disabled="False" + .returns('os-flv-disabled:disabled="False" os-flv-ext-data:ephemeral="0" disk="1" id="1" @@ -43,8 +41,81 @@ ram="512" rxtx_factor="1.0" swap="" vcpus="1"') - provider.create - expect(provider.exists?).to be_truthy + provider.create + expect(provider.exists?).to be_truthy + end + end + + context 'with project' do + before do + flavor_attrs.merge!( + :project => '3073e17b-fb7f-4524-bdcd-c54bc70e9da9' + ) + end + + it 'creates flavor' do + provider.class.expects(:openstack) + .with('flavor', 'create', '--format', 'shell', ['example', '--public', '--id', '1', '--ram', '512', '--disk', '1', '--vcpus', '1']) + .returns('os-flv-disabled:disabled="False" +os-flv-ext-data:ephemeral="0" +disk="1" +id="1" +name="example" +os-flavor-access:is_public="True" +ram="512" +rxtx_factor="1.0" +swap="" +vcpus="1"') + provider.class.expects(:openstack) + .with('flavor', 'set', ['example', '--project', '3073e17b-fb7f-4524-bdcd-c54bc70e9da9']) + provider.class.expects(:openstack) + .with('project', 'show', '--format', 'shell', '3073e17b-fb7f-4524-bdcd-c54bc70e9da9') + .returns('enabled="True" +name="admin" +id="3073e17b-fb7f-4524-bdcd-c54bc70e9da9" +domain_id="domain_one_id" +') + provider.create + expect(provider.exists?).to be_truthy + expect(provider.project).to eq('3073e17b-fb7f-4524-bdcd-c54bc70e9da9') + expect(provider.project_name).to eq('admin') + end + end + + context 'with project_name' do + before do + flavor_attrs.merge!( + :project_name => 'admin' + ) + end + + it 'creates flavor with project_name' do + provider.class.expects(:openstack) + .with('flavor', 'create', '--format', 'shell', ['example', '--public', '--id', '1', '--ram', '512', '--disk', '1', '--vcpus', '1']) + .returns('os-flv-disabled:disabled="False" +os-flv-ext-data:ephemeral="0" +disk="1" +id="1" +name="example" +os-flavor-access:is_public="True" +ram="512" +rxtx_factor="1.0" +swap="" +vcpus="1"') + provider.class.expects(:openstack) + .with('flavor', 'set', ['example', '--project', 'admin']) + provider.class.expects(:openstack) + .with('project', 'show', '--format', 'shell', 'admin') + .returns('enabled="True" +name="admin" +id="3073e17b-fb7f-4524-bdcd-c54bc70e9da9" +domain_id="domain_one_id" +') + provider.create + expect(provider.exists?).to be_truthy + expect(provider.project).to eq('3073e17b-fb7f-4524-bdcd-c54bc70e9da9') + expect(provider.project_name).to eq('admin') + end end end @@ -57,6 +128,26 @@ vcpus="1"') expect(provider.exists?).to be_falsey end end + + describe '#flush' do + context '.project' do + it 'updates flavor' do + provider.class.expects(:openstack) + .with('flavor', 'set', ['example', '--project', '3073e17b-fb7f-4524-bdcd-c54bc70e9da9']) + provider.project = '3073e17b-fb7f-4524-bdcd-c54bc70e9da9' + provider.flush + end + end + + context '.project_name' do + it 'updates flavor' do + provider.class.expects(:openstack) + .with('flavor', 'set', ['example', '--project', 'admin']) + provider.project_name = 'admin' + provider.flush + end + end + end end end