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

[Tech-Debt] Avoid use of deprecated net-tools #57541

Closed
OrangeDog opened this issue Jun 4, 2020 · 24 comments · Fixed by #64351
Closed

[Tech-Debt] Avoid use of deprecated net-tools #57541

OrangeDog opened this issue Jun 4, 2020 · 24 comments · Fixed by #64351
Assignees
Labels
Feature new functionality including changes to functionality and code refactors, etc. severity-high 2nd top severity, seen by most users, causes major problems tech-debt

Comments

@OrangeDog
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The linux net-tools package has been deprecated since 2011, and provides commands including arp, ifconfig, iptunnel, iwconfig, nameif, netstat, and route.

Some salt modules make use of these commands, causing failures when they're not available (#57513).

Describe the solution you'd like
Code should be rewritten, preferring first native python (e.g. pyroute2) then the iproute2 tools (e.g. ip), then only falling back to net-tools if that's all that is available. If every supported platform has already migrated to iproute2 then the fallback is not required.

Describe alternatives you've considered
Continue to depend on systems providing the net-tools package. This won't work forever.

Additional context
Replacement commands:

net-tools iproute2
arp ip neighbor
ifconfig ip addr, ip link, ip -stats
iptunnel ip tunnel
iwconfig iw
nameif ip link
netstat ss
route ip route
@OrangeDog OrangeDog added the Feature new functionality including changes to functionality and code refactors, etc. label Jun 4, 2020
@bryceml
Copy link
Contributor

bryceml commented Jun 4, 2020

The issue described here #55416

could probably be fixed at the same time.

@sagetherage sagetherage added the Magnesium Mg release after Na prior to Al label Jun 4, 2020
@sagetherage sagetherage added the severity-high 2nd top severity, seen by most users, causes major problems label Jun 4, 2020
@sagetherage sagetherage added this to the Approved milestone Jun 4, 2020
@sagetherage
Copy link
Contributor

I don't know if I would called this a feature - I see it as an enhancement to something that currently exists and is not working due to an upstream dependency deprecation some time ago (2011?!), but I don't have a template for enhancement :) so this works

@OrangeDog
Copy link
Contributor Author

@sagetherage the only options are Bug and Feature

@sagetherage
Copy link
Contributor

yes, I put those in place, mostly putting notes for myself :)

@OrangeDog
Copy link
Contributor Author

Closely related: Ubuntu doesn't use ifupdown any more, but netplan. The file /etc/network/interfaces is also not used.

@OrangeDog
Copy link
Contributor Author

OrangeDog commented Jun 9, 2020

I think the whole of debian_ip needs rewriting. Every config file and tool it uses is obsolete (for Ubuntu).
Related: #54791

@dmurphy18
Copy link
Contributor

Note: the old net-tools cannot be totally removed since still have to support older OS's, for example: Solaris 10, some of the underlying OS's on some switch/routers, AIX. But as stated above, have the net-tools as a fall back for when more modern implementations are not available.

@dmurphy18
Copy link
Contributor

dmurphy18 commented Jun 9, 2020

Capturing a thread from community slack here

From: @OrangeDog

also NetworkManager vs. systemd-networkd / ifupdown vs. netplat, etc.

From @terminalmage

I actually have implemented that at work and plan to submit upstream soon

So, need to do a root and branch update of networking when handling replacement of net-tools, allowing for older OS's (Solaris, AIX) and what is available on older Linux platforms, for example: Debian 9, Ubuntu 16.04, and FreeBSD too.

@terminalmage
Copy link
Contributor

I think you misread, @dmurphy18. I wasn't saying I did any of that, I was saying that I implemented network teaming already, which is entirely unrelated to this.

@OrangeDog
Copy link
Contributor Author

OrangeDog commented Jun 9, 2020

@dmurphy18 That should've been netplan, not netplat.

Debian 9 and Ubuntu 16.04 also have iproute2. So does Debian 8. So did Debian 4.

@OrangeDog
Copy link
Contributor Author

OrangeDog commented Jun 9, 2020

More related: #13085, #49078, #57618

The PR #54572 was merged, but then reverted at some point. The Suggests: ifupdown was also not removed.

This is possibly the cause of errors (which used to also cause stacktraces) in #6922. Ubuntu 18.04 and netplan are mentioned.

@dmurphy18
Copy link
Contributor

@terminalmage sorry about that, was grouping network work together, or at least anything with the word network in it :)

@sagetherage
Copy link
Contributor

likely we need an epic created in planning and listing out all the different tasks to track we don't miss something

@terminalmage
Copy link
Contributor

terminalmage commented Jun 11, 2020

I did a quick grep of the code and then checked the matching files. Here's what I found, though I welcome people doing their own analysis in case I missed something:

  • salt/modules/bridge.py - Seems to be used for BSD only. Not sure if this is a problem.
  • salt/modules/iwtools.py - Seems we can leave this alone since this module is specific to using iwtools
  • salt/modules/lxc.py - iproute2 already used here as well and is preferred
  • salt/modules/network.py - Both ss and netstat are supported, but netstat is preferred if both are present. Should ss be preferred moving forward?
  • salt/modules/nspawn.py - ifconfig is referenced in CLI examples in docstrings. Not sure if necessary to update.
  • salt/modules/ps.py - Both ss and netstat are supported here and are exposed as their own Salt functions, which are already conditionally loaded depending on whether either are installed. So, no change needs to be made here.
  • salt/modules/vagrant.py - ifconfig used in get_ssh_config(). No ip equivalent exists and we probably need to consider adding one (and preferring it).
  • salt/modules/win_network.py - netstat referenced, but this is Windows' version of it.
  • salt/modules/win_status.py - Same as win_network.py
  • salt/utils/network.py - Only a couple of the BSDs rely on ifconfig. Not sure if this is a problem.

@OrangeDog
Copy link
Contributor Author

OrangeDog commented Jun 11, 2020

  • salt/modules/network.py is using arp, which was the original issue I linked to
  • salt/modules/linux_ip.py is using ip, but using cmd strings instead of arg lists

There's also this changelog entry I'm not exactly sure what it refers to, but if arp isn't installed then it should be using ip.

net.arp will no longer be made available unless arp is installed

@sagetherage sagetherage added this to the Silicon milestone Mar 22, 2021
@bryceml bryceml removed their assignment Mar 22, 2021
@sagetherage sagetherage modified the milestones: Silicon, Approved Aug 12, 2021
@fignew
Copy link
Contributor

fignew commented May 16, 2022

Fedora 36 no longer has the iwtools package. This has broken the iwtools module for Saltstack.

@dmurphy18 dmurphy18 added the Phosphorus v3005.0 Release code name and version label May 16, 2022
@dmurphy18
Copy link
Contributor

@Ch3LL we probably need to check for those old tools usage before tagging Phosphorous

@dmurphy18 dmurphy18 added Sulfur v3006.0 release code name and version and removed Phosphorus v3005.0 Release code name and version labels May 17, 2022
@dmurphy18 dmurphy18 changed the title Avoid use of deprecated net-tools [Tech-Debt] Avoid use of deprecated net-tools May 17, 2022
@dmurphy18 dmurphy18 self-assigned this May 17, 2022
@waynew waynew removed the Sulfur v3006.0 release code name and version label Dec 16, 2022
@dmurphy18
Copy link
Contributor

dmurphy18 commented Apr 27, 2023

Quick check of replacement commands in master branch as April 2023:

net-tools iproute2 Comment
arp ip neighbour need to replace depending on OS
ifconfig ip addr, ip link, ip -stats need to replace depending on OS
iptunnel ip tunnel found no use
iwconfig iw limited to iwtools.py
nameif ip link found no use
netstat ss need to replace depending on OS
route ip route need to replace depending on OS

:) English spelling for neighbour man 8 ip-neighbour but command only looks at ip neigh

