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

Changing the key of network device to be both ipv4 + ipv6 #16619

Merged
merged 1 commit into from
Dec 12, 2017

Conversation

AlonaKaplan
Copy link
Contributor

A network device may not have ipv4 addresses at all, using ipv4 as the sole
key is not enough.

@AlonaKaplan
Copy link
Contributor Author

@pkliczewski @Ladas please review. This is needed for the regular (non graph) refresh.

@Ladas
Copy link
Contributor

Ladas commented Dec 8, 2017

@AlonaKaplan can you verify this doesn't break Vmware/Rhev providers specs? (you will need to run them locally)

@pkliczewski
Copy link
Contributor

The change looks good, please make sure that there are no regressions as @Ladas suggested.

@AlonaKaplan
Copy link
Contributor Author

Both vmware and ovirt specs pass. oVirt was also verified manually (web ui).

A network device may not have ipv4 addresses at all, using ipv4 as the sole
key is not enough.
@miq-bot
Copy link
Member

miq-bot commented Dec 10, 2017

Checked commit AlonaKaplan@3ec67f4 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

LGTM great change @AlonaKaplan !

@agrare agrare merged commit a270061 into ManageIQ:master Dec 12, 2017
@chargio
Copy link
Contributor

chargio commented Dec 12, 2017

Shouldn't this be either, not both?
You can have IPv4, IPv6, or both, they are independent

@agrare
Copy link
Member

agrare commented Dec 12, 2017

@chargio
Copy link
Contributor

chargio commented Dec 12, 2017

@agrare I was concerned that the identification is for all of them. A server with an IPv4 address and a server with the IPv6 address are the same, even if one of them is not working. There are differnt endpoints for the same server.

@agrare
Copy link
Member

agrare commented Dec 12, 2017

Sorry @chargio you lost me :D A NIC can have multiple ip addresses, can be more than one ipv4 address, a mix of ipv4 and ipv6 addresses, etc...

@agrare agrare added this to the Sprint 76 Ending Jan 1, 2018 milestone Dec 12, 2017
@AlonaKaplan
Copy link
Contributor Author

@miq-bot add_label gaprindashvili/yes

simaishi pushed a commit that referenced this pull request Jan 3, 2018
Changing the key of network device to be both ipv4 + ipv6
(cherry picked from commit a270061)
@simaishi
Copy link
Contributor

simaishi commented Jan 3, 2018

Gaprindashvili backport details:

$ git log -1
commit f05b40ef0e67495de4e5f2f48bb0b92940502636
Author: Adam Grare <agrare@redhat.com>
Date:   Tue Dec 12 08:44:03 2017 -0500

    Merge pull request #16619 from AlonaKaplan/ips
    
    Changing the key of network device to be both ipv4 + ipv6
    (cherry picked from commit a270061b26f89a05b270f5168e6c9ba70d8213c5)

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.

7 participants