Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying the datastore/network for OVF deployment. #670

Merged
merged 1 commit into from Nov 20, 2020

Conversation

lfu
Copy link
Member

@lfu lfu commented Nov 18, 2020

Allow the user to specify the datastore/network for the OVF deployment when creating the service template or through the service dialog.

Depends on ManageIQ/manageiq-ui-classic#7521.
Depends on #671.

@miq-bot add_label enhancement

@lfu lfu changed the title Allow specifying the datastore for OVF deployment. [WIP] Allow specifying the datastore for OVF deployment. Nov 18, 2020
@miq-bot miq-bot added the wip label Nov 18, 2020
@@ -28,6 +28,7 @@ def deployment_spec(opts)

spec = {"accept_all_EULA" => opts[:accept_all_eula]}
spec["name"] = opts[:vm_name] if opts[:vm_name].present?
spec["default_datastore_id"] = Storage.find_by(:id => opts[:storage_id]).ems_ref if opts[:storage_id].present?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be setting an ems_ref to an _id property? Also if the find_by returns nil .ems_ref is going to blow up, recommend .find_by()&.ems_ref

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default_datastore_id is the name that VMware uses. MIQ saves it as ems_ref for a storage.

@@ -45,7 +46,8 @@
:accept_all_eula => true,
:resource_pool_id => "2",
:ems_folder_id => "3",
:host_id => "1"
:host_id => "1",
:storage_id => "1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing?

@lfu lfu force-pushed the ovf_storage_i_15722 branch 2 times, most recently from 7a20f77 to 5532656 Compare November 19, 2020 14:24
@lfu lfu force-pushed the ovf_storage_i_15722 branch 2 times, most recently from 6dc9cef to a32137f Compare November 20, 2020 15:09
@lfu lfu changed the title [WIP] Allow specifying the datastore for OVF deployment. Allow specifying the datastore/network for OVF deployment. Nov 20, 2020
@miq-bot miq-bot removed the wip label Nov 20, 2020
@miq-bot
Copy link
Member

miq-bot commented Nov 20, 2020

Checked commit lfu@d06dab3 with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
3 files checked, 0 offenses detected
Everything looks fine. 🏆

@lfu
Copy link
Member Author

lfu commented Nov 20, 2020

@agrare @gtanzillo Please take a final review.

@gtanzillo gtanzillo self-assigned this Nov 20, 2020
@gtanzillo gtanzillo merged commit 6fb67cd into ManageIQ:master Nov 20, 2020
Fryguy pushed a commit that referenced this pull request Nov 30, 2020
Allow specifying the datastore/network for OVF deployment.

(cherry picked from commit 6fb67cd)
@Fryguy
Copy link
Member

Fryguy commented Nov 30, 2020

Kasparov backport details:

commit 8de47cb178651c9109e3169e334decda0ca90785 (HEAD -> kasparov, upstream/kasparov)
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Fri Nov 20 11:07:27 2020 -0500

    Merge pull request #670 from lfu/ovf_storage_i_15722

    Allow specifying the datastore/network for OVF deployment.

    (cherry picked from commit 6fb67cd5bf7bafeb3a4fbe4041bfea9d65ed7f07)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants