added taking over incomplete volume parameters #359
Conversation
|
Hi, |
You can check here which lines are affected: https://github.com/theforeman/foreman_fog_proxmox/pull/359/files |
| value.each do |index, dev_specs| | ||
|
|
||
| # Only if this contains only 1 set like: {"size"=>"xxGB"} | ||
| if #{index} > 0 && dev_specs.keys.count == 1 |
There was a problem hiding this comment.
it should be if index > 0 && dev_specs.keys.count == 1
| new_data['volumes_attributes'] = volumes_attributes | ||
| end | ||
| end | ||
| return new_data |
There was a problem hiding this comment.
No need to add return keyword as it is implicit.
There was a problem hiding this comment.
Nice, I didn't know that .. This is one of my firsts ruby spinsels ;-)
|
Hi Manisha15, can this be merged into main now ? |
I am testing and reviewing it. Then I'll merge it. Can you please squash the commits in one commit and create an issue for it along with appending the commit message with |
libc225so
force-pushed
the
fix_extra_non_specified_disks
branch
from
6182649
to
dedf741
Compare
|
Hi |
|
@libc225so There is no specific hammer cli command for proxmox. May I know the commands you used that gave the issue redarding volume attributes? |
|
Hi Manisha15 The options for the disk ( configured with --volume ), follow what is configured in the compute profile, but allows posibilities to address for different capacity. If your compute profile contains 1 disk, then you will get 1 disk. But there is an option ( simular as to use with VMWare ) to add extra disks, i.e. repeat the "--volume" parameter like this: --volume="'size=19GB'" So in this example, the 19GB refers to the disk from the compute profile But the parameters from hammer going to Foreman do not allow you to add any other information then just the size .. So the complete json that in the end is send to fog is this json string for the first disk. "0"=>{"storage_type"=>"hard_disk", "storage"=>"local-lvm", "controller"=>"virtio", "device"=>"0", "cache"=>"none", "size"=>"19GB", "id"=>"virtio0"}, As said, the parameters for volume do not provide possibilities to add for instance the type / and or storage etc ... So the only logical thing that happends is, that the json for disk 2 && 3 coming from Foreman to fog only contains the size parameter ( "1"=>{"size"=>"10GB"}, "2"=>{"size"=>"20GB"} ) But when this information is handed to fog, this is insufficient to create the disk in proxmox, because it also needs to know where and how to store that disk. Normally if you create a second disk, you keep the disks in 1 place, so it makes sense to copy over the information provided in the 1st disk configuration to the requested number 2 and 3. And that is exactly what the patch does, it fills in the missing parts in the json. On the other hand, if the information is correct, i.e. you have 2 disks configured in your compute profile, then these will come in simular like that 1st one from above .. and the patch will not touch it, because it only works on the combination for "if index > 0 && dev_specs.keys.count == 1" .. Hope this helps |
Hi,
I found out that when a new VM is to be created with a request for disks that are not in the ComputeProfile but simply added for example like this ( through hammer ) with: --volume='size=10GB' --volume='size=20GB' the disks do not show up properly in the json.
They show up like this:
"volumes_attributes"=>{
"0"=>{"storage_type"=>"hard_disk", "storage"=>"local-lvm", "controller"=>"virtio", "device"=>"0", "cache"=>"none", "size"=>"19GB", "id"=>"virtio0"},
"1"=>{"size"=>"10GB"},
"2"=>{"size"=>"20GB"}},
As an result disk 1 && 2 are disregarded and only the disk that has been mentioned in the ComputeProfile will get the size requested.
This patch adjusts the json to "copy" the settings from the first disk, into the next ones.
I Hope, you can consider this patch.
Thank you and regards,
Kees