Fixes #19623 - add comparison to portkey for vmware networks #4611
Conversation
|
@wiad, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ohadlevy, @isratrade and @ares to be potential reviewers. |
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
2 similar comments
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
|
Issues: #19623 |
|
[test upgrade] (this triggers our CI test) |
| @@ -40,6 +40,7 @@ def select_nic(fog_nics, nic) | |||
| all_networks = service.raw_networks(datacenter) | |||
| vm_network = all_networks.detect { |network| network._ref == nic_attrs['network'] } | |||
| vm_network ||= all_networks.detect { |network| network.name == nic_attrs['network'] } | |||
| vm_network ||= all_networks.detect { |network| network.key == nic_attrs['network'] } | |||
There was a problem hiding this comment.
Nitpick, you may as well put everything on the same line:
vm_network ||= all_networks.detect { |network| [network.name, network.key].include? nic_attrs['network'] }
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
theforeman-bot
added
Needs re-review
Needs testing
Waiting on contributor
and removed
Waiting on contributor
labels
|
@dLobatog: I can't test the issue described in the ticket, but I'll be happy to test this for regressions & merge this next week. |
There was a problem hiding this comment.
@timogoebel Don't worry, I tested just the basic case. The proper fix has to come through fog-vmware though. If tests pass I'll merge.
@wiad Remember to make the commits compliant with @theforeman-bot 's message tips 😉 #4611 (comment) It helps with our automation. Don't worry now though, I will just rebase your two commits into one with the right title.
|
@dLobatog yeah, I forgot the commit message style convention in the last commit but hoped it would be ok anyway - thanks! I'll remember it next time. |
|
[test upgrade] |
|
Thanks @wiad ! |
Details in http://projects.theforeman.org/issues/19623