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

better handling of Linux ip addresses #29209

Open
wants to merge 3 commits into
base: devel
from

Conversation

@marvin-sinister
Copy link

commented Sep 10, 2017

SUMMARY

This has multiple fixes, it puts primary ip addreses in a list under ansible_ifname:ipv4, prevents double entries of secondary ip addresses under ansible_ifname:ipv4_secondaries, also (mostly by accident) makes the fact format consistent with generic_bsd facts, where ansible_ifname:ipv4 is a list, unlike current linux where it's a dictionary.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

module_utils/facts

ANSIBLE VERSION
ansible 2.5.0 (devel ab7c850360) last updated 2017/09/10 21:11:37 (GMT +200)
  config file = None
  configured module search path = [u'/home/ansible/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/ansible/development/ansible-upstream/ansible/lib/ansible
  executable location = /home/ansible/development/ansible-upstream/ansible/bin/ansible
  python version = 2.7.13 (default, Jan 19 2017, 14:48:08) [GCC 6.3.0 20170118]
ADDITIONAL INFORMATION

The logic behind the module is wrong. At line 190 it checks whether the ip is secondary and if no ipv4 is present, so that can activate only once. After that at line 195 it doesn't check whether the ip is secondary.
The second bug is result of code after line 206, where there is no check if interface is same as device, and in that case the address is added second time if secondary.

With interface configured as follows:

2: ens192: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000
    link/ether 00:0c:29:dc:f2:12 brd ff:ff:ff:ff:ff:ff
    inet 192.168.0.151/24 brd 192.168.0.255 scope global ens192
       valid_lft forever preferred_lft forever
    inet 10.0.0.5/24 brd 10.0.0.255 scope global ens192
       valid_lft forever preferred_lft forever
    inet 10.0.0.8/24 scope global secondary ens192
       valid_lft forever preferred_lft forever
    inet6 fe80::375c:5c45:ba87:25c/64 scope link
       valid_lft forever preferred_lft forever

The result is:
Before:

        "ansible_ens192": {
            ...
            "ipv4": {
                "address": "192.168.0.151",
                "broadcast": "192.168.0.255",
                "netmask": "255.255.255.0",
                "network": "192.168.0.0"
            },
            "ipv4_secondaries": [
                {
                    "address": "10.0.0.5",
                    "broadcast": "10.0.0.255",
                    "netmask": "255.255.255.0",
                    "network": "10.0.0.0"
                },
                {
                    "address": "10.0.0.8",
                    "broadcast": "global",
                    "netmask": "255.255.255.0",
                    "network": "10.0.0.0"
                },
                {
                    "address": "10.0.0.8",
                    "broadcast": "global",
                    "netmask": "255.255.255.0",
                    "network": "10.0.0.0"
                }
            ],
            ...
        },

After:

        "ansible_ens192": {
            ...
            "ipv4": [
                {
                    "address": "192.168.0.151",
                    "broadcast": "192.168.0.255",
                    "netmask": "255.255.255.0",
                    "network": "192.168.0.0"
                },
                {
                    "address": "10.0.0.5",
                    "broadcast": "10.0.0.255",
                    "netmask": "255.255.255.0",
                    "network": "10.0.0.0"
                }
            ],
            "ipv4_secondaries": [
                {
                    "address": "10.0.0.8",
                    "broadcast": "global",
                    "netmask": "255.255.255.0",
                    "network": "10.0.0.0"
                }
            ],
            ...
        },
@marvin-sinister

This comment has been minimized.

Copy link
Author

commented Sep 12, 2017

Since this would break those that use current Linux output, we shoul probably look into alternative solution.

One posibility is leaving interface[iface]['ipv4'] as is, and put primary addresses in to interface[iface]['ipv4_primaries'] and secondary into interface[iface]['ipv4_secondaries'], but i suspect that will break something as well, since then ['ipv4] and ['ipv4_secondaries'] would not have all the addresses for some interfaces.

Other posibility is just to have first address as interface[iface]['ipv4'] and everything elese dumped into interface[iface]['ipv4_secondaries'] (which we do now, more or less) and not pretend that secondary has anything to do with ip commands definition of secondary ip address. Additionally dump all addresses into new fact, something like interface[iface]['ipv4_all'].

Deduplication part should not break anything.

@maxamillion

This comment has been minimized.

Copy link
Contributor

commented May 21, 2018

Sorry for the lag time on this, would you mind rebasing so we can get the review and merge? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.