Merge pull request chef#52 from opscode/adamed-oc-9368

OC-9368: image_os_type option should be compulsory in knife-openstack.
MsysTechnologiesllc · Aug 20, 2013 · 8740709 · 8740709
2 parents fe25f35 + 78c5ccd
commit 8740709
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 24 deletions.
diff --git 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_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

diff --git 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,21 +104,13 @@ 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? && (config[:ssh_key_name].nil? && Chef::Config[:knife][: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?
         end
       end

diff --git 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