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

No longer select 'Discovered virtual machine' as a default folder #11135

Merged

Conversation

syncrou
Copy link
Contributor

@syncrou syncrou commented Sep 8, 2016

Purpose or Intent

No longer default to the Discovered virtual machine folder when provisioning VMWare VM's.

Links

Steps for Testing/QA

Provision a VM from a dialog with :placement_auto set to true.
The VM will now show up off the root folder for the parent datacenter as opposed to the Discovered virtual machine folder.

@syncrou
Copy link
Contributor Author

syncrou commented Sep 8, 2016

@miq-bot add_label providers/vmware/infra, provisioning

@syncrou
Copy link
Contributor Author

syncrou commented Sep 8, 2016

@miq-bot add_label automate

@syncrou
Copy link
Contributor Author

syncrou commented Sep 8, 2016

@gmcculloug, @Fryguy -- Please review

--cc @agrare, @tinaafitz

vm_folder = "#{host_dc.folder_path}/vm"
find_folder("#{vm_folder}/Discovered virtual machine", host_dc) || find_folder(vm_folder, host_dc)
find_folder(vm_folder, host_dc)
Copy link
Member

Choose a reason for hiding this comment

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

Minor, but you do not need the vm_folder variable anymore since it is only used once.

find_folder("#{host_dc.folder_path}/vm", host_dc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - I only left it there for readability. I'll pull it out though as the find_folder method will still be easy to read since the string referenced by vm_folder is much shorter than it originally was.

For VMWare provisioning we allow the root folder as the default when
auto placement is set to true

https://www.pivotaltracker.com/story/show/130055823
@syncrou syncrou force-pushed the remove_discovered_virtual_machine branch from 60afbcd to 8ea3f2a Compare September 9, 2016 13:47
@miq-bot
Copy link
Member

miq-bot commented Sep 9, 2016

Checked commit syncrou@8ea3f2a with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 0 offenses detected
Everything looks good. 👍

@agrare
Copy link
Member

agrare commented Sep 9, 2016

Nice @syncrou !

@gmcculloug gmcculloug added bug and removed automate labels Sep 9, 2016
@gmcculloug gmcculloug merged commit 7ac58a9 into ManageIQ:master Sep 9, 2016
@gmcculloug gmcculloug added this to the Sprint 46 Ending Sep 12, 2016 milestone Sep 9, 2016
@agrare
Copy link
Member

agrare commented Oct 4, 2016

@gmcculloug this seems to be impacting users on darga (#11573) can this be backported?

@gmcculloug
Copy link
Member

@agrare Yes. @syncrou Is there a BZ you can associate to this change?

@chessbyte
Copy link
Member

@simaishi can you help out with Bugzilla for this based on #11573

@simaishi
Copy link
Contributor

chessbyte pushed a commit that referenced this pull request Oct 11, 2016
No longer select 'Discovered virtual machine' as a default folder
(cherry picked from commit 7ac58a9)

https://bugzilla.redhat.com/show_bug.cgi?id=1383470
@chessbyte
Copy link
Member

Darga Backport details:

$ git log
commit e66c3824aec855dc5b74f3652796f746657a8319
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Fri Sep 9 10:39:58 2016 -0400

    Merge pull request #11135 from syncrou/remove_discovered_virtual_machine

    No longer select 'Discovered virtual machine' as a default folder
    (cherry picked from commit 7ac58a9eddd6068e141a166de578834f59bd4441)

    https://bugzilla.redhat.com/show_bug.cgi?id=1383470

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

7 participants