From 9e838c0aa5d6a80d4799abff3c9b3a9157af2c96 Mon Sep 17 00:00:00 2001 From: siddheshwar-more Date: Fri, 9 Aug 2013 16:33:45 +0530 Subject: [PATCH 1/3] Rename some options, make image-os-type option mandatory for knife-openstack and rspec tests for same change --- .../cloud/openstack_server_create_options.rb | 4 ++-- lib/chef/knife/openstack_server_create.rb | 5 ++++- spec/unit/openstack_server_create_spec.rb | 21 +++++++++++++++++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/chef/knife/cloud/openstack_server_create_options.rb b/lib/chef/knife/cloud/openstack_server_create_options.rb index ff636d70..e75521c4 100644 --- a/lib/chef/knife/cloud/openstack_server_create_options.rb +++ b/lib/chef/knife/cloud/openstack_server_create_options.rb @@ -30,9 +30,9 @@ def self.included(includer) :default => ["default"], :proc => Proc.new { |groups| groups.split(',') } - option :ssh_key_name, + option :openstack_ssh_key_id, :short => "-S KEY", - :long => "--ssh-key KEY", + :long => "--openstack-ssh-key-id KEY", :description => "The OpenStack SSH keypair id", :proc => Proc.new { |key| Chef::Config[:knife][:openstack_ssh_key_id] = key } diff --git a/lib/chef/knife/openstack_server_create.rb b/lib/chef/knife/openstack_server_create.rb index 805dd1f0..3bee5cfe 100644 --- a/lib/chef/knife/openstack_server_create.rb +++ b/lib/chef/knife/openstack_server_create.rb @@ -105,7 +105,7 @@ def validate_params! if locate_config_value(:identity_file).nil? && locate_config_value(:ssh_password).nil? errors << "You must provide either Identity file or SSH Password." end - if !locate_config_value(:identity_file).nil? && (config[:ssh_key_name].nil? && Chef::Config[:knife][:openstack_ssh_key_id].nil?) + if !locate_config_value(:identity_file).nil? && locate_config_value(:openstack_ssh_key_id).nil? errors << "You must provide SSH Key." end elsif locate_config_value(:bootstrap_protocol) == 'winrm' @@ -115,6 +115,9 @@ def validate_params! else errors << "You must provide a valid bootstrap protocol. options [ssh/winrm]. For linux type images, options [ssh]" end + + 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? end end diff --git a/spec/unit/openstack_server_create_spec.rb b/spec/unit/openstack_server_create_spec.rb index cb03d718..d0249a33 100644 --- a/spec/unit/openstack_server_create_spec.rb +++ b/spec/unit/openstack_server_create_spec.rb @@ -47,15 +47,32 @@ end it "validates ssh params" do - Chef::Config[:knife][:image_os] = "other" + 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! end + it "raise_error on invalid image_os_type params" do + Chef::Config[:knife][:ssh_password] = "ssh_password" + 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! + end + + it "raise_error on mising image_os_type params" do + Chef::Config[:knife][:image_os_type] = "other_than_windows_linux" + 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! + end + context "bootstrap protocol: Ssh " do before(:each) do Chef::Config[:knife][:bootstrap_protocol] = "ssh" + Chef::Config[:knife][:image_os_type] = 'linux' end it "raise error when neither identity file nor SSH password is provided and exits immediately." do @@ -96,7 +113,7 @@ before(:each) do instance.configure_chef instance.config[:bootstrap_protocol] = 'winrm' - Chef::Config[:knife][:image_os] = 'windows' + Chef::Config[:knife][:image_os_type] = 'windows' end it "validates gracefully when winrm User and Winrm password are provided." do From d601a2a53eb25f27b956392b81acc8cfca40a802 Mon Sep 17 00:00:00 2001 From: siddheshwar-more Date: Fri, 9 Aug 2013 16:35:37 +0530 Subject: [PATCH 2/3] removed unwanted code from openstack_helper --- lib/chef/knife/openstack_helpers.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/chef/knife/openstack_helpers.rb b/lib/chef/knife/openstack_helpers.rb index f8532241..a19cdfdb 100644 --- a/lib/chef/knife/openstack_helpers.rb +++ b/lib/chef/knife/openstack_helpers.rb @@ -14,12 +14,6 @@ def primary_public_ip_address(addresses) return addresses['public'].last['addr'] if addresses['public'] end - def is_image_windows? - # Openstack image info does not have os type, so we use image_os_type cli option to interpret if we are creating windows server. - os_type = locate_config_value(:image_os) - os_type.nil? ? false : (os_type.downcase == 'windows') - end - def create_service_instance OpenstackService.new end From 78c5ccd4dce7b50f5d648605867aee67c2ba6b0f Mon Sep 17 00:00:00 2001 From: siddheshwar-more Date: Wed, 14 Aug 2013 10:57:15 +0530 Subject: [PATCH 3/3] Implemented review comments to abstract out validate params and use set_image_os_type --- lib/chef/knife/openstack_server_create.rb | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/lib/chef/knife/openstack_server_create.rb b/lib/chef/knife/openstack_server_create.rb index 3bee5cfe..74dd7776 100644 --- a/lib/chef/knife/openstack_server_create.rb +++ b/lib/chef/knife/openstack_server_create.rb @@ -35,6 +35,10 @@ class OpenstackServerCreate < ServerCreateCommand banner "knife openstack server create (options)" + def set_image_os_type + # Openstack does not provide a way to identify image os type, So image_os_type is taken from user. + end + def before_exec_command # setup the create options @create_options = { @@ -100,22 +104,11 @@ def before_bootstrap end def validate_params! + super errors = [] - if locate_config_value(:bootstrap_protocol) == 'ssh' - if locate_config_value(:identity_file).nil? && locate_config_value(:ssh_password).nil? - errors << "You must provide either Identity file or SSH Password." - end - if !locate_config_value(:identity_file).nil? && locate_config_value(:openstack_ssh_key_id).nil? - errors << "You must provide SSH Key." - end - elsif locate_config_value(:bootstrap_protocol) == 'winrm' - if locate_config_value(:winrm_password).nil? - errors << "You must provide Winrm Password." - end - else - errors << "You must provide a valid bootstrap protocol. options [ssh/winrm]. For linux type images, options [ssh]" - end - + + 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))) exit 1 if errors.each{|e| ui.error(e)}.any?