Fixed internal review comments and added specs

Signed-off-by: Piyush <piyush.awasthi@msystechnologies.com>

lib/chef/knife/ec2_server_create.rb

@@ -515,6 +515,7 @@ def run
         printed_tags = hashed_tags.map{ |tag, val| "#{tag}: #{val}" }.join(", ")
 
         hashed_volume_tags={}
+        volume_tags = locate_config_value(:volume_tags)
         volume_tags.map{ |t| key,val=t.split('='); hashed_volume_tags[key]=val} unless volume_tags.nil?
         printed_volume_tags = hashed_volume_tags.map{ |tag, val| "#{tag}: #{val}" }.join(", ")
 
@@ -627,11 +628,11 @@ def run
         msg_pair("Tags", printed_tags)
         msg_pair("SSH Key", @server.key_name)
         msg_pair("Root Device Type", @server.root_device_type)
+        msg_pair("Root Volume Tags", printed_volume_tags)
         if @server.root_device_type == "ebs"
           device_map = @server.block_device_mapping.first
           msg_pair("Root Volume ID", device_map['volumeId'])
           msg_pair("Root Device Name", device_map['deviceName'])
-          msg_pair("Root Volume Tags", printed_volume_tags)
           msg_pair("Root Device Delete on Terminate", device_map['deleteOnTermination'])
           msg_pair("Standard or Provisioned IOPS", device_map['volumeType'])
           msg_pair("IOPS rate", device_map['iops'])
@@ -962,6 +963,12 @@ def validate!
           exit 1
         end
 
+        volume_tags = locate_config_value(:volume_tags)
+        if !volume_tags.nil? and volume_tags.length != volume_tags.to_s.count('=')
+          ui.error("Volume Tags should be entered in a key = value pair")
+          exit 1
+        end
+
       end
 
       def tags
@@ -1454,15 +1461,6 @@ def evaluate_node_name(node_name)
         return node_name%server.id
       end
 
-      def volume_tags
-       volume_tags = locate_config_value(:volume_tags)
-        if !volume_tags.nil? and volume_tags.length != volume_tags.to_s.count('=')
-          ui.error("Volume Tags should be entered in a key = value pair")
-          exit 1
-        end
-       volume_tags
-      end
-
       def create_volume_tags(hashed_volume_tags)
         hashed_volume_tags.each_pair do |key,val|
           connection.tags.create :key => key, :value => val, :resource_id => @server.block_device_mapping.first['volumeId']

spec/unit/ec2_server_create_spec.rb

@@ -46,7 +46,8 @@
                            :public_ip_address => '75.101.253.10',
                            :private_dns_name => 'ip-10-251-75-20.ec2.internal',
                            :private_ip_address => '10.251.75.20',
-                           :root_device_type => 'not_ebs' } }
+                           :root_device_type => 'not_ebs',
+                           :block_device_mapping => [{'volumeId' => "456"}]  } }
 
   let (:server) { double(:id => "i-123" ) }
 
@@ -80,6 +81,7 @@
     end
 
     allow(ec2_connection).to receive(:tags).and_return double('create', :create => true)
+    allow(ec2_connection).to receive(:volume_tags).and_return double('create', :create => true)
     allow(ec2_connection).to receive_message_chain(:images, :get).and_return double('ami', :root_device_type => 'not_ebs', :platform => 'linux')
     allow(ec2_connection).to receive(:addresses).and_return [double('addesses', {
             :domain => 'standard',
@@ -281,7 +283,6 @@
       # default value of config[:ssh_password] is nil
       knife_ec2_create.config[:winrm_password] = "winrm_password"
       knife_ec2_create.config[:ssh_password] = nil
-
       expect(new_ec2_server).to receive(:wait_for).and_return(true)
       knife_ec2_create.run
       expect(knife_ec2_create.config[:ssh_password]).to eq("winrm_password")
@@ -455,6 +456,8 @@
       allow(ec2_servers).to receive(:create).and_return(new_ec2_server)
       allow(knife_ec2_create).to receive(:puts)
       allow(knife_ec2_create).to receive(:print)
+      allow(knife_ec2_create.ui).to receive(:error)
+      allow(knife_ec2_create.ui).to receive(:msg)
     end
 
     it "sets the Name tag to the instance id by default" do
@@ -490,6 +493,25 @@
 
   end
 
+  describe "when setting volume tags" do
+    before do
+      expect(Fog::Compute::AWS).to receive(:new).and_return(ec2_connection)
+      allow(knife_ec2_create).to receive(:bootstrap_for_linux_node).and_return double("bootstrap", :run => true)
+      allow(ec2_connection).to receive(:servers).and_return(ec2_servers)
+      allow(ec2_servers).to receive(:create).and_return(new_ec2_server)
+      allow(new_ec2_server).to receive(:wait_for).and_return(true)
+      allow(knife_ec2_create.ui).to receive(:error)
+    end
+
+    it "sets the volume tags as specified when given --volume-tags Key=Value" do
+      knife_ec2_create.config[:volume_tags] = ["VolumeTagKey=TestVolumeTagValue"]
+      expect(ec2_connection.tags).to receive(:create).with(:key => "VolumeTagKey",
+                                                        :value => "TestVolumeTagValue",
+                                                        :resource_id => new_ec2_server.block_device_mapping.first['volumeId'])
+      knife_ec2_create.run
+    end
+  end
+
   # This shared examples group can be used to house specifications that
   # are common to both the Linux and Windows bootstraping process. This
   # would remove a lot of testing duplication that is currently present.