You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by lu...@redhat.com on 2012/12/04 09:42:28 UTC

A few more CIMI fixes

Here are a few more fixes for CIMI that we uncovered today in
testing. There's still quite a bit of trouble with running against other
providers.

One of the sources of headaches is when a provider uses relative URI's
everywhere - I added a post helper to make sure we properly translate from
relative to absolute URI. We need to do that for places where we directly
call RestClient.put and RestClient.delete, too.

There's also at least one place where we construct a URL in test_helper.rb
(in teardown constructing the stop URL for a machine)

The tests also run very slowly - one reason is that we do not cache
anything; we especially hit the CEP a lot, which with the network I am on
is painful (2-3s for each CEP request) We should try and make the cep
method in test_helper cache the response (one for xml, one for json) to
speed up the tests.

Other than that, things are looking good ;)

David

[PATCH 4/6] CIMI tests: add helper for post

Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>

We need to make sure URL's are properly expanded when providers use
relative URL's. Therefore, using Restclient.post directly won't work.
---
 tests/cimi/test_helper.rb |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/tests/cimi/test_helper.rb b/tests/cimi/test_helper.rb
index 0e102c4..fccfcbc 100644
--- a/tests/cimi/test_helper.rb
+++ b/tests/cimi/test_helper.rb
@@ -141,6 +141,13 @@ module CIMI::Test::Methods
       RestClient.get absolute_url(path), headers(params)
     end
 
+    def post(path, body, params = {})
+      log_request(:post, path, :params => params, :body => body)
+      resp = RestClient.post absolute_url(path), body, headers(params)
+      log_response(:post, path, resp)
+      resp
+    end
+
     # Find the model class that can process the body of the HTTP response
     # +resp+
     def model_class(resp)
@@ -196,6 +203,22 @@ module CIMI::Test::Methods
       @log
     end
 
+    def log_request(method, path, opts = {})
+      log.debug("#{method.to_s.upcase} #{absolute_url(path)}")
+      if opts[:params]
+        h = headers(opts[:params])
+        h.keys.sort.each { |k| log.debug "  #{k}: #{h[k]}" }
+      end
+      log.debug opts[:body] if opts[:body]
+    end
+
+    def log_response(method, path, resp)
+      log.debug "--> #{resp.code} #{resp.headers[:content_type]}"
+      resp.headers.keys.each { |k| log.debug "#{k}: /#{resp.headers[k]}/" }
+      log.debug resp.body
+      log.debug "/#{method.to_s.upcase} #{absolute_url(path)}"
+    end
+
     def poll_state(machine, state)
       while not machine.state.upcase.eql?(state)
         puts state
-- 
1.7.7.6


[PATCH 2/6] CIMI: base_uri must end in a slash

Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>

---
 server/lib/cimi/models/cloud_entry_point.rb |    2 +-
 tests/cimi/test_helper.rb                   |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/lib/cimi/models/cloud_entry_point.rb b/server/lib/cimi/models/cloud_entry_point.rb
index c03c524..52a3015 100644
--- a/server/lib/cimi/models/cloud_entry_point.rb
+++ b/server/lib/cimi/models/cloud_entry_point.rb
@@ -39,7 +39,7 @@ class CIMI::Model::CloudEntryPoint < CIMI::Model::Base
       :name => context.driver.name,
       :description => "Cloud Entry Point for the Deltacloud #{context.driver.name} driver",
       :id => context.cloudEntryPoint_url,
-      :base_uri => context.base_uri,
+      :base_uri => context.base_uri + "/",
       :created => Time.now.xmlschema
     }))
   end
diff --git a/tests/cimi/test_helper.rb b/tests/cimi/test_helper.rb
index 9e5f6a3..009bac6 100644
--- a/tests/cimi/test_helper.rb
+++ b/tests/cimi/test_helper.rb
@@ -157,9 +157,9 @@ module CIMI::Test::Methods
       if path.start_with?("http")
         path
       elsif path.start_with?("/")
-        api.base_uri + path
+        api.base_uri + path[1, path.size]
       else
-        api.base_uri + "/#{path}"
+        api.base_uri + "#{path}"
       end
     end
 
-- 
1.7.7.6


Re: A few more CIMI fixes

Posted by "marios@redhat.com" <ma...@redhat.com>.
ACK && PUSH

I'll update dev.deltacloud.org now to reflect this

marios

