Merge pull request #260 from mattkirby/fix_check_disk

Remove propertyCollector from add_disk
puppetlabs · Jun 20, 2018 · d736b1e · d736b1e
2 parents 63fb231 + d1ae85c
commit d736b1e
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 73 deletions.
diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb
@@ -458,9 +458,12 @@ def add_disk(vm, size, datastore, connection, datacentername)
 
           vmdk_datastore = find_datastore(datastore, connection, datacentername)
           raise("Datastore '#{datastore}' does not exist in datacenter '#{datacentername}'") if vmdk_datastore.nil?
-          vmdk_file_name = "#{vm['name']}/#{vm['name']}_#{find_vmdks(vm['name'], datastore, connection, datacentername).length + 1}.vmdk"
 
+          datacenter = connection.serviceInstance.find_datacenter(datacentername)
           controller = find_disk_controller(vm)
+          disk_unit_number = find_disk_unit_number(vm, controller)
+          disk_count = vm.config.hardware.device.grep(RbVmomi::VIM::VirtualDisk).count
+          vmdk_file_name = "#{vm['name']}/#{vm['name']}_#{disk_count}.vmdk"
 
           vmdk_spec = RbVmomi::VIM::FileBackedVirtualDiskSpec(
             capacityKb: size.to_i * 1024 * 1024,
@@ -471,15 +474,15 @@ def add_disk(vm, size, datastore, connection, datacentername)
           vmdk_backing = RbVmomi::VIM::VirtualDiskFlatVer2BackingInfo(
             datastore: vmdk_datastore,
             diskMode: DISK_MODE,
-            fileName: "[#{vmdk_datastore.name}] #{vmdk_file_name}"
+            fileName: "[#{datastore}] #{vmdk_file_name}"
           )
 
           device = RbVmomi::VIM::VirtualDisk(
             backing: vmdk_backing,
             capacityInKB: size.to_i * 1024 * 1024,
             controllerKey: controller.key,
             key: -1,
-            unitNumber: find_disk_unit_number(vm, controller)
+            unitNumber: disk_unit_number
           )
 
           device_config_spec = RbVmomi::VIM::VirtualDeviceConfigSpec(
@@ -492,8 +495,8 @@ def add_disk(vm, size, datastore, connection, datacentername)
           )
 
           connection.serviceContent.virtualDiskManager.CreateVirtualDisk_Task(
-            datacenter: connection.serviceInstance.find_datacenter(datacentername),
-            name: "[#{vmdk_datastore.name}] #{vmdk_file_name}",
+            datacenter: datacenter,
+            name: "[#{datastore}] #{vmdk_file_name}",
             spec: vmdk_spec
           ).wait_for_completion
 
@@ -802,23 +805,6 @@ def find_vm(pool_name, vmname, connection)
           connection.searchIndex.FindByInventoryPath(propSpecs)
         end
 
-        def find_vmdks(vmname, datastore, connection, datacentername)
-          disks = []
-
-          vmdk_datastore = find_datastore(datastore, connection, datacentername)
-
-          vm_files = connection.serviceContent.propertyCollector.collectMultiple vmdk_datastore.vm, 'layoutEx.file'
-          vm_files.keys.each do |f|
-            vm_files[f]['layoutEx.file'].each do |l|
-              if l.name =~ /^\[#{vmdk_datastore.name}\] #{vmname}\/#{vmname}_([0-9]+).vmdk/
-                disks.push(l)
-              end
-            end
-          end
-
-          disks
-        end
-
         def get_base_vm_container_from(connection)
           view_manager = connection.serviceContent.viewManager
           view_manager.CreateContainerView(

diff --git a/spec/unit/providers/vsphere_spec.rb b/spec/unit/providers/vsphere_spec.rb
@@ -3331,57 +3331,6 @@
     end
   end
 
-  describe '#find_vmdks' do
-    let(:datastorename) { 'datastore' }
-    let(:connection_options) {{
-      :serviceContent => {
-        :datacenters => [
-          { :name => datacenter_name, :datastores => [datastorename] }
-        ]
-      }
-    }}
-
-    let(:collectMultiple_response) { {} }
-
-    before(:each) do
-      allow(connection.serviceContent.propertyCollector).to receive(:collectMultiple).and_return(collectMultiple_response)
-    end
-
-    context 'Searching all files for all VMs on a Datastore' do
-      # This is fairly fragile mocking
-      let(:collectMultiple_response) { {
-        'FakeVMObject1' => { 'layoutEx.file' =>
-        [
-          mock_RbVmomi_VIM_VirtualMachineFileLayoutExFileInfo({ :key => 101, :name => "[#{datastorename}] mock1/mock1_0.vmdk"})
-        ]},
-        vmname => { 'layoutEx.file' =>
-        [
-          # VMDKs which should match
-          mock_RbVmomi_VIM_VirtualMachineFileLayoutExFileInfo({ :key => 1, :name => "[#{datastorename}] #{vmname}/#{vmname}_0.vmdk"}),
-          mock_RbVmomi_VIM_VirtualMachineFileLayoutExFileInfo({ :key => 2, :name => "[#{datastorename}] #{vmname}/#{vmname}_1.vmdk"}),
-          # VMDKs which should not match
-          mock_RbVmomi_VIM_VirtualMachineFileLayoutExFileInfo({ :key => 102, :name => "[otherdatastore] #{vmname}/#{vmname}_0.vmdk"}),
-          mock_RbVmomi_VIM_VirtualMachineFileLayoutExFileInfo({ :key => 103, :name => "[otherdatastore] #{vmname}/#{vmname}.vmdk"}),
-          mock_RbVmomi_VIM_VirtualMachineFileLayoutExFileInfo({ :key => 104, :name => "[otherdatastore] #{vmname}/#{vmname}_abc.vmdk"}),
-        ]},
-      } }
-
-      it 'should return empty array if no VMDKs match the VM name' do
-        expect(subject.find_vmdks('missing_vm_name',datastorename,connection,datacenter_name)).to eq([])
-      end
-
-      it 'should return matching VMDKs for the VM' do
-        result = subject.find_vmdks(vmname,datastorename,connection,datacenter_name)
-        expect(result).to_not be_nil
-        expect(result.count).to eq(2)
-        # The keys for each VMDK should be less that 100 as per the mocks
-        result.each do |fileinfo|
-          expect(fileinfo.key).to be < 100
-        end
-      end
-    end
-  end
-
   describe '#get_base_vm_container_from' do
     it 'should return a recursive view of type VirtualMachine' do
       result = subject.get_base_vm_container_from(connection)