Merge "Add ability to set properties with glance_image"

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 id=(id)
-    fail('id is read only')
+  def properties=(value)
+    @property_flush[:properties] = value
   end
 
-  def id
-    @property_hash[:id]
+  def id=(id)
+    fail('id is read only')
   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

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
-
-

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

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