The instance_action method in server.rb is clearly expecting the individual drivers to return an "Instance" object when the request is successfully fulfilled. Unfortunately, the return value from the individual drivers is woefully inconsistent here; some of them return a new Instance object, and some do not.
Further, as per https://fedorahosted.org/pipermail/deltacloud-devel/2010-May/001130.html it seems like we should be doing a better job handling cases where we can create a full Instance object vs. a truncated one. To help fix both of these, the following patch: 1) Changes the haml file to only show the fields of the instance that are actually available. This means that if an individual driver cannot reasonably return good information about an instance after an action has been performed, it can at least return the href and id of the instance; it would then be up to the client to do another GET with the instance HREF to get additional information. 2) Fixes up the EC2 driver to return the expected instance object. In particular, we now throw an exception if the action fails, but if the follow-up request for instance information fails, we just return a truncated Instance object. In going over the drivers, it looks like: Mock - returns Instance object - good EC2 - (after this patch) returns Instance object - good GoGrid - does not return Instance object - bad OpenNebula - returns Instance object - good RackSpace - does not return Instance object - bad RHEV-M - returns Instance object - good Rimu - does not return Instance object - bad So if it is agreed this is the direction to go, I will fix up the other broken drivers as well. Signed-off-by: Chris Lalancette <[email protected]> --- server/lib/deltacloud/drivers/ec2/ec2_driver.rb | 54 +++++++++++++++++++-- server/views/instances/show.xml.haml | 57 +++++++++++++---------- 2 files changed, 81 insertions(+), 30 deletions(-) diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb index 69356e2..492a059 100644 --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb @@ -168,22 +168,64 @@ class EC2Driver < Deltacloud::BaseDriver def reboot_instance(credentials, id) ec2 = new_client(credentials) - safely do - ec2.reboot_instances( :instance_id => id ) + backup = ec2.reboot_instances( :instance_id => id ) + + begin + this_instance = ec2.describe_instances( :instance_id => id).reservationSet.item.first.instancesSet.item.first + convert_instance(this_instance, this_instance.ownerId) + rescue Exception => e + puts "ERROR: #{e.message}" + # at this point, the reboot has succeeded but our follow-up + # "describe_instances" failed for some reason. Create a simple Instance + # object with only the ID and new state in place + state = backup.instancesSet.item.first.currentState.name + Instance.new( { + :id => id, + :state => state, + :actions => instance_actions_for( state ), + } ) end end def stop_instance(credentials, id) ec2 = new_client(credentials) - safely do - ec2.terminate_instances( :instance_id => id ) + backup = ec2.terminate_instances( :instance_id => id ) + + begin + this_instance = ec2.describe_instances( :instance_id => id ).reservationSet.item.first.instancesSet.item.first + convert_instance(this_instance, this_instance.ownerId) + rescue Exception => e + puts "ERROR: #{e.message}" + # at this point, the stop has succeeded but our follow-up + # "describe_instances" failed for some reason. Create a simple Instance + # object with only the ID and new state in place + state = backup.instancesSet.item.first.currentState.name + Instance.new( { + :id => id, + :state => state, + :actions => instance_actions_for( state ), + } ) end end def destroy_instance(credentials, id) ec2 = new_client(credentials) - safely do - ec2.terminate_instances( :instance_id => id ) + backup = ec2.terminate_instances( :instance_id => id ) + + begin + this_instance = ec2.describe_instances( :instance_id => id).reservationSet.item.first.instancesSet.item.first + convert_instance(this_instance, this_instance.ownerId) + rescue Exception => e + puts "ERROR: #{e.message}" + # at this point, the destroy has succeeded but our follow-up + # "describe_instances" failed for some reason. Create a simple Instance + # object with only the ID and new state in place + state = backup.instancesSet.item.first.currentState.name + Instance.new( { + :id => id, + :state => state, + :actions => instance_actions_for( state ), + } ) end end diff --git a/server/views/instances/show.xml.haml b/server/views/instances/show.xml.haml index a4859b0..9ed1b89 100644 --- a/server/views/instances/show.xml.haml +++ b/server/views/instances/show.xml.haml @@ -2,27 +2,36 @@ %instance{:href => instance_url(@instance.id)} %id< [email protected] - %name< - [email protected] - %owner_id< - [email protected]_id - %image{:href => image_url(@instance.image_id)} - %realm{:href => realm_url(@instance.realm_id)} - %state< - [email protected] - - haml_tag :"hardware-profile", {:href => hardware_profile_url(@instance.instance_profile.id)} do - %id< - [email protected]_profile.id - - @instance.instance_profile.overrides.each do |p, v| - %property{:kind => 'fixed', :name => p, :value => v, :unit => Deltacloud::HardwareProfile::unit(p)} - %actions - - @instance.actions.compact.each do |instance_action| - %link{:rel => instance_action, :method => instance_action_method(instance_action), :href => self.send("#{instance_action}_instance_url", @instance.id)} - %public-addresses - - @instance.public_addresses.each do |address| - %address< - =address - %private-addresses - - @instance.private_addresses.each do |address| - %address< - =address + - if @instance.name + %name< + [email protected] + - if @instance.owner_id + %owner_id< + [email protected]_id + - if @instance.image_id + %image{:href => image_url(@instance.image_id)} + - if @instance.realm_id + %realm{:href => realm_url(@instance.realm_id)} + - if @instance.state + %state< + [email protected] + - if @instance.instance_profile + - haml_tag :"hardware-profile", {:href => hardware_profile_url(@instance.instance_profile.id)} do + %id< + [email protected]_profile.id + - @instance.instance_profile.overrides.each do |p, v| + %property{:kind => 'fixed', :name => p, :value => v, :unit => Deltacloud::HardwareProfile::unit(p)} + - if @instance.actions + %actions + - @instance.actions.compact.each do |instance_action| + %link{:rel => instance_action, :method => instance_action_method(instance_action), :href => self.send("#{instance_action}_instance_url", @instance.id)} + - if @instance.public_addresses + %public-addresses + - @instance.public_addresses.each do |address| + %address< + =address + - if @instance.private_addresses + %private-addresses + - @instance.private_addresses.each do |address| + %address< + =address -- 1.6.6.1 _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
