Declare Kubevirt's template as eligible for provision #16873
Conversation
|
@miq-bot add_labels gaprindashvili/no, enhancement, providers |
|
@pkliczewski please review |
| :required: false | ||
| :display: :edit | ||
| :data_type: :string | ||
| :sysprep_enabled: |
There was a problem hiding this comment.
Why do we need sysprep options? For MVP we only need cloud-init.
| :required: false | ||
| :display: :edit | ||
| :data_type: :integer | ||
| :placement_auto: |
There was a problem hiding this comment.
I think that placement is beyond us. The way we interact with kubevirt won't let us interfere with placement.
There was a problem hiding this comment.
removed the host placement.
| :service: | ||
| :description: Catalog | ||
| :fields: | ||
| :number_of_vms: |
There was a problem hiding this comment.
I think we need only one for now
There was a problem hiding this comment.
If this works let's keep it
| :notes_display: :show | ||
| :min_length: | ||
| :max_length: 255 | ||
| :pxe_image_id: |
There was a problem hiding this comment.
pxe and iso provisioning is out of the scope for mvp.
masayag
force-pushed
the
provision_vm_dialog
branch
from
c2fa887
to
1a2ef4d
Compare
|
@miq-bot assign @gmcculloug |
|
@masayag If you are not using the |
masayag
force-pushed
the
provision_vm_dialog
branch
from
1a2ef4d
to
c32dea5
Compare
|
@gmcculloug done, removed all of the unused entries from the dialog. Will add them when the provider will support them gradually. |
| :display: :edit | ||
| :default: native_clone | ||
| :data_type: :string | ||
| :linked_clone: |
There was a problem hiding this comment.
Is this a likely Kubevirt property?
There was a problem hiding this comment.
:linked_clone: is used by VMWare and Redhat to describe if the vm creation should be thinly-provisioned or not IIUC. It seems that there is no use case to support it for kubevirt. I'll remove it from the workflow and from yml.
The code that implements the provisioning was merged as part of the branch creation. You can find it here:
Kubevirt is an infra manager/provider.
|
@masayag Is there a workflow in progress (wip PR maybe) that would use this dialog? Is the Kubevirt provider being developer similar to a cloud or infra provider or something else? |
masayag
force-pushed
the
provision_vm_dialog
branch
from
c32dea5
to
29a1559
Compare
|
@gmcculloug, removed the The code that implements the provisioning was merged as part of the branch creation. You can find it here: Kubevirt is an infra manager/provider. |
|
@masayag I wasn't think about the provider repo when I initially reviewed this PR but it would make sense to move the dialog into the provider repo they way we do for Azure provider today: https://github.com/ManageIQ/manageiq-providers-azure/blob/master/content/miq_dialogs/miq_provision_azure_dialogs_template.yaml (The other dialogs will eventually move as well.) I think the other change needs to remain in the core repo for now. Sorry for realizing this so late. |
Kubevirt is a virtualization provider that allows to create virtual machines. For that purpose, the kubevirt's template should be added to the eligible for provision list of templates.
masayag
force-pushed
the
provision_vm_dialog
branch
from
29a1559
to
6960e36
Compare
masayag
changed the title
|
@gmcculloug updated both PRs (this and ManageIQ/manageiq-providers-kubevirt#25) Thanks for the review. |
|
Checked commit masayag@6960e36 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@masayag Could you link these two PRs to each other in the PR description, the top comment? Thanks. |
| super.where(:type => %w(ManageIQ::Providers::Redhat::InfraManager::Template | ||
| ManageIQ::Providers::Vmware::InfraManager::Template | ||
| ManageIQ::Providers::Microsoft::InfraManager::Template | ||
| ManageIQ::Providers::Kubevirt::InfraManager::Template)) |
There was a problem hiding this comment.
@agrare I don't think it needs to be part of this PR but do we have a way to do this in a more pluggable way?
There was a problem hiding this comment.
😱 this is really nasty, looking at this and the CloudManager::Template.eligible_for_provisioning methods I think they include.....all of them?
There was a problem hiding this comment.
Looks like all this really does is show Infra or Cloud templates, why don't they just do ManageIQ::Providers::InfraManager::Template.all or ManageIQ::Providers::CloudManager::Template.all ?
There was a problem hiding this comment.
From the Lifecycle -> Provisioning UI we show all templates, and that is not always an accurate list of what templates can be provisioned. I think the best example here is that we did not support Microsoft::InfraManager template provisioning for a while after the templates were available in inventory.
Being able to select a provider's template because it is available in inventory without the supporting provisioning workflow/dialog would lead to UI errors.
From a pluggable solution we need to be able to ask each provider if they support template provisioning and then we can query the appropriate templates.
|
@lfu updated both PRs. |
|
@gmcculloug looks good to me |
1 similar comment
|
@gmcculloug looks good to me |
gmcculloug
modified the milestones:
Sprint 78 Ending Jan 29, 2018,
Sprint 79 Ending Feb 12, 2018
Kubevirt is a virtualization provider that allows to create virtual
machines.
For that purpose, the kubevirt's template should be added to the
eligible for provision list of templates.
Kubevirt is a virtualization provider that allows to create virtual
machines. Currently, it is pretty limited and not fully defined,
therefore the initial dialog allows the user to specific the VM memory
only.
The provider PR that adds the vm provision dialog:
ManageIQ/manageiq-providers-kubevirt#25