Skip to content

Commit

Permalink
Merge pull request #224 from AbelHu/abelhu
Browse files Browse the repository at this point in the history 
Delete the possible unexpected node 'resources' before updating VMs.
  • Loading branch information
@AbelHu
AbelHu authored Nov 25, 2016
2 parents 5a44279 + ec0bad0 commit 813cee1
Show file tree
Hide file tree
Showing 4 changed files with 348 additions and 93 deletions.
20 changes: 17 additions & 3 deletions src/bosh_azure_cpi/lib/cloud/azure/azure_client2.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def create_virtual_machine(vm_params, network_interfaces, availability_set = nil
'name' => vm_params[:name],
'location' => vm_params[:location],
'type' => "#{REST_API_PROVIDER_COMPUTER}/#{REST_API_COMPUTER_VIRTUAL_MACHINES}",
'tags' => vm_params[:metadata],
'tags' => vm_params[:tags],
'properties' => {
'hardwareProfile' => {
'vmSize' => vm_params[:vm_size]
Expand Down Expand Up @@ -252,8 +252,8 @@ def restart_virtual_machine(name)
end

# Set tags for a virtual machine
# @param [String] name - Name of virtual machine.
# @param [Hash] metadata - metadata key/value pairs.
# @param [String] name - Name of virtual machine.
# @param [Hash] tags - tags key/value pairs.
#
# @return [Boolean]
#
Expand All @@ -266,6 +266,8 @@ def update_tags_of_virtual_machine(name, tags)
raise AzureNotFoundError, "update_tags_of_virtual_machine - cannot find the virtual machine by name \"#{name}\""
end

vm = remove_resources_from_vm(vm)

vm['tags'] = tags
http_put(url, vm)
end
Expand All @@ -287,6 +289,8 @@ def attach_disk_to_virtual_machine(name, disk_name, disk_uri, caching)
raise AzureNotFoundError, "attach_disk_to_virtual_machine - cannot find the virtual machine by name `#{name}'"
end

vm = remove_resources_from_vm(vm)

disk_info = DiskInfo.for(vm['properties']['hardwareProfile']['vmSize'])
lun = nil
data_disks = vm['properties']['storageProfile']['dataDisks']
Expand All @@ -310,6 +314,7 @@ def attach_disk_to_virtual_machine(name, disk_name, disk_uri, caching)
'vhd' => { 'uri' => disk_uri }
}
vm['properties']['storageProfile']['dataDisks'].push(new_disk)

@logger.info("attach_disk_to_virtual_machine - attach disk `#{disk_name}' to `#{lun}'")
http_put(url, vm)
disk = {
Expand All @@ -336,6 +341,8 @@ def detach_disk_from_virtual_machine(name, disk_name)
raise AzureNotFoundError, "detach_disk_from_virtual_machine - cannot find the virtual machine by name \"#{name}\""
end

vm = remove_resources_from_vm(vm)

@logger.debug("detach_disk_from_virtual_machine - virtual machine:\n#{JSON.pretty_generate(vm)}")
disk = vm['properties']['storageProfile']['dataDisks'].find { |disk| disk['name'] == disk_name}
raise Bosh::Clouds::DiskNotAttached.new(true),
Expand Down Expand Up @@ -1611,5 +1618,12 @@ def get_http_common_headers(response)
message += "x-ms-routing-request-id: #{response['x-ms-routing-request-id']}\n"
message
end

# Sometimes Azure returns VM information with a node 'resources' which contains all extensions' information.
# Azure will retrun 'InvalidRequestContent' if CPI does not delete the node 'resources'.
def remove_resources_from_vm(vm)
vm.delete('resources')
vm
end
end
end
146 changes: 118 additions & 28 deletions src/bosh_azure_cpi/spec/unit/azure_client2/attach_disk_to_virtual_machine_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
}

context "when token is valid, create operation is accepted and completed" do
let(:response_body) {
let(:request_body) {
{
"id" => "fake-id",
"name" => "fake-name",
Expand All @@ -54,42 +54,132 @@
"storageProfile" => {
"dataDisks" => [
{ "lun" => 0 },
{ "lun" => 1 }
{ "lun" => 1 },
{
"name" => disk_name,
"lun" => 2,
"createOption" => "Attach",
"caching" => 'ReadWrite',
"vhd" => { "uri" => disk_uri }
}
]
},
"hardwareProfile" => {
"vmSize" => "Standard_A5"
}
}
}.to_json
}
}

it "should raise no error" do
stub_request(:post, token_uri).to_return(
:status => 200,
:body => {
"access_token"=>valid_access_token,
"expires_on"=>expires_on
}.to_json,
:headers => {})
stub_request(:get, vm_uri).to_return(
:status => 200,
:body => response_body,
:headers => {})
stub_request(:put, vm_uri).to_return(
:status => 200,
:body => '',
:headers => {
"azure-asyncoperation" => operation_status_link
})
stub_request(:get, operation_status_link).to_return(
:status => 200,
:body => '{"status":"Succeeded"}',
:headers => {})
context "when VM's information does not contain 'resources'" do
let(:response_body) {
{
"id" => "fake-id",
"name" => "fake-name",
"location" => "fake-location",
"tags" => "fake-tags",
"properties" => {
"provisioningState" => "fake-state",
"storageProfile" => {
"dataDisks" => [
{ "lun" => 0 },
{ "lun" => 1 }
]
},
"hardwareProfile" => {
"vmSize" => "Standard_A5"
}
}
}.to_json
}

expect(
azure_client2.attach_disk_to_virtual_machine(vm_name, disk_name, disk_uri, caching)
).to eq(disk)
it "should raise no error" do
stub_request(:post, token_uri).to_return(
:status => 200,
:body => {
"access_token"=>valid_access_token,
"expires_on"=>expires_on
}.to_json,
:headers => {})
stub_request(:get, vm_uri).to_return(
:status => 200,
:body => response_body,
:headers => {})
stub_request(:put, vm_uri).with(body: request_body).to_return(
:status => 200,
:body => '',
:headers => {
"azure-asyncoperation" => operation_status_link
})
stub_request(:get, operation_status_link).to_return(
:status => 200,
:body => '{"status":"Succeeded"}',
:headers => {})

expect(
azure_client2.attach_disk_to_virtual_machine(vm_name, disk_name, disk_uri, caching)
).to eq(disk)
end
end

context "when VM's information contains 'resources'" do
let(:response_body) {
{
"id" => "fake-id",
"name" => "fake-name",
"location" => "fake-location",
"tags" => "fake-tags",
"properties" => {
"provisioningState" => "fake-state",
"storageProfile" => {
"dataDisks" => [
{ "lun" => 0 },
{ "lun" => 1 }
]
},
"hardwareProfile" => {
"vmSize" => "Standard_A5"
}
},
"resources" => [
{
"properties": {},
"id": "fake-id",
"name": "fake-name",
"type": "fake-type",
"location": "fake-location"
}
]
}.to_json
}

it "should raise no error" do
stub_request(:post, token_uri).to_return(
:status => 200,
:body => {
"access_token"=>valid_access_token,
"expires_on"=>expires_on
}.to_json,
:headers => {})
stub_request(:get, vm_uri).to_return(
:status => 200,
:body => response_body,
:headers => {})
stub_request(:put, vm_uri).with(body: request_body).to_return(
:status => 200,
:body => '',
:headers => {
"azure-asyncoperation" => operation_status_link
})
stub_request(:get, operation_status_link).to_return(
:status => 200,
:body => '{"status":"Succeeded"}',
:headers => {})

expect(
azure_client2.attach_disk_to_virtual_machine(vm_name, disk_name, disk_uri, caching)
).to eq(disk)
end
end
end

Expand Down
135 changes: 110 additions & 25 deletions src/bosh_azure_cpi/spec/unit/azure_client2/detach_disk_from_virtual_machine_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,32 +55,117 @@
}

