From f00d32ccf3bd715633bdafc420b71cc60e3efa17 Mon Sep 17 00:00:00 2001
From: Ferran Rodenas <frodenas@gmail.com>
Date: Tue, 26 Mar 2013 23:51:55 +0100
Subject: [PATCH 1/3] [openstack_cpi] Better over limit support

---
 bosh_openstack_cpi/lib/cloud/openstack.rb     |   1 -
 .../lib/cloud/openstack/cloud.rb              |  86 +++----
 .../lib/cloud/openstack/connection.rb         |  48 ----
 .../lib/cloud/openstack/helpers.rb            |  29 +++
 .../cloud/openstack/network_configurator.rb   |  14 +-
 .../lib/cloud/openstack/vip_network.rb        |  26 ++-
 bosh_openstack_cpi/spec/spec_helper.rb        |   8 +-
 bosh_openstack_cpi/spec/unit/cloud_spec.rb    |   3 +-
 .../spec/unit/connection_spec.rb              | 132 -----------
 bosh_openstack_cpi/spec/unit/helpers_spec.rb  | 220 +++++++++++++-----
 .../spec/unit/validate_deployment_spec.rb     |   3 +-
 11 files changed, 261 insertions(+), 309 deletions(-)
 delete mode 100644 bosh_openstack_cpi/lib/cloud/openstack/connection.rb
 delete mode 100644 bosh_openstack_cpi/spec/unit/connection_spec.rb

diff --git a/bosh_openstack_cpi/lib/cloud/openstack.rb b/bosh_openstack_cpi/lib/cloud/openstack.rb
index 168076decdd..ad147f6f045 100644
--- a/bosh_openstack_cpi/lib/cloud/openstack.rb
+++ b/bosh_openstack_cpi/lib/cloud/openstack.rb
@@ -20,7 +20,6 @@ module OpenStackCloud; end
 require "cloud"
 require "cloud/openstack/helpers"
 require "cloud/openstack/cloud"
-require "cloud/openstack/connection"
 require "cloud/openstack/registry_client"
 require "cloud/openstack/tag_manager"
 require "cloud/openstack/version"
diff --git a/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb b/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
index fed314c2cb4..42031a68f6a 100644
--- a/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
+++ b/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
@@ -42,7 +42,7 @@ def initialize(options)
         :openstack_region => @openstack_properties["region"],
         :openstack_endpoint_type => @openstack_properties["endpoint_type"]
       }
-      @openstack = Connection.new(:compute, openstack_params)
+      @openstack = Fog::Compute.new(openstack_params)
 
       glance_params = {
         :provider => "OpenStack",
@@ -53,7 +53,7 @@ def initialize(options)
         :openstack_region => @openstack_properties["region"],
         :openstack_endpoint_type => @openstack_properties["endpoint_type"]
       }
-      @glance = Connection.new(:image, glance_params)
+      @glance = Fog::Image.new(glance_params)
 
       registry_endpoint = @registry_properties["endpoint"]
       registry_user = @registry_properties["user"]
@@ -175,15 +175,15 @@ def create_stemcell(image_path, cloud_properties)
     def delete_stemcell(stemcell_id)
       with_thread_name("delete_stemcell(#{stemcell_id})") do
         @logger.info("Deleting stemcell `#{stemcell_id}'...")
-        image = @glance.images.find_by_id(stemcell_id)
+        image = with_openstack { @glance.images.find_by_id(stemcell_id) }
         if image
           kernel_id = image.properties["kernel_id"]
           if kernel_id
-            kernel = @glance.images.find_by_id(kernel_id)
+            kernel = with_openstack { @glance.images.find_by_id(kernel_id) }
             if kernel && kernel.properties["stemcell"]
               if kernel.properties["stemcell"] == image.name
                 @logger.info("Deleting kernel `#{kernel_id}'...")
-                kernel.destroy
+                with_openstack { kernel.destroy }
                 @logger.info("Kernel `#{kernel_id}' is now deleted")
               end
             end
@@ -191,17 +191,17 @@ def delete_stemcell(stemcell_id)
 
           ramdisk_id = image.properties["ramdisk_id"]
           if ramdisk_id
-            ramdisk = @glance.images.find_by_id(ramdisk_id)
+            ramdisk = with_openstack { @glance.images.find_by_id(ramdisk_id) }
             if ramdisk && ramdisk.properties["stemcell"]
               if ramdisk.properties["stemcell"] == image.name
                 @logger.info("Deleting ramdisk `#{ramdisk_id}'...")
-                ramdisk.destroy
+                with_openstack { ramdisk.destroy }
                 @logger.info("Ramdisk `#{ramdisk_id}' is now deleted")
               end
             end
           end
 
-          image.destroy
+          with_openstack { image.destroy }
           @logger.info("Stemcell `#{stemcell_id}' is now deleted")
         else
           @logger.info("Stemcell `#{stemcell_id}' not found. Skipping.")