On 04/12/12 10:42, lutter@redhat.com wrote:
> Here are a few more fixes for CIMI that we uncovered today in
> testing. There's still quite a bit of trouble with running against other
> providers.
> 
> One of the sources of headaches is when a provider uses relative URI's
> everywhere - I added a post helper to make sure we properly translate from
> relative to absolute URI. We need to do that for places where we directly
> call RestClient.put and RestClient.delete, too.
> 
> There's also at least one place where we construct a URL in test_helper.rb
> (in teardown constructing the stop URL for a machine)
> 
> The tests also run very slowly - one reason is that we do not cache
> anything; we especially hit the CEP a lot, which with the network I am on
> is painful (2-3s for each CEP request) We should try and make the cep
> method in test_helper cache the response (one for xml, one for json) to
> speed up the tests.
> 
> Other than that, things are looking good ;)
> 
> David
> 


[PATCH 5/6] CIMI tests: allow setting the Content-Type header for requests

Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>

---
 tests/cimi/test_helper.rb |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/tests/cimi/test_helper.rb b/tests/cimi/test_helper.rb
index fccfcbc..e967e55 100644
--- a/tests/cimi/test_helper.rb
+++ b/tests/cimi/test_helper.rb
@@ -181,12 +181,15 @@ module CIMI::Test::Methods
     def headers(params)
       headers = api.auth_header
       if params[:accept]
-        headers["Accept"] = "application/#{params.delete(:accept)}"
+        headers["Accept"] = "application/#{params[:accept]}"
       else
         # @content_type is set by the harness below
         # if it isn't, default to XML
         headers["Accept"] = @content_type || "application/xml"
       end
+      if params[:content_type]
+        headers["Content-Type"] = "application/#{params[:content_type]}"
+      end
       headers
     end
 
-- 
1.7.7.6


[PATCH 1/6] CIMI: make CIMI::Model::VolumeImage.find behave like Machine.find

Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>

---
 server/lib/cimi/models/volume.rb |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/server/lib/cimi/models/volume.rb b/server/lib/cimi/models/volume.rb
index 7951a99..6a6d42f 100644
--- a/server/lib/cimi/models/volume.rb
+++ b/server/lib/cimi/models/volume.rb
@@ -38,12 +38,15 @@ class CIMI::Model::Volume < CIMI::Model::Base
   end
 
   def self.find(id, context)
-    volumes = []
-    opts = ( id == :all ) ? {} : { :id => id }
-    volumes = context.driver.storage_volumes(context.credentials, opts)
-    volumes.collect!{ |volume| from_storage_volume(volume, context) }
-    return volumes.first unless volumes.length > 1 || volumes.length == 0
-    return volumes
+    creds = context.credentials
+    if id == :all
+      volumes = context.driver.storage_volumes(creds)
+      volumes.collect{ |volume| from_storage_volume(volume, context) }
+    else
+      volume = context.driver.storage_volumes(creds)
+      raise CIMI::Model::NotFound unless volume
+      from_storage_volume(volume)
+    end
   end
 
   def self.all(context); find(:all, context); end
-- 
1.7.7.6


[PATCH 3/6] CIMI tests: do not send any auth header if no user is set

Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>

---
 tests/cimi/test_helper.rb |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/tests/cimi/test_helper.rb b/tests/cimi/test_helper.rb
index 009bac6..0e102c4 100644
--- a/tests/cimi/test_helper.rb
+++ b/tests/cimi/test_helper.rb
@@ -50,6 +50,14 @@ module CIMI
         "Basic #{Base64.encode64("#{u}:#{p}")}"
       end
 
+      def auth_header
+        if @cimi["user"]
+          { "Authorization" => basic_auth }
+        else
+          {}
+        end
+      end
+
       def preferred
         @cimi["preferred"] || {}
       end
@@ -164,9 +172,7 @@ module CIMI::Test::Methods
     end
 
     def headers(params)
-      headers = {
-        'Authorization' => api.basic_auth
-      }
+      headers = api.auth_header
       if params[:accept]
         headers["Accept"] = "application/#{params.delete(:accept)}"
       else
-- 
1.7.7.6


[PATCH 6/6] CIMI tests: use post helper instead of RestClient.post

Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>

---
 tests/cimi/part2_test.rb  |    7 +++----
 tests/cimi/part3_test.rb  |    8 ++++----
 tests/cimi/part4_test.rb  |   20 ++++++++++----------
 tests/cimi/part5_test.rb  |    4 ++--
 tests/cimi/test_helper.rb |    4 ++--
 5 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/tests/cimi/part2_test.rb b/tests/cimi/part2_test.rb
