fog / fog-openstack

Fog for OpenStack Platform
MIT License
68 stars 130 forks source link

Add block_device_mapping attribute to image model #466

Closed alexander-demicev closed 5 years ago

alexander-demicev commented 5 years ago

This PR adds block_device_mapping attribute to image model. When snapshotting VM that was booted from volume, image and volume snapshots are created. This property contains a snapshot_id, and it's needed for having a reference to the volume snapshot. I don't like that block_device_mapping will be an unparsed JSON string and that's also an array: "block_device_mapping"=> "[{\"guest_format\": null, \"boot_index\": 0, \"delete_on_termination\": false, \"no_device\": null, \"snapshot_id\": \"94167803-1136-43f5-b2ff-1590d9bdf5f7\", \"device_name\": \"/dev/vda\", \"disk_bus\": \"virtio\", \"image_id\": null, \"source_type\": \"snapshot\", \"tag\": null, \"device_type\": \"disk\", \"volume_id\": null, \"destination_type\": \"volume\", \"volume_size\": 1}]", If there is a clean way to parse it in fog, I'll be happy to add it.

@aufi @gildub

theopenlab-ci[bot] commented 5 years ago

Build succeeded.

gildub commented 5 years ago

@alexander-demichev,

Have you tried with the :type option? For example

attribute :block_device_mapping, :type => :array
alexander-demicev commented 5 years ago

@gildub , Yes, it's creating an attribute like this block_device_mapping=["[{\"guest_format\": null, \"boot_index\": 0, \"delete_on_termination\": false, \"no_device\": null, \"snapshot_id\": \"51b0b1d7-897a-42a2-a8e8-dfe2c90dec39\", \"device_name\": \"/dev/vda\", \"disk_bus\": \"virtio\", \"image_id\": null, \"source_type\": \"snapshot\", \"tag\": null, \"device_type\": \"disk\", \"volume_id\": null, \"destination_type\": \"volume\", \"volume_size\": 1}]"]

gildub commented 5 years ago

@alexander-demichev, I wonder if this happening because the value is actually an array of array. Therefore the type array for the attribute works but doesn't interprets the content of it You might be better off with JSON.parse(block_device_mapping[0])

alexander-demicev commented 5 years ago

@gildub Do you mean that :type => :array is ok and I should just parse it with JSON.parse later, when using it?

sseago commented 5 years ago

@alexander-demichev So it looks like I won't need this PR after all for fixing the snapshot bug. The existing code in manageiq-smartstate tries to pull metadata from compute_service.images rather than image_service.images -- and the block_device_mapping metadata is already found in the compute service output. This means the existing fog-openstack works fine as long as I give it the compute service image object rather than the image service image object.

Having it show up here probably still makes sense, but this particular bug won't depend on this PR anymore.