-
Notifications
You must be signed in to change notification settings - Fork 62
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
Get mac addr if the selected profile is '<Empty>' or '<Template>' #227
Conversation
@masayag please review |
@miq-bot add_labels bug, gaprindashvili/yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix rubocop complaints
@@ -551,10 +551,24 @@ def get_mac_address_of_nic_on_requested_vlan(args) | |||
end | |||
|
|||
def find_mac_address_on_network(nics, vnic_profile_id) | |||
nic = nics.detect do |n| | |||
n.vnic_profile.id == vnic_profile_id | |||
if vnic_profile_id == '<Empty>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please extract '' to constant - it is used widely and we should define it in one place, so let's start with defining a constant for it in this PR, and in a follow PR replace other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will do it in a separate patch.
nic = nics.detect do |n| | ||
n.vnic_profile.nil? | ||
end | ||
elsif vnic_profile_id == '<Template>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here for
if nics.empty? | ||
_log.warn "Cannot find mac address, there are no NICs" | ||
else | ||
_log.warn "Cannot find mac address, cannot find NIC with vnic_profile=#{vnic_profile_id}" if nic.nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it sounds like there are two error messages in this line: one for mac address and the other for the profile.
Maybe 'or' between the two sentence parts or can we be more specific based on the detected nic ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
2cd65b5
to
4963f2f
Compare
the code looks okay, but a spec file is needed here. |
Verified this fix, Tested on CFME side, for RHV provider, VM provision from pxe:
|
…te>' In case the selected profile is '<Empty>' the mac of a nic with null profile will be retrieved. In case the selected profile is '<Template>' the mac of the first nic would be retrieved. Bug-Url: https://bugzilla.redhat.com/1552597 Signed-off-by: Alona Kaplan <alkaplan@redhat.com>
@masayag a spec was added, please re-review. |
Checked commit AlonaKaplan@1ec7344 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Get mac addr if the selected profile is '<Empty>' or '<Template>' (cherry picked from commit 0c70a56) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1561218
Gaprindashvili backport details:
|
In case the selected profile is '<Empty>' the mac of a nic with null
profile will be retrieved.
In case the selected profile is '<Template>' the mac of the first nic would
be retrieved.
Fixes https://bugzilla.redhat.com/1552597