@@ -240,11 +240,11 @@ def create_vm(agent_id, stemcell_id, resource_pool,
         nics = network_configurator.nics
         @logger.debug("Using NICs: `#{nics.join(', ')}'")
 
-        image = @openstack.images.find { |i| i.id == stemcell_id }
+        image = with_openstack { @openstack.images.find { |i| i.id == stemcell_id } }
         cloud_error("Image `#{stemcell_id}' not found") if image.nil?
         @logger.debug("Using image: `#{stemcell_id}'")
 
-        flavor = @openstack.flavors.find { |f| f.name == resource_pool["instance_type"] }
+        flavor = with_openstack { @openstack.flavors.find { |f| f.name == resource_pool["instance_type"] } }
         cloud_error("Flavor `#{resource_pool["instance_type"]}' not found") if flavor.nil?
         @logger.debug("Using flavor: `#{resource_pool["instance_type"]}'")
 
@@ -262,7 +262,7 @@ def create_vm(agent_id, stemcell_id, resource_pool,
         server_params[:availability_zone] = availability_zone if availability_zone
 
         @logger.debug("Using boot parms: `#{server_params.inspect}'")
-        server = @openstack.servers.create(server_params)
+        server = with_openstack { @openstack.servers.create(server_params) }
 
         @logger.info("Creating new server `#{server.id}'...")
         wait_resource(server, :active, :state)
@@ -286,9 +286,9 @@ def create_vm(agent_id, stemcell_id, resource_pool,
     def delete_vm(server_id)
       with_thread_name("delete_vm(#{server_id})") do
         @logger.info("Deleting server `#{server_id}'...")
-        server = @openstack.servers.get(server_id)
+        server = with_openstack { @openstack.servers.get(server_id) }
         if server
-          server.destroy
+          with_openstack { server.destroy }
           wait_resource(server, :terminated, :state, true)
 
           @logger.info("Deleting settings for server `#{server.id}'...")
@@ -306,7 +306,7 @@ def delete_vm(server_id)
     # @return [Boolean] True if the vm exists
     def has_vm?(server_id)
       with_thread_name("has_vm?(#{server_id})") do
-        server = @openstack.servers.get(server_id)
+        server = with_openstack { @openstack.servers.get(server_id) }
         !server.nil?
       end
     end
@@ -318,7 +318,7 @@ def has_vm?(server_id)
     # @return [void]
     def reboot_vm(server_id)
       with_thread_name("reboot_vm(#{server_id})") do
-        server = @openstack.servers.get(server_id)
+        server = with_openstack { @openstack.servers.get(server_id) }
         cloud_error("Server `#{server_id}' not found") unless server
 
         soft_reboot(server)
@@ -336,11 +336,12 @@ def configure_networks(server_id, network_spec)
       with_thread_name("configure_networks(#{server_id}, ...)") do
         @logger.info("Configuring `#{server_id}' to use the following " \
                      "network settings: #{network_spec.pretty_inspect}")
-
         network_configurator = NetworkConfigurator.new(network_spec)
-        server = @openstack.servers.get(server_id)
 
-        sg = @openstack.list_security_groups(server_id).body["security_groups"]
+        server = with_openstack { @openstack.servers.get(server_id) }
+        cloud_error("Server `#{server_id}' not found") unless server
+
+        sg = with_openstack { @openstack.list_security_groups(server_id).body["security_groups"] }
         actual = sg.collect { |s| s["name"] }.sort
         new = network_configurator.security_groups(@default_security_groups)
 
@@ -381,14 +382,14 @@ def create_disk(size, server_id = nil)
         }
 
         if server_id
-          server = @openstack.servers.get(server_id)
+          server = with_openstack { @openstack.servers.get(server_id) }
           if server && server.availability_zone
             volume_params[:availability_zone] = server.availability_zone
           end
         end
 
         @logger.info("Creating new volume...")
-        volume = @openstack.volumes.create(volume_params)
+        volume = with_openstack { @openstack.volumes.create(volume_params) }
 
         @logger.info("Creating new volume `#{volume.id}'...")
         wait_resource(volume, :available)
@@ -406,14 +407,14 @@ def create_disk(size, server_id = nil)
     def delete_disk(disk_id)
       with_thread_name("delete_disk(#{disk_id})") do
         @logger.info("Deleting volume `#{disk_id}'...")
-        volume = @openstack.volumes.get(disk_id)
+        volume = with_openstack { @openstack.volumes.get(disk_id) }
         if volume
           state = volume.status
           if state.to_sym != :available
             cloud_error("Cannot delete volume `#{disk_id}', state is #{state}")
           end
 
-          volume.destroy
+          with_openstack { volume.destroy }
           wait_resource(volume, :deleted, :status, true)
         else
           @logger.info("Volume `#{disk_id}' not found. Skipping.")
@@ -429,10 +430,10 @@ def delete_disk(disk_id)
     # @return [void]
     def attach_disk(server_id, disk_id)
       with_thread_name("attach_disk(#{server_id}, #{disk_id})") do
-        server = @openstack.servers.get(server_id)
+        server = with_openstack { @openstack.servers.get(server_id) }
         cloud_error("Server `#{server_id}' not found") unless server
 
-        volume = @openstack.volumes.get(disk_id)
+        volume = with_openstack { @openstack.volumes.get(disk_id) }
         cloud_error("Volume `#{disk_id}' not found") unless volume
 
         device_name = attach_volume(server, volume)
@@ -453,10 +454,10 @@ def attach_disk(server_id, disk_id)
     # @return [void]
     def detach_disk(server_id, disk_id)
       with_thread_name("detach_disk(#{server_id}, #{disk_id})") do
-        server = @openstack.servers.get(server_id)
+        server = with_openstack { @openstack.servers.get(server_id) }
         cloud_error("Server `#{server_id}' not found") unless server
 
-        volume = @openstack.volumes.get(disk_id)
+        volume = with_openstack { @openstack.volumes.get(disk_id) }
         cloud_error("Volume `#{disk_id}' not found") unless volume
 
         detach_volume(server, volume)
@@ -477,11 +478,13 @@ def detach_disk(server_id, disk_id)
     # @return [void]
     def set_vm_metadata(server_id, metadata)
       with_thread_name("set_vm_metadata(#{server_id}, ...)") do
-        server = @openstack.servers.get(server_id)
-        cloud_error("Server `#{server_id}' not found") unless server
+        with_openstack do
+          server = @openstack.servers.get(server_id)
+          cloud_error("Server `#{server_id}' not found") unless server
 
-        metadata.each do |name, value|
-          TagManager.tag(server, name, value)
+          metadata.each do |name, value|
+            TagManager.tag(server, name, value)
+          end
         end
       end
     end
@@ -506,7 +509,7 @@ def validate_deployment(old_manifest, new_manifest)
     # @note this is a private method that is public to make it easier to test
     def select_availability_zone(volumes, resource_pool_az)
       if volumes && !volumes.empty?
-        disks = volumes.map { |vid| @openstack.volumes.get(vid) }
+        disks = volumes.map { |vid| with_openstack { @openstack.volumes.get(vid) } }
         ensure_same_availability_zone(disks, resource_pool_az)
         disks.first.availability_zone
       else
@@ -627,7 +630,7 @@ def update_agent_settings(server)
     # @return [void]
     def soft_reboot(server)
       @logger.info("Soft rebooting server `#{server.id}'...")
-      server.reboot
+      with_openstack { server.reboot }
       wait_resource(server, :active, :state)
     end
 
@@ -638,7 +641,7 @@ def soft_reboot(server)
     # @return [void]
     def hard_reboot(server)
       @logger.info("Hard rebooting server `#{server.id}'...")
-      server.reboot(type = 'HARD')
+      with_openstack { server.reboot(type = 'HARD') }
       wait_resource(server, :active, :state)
     end
 
@@ -649,8 +652,7 @@ def hard_reboot(server)
     # @param [Fog::Compute::OpenStack::Volume] volume OpenStack volume
     # @return [String] Device name
     def attach_volume(server, volume)
-      volume_attachments = @openstack.get_server_volumes(server.id).
-                           body['volumeAttachments']
+      volume_attachments = with_openstack { @openstack.get_server_volumes(server.id).body['volumeAttachments'] }
       device_names = Set.new(volume_attachments.collect! { |v| v["device"] })
 
       new_attachment = nil
@@ -662,10 +664,9 @@ def attach_volume(server, volume)
         end
         @logger.info("Attaching volume `#{volume.id}' to `#{server.id}', " \
                      "device name is `#{dev_name}'")
-        if volume.attach(server.id, dev_name)
-          wait_resource(volume, :"in-use")
-          new_attachment = dev_name
-        end
+        with_openstack { volume.attach(server.id, dev_name) }
+        wait_resource(volume, :"in-use")
+        new_attachment = dev_name
         break
       end
       cloud_error("Server has too many disks attached") if new_attachment.nil?
@@ -680,8 +681,7 @@ def attach_volume(server, volume)
     # @param [Fog::Compute::OpenStack::Volume] volume OpenStack volume
     # @return [void]
     def detach_volume(server, volume)
-      volume_attachments = @openstack.get_server_volumes(server.id).
-                           body['volumeAttachments']
+      volume_attachments = with_openstack { @openstack.get_server_volumes(server.id).body['volumeAttachments'] }
       device_map = volume_attachments.collect! { |v| v["volumeId"] }
 
       unless device_map.include?(volume.id)
@@ -690,7 +690,7 @@ def detach_volume(server, volume)
       end
 
       @logger.info("Detaching volume `#{volume.id}' from `#{server.id}'...")
-      volume.detach(server.id, volume.id)
+      with_openstack { volume.detach(server.id, volume.id) }
       wait_resource(volume, :available)
     end
 
@@ -702,7 +702,7 @@ def detach_volume(server, volume)
     def upload_image(image_params)
       @logger.info("Creating new image...")
       started_at = Time.now
-      image = @glance.images.create(image_params)
+      image = with_openstack { @glance.images.create(image_params) }
       total = Time.now - started_at
       @logger.info("Created new image `#{image.id}', took #{total}s")
 
diff --git a/bosh_openstack_cpi/lib/cloud/openstack/connection.rb b/bosh_openstack_cpi/lib/cloud/openstack/connection.rb
deleted file mode 100644
index c8d6c287387..00000000000
--- a/bosh_openstack_cpi/lib/cloud/openstack/connection.rb
+++ /dev/null
@@ -1,48 +0,0 @@
-# Copyright (c) 2009-2013 VMware, Inc.
-
-module Bosh::OpenStackCloud
-  class Connection
-    include Helpers
-
-    MAX_RETRYAFTER_TIME = 10 # Max number of seconds before retrying a call
-
-    def initialize(service, params = {})
-      @logger = Bosh::Clouds::Config.logger
-      case service
-        when :compute then @connection = Fog::Compute.new(params)
-        when :image then @connection = Fog::Image.new(params)
-        else cloud_error("Service #{service} not supported by OpenStack CPI")
-      end
-    end
-
-    # Delegate all methods to Fog.
-    def method_missing(method, *arguments, &block)
-      return super unless @connection.respond_to?(method)
-
-      begin
-        @connection.send(method, *arguments, &block)
-      rescue Excon::Errors::RequestEntityTooLarge => e
-        # If we find a rate limit error, parse message, wait, and retry
-        retried = false
-        unless e.response.body.empty?
-          begin
-            message = JSON.parse(e.response.body)
-            if message["overLimit"] && message["overLimit"]["retryAfter"]
-              retryafter = message["overLimit"]["retryAfter"]
-              wait_time = [MAX_RETRYAFTER_TIME, retryafter.to_i].min
-              task_checkpoint
-              @logger.debug("OpenStack API overLimit, waiting #{wait_time} " +
-                            "seconds before retrying") if @logger
-              sleep(wait_time)
-              retried = true
-              retry
-            end
-          rescue JSON::ParserError
-            # do nothing
-          end
-        end
-        raise e unless retried
-      end
-    end
-  end
-end
\ No newline at end of file
diff --git a/bosh_openstack_cpi/lib/cloud/openstack/helpers.rb b/bosh_openstack_cpi/lib/cloud/openstack/helpers.rb
index 3b49673391f..7d674b939fa 100644
--- a/bosh_openstack_cpi/lib/cloud/openstack/helpers.rb
+++ b/bosh_openstack_cpi/lib/cloud/openstack/helpers.rb
@@ -6,6 +6,8 @@ module Bosh::OpenStackCloud
   module Helpers
 
     DEFAULT_TIMEOUT = 600 # Default timeout for target state (in seconds)
+    MAX_RETRIES = 10 # Max number of retries
+    DEFAULT_RETRY_TIMEOUT = 1 # Default timeout before retrying a call (in seconds)
 
     ##
     # Raises CloudError exception
@@ -16,6 +18,33 @@ def cloud_error(message)
       raise Bosh::Clouds::CloudError, message
     end
 
+    def with_openstack
+      retries = 0
+      begin
+        yield
+      rescue Excon::Errors::RequestEntityTooLarge => e
+        # If we find a rate limit error, parse message, wait, and retry
+        unless e.response.body.empty? || retries >= MAX_RETRIES
+          begin
+            message = JSON.parse(e.response.body)
+            overlimit = message["overLimit"] || message["overLimitFault"]
+            if overlimit
+              task_checkpoint
+              wait_time = overlimit["retryAfter"] || DEFAULT_RETRY_TIMEOUT
+              @logger.debug("OpenStack API overLimit, waiting #{wait_time} " +
+                                "seconds before retrying") if @logger
+              sleep(wait_time.to_i)
+              retries += 1
+              retry
+            end
+          rescue JSON::ParserError
+            # do nothing
+          end
+        end
+        raise e
+      end
+    end
+
     ##
     # Waits for a resource to be on a target state
     #
diff --git a/bosh_openstack_cpi/lib/cloud/openstack/network_configurator.rb b/bosh_openstack_cpi/lib/cloud/openstack/network_configurator.rb
index 2ca22543518..f499e8e4eab 100644
--- a/bosh_openstack_cpi/lib/cloud/openstack/network_configurator.rb
+++ b/bosh_openstack_cpi/lib/cloud/openstack/network_configurator.rb
@@ -73,12 +73,14 @@ def configure(openstack, server)
       else
         # If there is no vip network we should disassociate any floating IP
         # currently held by server (as it might have had floating IP before)
-        addresses = openstack.addresses
-        addresses.each do |address|
-          if address.instance_id == server.id
-            @logger.info("Disassociating floating IP `#{address.ip}' " \
-                         "from server `#{server.id}'")
-            address.server = nil
+        with_openstack do
+          addresses = openstack.addresses
+          addresses.each do |address|
+            if address.instance_id == server.id
+              @logger.info("Disassociating floating IP `#{address.ip}' " \
+                           "from server `#{server.id}'")
+              address.server = nil
+            end
           end
         end
       end
diff --git a/bosh_openstack_cpi/lib/cloud/openstack/vip_network.rb b/bosh_openstack_cpi/lib/cloud/openstack/vip_network.rb
index 6c2f84cb355..ce477ce68ca 100644
--- a/bosh_openstack_cpi/lib/cloud/openstack/vip_network.rb
+++ b/bosh_openstack_cpi/lib/cloud/openstack/vip_network.rb
@@ -29,19 +29,21 @@ def configure(openstack, server)
 
       # Check if the OpenStack floating IP is allocated. If true, disassociate
       # it from any server before associating it to the new server
-      address = openstack.addresses.find { |a| a.ip == @ip }
-      if address
-        unless address.instance_id.nil?
-          @logger.info("Disassociating floating IP `#{@ip}' " \
-                       "from server `#{address.instance_id}'")
-          address.server = nil
-        end
+      with_openstack do
+        address = openstack.addresses.find { |a| a.ip == @ip }
+        if address
+          unless address.instance_id.nil?
+            @logger.info("Disassociating floating IP `#{@ip}' " \
+                         "from server `#{address.instance_id}'")
+            address.server = nil
+          end
 
-        @logger.info("Associating server `#{server.id}' " \
-                     "with floating IP `#{@ip}'")
-        address.server = server
-      else
-        cloud_error("Floating IP #{@ip} not allocated")
+          @logger.info("Associating server `#{server.id}' " \
+                       "with floating IP `#{@ip}'")
+          address.server = server
+        else
+          cloud_error("Floating IP #{@ip} not allocated")
+        end
       end
     end
 
diff --git a/bosh_openstack_cpi/spec/spec_helper.rb b/bosh_openstack_cpi/spec/spec_helper.rb
index d7273ce466a..a60a61683c2 100644
--- a/bosh_openstack_cpi/spec/spec_helper.rb
+++ b/bosh_openstack_cpi/spec/spec_helper.rb
@@ -62,10 +62,10 @@ def mock_cloud(options = nil)
   addresses = double("addresses")
   snapshots = double("snapshots")
 
-  glance = double(Bosh::OpenStackCloud::Connection, :service => :image)
+  glance = double(Fog::Image)
   Fog::Image.stub(:new).and_return(glance)
 
-  openstack = double(Bosh::OpenStackCloud::Connection, :service => :compute)
+  openstack = double(Fog::Compute)
 
   openstack.stub(:servers).and_return(servers)
   openstack.stub(:images).and_return(images)
@@ -84,10 +84,10 @@ def mock_cloud(options = nil)
 def mock_glance(options = nil)
   images = double("images")
 
-  openstack = double(Bosh::OpenStackCloud::Connection, :service => :compute)
+  openstack = double(Fog::Compute)
   Fog::Compute.stub(:new).and_return(openstack)
 
-  glance = double(Bosh::OpenStackCloud::Connection, :service => :image)
+  glance = double(Fog::Image)
   glance.stub(:images).and_return(images)
 
   Fog::Image.stub(:new).and_return(glance)
diff --git a/bosh_openstack_cpi/spec/unit/cloud_spec.rb b/bosh_openstack_cpi/spec/unit/cloud_spec.rb
index e43268caff6..d599d275b70 100644
--- a/bosh_openstack_cpi/spec/unit/cloud_spec.rb
+++ b/bosh_openstack_cpi/spec/unit/cloud_spec.rb
@@ -8,7 +8,8 @@
   describe "creating via provider" do
 
     it "can be created using Bosh::Cloud::Provider" do
-      Bosh::OpenStackCloud::Connection.stub(:new)
+      Fog::Compute.stub(:new)
+      Fog::Image.stub(:new)
       cloud = Bosh::Clouds::Provider.create(:openstack, mock_cloud_options)
       cloud.should be_an_instance_of(Bosh::OpenStackCloud::Cloud)
     end
diff --git a/bosh_openstack_cpi/spec/unit/connection_spec.rb b/bosh_openstack_cpi/spec/unit/connection_spec.rb
deleted file mode 100644
index 6d9445b5127..00000000000
--- a/bosh_openstack_cpi/spec/unit/connection_spec.rb
+++ /dev/null
@@ -1,132 +0,0 @@
-# Copyright (c) 2009-2013 VMware, Inc.
-
-require "spec_helper"
-
-describe Bosh::OpenStackCloud::Connection do
-  before(:each) do
-    Bosh::Clouds::Config.stub(:task_checkpoint)
-  end
-
-  describe "Compute service" do
-    before(:each) do
-      @fog_compute = double(Fog::Compute)
-      Fog::Compute.stub(:new).and_return(@fog_compute)
-      @connection = Bosh::OpenStackCloud::Connection.new(:compute, {})
-    end
-
-    it "should respond to a Fog::Compute method" do
-      @fog_compute.should_receive(:respond_to?).with(:servers).and_return(true)
-      @fog_compute.should_receive(:servers)
-
-      @connection.servers
-    end
-
-    it "should raise an error if Fog::Compute doesn't respond to a method" do
-      @fog_compute.should_receive(:respond_to?).with(:pair).and_return(false)
-
-      expect {
-        @connection.pair
-      }.to raise_error(NoMethodError, /undefined method `pair' for/)
-    end
-  end
-
-  describe "Image service" do
-    before(:each) do
-      @fog_image = double(Fog::Image)
-      Fog::Image.stub(:new).and_return(@fog_image)
-      @connection = Bosh::OpenStackCloud::Connection.new(:image, {})
-    end
-
-    it "should respond to a Fog::Image method" do
-      @fog_image.should_receive(:respond_to?).with(:images).and_return(true)
-      @fog_image.should_receive(:images)
-
-      @connection.images
-    end
-
-    it "should raise an error if Fog::Image doesn't respond to a method" do
-      @fog_image.should_receive(:respond_to?).with(:pair).and_return(false)
-
-      expect {
-        @connection.pair
-      }.to raise_error(NoMethodError, /undefined method `pair' for/)
-    end
-  end
-
-  describe "Unknow service" do
-    it "should raise an unsupported service error" do
-      expect {
-        Bosh::OpenStackCloud::Connection.new(:pair, {})
-      }.to raise_error(Bosh::Clouds::CloudError, /Service pair not supported by OpenStack CPI/)
-    end
-  end
-
-  describe "RequestEntityTooLarge exception" do
-    before(:each) do
-      @fog_compute = double(Fog::Compute)
-      Fog::Compute.stub(:new).and_return(@fog_compute)
-      @connection = Bosh::OpenStackCloud::Connection.new(:compute, {})
-    end
-
-    it "should retry after the amount of seconds received at the response body" do
-      body = { "overLimit" => {
-                 "message" => "This request was rate-limited.",
-                 "code" => 413,
-                 "retryAfter" => "1",
-                 "details" => "Only 10 POST request(s) can be made to * every minute."}
-             }
-      response = Excon::Response.new(:body => JSON.dump(body))
-
-      @fog_compute.should_receive(:respond_to?).with(:servers).and_return(true)
-      @fog_compute.should_receive(:servers).
-        and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
-      @connection.should_receive(:sleep).with(1)
-      @fog_compute.should_receive(:servers)
-
-      @connection.servers
-    end
-
-    it "should retry after the max number of seconds before retrying a call" do
-      body = { "overLimit" => {
-                 "message" => "This request was rate-limited.",
-                 "code" => 413,
-                 "retryAfter" => "20",
-                 "details" => "Only 10 POST request(s) can be made to * every minute."}
-             }
-      response = Excon::Response.new(:body => JSON.dump(body))
-
-      @fog_compute.should_receive(:respond_to?).with(:servers).and_return(true)
-      @fog_compute.should_receive(:servers).
-        and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
-      @connection.should_receive(:sleep).with(10)
-      @fog_compute.should_receive(:servers)
-
-      @connection.servers
-    end
-
-    it "should raise an error if response has no body" do
-      response = Excon::Response.new(:body => "")
-
-      @fog_compute.should_receive(:respond_to?).with(:servers).and_return(true)
-      @fog_compute.should_receive(:servers).
-        and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
-
-      expect {
-        @connection.servers
-      }.to raise_error(Excon::Errors::RequestEntityTooLarge)
-    end
-
-    it "should raise an error if response has no overlimit message" do
-      body = "This request was rate-limited."
-      response = Excon::Response.new(:body => JSON.dump(body))
-
-      @fog_compute.should_receive(:respond_to?).with(:servers).and_return(true)
-      @fog_compute.should_receive(:servers).
-        and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
-
-      expect {
-        @connection.servers
-      }.to raise_error(Excon::Errors::RequestEntityTooLarge)
-    end
-  end
-end
diff --git a/bosh_openstack_cpi/spec/unit/helpers_spec.rb b/bosh_openstack_cpi/spec/unit/helpers_spec.rb
index 427a98167b2..4ded02b20c9 100644
--- a/bosh_openstack_cpi/spec/unit/helpers_spec.rb
+++ b/bosh_openstack_cpi/spec/unit/helpers_spec.rb
@@ -5,71 +5,169 @@
 
 describe Bosh::OpenStackCloud::Helpers do
   before(:each) do
+    @cloud = mock_cloud
     Bosh::Clouds::Config.stub(:task_checkpoint)
   end
 
-  it "should time out" do
-    cloud = mock_cloud
-
-    resource = double("resource")
-    resource.stub(:id).and_return("foobar")
-    resource.stub(:reload).and_return(cloud)
-    resource.stub(:status).and_return(:start)
-    cloud.stub(:sleep)
-
-    expect {
-      cloud.wait_resource(resource, :stop, :status, false, 0.1)
-    }.to raise_error Bosh::Clouds::CloudError, /Timed out/
-  end
-
-  it "should not time out" do
-    cloud = mock_cloud
-
-    resource = double("resource")
-    resource.stub(:id).and_return("foobar")
-    resource.stub(:reload).and_return(cloud)
-    resource.stub(:status).and_return(:start, :stop)
-    cloud.stub(:sleep)
-
-    cloud.wait_resource(resource, :stop, :status, false, 0.1)
-  end
-
-  it "should raise Bosh::Clouds::CloudError if state is error" do
-    cloud = mock_cloud
-
-    resource = double("resource")
-    resource.stub(:id).and_return("foobar")
-    resource.stub(:reload).and_return(cloud)
-    resource.stub(:status).and_return(:error)
-    cloud.stub(:sleep)
-
-    expect {
-      cloud.wait_resource(resource, :stop, :status, false, 0.1)
-    }.to raise_error Bosh::Clouds::CloudError, /state is error/
-  end
-
-  it "should raise Bosh::Clouds::CloudError if resource not found" do
-    cloud = mock_cloud
-
-    resource = double("resource")
-    resource.stub(:id).and_return("foobar")
-    resource.stub(:reload).and_return(nil)
-    cloud.stub(:sleep)
-
-    expect {
-      cloud.wait_resource(resource, :deleted, :status, false, 0.1)
-    }.to raise_error Bosh::Clouds::CloudError, /Resource not found/
+  describe "wait_resource" do
+    it "should time out" do
+      resource = double("resource")
+      resource.stub(:id).and_return("foobar")
+      resource.stub(:reload).and_return(@cloud)
+      resource.stub(:status).and_return(:start)
+      @cloud.stub(:sleep)
+
+      expect {
+        @cloud.wait_resource(resource, :stop, :status, false, 0.1)
+      }.to raise_error Bosh::Clouds::CloudError, /Timed out/
+    end
+
+    it "should not time out" do
+      resource = double("resource")
+      resource.stub(:id).and_return("foobar")
+      resource.stub(:reload).and_return(@cloud)
+      resource.stub(:status).and_return(:start, :stop)
+      @cloud.stub(:sleep)
+
+      @cloud.wait_resource(resource, :stop, :status, false, 0.1)
+    end
+
+    it "should raise Bosh::Clouds::CloudError if state is error" do
+      resource = double("resource")
+      resource.stub(:id).and_return("foobar")
+      resource.stub(:reload).and_return(@cloud)
+      resource.stub(:status).and_return(:error)
+      @cloud.stub(:sleep)
+
+      expect {
+        @cloud.wait_resource(resource, :stop, :status, false, 0.1)
+      }.to raise_error Bosh::Clouds::CloudError, /state is error/
+    end
+
+    it "should raise Bosh::Clouds::CloudError if resource not found" do
+      resource = double("resource")
+      resource.stub(:id).and_return("foobar")
+      resource.stub(:reload).and_return(nil)
+      @cloud.stub(:sleep)
+
+      expect {
+        @cloud.wait_resource(resource, :deleted, :status, false, 0.1)
+      }.to raise_error Bosh::Clouds::CloudError, /Resource not found/
+    end
+
+    it "should not raise and exception if resource not found" do
+      resource = double("resource")
+      resource.stub(:id).and_return("foobar")
+      resource.stub(:reload).and_return(nil)
+      resource.stub(:status).and_return(:deleted)
+      @cloud.stub(:sleep)
+
+      @cloud.wait_resource(resource, :deleted, :status, true, 0.1)
+    end
   end
 
-  it "should not raise and exception if resource not found" do
-    cloud = mock_cloud
-
-    resource = double("resource")
-    resource.stub(:id).and_return("foobar")
-    resource.stub(:reload).and_return(nil)
-    resource.stub(:status).and_return(:deleted)
-    cloud.stub(:sleep)
-
-    cloud.wait_resource(resource, :deleted, :status, true, 0.1)
+  describe "with_openstack" do
+    before(:each) do
+      @openstack = double("openstack")
+    end
+
+    it "should raise the exception if not RequestEntityTooLarge exception" do
+      response = Excon::Response.new(:body => "")
+
+      @openstack.should_receive(:servers)
+        .and_raise(Bosh::Clouds::CloudError)
+      @cloud.should_not_receive(:sleep)
+
+      expect {
+        @cloud.with_openstack do
+          @openstack.servers
+        end
+      }.to raise_error(Bosh::Clouds::CloudError)
+    end
+
+    it "should raise the exception if response has no body" do
+      response = Excon::Response.new(:body => "")
+
+      @openstack.should_receive(:servers)
+        .and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
+      @cloud.should_not_receive(:sleep)
+
+      expect {
+        @cloud.with_openstack do
+          @openstack.servers
+        end
+      }.to raise_error(Excon::Errors::RequestEntityTooLarge)
+    end
+
+    it "should raise the exception if response is not JSON" do
+      response = Excon::Response.new(:body => "foo = bar")
+
+      @openstack.should_receive(:servers)
+        .and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
+      @cloud.should_not_receive(:sleep)
+
+      expect {
+        @cloud.with_openstack do
+          @openstack.servers
+        end
+      }.to raise_error(Excon::Errors::RequestEntityTooLarge)
+    end
+
+    it "should retry the amount of seconds received at the response body" do
+      body = { "overLimit" => {
+          "message" => "This request was rate-limited.",
+          "code" => 413,
+          "retryAfter" => "5",
+          "details" => "Only 10 POST request(s) can be made to * every minute."}
+      }
+      response = Excon::Response.new(:body => JSON.dump(body))
+
+      @openstack.should_receive(:servers)
+        .and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
+      @cloud.should_receive(:sleep).with(5)
+      @openstack.should_receive(:servers).and_return(nil)
+
+      @cloud.with_openstack do
+        @openstack.servers
+      end
+    end
+
+    it "should retry the default number of seconds if not set at the response body" do
+      body = { "overLimitFault" => {
+          "message" => "This request was rate-limited.",
+          "code" => 413,
+          "details" => "Only 10 POST request(s) can be made to * every minute."}
+      }
+      response = Excon::Response.new(:body => JSON.dump(body))
+
+      @openstack.should_receive(:servers)
+        .and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
+      @cloud.should_receive(:sleep).with(1)
+      @openstack.should_receive(:servers).and_return(nil)
+
+      @cloud.with_openstack do
+        @openstack.servers
+      end
+    end
+
+    it "should retry the max number of retries" do
+      body = { "overLimit" => {
+          "message" => "This request was rate-limited.",
+          "code" => 413,
+          "retryAfter" => "5",
+          "details" => "Only 10 POST request(s) can be made to * every minute."}
+      }
+      response = Excon::Response.new(:body => JSON.dump(body))
+
+      @openstack.should_receive(:servers).exactly(11)
+        .and_raise(Excon::Errors::RequestEntityTooLarge.new("", "", response))
+      @cloud.should_receive(:sleep).with(5).exactly(10)
+
+      expect {
+        @cloud.with_openstack do
+          @openstack.servers
+        end
+      }.to raise_error(Excon::Errors::RequestEntityTooLarge)
+    end
   end
 end
diff --git a/bosh_openstack_cpi/spec/unit/validate_deployment_spec.rb b/bosh_openstack_cpi/spec/unit/validate_deployment_spec.rb
index 89c7d95757a..005d5c2c3b3 100644
--- a/bosh_openstack_cpi/spec/unit/validate_deployment_spec.rb
+++ b/bosh_openstack_cpi/spec/unit/validate_deployment_spec.rb
@@ -6,7 +6,8 @@
 describe Bosh::OpenStackCloud::Cloud do
 
   it "doesn't implement `validate_deployment'" do
-    Bosh::OpenStackCloud::Connection.stub(:new)
+    Fog::Compute.stub(:new)
+    Fog::Image.stub(:new)
     cloud = make_cloud
     expect {
       cloud.validate_deployment({}, {})

From e142a529c638c0595cfdda3211e063fddba4f3dd Mon Sep 17 00:00:00 2001
From: Ferran Rodenas <frodenas@gmail.com>
Date: Wed, 27 Mar 2013 00:17:14 +0100
Subject: [PATCH 2/3] [openstack_cpi] Use new fog method to retrieve security
 groups

---
 .../lib/cloud/openstack/cloud.rb              |  4 ++--
 .../spec/unit/configure_networks_spec.rb      | 21 ++++++++-----------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb b/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
index 42031a68f6a..bfd0d199a53 100644
--- a/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
+++ b/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
@@ -341,8 +341,8 @@ def configure_networks(server_id, network_spec)
         server = with_openstack { @openstack.servers.get(server_id) }
         cloud_error("Server `#{server_id}' not found") unless server
 
-        sg = with_openstack { @openstack.list_security_groups(server_id).body["security_groups"] }
-        actual = sg.collect { |s| s["name"] }.sort
+        sg = with_openstack { server.security_groups }
+        actual = sg.collect { |s| s.name }.sort
         new = network_configurator.security_groups(@default_security_groups)
 
         # If the security groups change, we need to recreate the VM
diff --git a/bosh_openstack_cpi/spec/unit/configure_networks_spec.rb b/bosh_openstack_cpi/spec/unit/configure_networks_spec.rb
index 49d5205f808..716c6ed1110 100644
--- a/bosh_openstack_cpi/spec/unit/configure_networks_spec.rb
+++ b/bosh_openstack_cpi/spec/unit/configure_networks_spec.rb
@@ -11,13 +11,12 @@
 
   it "forces recreation when security groups differ" do
     server = double("server", :id => "i-test", :name => "i-test")
-    sec_grp = double("security_group",
-                     :body => {"security_groups" => [{"name"=> "newgroup" }]})
+    security_group = double("security_groups", :name => "newgroups")
+
+    server.should_receive(:security_groups).and_return([security_group])
 
     cloud = mock_cloud do |openstack|
       openstack.servers.should_receive(:get).with("i-test").and_return(server)
-      openstack.should_receive(:list_security_groups).
-          with("i-test").and_return(sec_grp)
     end
 
     expect {
@@ -29,14 +28,13 @@
     server = double("server", :id => "i-test", :name => "i-test")
     address = double("address", :id => "a-test", :ip => "10.0.0.1",
                      :instance_id => nil)
-    sec_grp = double("security_group",
-                     :body => {"security_groups" => [{"name"=> "default" }]})
+    security_group = double("security_groups", :name => "default")
+
+    server.should_receive(:security_groups).and_return([security_group])
 
     cloud = mock_cloud do |openstack|
       openstack.servers.should_receive(:get).with("i-test").and_return(server)
       openstack.addresses.should_receive(:find).and_return(address)
-      openstack.should_receive(:list_security_groups).
-          with("i-test").and_return(sec_grp)
     end
 
     address.should_receive(:server=).with(server)
@@ -55,14 +53,13 @@
     server = double("server", :id => "i-test", :name => "i-test")
     address = double("address", :id => "a-test", :ip => "10.0.0.1",
                      :instance_id => "i-test")
-    sec_grp = double("security_group",
-                     :body => {"security_groups" => [{"name"=> "default" }]})
+    security_group = double("security_groups", :name => "default")
+
+    server.should_receive(:security_groups).and_return([security_group])
 
     cloud = mock_cloud do |openstack|
       openstack.servers.should_receive(:get).with("i-test").and_return(server)
       openstack.addresses.should_receive(:each).and_yield(address)
-      openstack.should_receive(:list_security_groups).
-          with("i-test").and_return(sec_grp)
     end
 
     address.should_receive(:server=).with(nil)

From f12939a3ca785702955f14d9861beae4b32894c3 Mon Sep 17 00:00:00 2001
From: Ferran Rodenas <frodenas@gmail.com>
Date: Wed, 27 Mar 2013 12:45:12 +0100
Subject: [PATCH 3/3] [bat] Add support for OpenStack manual networks

---
 bat/README.md                   | 47 +++++++++++++++++++++++++++++++++
 bat/templates/openstack.yml.erb | 45 ++++++++++++++++++++++++++-----
 2 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/bat/README.md b/bat/README.md
index f1069bd1fd3..b70357a1d42 100644
--- a/bat/README.md
+++ b/bat/README.md
@@ -72,6 +72,49 @@ properties:
     security_groups:
     - bat
 ```
+
+On OpenStack with DHCP:
+```yaml
+---
+cpi: openstack
+properties:
+  static_ip: 54.235.115.62 # floating IP to use for the bat-release jobs
+  uuid: 25569986-a7ed-4529-ba84-8a03e2c6c78f # BAT_DIRECTOR UUID
+  pool_size: 1
+  stemcell:
+    name: bosh-stemcell
+    version: latest
+  instances: 1
+  key_name: bosh # OpenStack key name
+  mbus: nats://nats:0b450ada9f830085e2cdeff6@10.42.49.80:4222 # Not used now, but don't remove
+```
+
+On OpenStack with manual networking (requires Quantum):
+```yaml
+---
+cpi: openstack
+properties:
+  static_ip: 54.235.115.62 # floating IP to use for the bat-release jobs
+  uuid: 25569986-a7ed-4529-ba84-8a03e2c6c78f # BAT_DIRECTOR UUID
+  pool_size: 1
+  stemcell:
+    name: bosh-stemcell
+    version: latest
+  instances: 1
+  key_name: bosh # OpenStack key name
+  mbus: nats://nats:0b450ada9f830085e2cdeff6@10.42.49.80:4222
+  network:
+    cidr: 10.0.1.0/24
+    reserved:
+    - 10.0.1.2 - 10.0.1.9
+    static:
+    - 10.0.1.10 - 10.0.1.30
+    gateway: 10.0.1.1
+    net_id: 4ef0b0ec-58c9-4478-8382-2099da773fdd #
+    security_groups:
+    - default
+```
+
 ## EC2 Networking Config
 
 ### On EC2 with AWS-provided DHCP networking
@@ -81,6 +124,10 @@ Add TCP port `4567` to the **default** security group.
 Create a **bat** security group in the same VPC the BAT_DIRECTOR is running in. Allow inbound access to TCP ports
  `22` and `4567` to the bat security group.
 
+## OpenStack Networking Config
+
+Add TCP ports `22` and `4567` to the **default** security group.
+
 ## Running BAT
 
 When all of the above is ready, running `bundle exec rake bat:env` will verify environment variables are set correctly.
diff --git a/bat/templates/openstack.yml.erb b/bat/templates/openstack.yml.erb
index f1134335e75..ce1d1c13406 100644
--- a/bat/templates/openstack.yml.erb
+++ b/bat/templates/openstack.yml.erb
@@ -7,11 +7,14 @@ release:
   version: <%= properties.release || "latest" %>
 
 compilation:
-  workers: 1
+  workers: 2
   network: default
   reuse_compilation_vms: true
   cloud_properties:
     instance_type: m1.small
+    <% if properties.key_name %>
+    key_name: <%= properties.key_name %>
+    <% end %>
 
 update:
   canaries: <%= properties.canaries || 1 %>
@@ -21,22 +24,49 @@ update:
   max_errors: 1
 
 networks:
+
+- name: static
+  type: vip
+  cloud_properties: {}
+
 - name: default
+<% if p('network.net_id', false) %>
+  type: manual
+  subnets:
+  - range: <%= properties.network.cidr %>
+    reserved:
+      <% properties.network.reserved.each do |range| %>
+      - <%= range %>
+      <% end %>
+    static:
+      <% properties.network.static.each do |range| %>
+      - <%= range %>
+      <% end %>
+    gateway: <%= properties.network.gateway %>
+    dns:
+      <% if_p('dns_nameserver') do |dns_nameserver| %>
+      - <%= dns_nameserver %>
+      <% end %>
+      <% if_p('network.dns') do |dns| %>
+      - <%= dns %>
+      <% end %>
+    cloud_properties:
+      security_groups: <%= p('network.security_groups') %>
+      net_id: <%= p('network.net_id') %>
+<% else %>
   type: dynamic
   <% if_p('dns_nameserver') do |dns_nameserver| %>
   dns:
     - <%= dns_nameserver %>
   <% end %>
-  <% if properties.security_group %>
+  <% if properties.security_groups %>
   cloud_properties:
     security_groups:
-      - <%= properties.security_group %>
+      - <%= properties.security_groups %>
   <% else %>
   cloud_properties: {}
   <% end %>
-- name: static
-  type: vip
-  cloud_properties: {}
+<% end %>
 
 resource_pools:
   - name: common
@@ -47,6 +77,9 @@ resource_pools:
       version: <%= properties.stemcell.version %>
     cloud_properties:
       instance_type: m1.small
+      <% if properties.key_name %>
+      key_name: <%= properties.key_name %>
+      <% end %>
     <% if properties.password %>
     env:
       bosh: