Parse quagga output without knowledge about hostname, so robust again…

…st hostname changes or mismatch (sonic-net#124)

* Parse quagga output without knowledge about hostname, so robust against
hostname changes or mismatch
* Fix sock init after close
* Fix bug
* Fix mock socket
* Reconnect less frequently, and make sock timeout 1s
* Handle parsing protocol error with timeout recv
* Fix regex bug
* Fix typo and wording
* Make regex more strict

.gitignore

@@ -17,7 +17,6 @@ dist/
 downloads/
 eggs/
 .eggs/
-lib/
 lib64/
 parts/
 sdist/
@@ -135,6 +134,5 @@ crashlytics.properties
 crashlytics-build.properties
 fabric.properties
 
-src/sonic_ax_impl/mibs/vendor/
 gh-release.patch
 tests/test_cpuUtilizationHandler.py

src/sonic_ax_impl/lib/perseverantsocket.py

@@ -6,8 +6,15 @@ def __init__(self, family=socket.AF_INET, type=socket.SOCK_STREAM, proto=0, file
  self.address_tuple = address_tuple
  self.family = family
  self.type = type
- self.sock = socket.socket(family=self.family, type=self.type, proto=proto, fileno=fileno, *args, **kwargs)
- self.sock.settimeout(10)
+ self.proto = proto
+ self.fileno = fileno
+ self.args = args
+ self.kwargs = kwargs
+ self._initsock()
+
+ def _initsock(self):
+ self.sock = socket.socket(family=self.family, type=self.type, proto=self.proto, fileno=self.fileno, *self.args, **self.kwargs)
+ self.sock.settimeout(1)
 
  @property
  def connected(self):
@@ -19,13 +26,15 @@ def connect(self, address_tuple):
 
  def reconnect(self):
  assert self.address_tuple is not None
+ if self._connected:
+ self.close()
  self.sock.connect(self.address_tuple)
  self._connected = True
 
  def close(self):
  self._connected = False
  self.sock.close()
- self.sock = socket.socket(self.family, self.type)
+ self._initsock()
 
  ## TODO: override __getattr__ to implement auto function call forwarding if not implemented
  def send(self, *args, **kwargs):

src/sonic_ax_impl/lib/quaggaclient.py

@@ -1,5 +1,6 @@
 import re
 import ipaddress
+import socket
 
 STATE_CODE = {
  "Idle": 1,
@@ -26,7 +27,7 @@ def parse_bgp_summary(summ):
  return bgpinfo
  if l.startswith('% No BGP neighbors found'): # in FRRouting (version 7.2)
  return bgpinfo
- if l.endswith('> '): # directly hostname prompt, in FRRouting (version 4.0)
+ if (l.endswith('> ') or l.endswith('# ')) and li == n - 1: # empty output followed by prompt, in FRRouting (version 4.0)
  return bgpinfo
  li += 1
 
@@ -88,10 +89,9 @@ def bgp_peer_tuple(dic):
 class QuaggaClient:
  HOST = '127.0.0.1'
  PORT = 2605
- PROMPT_PASSWORD = b'Password: '
+ PROMPT_PASSWORD = b'\x1fPassword: '
 
- def __init__(self, hostname, sock):
- self.prompt_hostname = ('\r\n' + hostname + '> ').encode()
+ def __init__(self, sock):
  self.sock = sock
  self.bgp_provider = 'Quagga'
 
@@ -110,10 +110,7 @@ def union_bgp_sessions(self):
  return neighbor_sessions
 
  def auth(self):
- cmd = b"zebra\n"
- self.sock.send(cmd)
-
- ## Nowadays we see 2 BGP stack
+ ## Nowadays we see 2 BGP stacks
  ## 1. Quagga (version 0.99.24.1)
  ## 2. FRRouting (version 7.2-sonic)
  banner = self.vtysh_recv()
@@ -123,6 +120,10 @@ def auth(self):
  self.bgp_provider = 'FRRouting'
  else:
  raise ValueError('Unexpected data recv for banner: {0}'.format(banner))
+
+ ## Send default user credential and receive the prompt
+ passwd = "zebra"
+ self.vtysh_run(passwd)
  return banner
 
  def vtysh_run(self, command):
@@ -133,11 +134,22 @@ def vtysh_run(self, command):
  def vtysh_recv(self):
  acc = b""
  while True:
- data = self.sock.recv(1024)
+ try:
+ data = self.sock.recv(1024)
+ except socket.timeout as e:
+ raise ValueError('Timeout recv acc=: {0}'.format(acc)) from e
  if not data:
- raise ValueError('Unexpected data recv: {0}'.format(acc))
+ raise ValueError('Unexpected data recv acc=: {0}'.format(acc))
  acc += data
- if acc.endswith(self.prompt_hostname):
+ ## 1. To match hostname
+ ## RFC 1123 Section 2.1
+ ## First char of hostname must be a letter or a digit
+ ## Hostname length <= 255
+ ## Hostname contains no whitespace characters
+ ## 2. To match the prompt line
+ ## The buffer may containers only prompt without return char
+ ## Or the buffer container some output followed by return char and prompt
+ if re.search(b'(^|\r\n)[a-zA-Z0-9][\\S]{0,254}[#>] $', acc):
  break
  if acc.endswith(QuaggaClient.PROMPT_PASSWORD):
  break

src/sonic_ax_impl/mibs/vendor/cisco/bgp4.py

@@ -11,28 +11,29 @@ def __init__(self):
  super().__init__()
  self.sock = PerseverantSocket(socket.AF_INET, socket.SOCK_STREAM
  , address_tuple=(QuaggaClient.HOST, QuaggaClient.PORT))
- self.QuaggaClient = QuaggaClient(socket.gethostname(), self.sock)
+ self.QuaggaClient = QuaggaClient(self.sock)
 
  self.session_status_map = {}
  self.session_status_list = []
 
  def reinit_data(self):
- pass
+ if not self.sock.connected:
+ try:
+ self.sock.reconnect()
+ mibs.logger.info('Connected quagga socket')
+ except (ConnectionRefusedError, socket.timeout) as e:
+ mibs.logger.debug('Failed to connect quagga socket. Retry later...: {}.'.format(e))
+ return
+ self.QuaggaClient.auth()
+ mibs.logger.info('Authed quagga socket')
 
  def update_data(self):
  self.session_status_map = {}
  self.session_status_list = []
 
  try:
  if not self.sock.connected:
- try:
- self.sock.reconnect()
- mibs.logger.info('Connected quagga socket')
- except (ConnectionRefusedError, socket.timeout) as e:
- mibs.logger.debug('Failed to connect quagga socket. Retry later...: {}.'.format(e))
- return
- self.QuaggaClient.auth()
- mibs.logger.info('Authed quagga socket')
+ return
 
  sessions = self.QuaggaClient.union_bgp_sessions()
 

tests/mock_tables/socket.py

@@ -4,6 +4,7 @@
 import unittest
 from unittest import TestCase, mock
 from unittest.mock import patch, mock_open, MagicMock
+from enum import Enum
 
 INPUT_DIR = os.path.dirname(os.path.abspath(__file__))
 modules_path = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
@@ -19,38 +20,50 @@
 def MockGetHostname():
  return 'str-msn2700-05'
 
+class State(Enum):
+ CLOSED = 0
+ BANNER = 1
+ INTERACTIVE = 2
+
 class MockSocket(_socket_class):
  def __init__(self, *args, **kwargs):
  super(MockSocket, self).__init__(*args, **kwargs)
- self._string_sent = b''
- self.prompt_hostname = MockGetHostname().encode()
+ self.prompt_hostname = (MockGetHostname() + '> ').encode()
+ self.state = State.CLOSED
 
  def connect(self, *args, **kwargs):
- pass
+ self.state = State.BANNER
+ self._string_sent = b''
 
  def send(self, *args, **kwargs):
  string = args[0]
  self._string_sent = string
  pass
 
  def recv(self, *args, **kwargs):
+ if self.state == State.CLOSED:
+ raise OSError("Transport endpoint is not connected")
+
+ if self.state == State.BANNER:
+ self.state = State.INTERACTIVE
+ return b'\r\nHello, this is Quagga (version 0.99.24.1).\r\nCopyright 1996-2005 Kunihiro Ishiguro, et al.\r\n\r\n\r\nUser Access Verification\r\n\r\n\xff\xfb\x01\xff\xfb\x03\xff\xfe"\xff\xfd\x1fPassword: '
+
+ if not self._string_sent or b'\n' not in self._string_sent:
+ raise socket.timeout
+
  try:
- if self._string_sent == b'':
- return b'\r\nHello, this is Quagga (version 0.99.24.1).\r\nCopyright 1996-2005 Kunihiro Ishiguro, et al.\r\n\r\n\r\nUser Access Verification\r\n\r\n\xff\xfb\x01\xff\xfb\x03\xff\xfe"\xff\xfd\x1fPassword: '
  if self._string_sent == b'zebra\n':
  return self.prompt_hostname
- if b'show ip bgp summary\n' in self._string_sent:
+ elif b'show ip bgp summary\n' in self._string_sent:
  filename = INPUT_DIR + '/bgpsummary_ipv4.txt'
  elif b'show ipv6 bgp summary\n' in self._string_sent:
  filename = INPUT_DIR + '/bgpsummary_ipv6.txt'
- elif b'\n' in self._string_sent:
- return self.prompt_hostname
  else:
- return None
+ return self.prompt_hostname
 
  with open(filename, 'rb') as f:
  ret = f.read()
- return ret + self.prompt_hostname
+ return ret + b'\r\n' + self.prompt_hostname
  finally:
  self._string_sent = b''
 

tests/test_vtysh.py

@@ -28,9 +28,6 @@ def setUpClass(cls):
  for updater in cls.lut.updater_instances:
  updater.update_data()
 
- def test_hostname(self):
- self.assertEqual(socket.gethostname(), 'str-msn2700-05')
-
  def test_getpdu_established(self):
  oid = ObjectIdentifier(20, 0, 0, 0, (1, 3, 6, 1, 4, 1, 9, 9, 187, 1, 2, 5, 1, 3, 1, 4, 10, 0, 0, 61))
  get_pdu = GetPDU(