saltstack / saltstack/salt

[BUG] 3003.1 tcp_keepalive probes not sent to idle minion if any connection is not idle

Open
#60,428 7 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description
TCP keepalive probes are sent to a minion if a zmq connection is idle for tcp_keepalive_idle time. This works as designed if there's no activity at all on the master and salt/presence/change events are seen on the event bus after tcp_keepalive_cnt * tcp_keepalive_intvl time a minion node does not return an ack packet.

However, if there's any activity on the zmq socket between master and any minion, even if that's not the minion that became unreachable, keepalive probes stop beings sent, and a node down is not being detected by the presence system. Also, while that node remains unreachable but the activity on the zmq socket stopped, no new keepalive probes are sent to the unreachable node, resulting in a missed presence/change event and the node never being added to manage.not_alived.

Setup
Linux tight-tiger-node1 5.4.0-52-generic #57-Ubuntu SMP Thu Oct 15 10:57:00 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

  • Relevant master config:
tcp_keepalive: True
tcp_keepalive_idle: 10
tcp_keepalive_cnt: 3
tcp_keepalive_intvl: 10

Steps to Reproduce the behavior

NOTE: 10.43.128.111 is master here, 10.43.128.114 one of the minions

  • Run a tcpdump capturing keepalive packets on master, eg:
$ tcpdump -ni any "host 10.43.128.114 and (host 10.43.128.111 and tcp port 4505) and ( tcp[tcpflags] == tcp-ack and len == 52 )"
13:15:40.902991 IP 10.43.128.111.4505 > 10.43.128.114.60228: Flags [.], ack 1, win 128, options [nop,nop,TS val 346993721 ecr 2634232174], length 0
13:15:40.903282 IP 10.43.128.114.60228 > 10.43.128.111.4505: Flags [.], ack 7184, win 348, options [nop,nop,TS val 2634242418 ecr 346809421], length 0
  • On the master, issue any module.function in a loop targeting any of the other minions, eg:
$ while true; do salt -G 'ipv4:10.43.128.111' cmd.run hostname; sleep 5; done
  • tcpdump now shows that keepalive probes aren't sent anymore from master to minion, we only see the ack packets from minion to master:
14:13:23.784085 IP 10.43.128.114.60228 > 10.43.128.111.4505: Flags [.], ack 8371, win 348, options [nop,nop,TS val 2637705299 ecr 350456602], length 0
14:13:26.054971 IP 10.43.128.114.60228 > 10.43.128.111.4505: Flags [.], ack 8836, win 348, options [nop,nop,TS val 2637707569 ecr 350458872], length 0
14:13:28.387538 IP 10.43.128.114.60228 > 10.43.128.111.4505: Flags [.], ack 9301, win 348, options [nop,nop,TS val 2637709902 ecr 350461205], length 0
14:13:30.619855 IP 10.43.128.114.60228 > 10.43.128.111.4505: Flags [.], ack 9766, win 348, options [nop,nop,TS val 2637712134 ecr 350463437], length 0
14:13:32.970185 IP 10.43.128.114.60228 > 10.43.128.111.4505: Flags [.], ack 10231, win 348, options [nop,nop,TS val 2637714485 ecr 350465788], length 0
  • Take down network on minion (10.43.128.114 in this case)
  • No salt/presence/change events are seen on the event bus
  • Abort the cmd.run loop on master. No salt/presence/change will be seen. Outage of a node has now gone unnoticed.

Expected behavior
Keepalive probes are sent on connections idle time exceeding tcp_keepalive_idle, regardless of other minion zmq activity.

Screenshots
NA

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3002.2
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.8.1
       libgit2: Not Installed
      M2Crypto: 0.33.0
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: Not Installed
        pygit2: Not Installed
        Python: 3.6.8 (default, Nov 16 2020, 16:55:22)
  python-gnupg: Not Installed
        PyYAML: 3.11
         PyZMQ: 17.0.0
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.1.4
 
System Versions:
          dist: centos 7 Core
        locale: UTF-8
       machine: x86_64
       release: 5.4.0-52-generic
        system: Linux
       version: CentOS Linux 7 Core

Additional context
On a side-note, it seems the presence system is pretty disconnected from the rest and completely dependent on tcp_keepalive. In case a presence event isn't picked up (yet), i would expect if salt "actively" finds a node is unresponsive, it would issue a salt/presence/change event as well. Example:

  1. Node goes down
  2. Before tcp_keepalive timeout is exceeded one manually runs a salt-run manage.down (or anything else that will detect a minion is not responding)
  3. Master knows the node is not responding -> issue a salt/presence/change event

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the master tcp_keepalive configuration, the ZeroMQ socket activity described in the reproduction, and the presence system's salt/presence/change event path. Reproduce with tcpdump while issuing commands to another minion, then take down the target network; done means the idle target still receives keepalive probes and presence/change reports it as unavailable.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
distributed-systems, networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.