@dmurphy18
Copy link
Contributor

Capturing my dev comments for future reference:

I did a quick grep of the code and then checked the matching files. Here's what I found, though I welcome people doing their own analysis in case I missed something:

salt/modules/bridge.py - Seems to be used for BSD only. Not sure if this is a problem.
salt/modules/iwtools.py - Seems we can leave this alone since this module is specific to using iwtools
salt/modules/lxc.py - iproute2 already used here as well and is preferred
salt/modules/network.py - Both ss and netstat are supported, but netstat is preferred if both are present. Should ss be preferred moving forward?
salt/modules/nspawn.py - ifconfig is referenced in CLI examples in docstrings. Not sure if necessary to update.
salt/modules/ps.py - Both ss and netstat are supported here and are exposed as their own Salt functions, which are already conditionally loaded depending on whether either are installed. So, no change needs to be made here.
salt/modules/vagrant.py - ifconfig used in get_ssh_config(). No ip equivalent exists and we probably need to consider adding one (and preferring it).
salt/modules/win_network.py - netstat referenced, but this is Windows' version of it.
salt/modules/win_status.py - Same as win_network.py
salt/utils/network.py - Only a couple of the BSDs rely on ifconfig. Not sure if this is a problem.

Quick check of replacement commands in master branch as April 2023:

chk net-tools iproute2 Comment

X arp ip neighbour need to replace depending on OS
X ifconfig ip addr, ip link, ip -stats need to replace depending on OS (man page has -stats, -s, but fails on Centos 7 and Ubuntu 22.04)
X iptunnel ip tunnel found no use
X iwconfig iw limited to iwtools.py
X nameif ip link found no use
X netstat ss need to replace depending on OS
route ip route need to replace depending on OS

ifconfig
X modules/bridge.py FreeBSF and OpenBSD still use ifconfig, use is okay
modules/vagrant.py get_ssh_config - need to update use of ifconfig and parsing of reply, Q. ip addr or ip link to find bridge address, inet and inet6
X modules/lxc.py info - code uses 'ip link show' first and falls back to 'ifconfig' if that fails
X run, run_stdout - change the example from using 'ifconfig -a' to 'ip addr show'
X modules/nspawn.py run, run_stdout - change the example from using 'ifconfig -a' to 'ip addr show'
X utils/networks.py _interfaces_ifconfig, linux_interfaces
- used as a backup to ip if it is not found.
X _netbsd_interfaces_ifconfig, _junos_interfaces_ifconfig, junos_interfaces, netbsd_interfaces
- uses for Juniper (BSD under the covers) and BSD

netstat
X modules/win_status.py master _win_remotes_on - makes use of netstat, might be command used by Windows, 'ss' is not available on Windows
X modules/win_network.py netstat - provides support for 'salt * network.netstat' command, 'ss' is not available on Windows
X modules/status.py netstats, linux_netstats, freebsd_netstats, bsd_netstats, sunos_netstats, aix_netstats
- provides support for 'salt * status.netstats' command - implies should there be a status.ss command ? NO
X netdev, freebsd_netdev, sun_netdev, aix_netdev - usage is OS specific and needs to remain for bsd, sun and aix
X all_status - uses netstats key to return netstats(), no problems 22.04 with no netstat, since still have /proc/net/netstat
X modules/saltcheck.py mentions network.netstat in comment code-block
X modules/ps.py netstat - netstat information for given process name,
- Note: there is a 'ss' command too, on 22.04 with no netstat issues error, but ps.ss works.
The error handling is done by decorators but wonder if should check and call ss if netstat not available, but that would imply
using dunder utils ad dunder grains, don't want to use dunder utils since trying to minimize that.
X modules/network.py netstat, _netstat_linux, _netstat_bsd, _netstat_sunos, _netstat_aix, _netstat_route_linux, _netstat_route_freebsd, _netstat_route_netbsd, _netstat_route_openbsd, _netstat_route_sunos, _netstat_route_aix, active_tcp
- Note: there is a 'ss' command too (_ss_linux, _ip_route_linux), No problems 22.04 with no netstat, falls back to _ss_linux
- Note: netstate will use _ss_linux if netstat not available
X utils/network.py _sunos_remotes_on, _openbsd_remotes_on, _windows_remotes_on, _aix_remotes_on - use of netstat limited to OS's that require its use

