Skip to content

Commit

Permalink
Merge pull request CenturyLinkCloud#84 from josh-barker/bug/additiona…
Browse files Browse the repository at this point in the history
…l-disks-added-twice

Fixes bug where additional disks added twice
  • Loading branch information
JJ Asghar authored Jun 4, 2018
2 parents 9fe8daf + a0aa48f commit 170df46
Show file tree
Hide file tree
Showing 4 changed files with 231 additions and 65 deletions.
62 changes: 7 additions & 55 deletions lib/chef/provisioning/vsphere_driver/driver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def initialize(driver_url, config)
end
end

attr_reader :connect_options, :vm_helper
attr_reader :connect_options

# Creates a new vm_helper if not already there
#
Expand Down Expand Up @@ -291,7 +291,7 @@ def ready_machine(action_handler, machine_spec, machine_options)
end

## Check if true available after added nic
@vm_helper.open_port?(machine_spec.location['ipaddress'], @vm_helper.port) unless machine_spec.location['ipaddress'].nil?
vm_helper.open_port?(machine_spec.location['ipaddress'], vm_helper.port) unless machine_spec.location['ipaddress'].nil?
machine
end

Expand All @@ -312,7 +312,7 @@ def setup_extra_nics(action_handler, bootstrap_options, vm, machine, machine_spe
bootstrap_options,
vm
)
if is_windows?(vm) && !new_nics.nil? && @vm_helper.open_port?(machine_spec.location['ipaddress'], @vm_helper.port)
if is_windows?(vm) && !new_nics.nil? && vm_helper.open_port?(machine_spec.location['ipaddress'], vm_helper.port)
new_nics.each do |nic|
nic_label = nic.device.deviceInfo.label
machine.execute_always(
Expand Down Expand Up @@ -687,57 +687,9 @@ def clone_vm(action_handler, bootstrap_options, machine_spec)
vm = vsphere_helper.find_vm(vm_folder, machine_name)
add_machine_spec_location(vm, machine_spec)

additional_disk_size_gb = Array(bootstrap_options[:additional_disk_size_gb])


vsphere_helper.update_main_disk_size_for(vm, bootstrap_options[:main_disk_size_gb])
vsphere_helper.set_additional_disks_for(vm, bootstrap_options[:datastore], bootstrap_options[:additional_disk_size_gb])
if !additional_disk_size_gb.empty? && bootstrap_options[:datastore].to_s.empty?
raise ':datastore must be specified when adding a disk to a cloned vm'
end

additional_disk_size_gb.each do |size|
size = size.to_i
next if size == 0
task = vm.ReconfigVM_Task(
spec: RbVmomi::VIM.VirtualMachineConfigSpec(
deviceChange: [
vsphere_helper.virtual_disk_for(
vm,
bootstrap_options[:datastore],
size
)
]
)
)
task.wait_for_completion
end


if bootstrap_options[:initial_iso_image]
d_obj = vm.config.hardware.device.select {|hw| hw.class == RbVmomi::VIM::VirtualCdrom}.first
backing = RbVmomi::VIM::VirtualCdromIsoBackingInfo(fileName: bootstrap_options[:initial_iso_image])
task = vm.ReconfigVM_Task(
spec: RbVmomi::VIM.VirtualMachineConfigSpec(
deviceChange: [
operation: :edit,
device: RbVmomi::VIM::VirtualCdrom(
backing: backing,
key: d_obj.key,
controllerKey: d_obj.controllerKey,
connectable: RbVmomi::VIM::VirtualDeviceConnectInfo(
startConnected: true,
connected: true,

allowGuestControl: true
)

)
]
)
)
task.wait_for_completion
end
vsphere_helper.set_initial_iso(vm, bootstrap_options[:initial_iso_file])

vm
end
Expand Down Expand Up @@ -818,7 +770,7 @@ def convergence_strategy_for(machine_spec, machine_options)
# @param [Object] vm The VM object from Chef-Provisioning
# @param [Object] action_handler taken from Chef provisioning for TODO.
def wait_for_transport(action_handler, machine_spec, machine_options, vm)
@vm_helper.find_port?(vm, machine_options[:bootstrap_options]) if vm_helper.port.nil?
vm_helper.find_port?(vm, machine_options[:bootstrap_options]) if vm_helper.port.nil?
transport = transport_for(
machine_spec,
machine_options[:bootstrap_options][:ssh]
Expand Down Expand Up @@ -865,7 +817,7 @@ def create_winrm_transport(host, options)
else
options[:winrm_transport].nil? ? :negotiate : options[:winrm_transport].to_sym
end
port = options[:port] || @vm_helper.port
port = options[:port] || vm_helper.port
winrm_options = {
user: (options[:user]).to_s,
pass: options[:password]
Expand Down Expand Up @@ -939,7 +891,7 @@ def ip_to_bootstrap(bootstrap_options, vm)
# Then check that it is reachable
until Time.now.utc - start_time > timeout
print '.'
return vm_ip.to_s if @vm_helper.open_port?(vm_ip, @vm_helper.port, 1)
return vm_ip.to_s if vm_helper.open_port?(vm_ip, vm_helper.port, 1)
sleep 1
end
raise "Timed out (#{timeout}s) waiting for ip #{vm_ip} to be connectable"
Expand Down
43 changes: 37 additions & 6 deletions lib/chef/provisioning/vsphere_driver/vsphere_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def datacenter
vim # ensure connection is valid
@datacenter ||= begin
rootFolder = vim.serviceInstance.content.rootFolder
dc = traverse_folders_for_dc(vim.rootFolder, datacenter_name) || abort('vSphere Datacenter not found [#{datacenter_name}]')
dc = traverse_folders_for_dc(vim.rootFolder, datacenter_name) || abort("vSphere Datacenter not found [#{datacenter_name}]")
end
end

Expand Down Expand Up @@ -259,12 +259,12 @@ def virtual_disk_for(vm, datastore, size_gb)
# @param [Subject] datastore the datastore the disk will be created on.
# @param [Subject] size_gb the size of the disk.
def set_additional_disks_for(vm, datastore, additional_disk_size_gb)
(additional_disk_size_gb.is_a?(Array) ? additional_disk_size_gb : [additional_disk_size_gb]).each do |size|
raise ':datastore must be specified when adding a disk to a cloned vm' if datastore.to_s.empty? && additional_disk_size_gb

Array(additional_disk_size_gb).each do |size|
size = size.to_i
next if size.zero?
if datastore.to_s.empty?
raise ':datastore must be specified when adding a disk to a cloned vm'
end
next if size <= 0

task = vm.ReconfigVM_Task(
spec: RbVmomi::VIM.VirtualMachineConfigSpec(
deviceChange: [
Expand All @@ -280,6 +280,37 @@ def set_additional_disks_for(vm, datastore, additional_disk_size_gb)
end
end

# Mounts the an iso on the first virtual CD ROm
#
# @param [Object] vm the VM object.
# @param [Subject] name of the iso file
def set_initial_iso(vm, initial_iso_image)
return unless initial_iso_image

d_obj = vm.config.hardware.device.select { |hw| hw.class == RbVmomi::VIM::VirtualCdrom }.first
backing = RbVmomi::VIM::VirtualCdromIsoBackingInfo(fileName: initial_iso_image)

task = vm.ReconfigVM_Task(
spec: RbVmomi::VIM.VirtualMachineConfigSpec(
deviceChange: [
operation: :edit,
device: RbVmomi::VIM::VirtualCdrom(
backing: backing,
key: d_obj.key,
controllerKey: d_obj.controllerKey,
connectable: RbVmomi::VIM::VirtualDeviceConnectInfo(
startConnected: true,
connected: true,

allowGuestControl: true
)
)
]
)
)
task.wait_for_completion
end

# Updates the main virtual disk for the VM
# This can only add capacity to the main disk, it is not possible to reduce the capacity.
#
Expand Down
8 changes: 4 additions & 4 deletions spec/unit_tests/clone_spec_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
end

it 'raises an error' do
expect { subject }.to raise_error
expect { subject }.to raise_error(RuntimeError)
end
end

Expand Down Expand Up @@ -139,23 +139,23 @@
let(:hostname) { 'my_host' }

it 'raises an error' do
expect { subject }.to raise_error
expect { subject }.to raise_error(RuntimeError)
end
end

context 'starting with a dash' do
let(:hostname) { '-myhost' }

it 'raises an error' do
expect { subject }.to raise_error
expect { subject }.to raise_error(RuntimeError)
end
end

context 'ending with a dash' do
let(:hostname) { 'myhost-' }

it 'raises an error' do
expect { subject }.to raise_error
expect { subject }.to raise_error(RuntimeError)
end
end
end
Expand Down
183 changes: 183 additions & 0 deletions spec/unit_tests/vsphere_helpers_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
require 'chef/provisioning/vsphere_driver'

describe ChefProvisioningVsphere::VsphereHelper do
let(:subject) do
connection_opts = {
host: 'fake.host.com',
port: 443
}
ChefProvisioningVsphere::VsphereHelper.new(connection_opts, 'fake datacenter')
end
let(:vm) { vm = double('vm') }
let(:task) { double('task', wait_for_completion: true) }

describe '#set_additional_disks_for' do
before do
allow(vm).to receive(:disks).and_return(['root_disk'], ['root_disk', 'first_disk'])
end

context 'when datastore is missing' do
let(:datastore) { nil }
it 'and no extra disks, nothing is raised' do
additional_disk_size_gb = nil
expect { subject.set_additional_disks_for vm, datastore, additional_disk_size_gb }.not_to raise_error
end

it 'and has 1 disk, error is raised' do
additional_disk_size_gb = 1
expect { subject.set_additional_disks_for vm, datastore, additional_disk_size_gb }.to raise_error(RuntimeError)
end

it 'and has multiple disks, error is raised' do
additional_disk_size_gb = [1, 2]
expect { subject.set_additional_disks_for vm, datastore, additional_disk_size_gb }.to raise_error(RuntimeError)
end
end

context 'when datastore is present' do
let(:datastore) { 'some datastore' }
let(:disk_1) do
{
spec: RbVmomi::VIM.VirtualMachineConfigSpec(
deviceChange: [
RbVmomi::VIM::VirtualDeviceConfigSpec(
operation: :add,
fileOperation: :create,
device: RbVmomi::VIM.VirtualDisk(
key: 1,
backing: RbVmomi::VIM.VirtualDiskFlatVer2BackingInfo(
fileName: "[#{datastore}]",
diskMode: 'persistent',
thinProvisioned: true
),
capacityInKB: 1 * 1024 * 1024,
controllerKey: 1000,
unitNumber: 1
)
)
]
)
}
end

let(:disk_2) do
{
spec: RbVmomi::VIM.VirtualMachineConfigSpec(
deviceChange: [
RbVmomi::VIM::VirtualDeviceConfigSpec(
operation: :add,
fileOperation: :create,
device: RbVmomi::VIM.VirtualDisk(
key: 2,
backing: RbVmomi::VIM.VirtualDiskFlatVer2BackingInfo(
fileName: "[#{datastore}]",
diskMode: 'persistent',
thinProvisioned: true
),
capacityInKB: 2 * 1024 * 1024,
controllerKey: 1000,
unitNumber: 2
)
)
]
)
}
end

it 'and no extra disks, nothing is created' do
additional_disk_size_gb = nil
expect(vm).not_to receive(:ReconfigVM_Task)
subject.set_additional_disks_for vm, datastore, additional_disk_size_gb
end

it 'and disk is a string, nothing is created' do
additional_disk_size_gb = ['not a number']
expect(vm).not_to receive(:ReconfigVM_Task)
subject.set_additional_disks_for vm, datastore, additional_disk_size_gb
end

it 'and has 0 GB, nothing is created' do
additional_disk_size_gb = [0]
expect(vm).not_to receive(:ReconfigVM_Task)
subject.set_additional_disks_for vm, datastore, additional_disk_size_gb
end

it 'and has -1 GB, nothing is created' do
additional_disk_size_gb = [-1]
expect(vm).not_to receive(:ReconfigVM_Task)
subject.set_additional_disks_for vm, datastore, additional_disk_size_gb
end

it 'and has 1 x 1 GB, create disk' do
additional_disk_size_gb = 1
expect(vm).to receive(:ReconfigVM_Task).with(disk_1).and_return(task)
subject.set_additional_disks_for vm, datastore, additional_disk_size_gb
end

it 'and has 2 disks, create 2 disks' do
additional_disk_size_gb = [1, 2]

expect(vm).to receive(:ReconfigVM_Task).with(disk_1).and_return(task)
expect(vm).to receive(:ReconfigVM_Task).with(disk_2).and_return(task)
subject.set_additional_disks_for vm, datastore, additional_disk_size_gb
end

it 'and has 3 disks, including 1 of 0 size, create 2 disks' do
additional_disk_size_gb = [0, 1, 2]
expect(vm).to receive(:ReconfigVM_Task).with(disk_1).and_return(task)
expect(vm).to receive(:ReconfigVM_Task).with(disk_2).and_return(task)

subject.set_additional_disks_for vm, datastore, additional_disk_size_gb
end
end
end

describe '#set_initial_iso' do
let(:cd_rom) do
double('cd_rom',
class: RbVmomi::VIM::VirtualCdrom,
key: 'some key',
controllerKey: 'some controller key'
)
end

let(:fake_backing) do
RbVmomi::VIM::VirtualCdromIsoBackingInfo(fileName: 'some_iso.iso')
end

let(:iso_spec) do
{
spec: RbVmomi::VIM.VirtualMachineConfigSpec(
deviceChange: [
operation: :edit,
device: RbVmomi::VIM::VirtualCdrom(
backing: fake_backing,
key: 'some key',
controllerKey: 'some controller key',
connectable: RbVmomi::VIM::VirtualDeviceConnectInfo(
startConnected: true,
connected: true,
allowGuestControl: true
)
)
]
)
}
end

before do
allow(vm).to receive_message_chain(:config, :hardware, :device)
.and_return([cd_rom])
end

it 'does nothing when no iso' do
expect(vm).not_to receive(:ReconfigVM_Task)
subject.set_initial_iso vm, nil
end

it 'sets initial iso' do
expect(vm).to receive(:ReconfigVM_Task).with(iso_spec).and_return(task)
subject.set_initial_iso vm, 'some_iso.iso'
end
end
end

0 comments on commit 170df46

Please sign in to comment.