Filter openstack networks in provisioning #17055
Conversation
| cloud_network.class == ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Public || | ||
| cloud_network.class == ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Private | ||
| end | ||
| networks.any? |
There was a problem hiding this comment.
Few comments on this part of code:
Filter if the CloudNetwork is or is not OpenStack network can be done also by lookup for ancestors of the class (not pushing for change, it's just my preference, example: cloud_network.class.ancestors.include? "ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork")
A bit high level comment, but in this change we expect that if network is from Openstack , then it has a method eligible_for_vm_provisioning, which might not be clear in future. I'd suggest adding eligible_for_vm_provisioning method also to CloudNetwork.
Nit: We don't need networks local variable here, it should be enough ask for any? just after the block end.
Nit2: It would be great not use openstack keyword, but have a generic code (e.g. by adding eligible_for_vm_provisioning method to CloudNetwork model).
alexander-demicev
force-pushed
the
filter-cloud-networks
branch
from
827a283 to
0b4d894
Compare
alexander-demicev
changed the title
alexander-demicev
force-pushed
the
filter-cloud-networks
branch
from
0b4d894 to
55abeb3
Compare
|
Checked commit alexander-demicev@55abeb3 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@Ladas Hi, if you have minute, could you check this PR? Thanks |
|
|
||
| def openstack?(networks) | ||
| networks.select do |cloud_network| | ||
| cloud_network.class.ancestors.include?("ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork") |
There was a problem hiding this comment.
core code can't have any mentions of OpenStack, do this via support mixin or use specific allowed_cloud_networks in OpenStack plugin
|
@Ladas thanks for advice, logic moved to openstack repo ManageIQ/manageiq-providers-openstack#238. Closing this PR. |
This PR filters openstack cloud networks that do not have subnets. Network requires a subnet in order to be used for provisioning.
BZ
@aufi @lpichler Can you review it?