Merge "Don't use copy from but download the image locally"

lib/puppet/provider/glance_image/openstack.rb

@@ -1,4 +1,6 @@
 require File.join(File.dirname(__FILE__), '..','..','..', 'puppet/provider/glance')
+require 'tempfile'
+require 'net/http'
 
 Puppet::Type.type(:glance_image).provide(
   :openstack,
@@ -10,10 +12,9 @@ Puppet::Type.type(:glance_image).provide(
 
   @credentials = Puppet::Provider::Openstack::CredentialsV2_0.new
 
-  # 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'
+  # TODO(flaper87): v2 is now the default. Force the use of v2,
+  # to avoid supporting both versions and other edge cases.
+  ENV['OS_IMAGE_API_VERSION'] = '2'
 
   def initialize(value={})
     super(value)
@@ -21,13 +22,30 @@ Puppet::Type.type(:glance_image).provide(
   end
 
   def create
+    temp_file = false
     if @resource[:source]
       # copy_from cannot handle file://
       if @resource[:source] =~ /^\// # local file
         location = "--file=#{@resource[:source]}"
       else
-        location = "--copy-from=#{@resource[:source]}"
+        temp_file = Tempfile.new('puppet-glance-image')
+
+        uri = URI(@resource[:source])
+        Net::HTTP.start(uri.host, uri.port,
+                        :use_ssl => uri.scheme == 'https') do |http|
+          request = Net::HTTP::Get.new uri
+          http.request request do |response|
+            open temp_file.path, 'w' do |io|
+              response.read_body do |segment|
+                io.write(segment)
               end
+            end
+          end
+        end
+
+        location = "--file=#{temp_file.path}"
+      end
+
     # location cannot handle file://
     # location does not import, so no sense in doing anything more than this
     elsif @resource[:location]
@@ -46,8 +64,14 @@ Puppet::Type.type(:glance_image).provide(
     opts << props_to_s(@resource[:properties]) if @resource[:properties]
     opts << location
 
+    begin
       @property_hash = self.class.request('image', 'create', opts)
       @property_hash[:ensure] = :present
+    ensure
+      if temp_file
+        temp_file.close(true)
+      end
+    end
   end
 
   def exists?
@@ -101,7 +125,7 @@ Puppet::Type.type(:glance_image).provide(
       new(
         :ensure           => :present,
         :name             => attrs[:name],
-        :is_public        => attrs[:is_public].downcase.chomp == 'true'? true : false,
+        :is_public        => attrs[:visibility].downcase.chomp == 'public'? true : false,
         :container_format => attrs[:container_format],
         :id               => attrs[:id],
         :disk_format      => attrs[:disk_format],

releasenotes/notes/force-use-of-v2-2c50d7c5a6467107.yaml

@@ -0,0 +1,13 @@
+---
+prelude: >
+    Glance API v1 is now deprecated and tools depending on it should move onto
+    as their default, and perhaps only supported, version.
+deprecations:
+  - |
+    This change deprecates the use of v1 (by forcing v2). This assumes the
+    Glance service it will talk to has v2 enabled. In order to guarantee
+    compatibility, this changes introduces a similar, fake, copy-from behavior
+    which downloads the remote image into the server (a temporary file) where
+    the provider is running and then uploads it to Glance. Consumers of this
+    provider must be aware of space limitations and the possibility of there not
+    being enough space for this operation to succeed.

spec/unit/provider/glance_image_spec.rb

@@ -22,7 +22,7 @@ describe provider_class do
         :is_public        => 'yes',
         :container_format => 'bare',
         :disk_format      => 'qcow2',
-        :source           => 'http://example.com/image1.img',
+        :source           => '/var/tmp/image1.img',
         :min_ram          => 1024,
         :min_disk         => 1024,
       }
@@ -40,7 +40,7 @@ describe provider_class 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', '--copy-from=http://example.com/image1.img' ])
+                        .with('image', 'create', '--format', 'shell', ['image1', '--public', '--container-format=bare', '--disk-format=qcow2', '--min-disk=1024', '--min-ram=1024', '--file=/var/tmp/image1.img'])
                         .returns('checksum="ee1eca47dc88f4879d8a229cc70a07c6"
 container_format="bare"
 created_at="2016-03-29T20:52:24Z"
@@ -92,7 +92,7 @@ deleted="False"
 deleted_at="None"
 disk_format="qcow2"
 id="5345b502-efe4-4852-a45d-edaba3a3acc6"
-is_public="True"
+visibility="public"
 min_disk="1024"
 min_ram="1024"
 name="image1"
@@ -182,7 +182,7 @@ deleted="False"
 deleted_at="None"
 disk_format="qcow2"
 id="5345b502-efe4-4852-a45d-edaba3a3acc6"
-is_public="True"
+visibility="public"
 min_disk="1024"
 min_ram="1024"
 name="image1"
@@ -272,7 +272,7 @@ disk_format="qcow2"
 id="2b4be0b8-aec0-43af-a404-33c3335a0b3f"
 min_disk="0"
 min_ram="0"
-is_public="True"
+visibility="public"
 name="image1"
 owner="None"
 properties="{}"