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

states/network.py managed() - no way to manage IP aliases in 2014.7 #18168

Closed
aleksmm opened this issue Nov 17, 2014 · 22 comments
Closed

states/network.py managed() - no way to manage IP aliases in 2014.7 #18168

aleksmm opened this issue Nov 17, 2014 · 22 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior fixed-pls-verify fix is linked, bug author to confirm fix P2 Priority 2 Platform Relates to OS, containers, platform-based utilities like FS, system based apps Regression The issue is a bug that breaks functionality known to work in previous releases. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@aleksmm
Copy link

aleksmm commented Nov 17, 2014

Lets consider this sls:

eth0:
  network.managed:
    - name: "eth0"
    - enabled: True
    - type: eth
    - proto: none
    - ipaddr: "10.10.223.30"
    - netmask: "255.255.255.0"
    - dns:
      - 8.8.8.8
      - 8.8.4.4

eth0_1:
  network.managed:
    - name: "eth0:1"
    - enabled: True
    - type: eth
    - ipaddr: "10.10.223.29"
    - netmask: "255.255.255.0"

eth0_2:
  network.managed:
    - name: "eth0:2"
    - enabled: True
    - type: eth
    - ipaddr: "10.10.223.38"
    - netmask: "255.255.255.0"

In 2014.1 you would have eth0 with 3 aliases on it because in modules/rh_ip.py up() it runs "ifup eth0:1" which legit on RHEL systems

In 2014.7 you cant do that anymore, because salt.utils.network.interfaces() returns new format of array:

{'lo': {'hwaddr': '00:00:00:00:00:00', 'up': True, 'inet': [{'broadcast': None, 'netmask': '255.0.0.0', 'label': 'lo', 'address': '127.0.0.1'}], 'inet6': [{'prefixlen': '128', 'address': '::1'}]}, 'eth0': {'hwaddr': 'd6:be:8f:2c:bc:57', 'inet6': [{'prefixlen': '64', 'address': 'fe80::d4be:8fff:fe2c:bc57'}], 'up': True, 'inet': [{'broadcast': '10.10.223.255', 'netmask': '255.255.255.0', 'label': 'eth0', 'address': '10.10.223.30'}], 'secondary': [{'broadcast': '10.10.223.255', 'netmask': '255.255.255.0', 'label': 'eth0:1', 'type': 'inet', 'address': '10.10.223.29'}, {'broadcast': '10.10.223.255', 'netmask': '255.255.255.0', 'label': 'eth0:2', 'type': 'inet', 'address': '10.10.223.38'}]}}

Which in short is {'lo':{}, 'eth0':{'secondary': [{'label': 'eth0:1'}, {'label': 'eth0:2'}]}, so when you have
states/network.py, line 282: interface_status = salt.utils.network.interfaces()[name].get('up') you will get exception trying to get name="eth0:1" from the dict.

Obviously the solution is either:

  • revert back to old dist format and consider "eth0:1" and etc as separate interfaces, as it used to be
  • add to type "alias" and treat the interface the other way, like with type="bond"
@basepi
Copy link
Contributor

basepi commented Nov 18, 2014

I think it probably makes the most sense to update the state to match the new dictionary format. We'll definitely investigate this.

@rallytime rallytime added ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists Regression The issue is a bug that breaks functionality known to work in previous releases. and removed ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. labels Nov 18, 2014
@rallytime rallytime added this to the Approved milestone Nov 18, 2014
thatch45 added a commit to thatch45/salt that referenced this issue Nov 20, 2014
Change the network lookup to be more robust and find aliases
@thatch45 thatch45 mentioned this issue Nov 20, 2014
@thatch45 thatch45 self-assigned this Nov 20, 2014
@thatch45
Copy link
Member

Ok, that PR should do it, it checks for the interface in the nested data and does not make bad assumptions as to the presence of interfaces

thatch45 added a commit that referenced this issue Nov 20, 2014
@justyns
Copy link

justyns commented Nov 21, 2014

I was having the same issue with 2014.7.0 and downloaded https://raw.githubusercontent.com/saltstack/salt/2014.7/salt/states/network.py which contains your changes, and it did fix the issue for me. Interface aliases are working correctly again, at least on centos.

Just chiming in to confirm this fixes the issue for me at least.

@rallytime rallytime added fixed-pls-verify fix is linked, bug author to confirm fix and removed in progress labels Nov 21, 2014
@rallytime
Copy link
Contributor

Thanks for the update @justyns - that is great to know!

@aleksmm Can you give this a try as well? Feel free to close this issue if the bug has been fixed for you.

@aleksmm
Copy link
Author

aleksmm commented Nov 21, 2014

@rallytime you have done exactly what was needed, it will work for sure. I will test this once it will be released in epel package.

@rallytime
Copy link
Contributor

@aleksmm Awesome! Glad we could get that fixed up.

@aleksmm
Copy link
Author

aleksmm commented Nov 21, 2014

@rallytime you may close the ticket, I will reopen in case it wont work.

@rallytime
Copy link
Contributor

Sounds good!

@eliasp
Copy link
Contributor

eliasp commented Jan 6, 2015

@rallytime Still seeing a problem here on Ubuntu 14.04 - could you re-open this issue?

@thatch45 The problem: On each run, network.managed reports 1 change:

----------
          ID: eth1:0
    Function: network.managed
      Result: True
     Comment: Interface eth1:0 is up to date.
     Started: 15:55:19.641703
    Duration: 340.381 ms
     Changes:   
              ----------
              status:
                  Interface eth1:0 is up

The problem seems to be caused by salt.utils.network.interfaces() not returning any secondary interfaces at all on Ubuntu, only primary interfaces.

First I assumed this happened because salt.utils.network.linux_interfaces() uses ip, which shows secondary interfaces only as labels in ip addr show (which is actually the right thing to do):

    2: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN group default qlen 1000
        link/ether 1c:bd:b9:da:3e:47 brd ff:ff:ff:ff:ff:ff
        inet 192.168.0.1/30 brd 192.168.0.3 scope global eth1
           valid_lft forever preferred_lft forever
        inet 192.168.1.1/30 brd 192.168.1.3 scope global eth1:0
           valid_lft forever preferred_lft forever
        inet6 fe80::1ebd:b9ff:feda:3e47/64 scope link 
           valid_lft forever preferred_lft forever

But after temporarily moving /bin/ip away I got the same results from salt.utils.network.interfaces() when forcing it to use the ifconfig based backend.

Running ifconfig -a directly shows the primary and secondary interface:

    eth1      Link encap:Ethernet  HWaddr 1c:bd:b9:da:3e:47  
              inet addr:192.168.0.1  Bcast:192.168.0.3  Mask:255.255.255.252
              inet6 addr: fe80::1ebd:b9ff:feda:3e47/64 Scope:Link
              UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
              RX packets:83916786 errors:170426 dropped:373223 overruns:148291 frame:0
              TX packets:45370983 errors:0 dropped:0 overruns:0 carrier:0
              collisions:0 txqueuelen:1000 
              RX bytes:126356682846 (126.3 GB)  TX bytes:3201221929 (3.2 GB)

    eth1:0    Link encap:Ethernet  HWaddr 1c:bd:b9:da:3e:47  
              inet addr:192.168.1.1  Bcast:192.168.1.3  Mask:255.255.255.252
              UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1

So it looks like the problem lies in _interfaces_ip() and _interfaces_ifconfig() having trouble parsing the secondary interfaces.

@rallytime rallytime reopened this Jan 6, 2015
@rallytime
Copy link
Contributor

@eliasp Sorry I never responded here. What version of salt are you seeing this on?

@eliasp
Copy link
Contributor

eliasp commented Feb 24, 2015

@rallytime Seeing this on:
OS: Ubuntu 14.04.1 amd64
Salt:

               Salt: 2014.7.1
             Python: 2.7.6 (default, Mar 22 2014, 22:59:56)
             Jinja2: 2.7.2
           M2Crypto: 0.21.1
     msgpack-python: 0.3.0
       msgpack-pure: Not Installed
           pycrypto: 2.6.1
            libnacl: Not Installed
             PyYAML: 3.10
              ioflo: Not Installed
              PyZMQ: 14.0.1
               RAET: Not Installed
                ZMQ: 4.0.4
               Mako: 0.9.1

@rallytime
Copy link
Contributor

Ok great thanks for the update!

@rallytime rallytime removed the fixed-pls-verify fix is linked, bug author to confirm fix label Feb 24, 2015
@eliasp
Copy link
Contributor

eliasp commented Mar 14, 2015

This problem also shows up in a slightly different scenario:
When applying this state to a Minion, where the physical interface doesn't exist at all, it still reports:

----------
          ID: eth1
    Function: network.managed
      Result: True
     Comment: Interface eth1 is up to date.
     Started: 13:44:52.982013
    Duration: 196.325 ms
     Changes:   
              ----------
              status:
                  Interface eth1 is up
----------
          ID: eth1:0
    Function: network.managed
      Result: True
     Comment: Interface eth1:0 is up to date.
     Started: 13:44:53.178423
    Duration: 182.302 ms
     Changes:   
              ----------
              status:
                  Interface eth1:0 is up

… instead of failing because eth1 doesn't exist at all.

@dmurphy18 dmurphy18 added severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P2 Priority 2 Platform Relates to OS, containers, platform-based utilities like FS, system based apps info-needed waiting for more info and removed severity-low 4th level, cosemtic problems, work around exists labels Jun 16, 2015
@dmurphy18
Copy link
Contributor

Can you retest this issue on the latest 2015.5.2 to check that it is resolved, thanks,

@rallytime rallytime added fixed-pls-verify fix is linked, bug author to confirm fix and removed info-needed waiting for more info labels Jun 27, 2015
@fbretel
Copy link
Contributor

fbretel commented Jul 23, 2015

Same problem here on a RHEL5 box: network.managed reports 1 change, for each ip alias, on each run.

           Salt: 2015.5.3
         Python: 2.6.8 (unknown, Feb 18 2015, 08:19:08)
         Jinja2: 2.5.5
       M2Crypto: 0.21.1
 msgpack-python: 0.4.5
   msgpack-pure: Not Installed
       pycrypto: 2.3
        libnacl: Not Installed
         PyYAML: 3.08
          ioflo: Not Installed
          PyZMQ: 14.5.0
           RAET: Not Installed
            ZMQ: 4.0.5
           Mako: Not Installed
        Tornado: Not Installed

@fbretel
Copy link
Contributor

fbretel commented Jul 23, 2015

salt.utils.network.interfaces() is returning secondary interfaces though.

@rallytime rallytime removed the fixed-pls-verify fix is linked, bug author to confirm fix label Jul 28, 2015
@timwsuqld
Copy link

I'm also experiencing something I believe is related. My alias interfaces report a change ever highstate run, regardless if they have actually changed.

          ID: int_ens192_38
    Function: network.managed
        Name: ens192:38
      Result: True
     Comment: Interface ens192:38 is up to date.
     Started: 15:00:12.257754
    Duration: 954.728 ms
     Changes:   
              ----------
              status:
                  Interface ens192:38 is up

@cmarzullo
Copy link

I too am having the same issue as @timwsuqld

----------
          ID: network_interface_eth0:0
    Function: network.managed
        Name: eth0:0
      Result: True
     Comment: Interface eth0:0 is up to date.
     Started: 12:51:04.534235
    Duration: 495.103 ms
     Changes:
              ----------
              status:
                  Interface eth0:0 is up
    network_interface_eth0:0:
        ----------
        __env__:
            base
        __sls__:
            networking.config
        network:
            |_
              ----------
              name:
                  eth0:0
            |_
              ----------
              type:
                  eth
            |_
              ----------
              netmask:
                  255.255.0.0
            |_
              ----------
              ipaddr:
                  192.168.101.1
            |_
              ----------
              enabled:
                  True
            |_
              ----------
              proto:
                  static
            - managed
            |_
              ----------
              order:
                  10001
salt-call --versions-report
Salt Version:
           Salt: 2016.3.0

Dependency Versions:
           cffi: 0.8.6
       cherrypy: Not Installed
       dateutil: 2.2
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.3
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.2
   mysql-python: 1.2.3
      pycparser: 2.10
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.9 (default, Mar  1 2015, 12:57:24)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.4.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: debian 8.5
        machine: x86_64
        release: 4.5.0-x86_64-linode65
         system: Linux
        version: debian 8.5

@meggiebot
Copy link

This is now fixed on the head of develop. Please re-open if this issue arises again.

@Foxlik
Copy link

Foxlik commented Mar 7, 2017

Sorry for reviving this thread, but I do have a favor to ask. Could this please get backported to stable? I'd love to stop patching the module myself. Thanks in advance.

@eliasp
Copy link
Contributor

eliasp commented Mar 7, 2017

@Foxlik just use Dynamic Module Distribution - no need to patch it manually every time you update.

@Foxlik
Copy link

Foxlik commented Mar 7, 2017

@eliasp ah, great idea. thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior fixed-pls-verify fix is linked, bug author to confirm fix P2 Priority 2 Platform Relates to OS, containers, platform-based utilities like FS, system based apps Regression The issue is a bug that breaks functionality known to work in previous releases. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests