From d5feb4001276d6b12379c1e70908edaa064e3a82 Mon Sep 17 00:00:00 2001 From: siddheshwar-more Date: Wed, 14 Aug 2013 15:55:58 +0530 Subject: [PATCH 1/3] Implemented changes to set valid exit codes for knife-openstack --- lib/chef/knife/openstack_server_create.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/chef/knife/openstack_server_create.rb b/lib/chef/knife/openstack_server_create.rb index 74dd7776..4f806e82 100644 --- a/lib/chef/knife/openstack_server_create.rb +++ b/lib/chef/knife/openstack_server_create.rb @@ -23,6 +23,7 @@ require 'chef/knife/cloud/openstack_server_create_options' require 'chef/knife/cloud/openstack_service' require 'chef/knife/cloud/openstack_service_options' +require 'chef/knife/cloud/exceptions' class Chef class Knife @@ -70,11 +71,16 @@ def after_exec_command #floating requested without value if floating_address.nil? free_floating = addresses.find_index {|a| a.fixed_ip.nil?} - if free_floating.nil? #no free floating IP found - ui.error("Unable to assign a Floating IP from allocated IPs.") - exit 1 - else - floating_address = addresses[free_floating].ip + begin + if free_floating.nil? #no free floating IP found + ui.fatal("Unable to assign a Floating IP from allocated IPs.") + raise CloudExceptions::ServerSetupError + else + floating_address = addresses[free_floating].ip + end + rescue CloudExceptions::ServerSetupError => e + cleanup_on_failure + raise e end end server.associate_address(floating_address) @@ -98,7 +104,7 @@ def before_bootstrap Chef::Log.debug("Bootstrap IP Address: #{bootstrap_ip_address}") if bootstrap_ip_address.nil? ui.error("No IP address available for bootstrapping.") - raise "No IP address available for bootstrapping." + raise CloudExceptions::BootstrapError end config[:bootstrap_ip_address] = bootstrap_ip_address end @@ -111,7 +117,7 @@ def validate_params! errors << "You must provide --image-os-type option [windows/linux]" if ! (%w(windows linux).include?(locate_config_value(:image_os_type))) - exit 1 if errors.each{|e| ui.error(e)}.any? + raise CloudExceptions::ValidationError if errors.each{|e| ui.error(e)}.any? end end end From d2e00b12ef04bdc18d148ca3471ff699034e0328 Mon Sep 17 00:00:00 2001 From: siddheshwar-more Date: Wed, 14 Aug 2013 15:56:45 +0530 Subject: [PATCH 2/3] Updated rspec tests --- spec/unit/openstack_flavor_list_spec.rb | 6 +++--- spec/unit/openstack_group_list_spec.rb | 6 +++--- spec/unit/openstack_image_list_spec.rb | 6 +++--- spec/unit/openstack_server_create_spec.rb | 16 ++++++++-------- spec/unit/openstack_server_delete_spec.rb | 6 +++--- spec/unit/openstack_server_list_spec.rb | 6 +++--- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/spec/unit/openstack_flavor_list_spec.rb b/spec/unit/openstack_flavor_list_spec.rb index b28acabc..63862601 100644 --- a/spec/unit/openstack_flavor_list_spec.rb +++ b/spec/unit/openstack_flavor_list_spec.rb @@ -23,19 +23,19 @@ it "raise error on openstack_username missing" do Chef::Config[:knife].delete(:openstack_username) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Username' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_password missing" do Chef::Config[:knife].delete(:openstack_password) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Password' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_auth_url missing" do Chef::Config[:knife].delete(:openstack_auth_url) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Auth Url' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end end diff --git a/spec/unit/openstack_group_list_spec.rb b/spec/unit/openstack_group_list_spec.rb index ccc7dc29..81fa4e07 100644 --- a/spec/unit/openstack_group_list_spec.rb +++ b/spec/unit/openstack_group_list_spec.rb @@ -23,19 +23,19 @@ it "raise error on openstack_username missing" do Chef::Config[:knife].delete(:openstack_username) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Username' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_password missing" do Chef::Config[:knife].delete(:openstack_password) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Password' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_auth_url missing" do Chef::Config[:knife].delete(:openstack_auth_url) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Auth Url' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end end diff --git a/spec/unit/openstack_image_list_spec.rb b/spec/unit/openstack_image_list_spec.rb index 4a834923..a5bb9a0c 100644 --- a/spec/unit/openstack_image_list_spec.rb +++ b/spec/unit/openstack_image_list_spec.rb @@ -23,19 +23,19 @@ it "raise error on openstack_username missing" do Chef::Config[:knife].delete(:openstack_username) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Username' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_password missing" do Chef::Config[:knife].delete(:openstack_password) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Password' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_auth_url missing" do Chef::Config[:knife].delete(:openstack_auth_url) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Auth Url' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end end diff --git a/spec/unit/openstack_server_create_spec.rb b/spec/unit/openstack_server_create_spec.rb index d0249a33..4a9b9b7b 100644 --- a/spec/unit/openstack_server_create_spec.rb +++ b/spec/unit/openstack_server_create_spec.rb @@ -37,20 +37,20 @@ it "raise error on openstack_username missing and exit immediately." do Chef::Config[:knife].delete(:openstack_username) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Username' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_auth_url missing and exit immediately." do Chef::Config[:knife].delete(:openstack_auth_url) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Auth Url' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "validates ssh params" do Chef::Config[:knife][:image_os_type] = "linux" Chef::Config[:knife][:bootstrap_protocol] = "ssh" instance.ui.should_receive(:error).with("You must provide either Identity file or SSH Password.") - instance.validate_params! + expect { instance.validate_params! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise_error on invalid image_os_type params" do @@ -58,7 +58,7 @@ Chef::Config[:knife][:openstack_ssh_key_id] = "ssh_key" Chef::Config[:knife][:image_os_type] = "other_than_windows_linux" instance.ui.should_receive(:error).with("You must provide --image-os-type option [windows/linux]") - instance.validate_params! + expect { instance.validate_params! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise_error on mising image_os_type params" do @@ -66,7 +66,7 @@ Chef::Config[:knife][:ssh_password] = "ssh_password" Chef::Config[:knife][:openstack_ssh_key_id] = "ssh_key" instance.ui.should_receive(:error).with("You must provide --image-os-type option [windows/linux]") - instance.validate_params! + expect { instance.validate_params! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end context "bootstrap protocol: Ssh " do @@ -79,14 +79,14 @@ Chef::Config[:knife][:identity_file] = nil Chef::Config[:knife][:ssh_password] = nil instance.ui.should_receive(:error).with("You must provide either Identity file or SSH Password.") - instance.validate_params! + expect { instance.validate_params! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error when Identity file is provided but SSH key is not provided and exits immediately." do Chef::Config[:knife][:identity_file] = "identity_file_path" Chef::Config[:knife][:openstack_ssh_key_id] = nil instance.ui.should_receive(:error).with("You must provide SSH Key.") - instance.validate_params! + expect { instance.validate_params! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "validates gracefully when SSH password is provided." do @@ -132,7 +132,7 @@ Chef::Config[:knife][:winrm_password] = nil instance.config[:winrm_password] = nil instance.ui.should_receive(:error).with("You must provide Winrm Password.") - instance.validate_params! + expect { instance.validate_params! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end end end diff --git a/spec/unit/openstack_server_delete_spec.rb b/spec/unit/openstack_server_delete_spec.rb index 723257a3..5d28239f 100644 --- a/spec/unit/openstack_server_delete_spec.rb +++ b/spec/unit/openstack_server_delete_spec.rb @@ -26,19 +26,19 @@ it "raise error on openstack_username missing" do Chef::Config[:knife].delete(:openstack_username) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Username' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_password missing" do Chef::Config[:knife].delete(:openstack_password) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Password' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_auth_url missing" do Chef::Config[:knife].delete(:openstack_auth_url) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Auth Url' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end end diff --git a/spec/unit/openstack_server_list_spec.rb b/spec/unit/openstack_server_list_spec.rb index ed0e5fe2..494689bb 100644 --- a/spec/unit/openstack_server_list_spec.rb +++ b/spec/unit/openstack_server_list_spec.rb @@ -23,19 +23,19 @@ it "raise error on openstack_username missing" do Chef::Config[:knife].delete(:openstack_username) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Username' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_password missing" do Chef::Config[:knife].delete(:openstack_password) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Password' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end it "raise error on openstack_auth_url missing" do Chef::Config[:knife].delete(:openstack_auth_url) instance.ui.should_receive(:error).with("You did not provide a valid 'Openstack Auth Url' value.") - instance.validate! + expect { instance.validate! }.to raise_error(Chef::Knife::Cloud::CloudExceptions::ValidationError) end end From 433d2da87df54ccc52293e0a3e5953eae815acc0 Mon Sep 17 00:00:00 2001 From: siddheshwar-more Date: Fri, 16 Aug 2013 16:17:02 +0530 Subject: [PATCH 3/3] Implemented review comments to send error message --- lib/chef/knife/openstack_server_create.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/chef/knife/openstack_server_create.rb b/lib/chef/knife/openstack_server_create.rb index 4f806e82..938481c8 100644 --- a/lib/chef/knife/openstack_server_create.rb +++ b/lib/chef/knife/openstack_server_create.rb @@ -73,8 +73,9 @@ def after_exec_command free_floating = addresses.find_index {|a| a.fixed_ip.nil?} begin if free_floating.nil? #no free floating IP found - ui.fatal("Unable to assign a Floating IP from allocated IPs.") - raise CloudExceptions::ServerSetupError + error_message = "Unable to assign a Floating IP from allocated IPs." + ui.fatal(error_message) + raise CloudExceptions::ServerSetupError, error_message else floating_address = addresses[free_floating].ip end @@ -103,8 +104,9 @@ def before_bootstrap bootstrap_ip_address = primary_private_ip_address(server.addresses) if config[:private_network] Chef::Log.debug("Bootstrap IP Address: #{bootstrap_ip_address}") if bootstrap_ip_address.nil? - ui.error("No IP address available for bootstrapping.") - raise CloudExceptions::BootstrapError + error_message = "No IP address available for bootstrapping." + ui.error(error_message) + raise CloudExceptions::BootstrapError, error_message end config[:bootstrap_ip_address] = bootstrap_ip_address end @@ -116,8 +118,8 @@ def validate_params! errors << "You must provide SSH Key." if locate_config_value(:bootstrap_protocol) == 'ssh' && !locate_config_value(:identity_file).nil? && locate_config_value(:openstack_ssh_key_id).nil? errors << "You must provide --image-os-type option [windows/linux]" if ! (%w(windows linux).include?(locate_config_value(:image_os_type))) - - raise CloudExceptions::ValidationError if errors.each{|e| ui.error(e)}.any? + error_message = "" + raise CloudExceptions::ValidationError, error_message if errors.each{|e| ui.error(e); error_message = "#{error_message} #{e}."}.any? end end end