saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.14k stars 5.47k forks source link

[BUG] status.master broken on Windows #66716

Open amalaguti opened 3 months ago

amalaguti commented 3 months ago

Description The funcion status.master does not show connections to master ports. The minion is configured in multimaster and is currently connected to both masters.

While netstat shows the following information

PS C:\Users\adrian> netstat -na | Select-String "172.21.0." | Select-String ":4505" | Select-String ESTABLISHED

  TCP    172.21.0.8:61266       172.21.0.10:4505       ESTABLISHED
  TCP    172.21.0.8:61288       172.21.0.11:4505       ESTABLISHED

The module status.master returns False

PS C:\Users\adrian> salt-call status.master master=saltmaster-pip -l warning
local:
    False

Reviewing the code in modules/win_status.py the status.master() function calls _get_connected_ips(port) which usues psutils.net_connections() and this function fails to check a established connection on port (default 4505) I think it fails due it's checking on laddr.port (local) and I think it should check on raddr.port (and raddr.ip)

def _get_connected_ips(port):
    """
    List all connections on the system that have an established connection on
    the passed port. This uses psutil.net_connections instead of netstat to be
    locale agnostic.
    """
    connected_ips = set()
    # Let's use psutil to be non-locale specific
    conns = psutil.net_connections()

    for conn in conns:
        if conn.status == psutil.CONN_ESTABLISHED:
            # Checking on my masters ip addresses port 4505 
            if conn.raddr.ip in ['172.21.0.10', '172.21.0.11'] and conn.raddr.port == 4505:
                log.warning(f">>>> ADRIAN psutil.net_connections() {conn.status} {conn.laddr.ip}:{conn.laddr.port} --> {conn.raddr.ip}:{conn.raddr.port}")
            if conn.laddr.port == port:
                connected_ips.add(conn.laddr.ip)

    return connected_ips

The warning logging messages seems to be fine

PS C:\Users\adrian> salt-call status.master master=saltmaster-pip -l warning
[WARNING ] >>>> ADRIAN psutil.net_connections() ESTABLISHED 172.21.0.8:61410 --> 172.21.0.10:4505
[WARNING ] >>>> ADRIAN psutil.net_connections() ESTABLISHED 172.21.0.8:61432 --> 172.21.0.11:4505
local:
    False

So I guess changing the code to check on raddr.ip and raddr.port should make this module work fine

Setup 3006.8 minion in multimaster setup (active/active)

Steps to Reproduce the behavior Configure a Windows minion in multimaster setup (active/active) and run salt-call state.master master=<master>

Expected behavior Should return True if the master is reachable

NOTE: Actually there are many issues about minion disconnecting from masters, I think this should be reviewed asap. May have impact in related minion communication issues

amalaguti commented 3 months ago

@twangboy fyi