context "when token is valid, create operation is accepted and completed" do
it "should raise no error" do
stub_request(:post, token_uri).to_return(
:status => 200,
:body => {
"access_token"=>valid_access_token,
"expires_on"=>expires_on
}.to_json,
:headers => {})
stub_request(:get, vm_uri).to_return(
:status => 200,
:body => response_body,
:headers => {})
stub_request(:put, vm_uri).to_return(
:status => 200,
:body => '',
:headers => {
"azure-asyncoperation" => operation_status_link
})
stub_request(:get, operation_status_link).to_return(
:status => 200,
:body => '{"status":"Succeeded"}',
:headers => {})
let(:request_body) {
{
"id" => "fake-id",
"name" => "fake-name",
"location" => "fake-location",
"tags" => "fake-tags",
"properties" => {
"provisioningState" => "fake-state",
"storageProfile" => {
"dataDisks" => [
{
"name" => "wrong name",
"lun" => 0
}
]
}
}
}
}

expect {
azure_client2.detach_disk_from_virtual_machine(vm_name, disk_name)
}.not_to raise_error
context "when VM's information does not contain 'resources'" do
it "should raise no error" do
stub_request(:post, token_uri).to_return(
:status => 200,
:body => {
"access_token"=>valid_access_token,
"expires_on"=>expires_on
}.to_json,
:headers => {})
stub_request(:get, vm_uri).to_return(
:status => 200,
:body => response_body,
:headers => {})
stub_request(:put, vm_uri).with(body: request_body).to_return(
:status => 200,
:body => '',
:headers => {
"azure-asyncoperation" => operation_status_link
})
stub_request(:get, operation_status_link).to_return(
:status => 200,
:body => '{"status":"Succeeded"}',
:headers => {})

expect {
azure_client2.detach_disk_from_virtual_machine(vm_name, disk_name)
}.not_to raise_error
end
end

context "when VM's information contains 'resources'" do
let(:response_body_with_resources) {
{
"id" => "fake-id",
"name" => "fake-name",
"location" => "fake-location",
"tags" => "fake-tags",
"properties" => {
"provisioningState" => "fake-state",
"storageProfile" => {
"dataDisks" => [
{
"name" => disk_name,
"lun" => 1
},
{
"name" => "wrong name",
"lun" => 0
}
]
}
},
"resources" => [
{
"properties": {},
"id": "fake-id",
"name": "fake-name",
"type": "fake-type",
"location": "fake-location"
}
]
}.to_json
}

it "should raise no error" do
stub_request(:post, token_uri).to_return(
:status => 200,
:body => {
"access_token"=>valid_access_token,
"expires_on"=>expires_on
}.to_json,
:headers => {})
stub_request(:get, vm_uri).to_return(
:status => 200,
:body => response_body_with_resources,
:headers => {})
stub_request(:put, vm_uri).with(body: request_body).to_return(
:status => 200,
:body => '',
:headers => {
"azure-asyncoperation" => operation_status_link
})
stub_request(:get, operation_status_link).to_return(
:status => 200,
:body => '{"status":"Succeeded"}',
:headers => {})

expect {
azure_client2.detach_disk_from_virtual_machine(vm_name, disk_name)
}.not_to raise_error
end
end
end

Expand Down
Loading

0 comments on commit 813cee1

Please sign in to comment.