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