Skip to content

Commit

Permalink
topotato: use integrated config for everything
Browse files Browse the repository at this point in the history
With `mgmtd` getting its tentacles into everything, per-daemon configs
don't really work anymore.  Just use integrated config everywhere.

This just concatenates the per-daemon configs, so some logging
statements are duplicate.  Doesn't matter for the time being.

Signed-off-by: David Lamparter <[email protected]>
  • Loading branch information
eqvinox committed Jul 8, 2024
1 parent 2833767 commit 2eb0d95
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 31 deletions.
76 changes: 45 additions & 31 deletions topotato/frr/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def deprecated(fn): # type: ignore
from ..network import TopotatoNetwork
from ..topobase import CallableNS
from .templating import TemplateUtils, jenv
from .exceptions import FRRStartupVtyshConfigFail

if typing.TYPE_CHECKING:
from .. import toponom
Expand Down Expand Up @@ -90,13 +91,6 @@ class FRRSetup:
daemons_all.extend("eigrpd pimd pim6d ldpd nhrpd sharpd pathd pbrd".split())
daemons_all.extend("bfdd vrrpd".split())

daemons_integrated_only: ClassVar[FrozenSet[str]] = frozenset(
"pim6d staticd mgmtd".split()
)
"""
Daemons that don't have their config loaded with ``-f`` on startup
"""

daemons_mgmtd: ClassVar[FrozenSet[str]] = frozenset(
"staticd ripd ripngd zebra".split()
)
Expand Down Expand Up @@ -535,6 +529,8 @@ class FRRRouterNS(TopotatoNetwork.RouterNS, CallableNS):
varlibdir: Optional[str]
rtrcfg: Dict[str, str]
livelogs: Dict[str, LiveLog]
frrconfpath: str
merged_cfg: str

