Allow people to overprovision and remove 10% check

Fixes chef#184

Refactor to get some methods out of base that are only used here.

Remove check for needing 10% overhead.
If we're thin provisioning, then only require 1G

lib/chef/knife/base_vsphere_command.rb

@@ -245,44 +245,6 @@ def find_pool(poolName)
         base_entity
       end
 
-      def choose_datastore(dstores, size)
-        vmdk_size_b = size.to_i * 1024 * 1024 * 1024
-
-        candidates = []
-        dstores.each do |store|
-          avail = number_to_human_size(store.summary[:freeSpace])
-          cap = number_to_human_size(store.summary[:capacity])
-          puts "#{ui.color('Datastore', :cyan)}: #{store.name} (#{avail}(#{store.summary[:freeSpace]}) / #{cap})"
-
-          # vm's can span multiple datastores, so instead of grabbing the first one
-          # let's find the first datastore with the available space on a LUN the vm
-          # is already using, or use a specified LUN (if given)
-
-          next unless (store.summary[:freeSpace] - vmdk_size_b) > 0
-          # also let's not use more than 90% of total space to save room for snapshots.
-          cap_remains = 100 * ((store.summary[:freeSpace].to_f - vmdk_size_b.to_f) / store.summary[:capacity].to_f)
-          candidates.push(store) if cap_remains.to_i > 10
-        end
-        if candidates.length > 0
-          vmdk_datastore = candidates[0]
-        else
-          puts 'Insufficient space on all LUNs designated or assigned to the virtual machine. Please specify a new target.'
-          vmdk_datastore = nil
-        end
-        vmdk_datastore
-      end
-
-      def find_datastores_regex(regex)
-        stores = []
-        puts "Looking for all datastores that match /#{regex}/"
-        dc = datacenter
-        base_entity = dc.datastore
-        base_entity.each do |ds|
-          stores.push ds if ds.name.match(/#{regex}/)
-        end
-        stores
-      end
-
       def find_datastore(dsName)
         dc = datacenter
         base_entity = dc.datastore

lib/chef/knife/vsphere_vm_vmdk_add.rb

@@ -17,7 +17,6 @@ class Chef::Knife::VsphereVmVmdkAdd < Chef::Knife::BaseVsphereCommand
   option :vmdk_type,
          long: '--vmdk-type TYPE',
          description: 'Type of VMDK',
-         # this is a bad idea as it will let you overcommit SAN by 400% or more. thick is a more "sane" default
          default: 'thin'
 
   option :target_lun,
@@ -36,8 +35,8 @@ def run
       exit 1
     end
 
-    size = @name_args[1]
-    if size.nil?
+    size_gb = @name_args[1]
+    if size_gb.nil?
       ui.fatal 'You need a VMDK size!'
       show_usage
       exit 1
@@ -50,15 +49,15 @@ def run
     fatal_exit "Could not find #{vmname}" unless vm
 
     target_lun = get_config(:target_lun) unless get_config(:target_lun).nil?
-    vmdk_size_kb = size.to_i * 1024 * 1024
+    vmdk_size_kb = size_gb.to_i * 1024 * 1024
 
     if target_lun.nil?
-      vmdk_datastore = choose_datastore(vm.datastore, size)
-      exit(-1) if vmdk_datastore.nil?
+      vmdk_datastore = choose_datastore(vm.datastore, thin_provisioning? ? 1 : size_gb)
+      fatal_exit('Insufficient space on all LUNs designated or assigned to the virtual machine. Please specify a new target.') if vmdk_datastore.nil?
     else
       vmdk_datastores = find_datastores_regex(target_lun)
-      vmdk_datastore = choose_datastore(vmdk_datastores, size)
-      exit(-1) if vmdk_datastore.nil?
+      vmdk_datastore = choose_datastore(vmdk_datastores, thin_provisioning? ? 1 : size_gb)
+      fatal_exit('Insufficient space on all LUNs designated or assigned to the virtual machine. Please specify a new target.') if vmdk_datastore.nil?
       vmdk_dir = "[#{vmdk_datastore.name}] #{vmname}"
       # create the vm folder on the LUN or subsequent operations will fail.
       unless vmdk_datastore.exists? vmname
@@ -82,8 +81,11 @@ def run
     pc = vmdk_datastore._connection.serviceContent.propertyCollector
     vms = vmdk_datastore.vm
     vm_files = pc.collectMultiple vms, 'layoutEx.file'
+    puts "here"
     vm_files.keys.each do |vmFile|
+      puts vmFile
       vm_files[vmFile]['layoutEx.file'].each do |layout|
+        puts "  #{layout.name}"
         if layout.name.match(/^\[#{vmdk_datastore.name}\] #{vmname}\/#{vmname}_([0-9]+).vmdk/)
           num = Regexp.last_match(1)
           next_vmdk = num.to_i + 1 if next_vmdk <= num.to_i
@@ -104,7 +106,7 @@ def run
         diskType: vmdk_type
       )
       ui.info 'Creating VMDK'
-      ui.info "#{ui.color 'Capacity:', :cyan} #{size} GB"
+      ui.info "#{ui.color 'Capacity:', :cyan} #{size_gb} GB"
       ui.info "#{ui.color 'Disk:', :cyan} #{vmdk_name}"
 
       if get_config(:noop)
@@ -242,4 +244,44 @@ def run
       vm.ReconfigVM_Task(spec: vm_config_spec).wait_for_completion
     end
   end
+
+  private
+  def thin_provisioning?
+    get_config(:vmdk_type) == 'thin'
+  end
+
+  def choose_datastore(dstores, minfree_gb)
+    minfree_b = minfree_gb.to_i * 1024 * 1024 * 1024
+
+    candidates = dstores.select do |store|
+      avail = number_to_human_size(store.summary[:freeSpace])
+      cap = number_to_human_size(store.summary[:capacity])
+      puts "#{ui.color('Datastore', :cyan)}: #{store.name} (#{avail}(#{store.summary[:freeSpace]}) / #{cap})"
+
+      # vm's can span multiple datastores, so instead of grabbing the first one
+      # let's find the first datastore with the available space on a LUN the vm
+      # is already using, or use a specified LUN (if given)
+
+      (store.summary[:freeSpace] - minfree_b) > 0
+    end
+    candidates.first
+    if candidates.length > 0
+      vmdk_datastore = candidates[0]
+    else
+      vmdk_datastore = nil
+    end
+    vmdk_datastore
+  end
+
+  def find_datastores_regex(regex)
+    stores = []
+    puts "Looking for all datastores that match /#{regex}/"
+    dc = datacenter
+    base_entity = dc.datastore
+    base_entity.each do |ds|
+      stores.push ds if ds.name.match(/#{regex}/)
+    end
+    stores
+  end
+
 end