Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance request attach disk to take more than one disk #401

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/fog/azurerm/models/compute/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,11 @@ def detach_data_disk(disk_name, async = false)
end

def attach_managed_disk(disk_name, disk_resource_group, async = false)
response = service.attach_data_disk_to_vm(data_disk_params(disk_name, nil, nil, disk_resource_group), async)
attach_managed_disks(disk_name, disk_resource_group, async)
end

def attach_managed_disks(disk_names, disk_resource_group, async = false)
response = service.attach_data_disk_to_vm(data_disk_params(disk_names, nil, nil, disk_resource_group), async)
async ? create_fog_async_response(response) : merge_attributes(Fog::Compute::AzureRM::Server.parse(response))
end

Expand Down
39 changes: 27 additions & 12 deletions lib/fog/azurerm/requests/compute/attach_data_disk_to_vm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,25 @@ def attach_data_disk_to_vm(disk_params, async)
# Variable un-packing for easy access
vm_name = disk_params[:vm_name]
vm_resource_group = disk_params[:vm_resource_group]
disk_name = disk_params[:disk_name]
disk_names = disk_params[:disk_name]
disk_resource_group = disk_params[:disk_resource_group]
disk_size = disk_params[:disk_size_gb]
storage_account_name = disk_params[:storage_account_name]

msg = "Attaching Data Disk #{disk_name} to Virtual Machine #{vm_name} in Resource Group #{vm_resource_group}"
disk_names = disk_names.is_a?(Array) ? disk_names : [disk_names]

msg = "Attaching Data Disk #{disk_names.join(', ')} to Virtual Machine #{vm_name} in Resource Group #{vm_resource_group}"
Fog::Logger.debug msg
vm = get_virtual_machine_instance(vm_resource_group, vm_name)
lun = get_logical_unit_number(vm.storage_profile.data_disks)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
lun = get_logical_unit_number(vm.storage_profile.data_disks)
lun = get_logical_unit_number(vm.storage_profile.data_disks)

This line is causing your PR to fail. Since in your code, you are getting the value of LUN only once, it throws an exception when it tries to add a new data disk to that same LUN.

access_key = get_storage_access_key(vm_resource_group, storage_account_name) if storage_account_name

data_disks = get_list_data_disks(disk_names, disk_size, lun, storage_account_name, access_key, disk_resource_group)

# Attach data disk to VM
if storage_account_name
# Un-managed data disk
access_key = get_storage_access_key(vm_resource_group, storage_account_name)
data_disk = get_unmanaged_disk_object(disk_name, disk_size, lun, storage_account_name, access_key)
elsif disk_resource_group
# Managed data disk
data_disk = get_data_disk_object(disk_resource_group, disk_name, lun)
data_disks.each do |data_disk|
vm.storage_profile.data_disks.push(data_disk)
end
vm.storage_profile.data_disks.push(data_disk)

begin
if async
response = @compute_mgmt_client.virtual_machines.create_or_update_async(vm_resource_group, vm_name, vm)
Expand All @@ -43,13 +42,29 @@ def attach_data_disk_to_vm(disk_params, async)
if async
response
else
Fog::Logger.debug "Data Disk #{disk_name} attached to Virtual Machine #{vm_name} successfully."
Fog::Logger.debug "Data Disk #{disk_names.join(', ')} attached to Virtual Machine #{vm_name} successfully."
virtual_machine
end
end

private

def get_list_data_disks(disk_names, disk_size, lun, storage_account_name, access_key, disk_resource_group)
data_disks = []
disk_names.each do |disk_name|
# Attach data disk to VM
if storage_account_name
# Un-managed data disk
data_disk = get_unmanaged_disk_object(disk_name, disk_size, lun, storage_account_name, access_key)
elsif disk_resource_group
# Managed data disk
data_disk = get_data_disk_object(disk_resource_group, disk_name, lun)
end
data_disks += [data_disk]
end
data_disks
end

def get_virtual_machine_instance(resource_group, vm_name)
msg = "Getting Virtual Machine #{vm_name} from Resource Group #{resource_group}"
Fog::Logger.debug msg
Expand Down
11 changes: 11 additions & 0 deletions test/models/compute/test_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -234,4 +234,15 @@ def test_detach_managed_disk_response
assert_instance_of Fog::AzureRM::AsyncResponse, @server.detach_managed_disk('managed_disk_name', true)
end
end

def test_attach_managed_disks_response
response = ApiStub::Models::Compute::Server.create_linux_virtual_machine_response(@compute_client)
@service.stub :attach_data_disk_to_vm, response do
assert_instance_of Fog::Compute::AzureRM::Server, @server.attach_managed_disks(%w(managed_disk_name1 managed_disk_name2), 'resoure_group')
end
async_response = Concurrent::Promise.execute { 10 }
@service.stub :attach_data_disk_to_vm, async_response do
assert_instance_of Fog::AzureRM::AsyncResponse, @server.attach_managed_disks(%w(managed_disk_name1 managed_disk_name2), 'resoure_group', true)
end
end
end
11 changes: 11 additions & 0 deletions test/requests/compute/test_attach_data_disk_to_vm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,15 @@ def test_get_storage_key_failure
end
end
end

def test_detach_multiple_data_disks_from_vm_success
@virtual_machines.stub :get, @get_vm_response do
@disks.stub :get, @get_managed_disk_response do
@virtual_machines.stub :create_or_update, @get_vm_managed_disk_response do
input_params = { vm_name: 'fog-test-vm', vm_resource_group: 'fog-test-rg', disk_name: %w(mydatadisk1 mydatadisk2), disk_size_gb: nil, storage_account_name: nil }
assert_equal @service.attach_data_disk_to_vm(input_params, false), @get_vm_managed_disk_response
end
end
end
end
end