Set the location property of managed disks to ensure disk saving #143
Conversation
| @@ -308,6 +308,7 @@ def populate_hardware_hash_with_disks(hardware_disks_array, instance) | |||
| disk_size = disk.respond_to?(:disk_size_gb) ? disk.disk_size_gb * 1.gigabyte : 0 | |||
| disk_name = disk.name | |||
| disk_location = disk.try(:vhd).try(:uri) | |||
| disk_location = disk.try(:vhd).try(:uri) || disk.try(:managed_disk).try(:id) | |||
There was a problem hiding this comment.
Did you mean to drop the previous line?
bronaghs
force-pushed
the
save_managed_disks
branch
from
2a0bb73
to
ff7cfc3
Compare
|
@djberg96 - yes. |
|
Just thinking out loud here, we run the refresh |
|
@djberg96 - The reason why the spec passes is because the problem only arises when there are 2 or more managed disks on an instance. |
|
@bronaghs oh, good catch! |
bronaghs
force-pushed
the
save_managed_disks
branch
from
ff7cfc3
to
6e93c47
Compare
bronaghs
changed the title
bronaghs
force-pushed
the
save_managed_disks
branch
from
6e93c47
to
e9ff1bc
Compare
|
Checked commit bronaghs@e9ff1bc with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 |
|
Looks good to me! 👍 |
| @@ -307,7 +307,7 @@ def populate_hardware_hash_with_disks(hardware_disks_array, instance) | |||
| data_disks.each do |disk| | |||
| disk_size = disk.respond_to?(:disk_size_gb) ? disk.disk_size_gb * 1.gigabyte : 0 | |||
| disk_name = disk.name | |||
| disk_location = disk.try(:vhd).try(:uri) | |||
| disk_location = disk.try(:vhd).try(:uri) || disk.try(:managed_disk).try(:id) | |||
There was a problem hiding this comment.
|| will get naughty if the first method exists but returns an empty string...
We had that problem 3 times in amazon.
But I guess the azure sdk does a better job here :)
|
@bronaghs Please create a Fine PR for this. Spec files are conflicting a lot, so it's better that gets resolved by you. |
This fixes a bug where managed disks are saved to the database when a provider is first added but are deleted upon subsequent refreshes of the same provider.
Background
The saving of instance disks involves creating an array of disks already associated with the VM from the previous refresh, and one by one, each disk is removed as it is saved. What is left is old disks that no longer exist on the VM and will be deleted.
This array of pre-existing disks is created from the VM.disks association which is then indexed using the TypedIndex class.
This indexing uses the
locationproperty of a disk as a key. If the location is missing, as is the case with managed disks, that disk will no longer be associated with the instance, will remain in the array of pre-existing disks and will eventually be deleted, see here and here.Steps to reproduce
After this PR is merged, the managed data disks will persist after a second refresh.
https://bugzilla.redhat.com/show_bug.cgi?id=1503866