Ignore virtual interfaces in sysinfo Launchpad #1944204 #97
Conversation
There was a problem hiding this comment.
Please, correct me if I'm wrong; are aliases supposed to show up by default now? A quick test:
$ sudo ifconfig wlan0:1 192.168.100.100
$ ls /sys/devices/virtual/net/wlan*
zsh: no matches found: /sys/devices/virtual/net/wlan*
$ python -c "import netifaces; print(netifaces.interfaces())"
['lo', 'wlan0', 'enx083a88615013', 'lxdbr0', 'veth7af8c96a', 'veth2cebdd17', 'wlan0:1']Changes otherwise make sense and look great.
|
Thanks @maxiberta I copied the alias code back in |
landscape/lib/network.py
Outdated
| return interfaces | ||
|
|
||
|
|
||
| def get_active_device_info(skip_alias=True, extended=False): | ||
| """ | ||
| Returns a dictionary containing information on each active network | ||
| interface present on a machine. |
There was a problem hiding this comment.
This comment isn't accurate anymore, since you now list "active physical network interfaces".
landscape/lib/network.py
Outdated
| if interface in skipped_interfaces: | ||
| continue | ||
| if skip_vlan and "." in interface: | ||
| if interface in virtual_interfaces: |
There was a problem hiding this comment.
Have you looked at the various callsites of this function?
It looks like this is used by a networkdevice monitor, which would alter monitoring of those interfaces by landscape-client.
This looks like it's altering a bit more than just sysinfo.
|
There are a few things which bug me with this change, but the most important is the side effects to client monitoring, which is not related to sysinfo. Also, I don't think the filtering of virtual interfaces is quite the right approach. In some network configurations, the physical interfaces have no addresses and the "main" IP is actually on a vlan or bridge. (if you have a DDWRT home router, that's what you'll see). The suggestion from the bug report of checking the default route interface almost always point to the interface of interest (unless the machine is air-gapped). |
|
It seemed simpler to me to do it this way reading the virtual devices, but it sounds like using the default routes is the way to go. For the side effects, I could add a flag so that |
There was a problem hiding this comment.
+1 feel free to disregard the nitpick.
Also, to answer your question, the "default" special gateway value is always there, as per docs.
Works fine with multi ipv6 v4 and other weird setups I could throw at it (dual routes, metrics, bridging).
I think there are still odd cases (or perhaps netiface is smarter than my fake network loops), but for the purpose of sysinfo this new behaviour is desirable.
| device_info = get_active_device_info(extended=False, default_only=True) | ||
| interfaces = [i["interface"] for i in device_info] | ||
| self.assertIn(default_iface, interfaces) | ||
| self.assertEqual(len(interfaces), 1) |
There was a problem hiding this comment.
nitpick: since you properly isolated this test, the content interfaces should be fixed and could be a single assertEqual() on a set.
No description provided.