huawei bgp: fix patch ordering and nested config re-apply on IP peer asn-number change#566
Open
vaneuk wants to merge 1 commit into
Open
huawei bgp: fix patch ordering and nested config re-apply on IP peer asn-number change#566vaneuk wants to merge 1 commit into
vaneuk wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Huawei does not allow changing the AS number of an existing IP BGP peer in place:
There is also no
undo peer X as-number Ycommand for an IP peer — youcan't surgically remove only the as-number. The only way to change an IP
peer's as-number is to fully recreate the peer:
undo peer Xdestroys every per-peer setting under every family block,so the nested configuration must be re-applied after recreation.
After
undo peer X, any per-option undo on that peer fails:The goal is for annet to produce clean patches: no commands that the device will reject, even if the deployment runner would mask the errors.
This drives both the ordering choices and the diff_logic.
Today, given the running config:
and a target that changes only the as-number to
1234, annet produces a patch the device rejects:After this PR:
A full peer deletion is also cleaner. Before:
After:
Peers under a VPN address-family
A
peer <ip>can also be declared inside avpn-instanceblock (ipv4-family vpn-instance EXAMPLE). The same recreate-and-re-apply logic applies, scoped to that block —undo peer <ip>is emitted inside the vpn-instance and only wipes that block's rows.As-number change of a VPN peer (the unchanged route-policy is re-applied after the wipe):
Full removal of a VPN peer collapses to a single in-block
undo peer <ip>(a sibling peer in the same block is untouched):The vpn-instance boundary is a separate peer namespace: the same IP can be a
peer in two different vpn-instances, or both a global peer and a VPN peer,
and a wipe in one scope must never leak into another. These isolation cases
need their full multi-VRF before/after to make sense, so see the test
fixtures rather than an excerpt here:
huawei_peer_as_number_change_vpn_isolation.yamland
huawei_peer_full_removal_vpn_isolation.yaml.Scope
This PR fixes IP peer as-number change and full removal, whether the
peer is declared at the bgp top level or inside an address-family /
vpn-instance block (e.g.
ipv4-family vpn-instance EXAMPLE).Peer-group removal is out of scope.
Changes
annet/rulebook/texts/huawei.order— inside thebgpblock, addtwo end-anchored rules above the family sub-block:
undo peer ~/[^\s]+$/ %order_reverse— matches exactly 3-tokenundo peer X(IP peer destroy). The~/regex/escape hatch is usedto inject a literal
$end-anchor; otherwiseundo peer *would bea greedy prefix pattern shadowing every more-specific
undo peer …rule deeper in the file.
undo peer * as-number %order_reverse— matchesundo peer X as-number(4-token, peer-group as-number undo). Terminates at theliteral
as-number, so it doesn't bleed ontoundo peer X connect-interfaceetc.Other per-option
undo peer …rules (description,* * *,* *)stay at their original after-family positions.
annet/rulebook/texts/huawei.rul— attach%diff_logic=annet.rulebook.huawei.bgp.reapply_after_as_number_changeto the
bgprule.annet/rulebook/huawei/bgp.py— newreapply_after_as_number_changethat runs at diff-build time over the
bgpsubtree, recursing block byblock (
_rewrite_block). Thepeer <ip> as-number …row can live at thebgp top level or inside a family / vpn-instance block, so each block
computes:
wiped— IPs whosepeer <ip> as-number …row is in this block'soldbut not
new(full removal or as-number value change), unioned withwiped IPs inherited from enclosing blocks (a top-level
undo peer <ip>wipes the global family blocks too).
huawei.bgp.peeremitsundo peer <ip>for these.recreated— subset that still has apeer <ip> as-number …row innew(renumbered, not deleted).And rewrites the diff:
peer <ip> …REMOVED row whose IP is inwiped(redundant after
undo peer <ip>and would error against a nonexistentpeer) — except the REMOVED
peer <ip> as-number Yrow, which is kept asthe trigger that drives
huawei.bgp.peerto emitundo peer <ip>.peer <ip> …AFFECTED rows to ADDED when the IP is inrecreated, so the re-emit happens afterundo peer <ip>.vpn-instanceboundary: a vpn-instancepeer is a separate namespace and its
undo peer <ip>only wipes withinthat block, so an identically-numbered peer in another vpn-instance is
left untouched.
No
huawei.orderchange is needed for the vpn-instance case — orderinginside a family block already emits
undo peerbeforepeer as-numberbefore the re-applied options.
Tests
huawei_undo_peer_group_as_number.yaml—undo peer CD_LU as-numbermoves from after-family to before-family (caught by the new 4-token
end-anchored rule).
huawei_undo_peer2.yaml(Initial release based on r13205266 #1, Add workflow .github/publish-to-pypi.yml #2, Fix annet deploy #4) — previously asserted redundantper-option undos that would error against a wiped peer; now assert
the cleaner single
undo peer Xoutput.tests/annet/test_patch/covering IPv4 / IPv6 as-number changes, peer-group as-number rename
(regression for the 4-token order rule), an unrelated sibling peer
(untouched), a route-policy rename (no spurious undo), and full peer
removal for both v4 and v6.
IPv4 / IPv6 as-number change (unchanged route-policy re-applied), full
removal (collapses to a single
undo peer <ip>, sibling peer untouched),and three isolation cases proving a wipe in one scope never leaks into
another:
as-number changes (the VPN peer is untouched);
VRF's peer is removed (the other VRF's peer, with its route-policy,
survives).
192.0.2.0/24and2001:db8::/32.