index c273614..cc6041d 100644
--- a/tests/cimi/part2_test.rb
+++ b/tests/cimi/part2_test.rb
@@ -56,8 +56,8 @@ end
     cep_json = cep(:accept => :json)
     #discover the 'addURI' for creating Machine
     add_uri = discover_uri_for("add", "machines")
-    RestClient.post(add_uri,
-      "<Machine>" +
+    post(add_uri,
+      "<MachineCreate xmlns=\"#{CIMI::Test::CIMI_NAMESPACE}\">" +
         "<name>cimi_machine_" + fmt.to_s() + "</name>" +
         "<machineTemplate>" +
           "<machineConfig " +
@@ -65,8 +65,7 @@ end
           "<machineImage " +
             "href=\"" + get_a(cep_json, "machineImage") + "\"/>" +
         "</machineTemplate>" +
-      "</Machine>",
-      {'Authorization' => api.basic_auth, :accept => fmt})
+      "</MachineCreate>", :content_type => :xml)
   end
 
   it "should add resource for cleanup" do
diff --git a/tests/cimi/part3_test.rb b/tests/cimi/part3_test.rb
index d7bb45b..9703618 100644
--- a/tests/cimi/part3_test.rb
+++ b/tests/cimi/part3_test.rb
@@ -39,7 +39,7 @@ class CreateNewMachineFromMachineTemplate < CIMI::Test::Spec
   #create a machineTemplate for use in these tests:
   cep_json = cep(:accept => :json)
   mach_templ_add_uri = discover_uri_for("add", "machineTemplates")
-  mach_templ_created = RestClient.post(mach_templ_add_uri,
+  mach_templ_created = post(mach_templ_add_uri,
     "<MachineTemplateCreate>" +
       "<name>cimi_machineTemplate1</name>"+
       "<description>A CIMI MachineTemplate, created by part3_test.rb</description>"+
@@ -49,7 +49,7 @@ class CreateNewMachineFromMachineTemplate < CIMI::Test::Spec
       "<machineImage " +
         "href=\"" + get_a(cep_json, "machineImage") + "\"/>" +
     "</MachineTemplateCreate>",
-    {'Authorization' => api.basic_auth, :accept => :json})
+    :accept => :json, :content_type => :xml)
 
   # 3.1: Query the CEP
   model :machineTemplate  do |fmt|
@@ -91,14 +91,14 @@ class CreateNewMachineFromMachineTemplate < CIMI::Test::Spec
     cep_json = cep(:accept => :json)
     #discover the 'addURI' for creating Machine
     add_uri = discover_uri_for("add", "machines")
-    RestClient.post(add_uri,
+    post(add_uri,
       "<Machine>" +
         "<name>cimi_machine_from_template" + fmt.to_s() + "</name>" +
         "<description> Created machine from template" + fmt.to_s() + "</description>" +
         "<machineTemplate " +
           "href=\"" + get_a(cep_json, "machineTemplate")+ "\"/>" +
       "</Machine>",
-    {'Authorization' => api.basic_auth, :accept => fmt})
+         :accept => fmt, :content_type => :xml)
   end
 
   it "should add resource for cleanup" do
diff --git a/tests/cimi/part4_test.rb b/tests/cimi/part4_test.rb
index f065104..72f22af 100644
--- a/tests/cimi/part4_test.rb
+++ b/tests/cimi/part4_test.rb
@@ -53,8 +53,8 @@ class AddVolumeToMachine < CIMI::Test::Spec
 # Create a machine to attach the volume
    cep_json = cep(:accept => :json)
    machine_add_uri = discover_uri_for("add", "machines")
-   machine = RestClient.post(machine_add_uri,
-     "<Machine>" +
+   machine = post(machine_add_uri,
+     "<MachineCreate xmlns=\"#{CIMI::Test::CIMI_NAMESPACE}\">" +
        "<name>cimi_machine</name>" +
        "<machineTemplate>" +
          "<machineConfig " +
@@ -62,13 +62,13 @@ class AddVolumeToMachine < CIMI::Test::Spec
          "<machineImage " +
            "href=\"" + get_a(cep_json, "machineImage") + "\"/>" +
        "</machineTemplate>" +
-     "</Machine>",
-     {'Authorization' => api.basic_auth, :accept => :json})
+     "</MachineCreate>",
+                  {:accept => :json, :content_type => :xml})
 
   # 4.3:  Create a new Volume
   model :volume do |fmt|
     volume_add_uri = discover_uri_for("add", "volumes")
