From d0957fb622a0f5f4f6e038aa22d87ee61a2f1ad2 Mon Sep 17 00:00:00 2001 From: Michael Polenchuk Date: Tue, 29 Mar 2016 15:34:42 +0300 Subject: [PATCH] Add ability to set properties with glance_image This change updates the glance_image provider to support the ability to specify properties for an image. This change only adds the support for setting properties. The conversion and update to using the v2 will require further rework of this provider. Change-Id: I22b92c5ccd0f77c837e9abe987cee07c7d90867e Co-Authored-By: Alex Schultz --- lib/puppet/provider/glance_image/openstack.rb | 95 ++++++------- lib/puppet/type/glance_image.rb | 33 ++++- spec/acceptance/basic_glance_spec.rb | 17 ++- spec/unit/provider/glance_image_spec.rb | 125 +++++++++++++++--- 4 files changed, 193 insertions(+), 77 deletions(-) diff --git a/lib/puppet/provider/glance_image/openstack.rb b/lib/puppet/provider/glance_image/openstack.rb index 26f180fe..20760eab 100644 --- a/lib/puppet/provider/glance_image/openstack.rb +++ b/lib/puppet/provider/glance_image/openstack.rb @@ -10,10 +10,9 @@ Puppet::Type.type(:glance_image).provide( @credentials = Puppet::Provider::Openstack::CredentialsV2_0.new - # glanceclient support `image create` (in v2 API) but not openstackclient - # openstackclient now uses image v2 API by default. - # in the meantime it's implemented in openstackclient, hardcode version - # see https://bugs.launchpad.net/python-openstackclient/+bug/1405562 + # TODO(aschultz): v2 is now supported but the options are different and + # it doesn't support the source being remote. We'll have to rework this + # to support v2 ENV['OS_IMAGE_API_VERSION'] = '1' def initialize(value={}) @@ -22,33 +21,31 @@ Puppet::Type.type(:glance_image).provide( end def create - if resource[:source] + if @resource[:source] # copy_from cannot handle file:// - if resource[:source] =~ /^\// # local file - location = "--file=#{resource[:source]}" + if @resource[:source] =~ /^\// # local file + location = "--file=#{@resource[:source]}" else - location = "--copy-from=#{resource[:source]}" + location = "--copy-from=#{@resource[:source]}" end # location cannot handle file:// # location does not import, so no sense in doing anything more than this - elsif resource[:location] - location = "--location=#{resource[:location]}" + elsif @resource[:location] + location = "--location=#{@resource[:location]}" else raise(Puppet::Error, "Must specify either source or location") end - properties = [resource[:name]] - if resource[:is_public] == :true - properties << "--public" - else - # This is the default, but it's nice to be verbose - properties << "--private" - end - properties << "--container-format=#{resource[:container_format]}" - properties << "--disk-format=#{resource[:disk_format]}" - properties << "--min-disk=#{resource[:min_disk]}" if resource[:min_disk] - properties << "--min-ram=#{resource[:min_ram]}" if resource[:min_ram] - properties << location - @property_hash = self.class.request('image', 'create', properties) + opts = [@resource[:name]] + + opts << (@resource[:is_public] == :true ? '--public' : '--private') + opts << "--container-format=#{@resource[:container_format]}" + opts << "--disk-format=#{@resource[:disk_format]}" + opts << "--min-disk=#{@resource[:min_disk]}" if @resource[:min_disk] + opts << "--min-ram=#{@resource[:min_ram]}" if @resource[:min_ram] + opts << props_to_s(@resource[:properties]) if @resource[:properties] + opts << location + + @property_hash = self.class.request('image', 'create', opts) @property_hash[:ensure] = :present end @@ -57,10 +54,12 @@ Puppet::Type.type(:glance_image).provide( end def destroy - self.class.request('image', 'delete', resource[:name]) + self.class.request('image', 'delete', @resource[:name]) @property_hash.clear end + mk_resource_methods + def is_public=(value) @property_flush[:is_public] = value end @@ -73,18 +72,10 @@ Puppet::Type.type(:glance_image).provide( @property_flush[:disk_format] = value end - def disk_format - @property_hash[:disk_format] - end - def container_format=(value) @property_flush[:container_format] = value end - def container_format - @property_hash[:container_format] - end - def min_ram=(value) @property_flush[:min_ram] = value end @@ -93,18 +84,19 @@ Puppet::Type.type(:glance_image).provide( @property_flush[:min_disk] = value end + def properties=(value) + @property_flush[:properties] = value + end + def id=(id) fail('id is read only') end - def id - @property_hash[:id] - end - def self.instances - list = request('image', 'list', '--long') + list = request('image', 'list') list.collect do |image| attrs = request('image', 'show', image[:id]) + properties = Hash[attrs[:properties].scan(/(\S+)='([^']*)'/)] rescue nil new( :ensure => :present, :name => attrs[:name], @@ -112,8 +104,9 @@ Puppet::Type.type(:glance_image).provide( :container_format => attrs[:container_format], :id => attrs[:id], :disk_format => attrs[:disk_format], - :min_disk => attrs['min_disk'], - :min_ram => attrs['min_ram'] + :min_disk => attrs[:min_disk], + :min_ram => attrs[:min_ram], + :properties => properties ) end end @@ -128,17 +121,25 @@ Puppet::Type.type(:glance_image).provide( end def flush - properties = [resource[:name]] if @property_flush - (properties << '--public') if @property_flush[:is_public] == :true - (properties << '--private') if @property_flush[:is_public] == :false - (properties << "--container-format=#{@property_flush[:container_format]}") if @property_flush[:container_format] - (properties << "--disk-format=#{@property_flush[:disk_format]}") if @property_flush[:disk_format] - (properties << "--min-ram=#{@property_flush[:min_ram]}") if @property_flush[:min_ram] - (properties << "--min-disk=#{@property_flush[:min_disk]}") if @property_flush[:min_disk] - self.class.request('image', 'set', properties) + opts = [@resource[:name]] + + (opts << '--public') if @property_flush[:is_public] == :true + (opts << '--private') if @property_flush[:is_public] == :false + (opts << "--container-format=#{@property_flush[:container_format]}") if @property_flush[:container_format] + (opts << "--disk-format=#{@property_flush[:disk_format]}") if @property_flush[:disk_format] + (opts << "--min-ram=#{@property_flush[:min_ram]}") if @property_flush[:min_ram] + (opts << "--min-disk=#{@property_flush[:min_disk]}") if @property_flush[:min_disk] + (opts << props_to_s(@property_flush[:properties])) if @property_flush[:properties] + + self.class.request('image', 'set', opts) @property_flush.clear end end + private + + def props_to_s(props) + props.flat_map{ |k, v| ['--property', "#{k}=#{v}"] } + end end diff --git a/lib/puppet/type/glance_image.rb b/lib/puppet/type/glance_image.rb index d731f39f..86b6f43a 100644 --- a/lib/puppet/type/glance_image.rb +++ b/lib/puppet/type/glance_image.rb @@ -13,13 +13,14 @@ Puppet::Type.newtype(:glance_image) do source => 'http://uec-images.ubuntu.com/releases/precise/release/ubuntu-12.04-server-cloudimg-amd64-disk1.img' min_ram => 1234, min_disk => 1234, + properties => { 'img_key' => img_value } } Known problems / limitations: - * All images are managed by the glance service. + * All images are managed by the glance service. This means that since users are unable to manage their own images via this type, is_public is really of no use. You can probably hide images this way but that's all. - * As glance image names do not have to be unique, you must ensure that your glance + * As glance image names do not have to be unique, you must ensure that your glance repository does not have any duplicate names prior to using this. * Ensure this is run on the same server as the glance-api service. @@ -74,21 +75,39 @@ Puppet::Type.newtype(:glance_image) do newvalues(/\S+/) end - newparam(:min_ram) do + newproperty(:min_ram) do desc "The minimal ram size" newvalues(/\d+/) end - newparam(:min_disk) do + newproperty(:min_disk) do desc "The minimal disk size" newvalues(/\d+/) end + newproperty(:properties) do + desc "The set of image properties" + + munge do |value| + return value if value.is_a? Hash + + # wrap property value in commas + value.gsub!(/=(\w+)/, '=\'\1\'') + Hash[value.scan(/(\S+)='([^']*)'/)] + end + + validate do |value| + return true if value.is_a? Hash + + value.split(',').each do |property| + raise ArgumentError, "Key/value pairs should be separated by an =" unless property.include?('=') + end + end + end + # Require the Glance service to be running autorequire(:service) do - ['glance'] + ['glance-api', 'glance-registry'] end end - - diff --git a/spec/acceptance/basic_glance_spec.rb b/spec/acceptance/basic_glance_spec.rb index a327006a..755e8fe8 100644 --- a/spec/acceptance/basic_glance_spec.rb +++ b/spec/acceptance/basic_glance_spec.rb @@ -47,6 +47,9 @@ describe 'glance class' do disk_format => 'qcow2', is_public => 'yes', source => 'http://download.cirros-cloud.net/0.3.1/cirros-0.3.1-x86_64-disk.img', + min_ram => '64', + min_disk => '1024', + properties => { 'icanhaz' => 'cheezburger' }, } EOS @@ -77,11 +80,21 @@ describe 'glance class' do end describe 'glance images' do - it 'should create a glance image' do - shell('openstack --os-username glance --os-password a_big_secret --os-tenant-name services --os-auth-url http://127.0.0.1:5000/v2.0 image list') do |r| + it 'should create a glance image with proper attributes' do + glance_env_opts = '--os-username glance --os-password a_big_secret --os-tenant-name services --os-auth-url http://127.0.0.1:5000/v2.0' + shell("openstack #{glance_env_opts} image list") do |r| expect(r.stdout).to match(/test_image/) expect(r.stderr).to be_empty end + shell("openstack #{glance_env_opts} image show test_image --format shell") do |r| + expect(r.stdout).to match(/visibility="public"/) + expect(r.stdout).to match(/container_format="bare"/) + expect(r.stdout).to match(/disk_format="qcow2"/) + expect(r.stdout).to match(/properties="icanhaz='cheezburger'"/) + expect(r.stdout).to match(/min_ram="64"/) + expect(r.stdout).to match(/min_disk="1024"/) + expect(r.stderr).to be_empty + end end end end diff --git a/spec/unit/provider/glance_image_spec.rb b/spec/unit/provider/glance_image_spec.rb index 51803cc2..8bd183ec 100644 --- a/spec/unit/provider/glance_image_spec.rb +++ b/spec/unit/provider/glance_image_spec.rb @@ -39,31 +39,26 @@ describe provider_class do it_behaves_like 'authenticated with environment variables' do describe '#create' do it 'creates an image' do - provider.class.stubs(:openstack) - .with('image', 'list', '--quiet', '--format', 'csv', '--long') - .returns('"ID","Name","Disk Format","Container Format","Size","Status" -"534 5b502-efe4-4852-a45d-edaba3a3acc6","image1","raw","bare",1270,"active" -') provider.class.stubs(:openstack) .with('image', 'create', '--format', 'shell', ['image1', '--public', '--container-format=bare', '--disk-format=qcow2', '--min-disk=1024', '--min-ram=1024', '--copy-from=http://example.com/image1.img' ]) - .returns('checksum="09b9c392dc1f6e914cea287cb6be34b0" + .returns('checksum="ee1eca47dc88f4879d8a229cc70a07c6" container_format="bare" -created_at="2015-04-08T18:28:01" -deleted="False" -deleted_at="None" +created_at="2016-03-29T20:52:24Z" disk_format="qcow2" -id="5345b502-efe4-4852-a45d-edaba3a3acc6" -is_public="True" +file="/v2/images/8801c5b0-c505-4a15-8ca3-1d2383f8c015/file" +id="8801c5b0-c505-4a15-8ca3-1d2383f8c015" min_disk="1024" min_ram="1024" name="image1" -owner="None" -properties="{}" +owner="5a9e521e17014804ab8b4e8b3de488a4" protected="False" -size="1270" +schema="/v2/schemas/image" +size="13287936" status="active" -updated_at="2015-04-10T18:18:18" +tags="" +updated_at="2016-03-29T20:52:40Z" virtual_size="None" +visibility="public" ') provider.create expect(provider.exists?).to be_truthy @@ -73,9 +68,6 @@ virtual_size="None" describe '#destroy' do it 'destroys an image' do - provider.class.stubs(:openstack) - .with('image', 'list', '--quiet', '--format', 'csv') - .returns('"ID","Name","Disk Format","Container Format","Size","Status"') provider.class.stubs(:openstack) .with('image', 'delete', 'image1') provider.destroy @@ -87,9 +79,9 @@ virtual_size="None" describe '.instances' do it 'finds every image' do provider.class.stubs(:openstack) - .with('image', 'list', '--quiet', '--format', 'csv', '--long') - .returns('"ID","Name","Disk Format","Container Format","Size","Status" -"5345b502-efe4-4852-a45d-edaba3a3acc6","image1","raw","bare",1270,"active" + .with('image', 'list', '--quiet', '--format', 'csv', []) + .returns('"ID","Name","Status" +"5345b502-efe4-4852-a45d-edaba3a3acc6","image1","active" ') provider.class.stubs(:openstack) .with('image', 'show', '--format', 'shell', '5345b502-efe4-4852-a45d-edaba3a3acc6') @@ -118,4 +110,95 @@ virtual_size="None" end end + + describe 'when managing an image with properties' do + + let(:tenant_attrs) do + { + :ensure => 'present', + :name => 'image1', + :is_public => 'yes', + :container_format => 'bare', + :disk_format => 'qcow2', + :source => '/var/tmp/image1.img', + :min_ram => 1024, + :min_disk => 1024, + :properties => { 'something' => 'what', 'vmware_disktype' => 'sparse' } + } + end + + let(:resource) do + Puppet::Type::Glance_image.new(tenant_attrs) + end + + let(:provider) do + provider_class.new(resource) + end + + it_behaves_like 'authenticated with environment variables' do + describe '#create' do + it 'creates an image' do + provider.class.stubs(:openstack) + .with('image', 'create', '--format', 'shell', ['image1', '--public', '--container-format=bare', '--disk-format=qcow2', '--min-disk=1024', '--min-ram=1024', ['--property', 'something=what', '--property', 'vmware_disktype=sparse'], '--file=/var/tmp/image1.img' ]) + .returns('checksum="ee1eca47dc88f4879d8a229cc70a07c6" +container_format="bare" +created_at="2016-03-29T20:52:24Z" +disk_format="qcow2" +file="/v2/images/8801c5b0-c505-4a15-8ca3-1d2383f8c015/file" +id="8801c5b0-c505-4a15-8ca3-1d2383f8c015" +min_disk="1024" +min_ram="1024" +name="image1" +owner="5a9e521e17014804ab8b4e8b3de488a4" +properties="something=\'what\', vmware_disktype=\'sparse\'" +protected="False" +schema="/v2/schemas/image" +size="13287936" +status="active" +tags="" +updated_at="2016-03-29T20:52:40Z" +virtual_size="None" +visibility="public" +') + provider.create + expect(provider.exists?).to be_truthy + end + end + end + + describe '.instances' do + it 'finds every image' do + provider.class.stubs(:openstack) + .with('image', 'list', '--quiet', '--format', 'csv', []) + .returns('"ID","Name","Status" +"5345b502-efe4-4852-a45d-edaba3a3acc6","image1","active" +') + provider.class.stubs(:openstack) + .with('image', 'show', '--format', 'shell', '5345b502-efe4-4852-a45d-edaba3a3acc6') + .returns('checksum="09b9c392dc1f6e914cea287cb6be34b0" +container_format="bare" +created_at="2015-04-08T18:28:01" +deleted="False" +deleted_at="None" +disk_format="qcow2" +id="5345b502-efe4-4852-a45d-edaba3a3acc6" +is_public="True" +min_disk="1024" +min_ram="1024" +name="image1" +owner="None" +properties="something=\'what\', vmware_disktype=\'sparse\'" +protected="False" +size="1270" +status="active" +updated_at="2015-04-10T18:18:18" +virtual_size="None" +') + instances = provider_class.instances + expect(instances.count).to eq(1) + end + end + + end + end