Add the volume to the root disk (vda) if the selected flavor has a ro… #894
Conversation
…ot disk size of 0 during instance provisioning
|
We should have spec tests covering this new case, I'll write some up and push to your branch if that is okay with you @Dhamo1107 |
|
Specs look like they fail to start due to ManageIQ/manageiq-schema#757 this should be resolved shortly |
| if instance_type.root_disk_size == 0 # Handle root disk logic only when root_disk_size is 0 | ||
| if index == 0 # The first volume should be the root disk, typically assigned to vda |
There was a problem hiding this comment.
This might be better done up above like
# Handle root disk logic only when root_disk_size is 0
if instance_type.root_disk_size > 0
volumes_attrs_list = [default_volume_attributes]
else
volumes_attrs_list = []
# The first volume should be the root disk, typically assigned to vda
requested_volumes.first&.merge!(:imageRef => source.ems_ref, :bootable => true, :boot_index => 0)
end
NOTE do the other requested volumes have to have boot_index = -1? It doesn't appear that they did before.
There was a problem hiding this comment.
I don't really like the -1, but if that we there previously, then it's fine. I would think null would be a better choice.
agrare
force-pushed
the
add-volume-to-root-disk
branch
from
835a879
to
8eb0bdc
Compare
| allow(service).to receive_message_chain('volumes.create').and_return @volume | ||
| allow(@task.source.ext_management_system).to receive(:with_provider_connection)\ | ||
| before do | ||
| allow(service).to receive_message_chain('volumes.create').and_return(volume) |
There was a problem hiding this comment.
I see this was here already, but I generally prefer
| allow(service).to receive_message_chain('volumes.create').and_return(volume) | |
| allow(service).to receive_message_chain(:volumes, :create).and_return(volume) |
There was a problem hiding this comment.
Yeah it was here previously, I do have a "cleanup specs" commit in here already though so I'd rather change all of them to be consistent
|
Overall LGTM- have one question about -1 vs null |
This spec made heavy use of instance vars rather than `let()` which makes it more difficult to override these in sub-contexts.
If a flavor has no root disk we have to set up the bootable volume manually based on the first requested volume.
agrare
force-pushed
the
add-volume-to-root-disk
branch
from
8eb0bdc
to
687edb5
Compare
This Pull Request addresses the handling of root disk provisioning when using flavors with a root disk size of 0. Specifically, if a flavor has a root disk size of 0, the volume provided during provisioning will be designated as the root disk (vda). Conversely, if the flavor has a non-zero root disk size (e.g., 100GB), the specified volume will be treated as an additional volume (vdb).
Before the change:
After the change:
Volume specified during provisioning is correctly assigned as the root disk (vda) if the flavor's root disk size is 0 (25GB volume specified on the add volume page is assigned under vda).
Additional volumes specified will automatically come under vdb when the flavor has a root disk size greater than 0 (With a 100GB root disk size flavor, the root disk is assigned under vda, while the 50GB volume specified on the add volume page is assigned under vdb).
Fixes [#892]