Skip to content

topotests: support ExaBGP 5.x and update the in-tree installs to 5.0.9#22336

Open
higebu wants to merge 4 commits into
FRRouting:masterfrom
higebu:topotests-exabgp-5x
Open

topotests: support ExaBGP 5.x and update the in-tree installs to 5.0.9#22336
higebu wants to merge 4 commits into
FRRouting:masterfrom
higebu:topotests-exabgp-5x

Conversation

@higebu

@higebu higebu commented Jun 12, 2026

Copy link
Copy Markdown

The topotests only support ExaBGP 4.2, but its branch no longer
receives new features; newer address families such as BGP-MUP
(#22311) need ExaBGP 5.x.

This PR makes TopoExaBGP.start detect the installed ExaBGP version
and use the matching launch form (the 4.x behavior is unchanged),
and updates the in-tree installs (tests/topotests/Dockerfile,
docker/ubuntu-ci/Dockerfile, doc/developer/topotests.rst) from
4.2.21 to the 5.0.9 release. The NetDEF CI agents provision their
own ExaBGP separately and can switch independently: see #22320.

None of the 5.x breaking changes affect the in-tree tests: all
exabgp.env files use encoder=text, no test drives BGP-LS through
exabgp, and the prefix-sid tests announce the attribute as raw bytes.

Verified bgp_prefix_sid, bgp_vrf_md5_peering and bgp_peer_shut via
tests/topotests/docker/frr-topotests.sh with both 4.2.21 and 5.0.9:
5 passed each. As a side effect this fixes a hang when running the
topotests that way with any exabgp >= 4.2: frr-topotests.sh runs
pytest as the container's PID 1, and exabgp's daemonise() refuses
to detach from a PID 1 parent. No standard setup hits this today --
the published topotests image still ships ExaBGP 3.4.17, which the
version probe rejects fail-fast, and the docker/ubuntu-ci flow runs
with --init -- but a locally built topotests image (4.2.21)
reproduces it. See the first commit message for details.

@frrbot frrbot Bot added docker documentation tests Topotests, make check, etc labels Jun 12, 2026
@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds ExaBGP 5.x support to the topotests framework by auto-detecting the installed version and branching on 4.x vs 5.x behaviour, then updates all in-tree installs (both Dockerfiles and the developer doc) from a pinned 4.x commit to the 5.0.9 tag.

  • topogen.py: get_exabgp_cmd now probes both --version (4.x) and the version subcommand (5.x), caching the result in _EXABGP_VERSION_CACHE; TopoExaBGP.start consults that cache to skip the mkfifo dance on 5.x, use the server subcommand with EXABGP_ENVFILE, and redirect stdio to /dev/null to fix a daemonize-blocking hang.
  • Dockerfiles / topotests.rst: ExaBGP install reference changed from a full SHA pin (@065905…) to the @5.0.9 version tag across all three locations.

Confidence Score: 4/5

Safe to merge; all behaviour changes are confined to the topotest harness and have been manually verified against both 4.2.21 and 5.0.9.

The version-detection and caching logic is correct, the 5.x CLI branching matches the upstream migration guide, and the /dev/null redirect fix for the daemonize-blocking bug is sound. The only non-trivial note is the switch from an immutable commit SHA to a version tag for the pip install, which is a minor reproducibility trade-off but does not affect runtime correctness.

All three Dockerfile / doc locations now pin to the @5.0.9 tag rather than a commit hash; tests/topotests/lib/topogen.py carries the core version-branching logic.

Important Files Changed

Filename Overview
tests/topotests/lib/topogen.py Adds version detection/caching for ExaBGP 4.x vs 5.x, conditionally skips mkfifo creation for 5.x, switches to the server subcommand and EXABGP_ENVFILE for 5.x, and redirects stdio to /dev/null to fix a blocking-on-daemonize issue. Logic is sound; the fallback "4.2.11" in the cache lookup is unreachable in practice.
docker/ubuntu-ci/Dockerfile Updates ExaBGP install from pinned commit hash to version tag 5.0.9; functionally correct but slightly less reproducible than the prior commit-hash pin.
tests/topotests/Dockerfile Same ExaBGP version-tag update as ubuntu-ci Dockerfile; same minor reproducibility note applies.
doc/developer/topotests.rst Documentation updated to install ExaBGP via @5.0.9 tag; matches the Dockerfile changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[TopoExaBGP.start] --> B[get_exabgp_cmd]
    B --> C{exabgp binary found?}
    C -- yes --> D[probe: exabgp --version]
    D --> E{regex match?}
    E -- yes --> F[cache version, return exacmd]
    E -- no --> G[probe: exabgp version]
    G --> H{regex match?}
    H -- yes --> F
    H -- no --> I[try python3 -m exabgp ...]
    I --> J{version ok?}
    J -- yes --> F
    J -- no --> K[return None - assert fails]
    C -- no --> I
    F --> L[version >= 4.2.11 confirmed]
    L --> M[lookup _EXABGP_VERSION_CACHE]
    M --> N{is 5.x?}
    N -- no --> O[mkfifo /var/run/exabgp.in/.out - exabgp -e env cfg]
    N -- yes --> P[skip mkfifo - EXABGP_ENVFILE=... - exabgp server cfg]
    O --> Q[run + redirect stdio to /dev/null]
    P --> Q
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
docker/ubuntu-ci/Dockerfile:110
**Git tag instead of pinned commit hash**

The previous reference used a full commit SHA (`@0659057837cd...`), which is immutable and guarantees the exact bytes installed. Switching to a tag (`@5.0.9`) means that if the tag is ever force-pushed or recreated on the upstream repo, a fresh `docker build` could silently pull in a different commit. The same change is made in `tests/topotests/Dockerfile` and `doc/developer/topotests.rst`. Consider using the resolved commit SHA for `5.0.9` to keep the same level of pinning.

Reviews (1): Last reviewed commit: "doc: update for exabgp 5" | Re-trigger Greptile

Comment thread docker/ubuntu-ci/Dockerfile Outdated
python3 -m pip install pyyaml && \
python3 -m pip install xmltodict && \
python3 -m pip install git+https://github.com/Exa-Networks/exabgp@0659057837cd6c6351579e9f0fa47e9fb7de7311
python3 -m pip install git+https://github.com/Exa-Networks/exabgp@5.0.9

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Git tag instead of pinned commit hash

The previous reference used a full commit SHA (@0659057837cd...), which is immutable and guarantees the exact bytes installed. Switching to a tag (@5.0.9) means that if the tag is ever force-pushed or recreated on the upstream repo, a fresh docker build could silently pull in a different commit. The same change is made in tests/topotests/Dockerfile and doc/developer/topotests.rst. Consider using the resolved commit SHA for 5.0.9 to keep the same level of pinning.

Prompt To Fix With AI
This is a comment left during a code review.
Path: docker/ubuntu-ci/Dockerfile
Line: 110

Comment:
**Git tag instead of pinned commit hash**

The previous reference used a full commit SHA (`@0659057837cd...`), which is immutable and guarantees the exact bytes installed. Switching to a tag (`@5.0.9`) means that if the tag is ever force-pushed or recreated on the upstream repo, a fresh `docker build` could silently pull in a different commit. The same change is made in `tests/topotests/Dockerfile` and `doc/developer/topotests.rst`. Consider using the resolved commit SHA for `5.0.9` to keep the same level of pinning.

How can I resolve this? If you propose a fix, please make it concise.

@donaldsharp

Copy link
Copy Markdown
Member

I'm not terribly interested in having the topotests work with different versions of exabgp. Please upgrade everything to 5.whatever if that is what you want. This just increases our support load for exabgp, which I'm not terribly interested in doing.

@mjstapp

mjstapp commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

I think I'm with Donald - if the "current" version is 5, let's see about moving to 5.

@higebu higebu force-pushed the topotests-exabgp-5x branch 2 times, most recently from 9b6d914 to 1494dbb Compare June 13, 2026 04:58
higebu added 4 commits June 16, 2026 20:08
Probe and launch exabgp using the 5.x subcommands (`version`,
`server` with EXABGP_ENVFILE) and raise the minimum to 5.0.0.  Drop
the /var/run/exabgp.{in,out} fifos, which 5.x refuses to start
against, and redirect the launch stdio to /dev/null so the daemonized
exabgp does not block self.run() on an open pipe.

Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
Install the ExaBGP 5.0.9 release.  Newer address families used by the
topotests (for example BGP-MUP) require ExaBGP 5.x.

Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
Install the ExaBGP 5.0.9 release.

Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
Point the topotests install instructions at the ExaBGP 5.0.9 release.

Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
@higebu higebu force-pushed the topotests-exabgp-5x branch from 1494dbb to 8361645 Compare June 16, 2026 11:10
@higebu

higebu commented Jun 16, 2026

Copy link
Copy Markdown
Author

@donaldsharp @mjstapp Thank you for your comments. Updated to target ExaBGP 5 only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants