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

Filter out the hosts with the selected network. #14946

Merged
merged 1 commit into from
May 1, 2017

Conversation

lfu
Copy link
Member

@lfu lfu commented Apr 28, 2017

Once a network is selected, the available hosts should be filtered out to the hosts that are on the selected network.

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

@miq-bot assign @gmcculloug
@miq-bot add_label bug, fine/yes, provisioning
cc @agrare

Once a network is selected, the available hosts should be filtered out to the hosts that are on the selected network.

https://bugzilla.redhat.com/show_bug.cgi?id=1435814
@miq-bot
Copy link
Member

miq-bot commented Apr 28, 2017

Checked commit lfu@4c80745 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Member

@bdunne bdunne left a comment

Choose a reason for hiding this comment

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

👍 LGTM

return all_hosts unless vlan_name

_log.info "Filtering hosts with the following network: <#{vlan_name}>"
all_hosts.reject { |h| !h.lans.pluck(:name).include?(vlan_name) }
Copy link
Member

Choose a reason for hiding this comment

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

@lfu Where you able to verify if this is save for all Infra providers? Is the host -> lan relationship always going to work here?

cc @agrare

Copy link
Member

Choose a reason for hiding this comment

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

@gmcculloug yes host -> lan will work for all infra providers (at least RHV and SCVMM, I don't see any lans in openstack::infra)

Copy link
Member Author

Choose a reason for hiding this comment

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

Openstack infra is only for managing the undercloud - registering new baremetal nodes, adding more compute nodes, etc. In other words, there is no provisioning for Openstack Infra.

@bdunne bdunne merged commit acf7681 into ManageIQ:master May 1, 2017
@bdunne bdunne added this to the Sprint 60 Ending May 8, 2017 milestone May 1, 2017
@agrare
Copy link
Member

agrare commented May 1, 2017

The only thing we should check on other providers (RHV/SCVMM) is how this behaves if you have two networks with the same name, since those two providers don't use the name for the UID like VMware does. I don't think we have the UID in @values[:vlan] though

@lfu
Copy link
Member Author

lfu commented May 8, 2017

The available hosts for RHEVM provisioning is limited to the same data center where the template is located. And it is not allowed to have two networks with the same name per data center.
So using network name as the identifier in @values[:vlan] would work for RHEVM.

@bronaghs How about SCVMM? Is it possible to have two networks with the same name in SCVMM that the selected host would actually be the wrong one and is not on the selected network?

simaishi pushed a commit that referenced this pull request Jun 2, 2017
@simaishi
Copy link
Contributor

simaishi commented Jun 2, 2017

Fine backport details:

$ git log -1
commit 162577f44fa1b81076f83ae4c6e708cb0538f6c6
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Mon May 1 10:19:16 2017 -0400

    Merge pull request #14946 from lfu/hosts_with_selected_vlan_1435814
    
    Filter out the hosts with the selected network.
    (cherry picked from commit acf768135451b7dc174fe2586b594beedc293a47)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1458363

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.

6 participants