arp
X modules/suse_ip.py _parse_settings_arp, _parse_settings_eth
- Note: _parse_setting_arp used in bond interface functions
_parse_settings_eth makes use of net.bridge.bridge-nf-call-arptables and aprcheck in opts
Specific to SUSE platform, hence not our issue
X modules/nftables.py _NFTABLES_FAMILIES
- _NFTABLES_FAMILIES has arp, but also has ip but short hand for ipv4 etc.
nothing to do here, since 'arp' is part of nftables, so need leave it in
X modules/panos.py get_arp - panos is for Palo Alto compatibility, so probably platform specific
X modules/napalm_network.py arp - arp is using proxy_napalm_wrap implying requires NAPALM, hence not our issue
X modules/rh_ip.py _parse_settings_arp, _parse_settings_eth - similar solution to suse_ip.py required
- Note: _parse_setting_arp used in bond interface functions
_parse_settings_eth makes use of net.bridge.bridge-nf-call-arptables and aprcheck in opts
Specific to RedHat platform, hence not our issue
X modules/network.py arp - has decorator doing path.which check for arp available, but nothing using 'ip neighbor'.a
- do we do a full ip [addr|neighbor|link|tunnel|etc] implementation to complainment arp etc
A. wrote ip_neighs and ip_neighs6 similar to ip_addrs and ip_addrs6 to show tables
Given that interfaces in utils/network.py already utilises ip addr, ip link, etc. don't think we need to
Cannot fail back to using ip neighbor, since decorator used to check for arp.
Also have ip_addrs, ip_addrs6, iphexval, ip_networks, ip_networks6
X modules/iptables.py _parser
- _parser is checking arguments for --arp, --noarp as part of parsing the input iptables
Given the move to nftables, consider iptables deprecated and as such arp usage not an issue
X proxy/napalm.py used as 'net.arp' as examples used in comment block for head of file.
- Note: this is a napalm module, hence not our issue
On RHEL 9 error: 'net.arp' is not available.
X runners/net.py findarp, find - checking ARP tables, e.g arp_device, arp_int, arp_mac and arp_ip
- Note: this is a napalm module, hence not our issue
On RHEL 9 error: 'net' virtual returned False: The napalm module could not be imported

route
X grains/core.py default_gateway
leverages 'ip -[4|6] route show'
X modules/suse_ip.py _parse_routes, build_routes, get_routes, /etc/sysconfig/network/routes
also need to consider jinja files in templates
Note: this looks to be platform specific in commands, don't see any issue
X modules/neutronng.py Platform specific for OpenStack, and use is input parameters, don't see any issue
X modules/linux_ip.py _parse_routes, get_routes, /proc/net/route
also need to consider jinja files in templates
Note: Why does this module not have 'build_routes' ?
modules/debian_ip.py _parse_routes, _write_file_routes, build_routes, get_routes, /etc/network/routes
also need to consider jinja files in templates
Also to consider is the use of if-up/if-down, believe there is an issue for this
where we should be using NetworkManager rather than the deprecated methods.
linux_ip.py makes use of 'ip link set {} <up|down>'
X modules/rh_ip.py _parse_routes, build_routes, get_routes, /etc/sysconfig/network-scripts
also need to consider jinja files in templates
Note: this looks to be platform specific in commands, don't see any issue
X modules/network.py _ip_route_linux, get_route, build_routes
makes use of 'ip -4 show table main', 'ip -6 show table all', ip route get'
X modules/nilrt_ip.py Platform for NI real-time Linux, not an issue for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. severity-high 2nd top severity, seen by most users, causes major problems tech-debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.