def __init__(
self, instance: TopotatoNetwork, name: str, configs: _FRRConfigProtocol
Expand Down Expand Up @@ -565,6 +561,8 @@ def _logwatch(self, evt: TimedElement):
return

logmsg = cast(LogMessage, evt)
# FIXME: this will no longer trigger with integrated config
# as the error is reported by vtysh (in _load_config) instead
if logmsg.uid == "SHWNK-NWT5S":
raise FRRInvalidConfigFail(logmsg.router.name, logmsg.daemon, logmsg.text)

Expand Down Expand Up @@ -603,25 +601,50 @@ def start_run(self):
super().start_run()

self.rtrcfg = self._configs.get(self.name, {})
self.frrconfpath = self.tempfile("frr.conf")

# TODO: convert to integrated config in tests rather than crudely merge here
self.merged_cfg = "\n".join(
self.rtrcfg.get(daemon, "") for daemon in self._configs.daemons
)

with open(self.frrconfpath, "w", encoding="utf-8") as fd:
fd.write(self.merged_cfg)

for daemon in self._configs.daemons:
if daemon not in self.rtrcfg:
continue
self.logfiles[daemon] = self.tempfile("%s.log" % daemon)
self.start_daemon(daemon)
self.start_daemon(daemon, defer_config=True)

def start_daemon(self, daemon: str):
frrpath = self.frr.frrpath
binmap = self.frr.binmap
# one-pass load all daemons
self._load_config()

use_integrated = True # daemon in self.frr.daemons_integrated_only
def _load_config(self, daemon=None):
daemon_arg = ["-d", daemon] if daemon else []

if use_integrated:
cfgpath = self.tempfile("integrated-" + daemon + ".conf")
else:
cfgpath = self.tempfile(daemon + ".conf")
with open(cfgpath, "w", encoding="utf-8") as fd:
fd.write(self.rtrcfg[daemon])
vtysh = self._vtysh(
daemon_arg + ["-f", self.frrconfpath], stderr=subprocess.PIPE
)
out, err = vtysh.communicate()
out, err = out.decode("UTF-8"), err.decode("UTF-8")

_logger.debug("stdout from config load: %r", out)
_logger.debug("stderr from config load: %r", err)

if 0 < vtysh.returncode < 128:
raise FRRStartupVtyshConfigFail(
self.name, vtysh.returncode, out, err, self.merged_cfg
)
if vtysh.returncode != 0:
# terminated by signal
raise subprocess.CalledProcessError(
vtysh.returncode, vtysh.args, vtysh.stdout, vtysh.stderr
)

def start_daemon(self, daemon: str, defer_config=False):
frrpath = self.frr.frrpath
binmap = self.frr.binmap

assert self.rundir is not None

Expand All @@ -636,13 +659,6 @@ def start_daemon(self, daemon: str):
"-d",
]
)
if not use_integrated:
cmdline.extend(
[
"-f",
cfgpath,
]
)
cmdline.extend(
[
"--log",
Expand Down Expand Up @@ -673,11 +689,8 @@ def start_daemon(self, daemon: str):
)
self.pids[daemon] = pid

if use_integrated:
# FIXME: do something with the output
self._vtysh(["-d", daemon, "-f", cfgpath]).communicate()
if daemon in self.frr.daemons_mgmtd and "mgmtd" in self.frr.daemons:
self._vtysh(["-d", "mgmtd", "-f", cfgpath]).communicate()
if not defer_config:
self._load_config(daemon)

def start_post(self, timeline, failed: List[Tuple[str, str]]):
for daemon in self._configs.daemons:
Expand Down Expand Up @@ -746,14 +759,15 @@ def end(self):

super().end()

def _vtysh(self, args: List[str]) -> subprocess.Popen:
def _vtysh(self, args: List[str], **kwargs) -> subprocess.Popen:
assert self.rundir is not None

frrpath = self.frr.frrpath
execpath = os.path.join(frrpath, "vtysh/vtysh")
return self.popen(
[execpath] + ["--vty_socket", self.rundir] + args,
stdout=subprocess.PIPE,
**kwargs,
)

def vtysh_exec(self, timeline: Timeline, cmds, timeout=5.0):
Expand Down
87 changes: 87 additions & 0 deletions topotato/frr/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0-or-later
# Copyright (C) 2018-2021 David Lamparter for NetDEF, Inc.
"""
Exceptions raised by topotato FRR integration.
"""
# pylint: disable=duplicate-code

import re
from typing import Optional
import attr

from _pytest._code import ExceptionInfo
from _pytest._code.code import TerminalRepr, ReprFileLocation
from _pytest._io import TerminalWriter

from ..exceptions import TopotatoFail


class FRRStartupVtyshConfigFail(TopotatoFail):
"""
The initial call to vtysh to load the integrated config failed.
This will commonly happen if there is a mistake in the config.
"""

router: str
returncode: int
stdout: str
stderr: str

# pylint: disable=too-many-arguments
def __init__(
self, router: str, returncode: int, stdout: str, stderr: str, config: str
):
self.router = router
self.returncode = returncode
self.stdout = stdout
self.stderr = stderr
self.config = config
super().__init__()

def __str__(self):
return f"{self.router}/startup-config-load"

@attr.s(eq=False, auto_attribs=True)
class TopotatoRepr(TerminalRepr):
excinfo: ExceptionInfo

@property
def reprcrash(self) -> Optional["ReprFileLocation"]:
# FIXME: figure out proper API?
# pylint: disable=protected-access
return self.excinfo._getreprcrash()

def _errlinenos(self):
exc = self.excinfo.value
errlinenos = set()
for line in exc.stderr.splitlines():
if m := re.match(r"line (\d+):", line):
errlinenos.add(int(m.group(1)))
return errlinenos

def toterminal(self, tw: TerminalWriter) -> None:
exc = self.excinfo.value
tw.line("")
tw.sep(
" ",
f"startup integrated-config load failed on {exc.router} (status {exc.returncode})",
red=True,
bold=True,
)
tw.line("")

tw.line("stdout:")
for line in exc.stdout.splitlines():
tw.line(f"\t{line}")

tw.line("stderr:")
for line in exc.stderr.splitlines():
tw.line(f"\t{line}")

errlinenos = self._errlinenos()
tw.line("configuration:")
for i, line in enumerate(exc.config.rstrip("\n").splitlines()):
lineno = i + 1
tw.line(f"{lineno:4d}\t{line}", red=lineno in errlinenos)

0 comments on commit 2eb0d95

Please sign in to comment.