-    RestClient.post(volume_add_uri,
+    post(volume_add_uri,
       "<Volume>" +
         "<name>cimi_volume_" + fmt.to_s() +"</name>" +
         "<description>volume for testing</description>" +
@@ -77,7 +77,7 @@ class AddVolumeToMachine < CIMI::Test::Spec
           "</volumeConfig>" +
         "</volumeTemplate>" +
       "</Volume>",
-    {'Authorization' => api.basic_auth, :accept => fmt})
+         :accept => fmt, :content_type => :xml)
   end
 
   it "should add resource machine resource for cleanup", :only => :json do
@@ -105,7 +105,7 @@ class AddVolumeToMachine < CIMI::Test::Spec
 
   log.info(machine.json["id"].to_s() + " is the machine id")
   volume_add_uri = discover_uri_for("add", "volumes")
-  volume = RestClient.post(volume_add_uri,
+  volume = post(volume_add_uri,
   "<Volume>" +
     "<name>cimi_volume_for_attach</name>" +
     "<description>volume for attach testing</description>" +
@@ -114,18 +114,18 @@ class AddVolumeToMachine < CIMI::Test::Spec
       "</volumeConfig>" +
     "</volumeTemplate>" +
   "</Volume>",
-{'Authorization' => api.basic_auth, :accept => :json})
+  :accept => :json, :content_type => :xml)
 
   log.info(volume.json["id"].to_s() + " is the volume id")
   # 4.4: Attach the new Volume to a Machine
   model :machineWithVolume, :only => :xml do
   attach_uri = discover_uri_for_subcollection("add", machine.json['id'], "volumes")
-    RestClient.post(attach_uri,
+    post(attach_uri,
     "<MachineVolume xmlns=\"http://schemas.dmtf.org/cimi/1/MachineVolume\">" +
     "<initialLocation>/dev/sdf</initialLocation>" +
     "<volume href=\"" + volume.json["id"] + "\"/>" +
     "</MachineVolume>",
-    {'Authorization' => api.basic_auth, :accept => :xml})
+    :accept => :xml, :content_type => :xml)
   end
 
   it "should have a response code equal to 201 for attaching a volume", :only => :xml do
diff --git a/tests/cimi/part5_test.rb b/tests/cimi/part5_test.rb
index f1ce6c9..185668a 100644
--- a/tests/cimi/part5_test.rb
+++ b/tests/cimi/part5_test.rb
@@ -34,7 +34,7 @@ class ManipulateAMachine < CIMI::Test::Spec
   cep_json = cep(:accept => :json)
   #discover machine create URI:
   machine_add_uri = discover_uri_for("add", "machines")
-  machine_created = RestClient.post(machine_add_uri,
+  machine_created = post(machine_add_uri,
     "<Machine>" +
       "<name>cimi_machine_part5</name>" +
       "<machineTemplate>" +
@@ -44,7 +44,7 @@ class ManipulateAMachine < CIMI::Test::Spec
           "href=\"" + get_a(cep_json, "machineImage") + "\"/>" +
       "</machineTemplate>" +
     "</Machine>",
-    {'Authorization' => api.basic_auth, :accept => :json})
+    :accept => :json, :content_type => :xml)
 
   model :machine do |fmt|
     get machine_created.json["id"], :accept => fmt
diff --git a/tests/cimi/test_helper.rb b/tests/cimi/test_helper.rb
index e967e55..0d318bf 100644
--- a/tests/cimi/test_helper.rb
+++ b/tests/cimi/test_helper.rb
@@ -233,11 +233,11 @@ module CIMI::Test::Methods
 
     def machine_stop_start(machine, action, state)
       uri = discover_uri_for(action, "", machine.operations)
-      response = RestClient.post( uri,
+      response = post( uri,
             "<Action xmlns=\"http://schemas.dmtf.org/cimi/1\">" +
               "<action> http://http://schemas.dmtf.org/cimi/1/action/" + action + "</action>" +
             "</Action>",
-            {'Authorization' => api.basic_auth, :accept => :xml })
+            :accept => :xml, :content_type => :xml)
       response.code.must_equal 202
       poll_state(machine(:refetch => true), state)
       machine(:refetch => true).state.upcase.must_equal state
-- 
1.7.7.6