From c8f942da271c0c1860d4e5305f4c8349a212682f Mon Sep 17 00:00:00 2001 From: ryskn Date: Fri, 3 Jul 2026 09:52:23 +0900 Subject: [PATCH] fix(srv6): don't wedge EgressPolicy in Terminating on un-encodable withdraw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reconcileDelete rebuilt the withdraw advert from status.SRPolicy and returned any error, blocking finalizer removal. Under the default --bgp-encoding=sr-policy, BuildPath fails permanently when BSID or EndpointAddr is empty, and config.Validate treats bsid as optional, so a bsid-less color never goes Ready yet carries a non-nil status.SRPolicy. Deleting it made Withdraw fail every reconcile, wedging the object in Terminating (freeable only by manually patching finalizers). Nothing was ever announced, so the block was pure downside. - reconcileDelete now calls BuildPath() first to separate deterministic encode failures (log and drop the finalizer — nothing to withdraw) from transient transport errors (keep the finalizer and retry) - main.go requires a bsid per color when --bgp-encoding=sr-policy, failing fast at load rather than reaching the un-deletable state --- srv6egress/cmd/bgp-controller/main.go | 10 ++++++++++ .../internal/controller/egresspolicy_controller.go | 10 +++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/srv6egress/cmd/bgp-controller/main.go b/srv6egress/cmd/bgp-controller/main.go index b543d8a5..77b5526b 100644 --- a/srv6egress/cmd/bgp-controller/main.go +++ b/srv6egress/cmd/bgp-controller/main.go @@ -98,6 +98,16 @@ func main() { var encoder bgp.Encoder switch bgpEncoding { case "sr-policy": + // The headend keys its SR Policy install on the BSID, so under this + // encoding every color needs one. Fail fast at load instead of letting + // a bsid-less color go non-Ready yet un-deletable (BuildPath would fail + // on both Announce and Withdraw). + for color, cc := range cfg.Colors { + if cc.BSID == "" { + log.Error(fmt.Errorf("color %d: bsid is required with --bgp-encoding=sr-policy", color), "invalid config") + os.Exit(1) + } + } encoder = bgp.NewSRPolicyEncoder(bgp.SRPolicyOptions{}) case "color-route": encoder = bgp.NewColoredEncoder() diff --git a/srv6egress/internal/controller/egresspolicy_controller.go b/srv6egress/internal/controller/egresspolicy_controller.go index b3ea91ee..306206a7 100644 --- a/srv6egress/internal/controller/egresspolicy_controller.go +++ b/srv6egress/internal/controller/egresspolicy_controller.go @@ -226,7 +226,15 @@ func (r *EgressPolicyReconciler) reconcileDelete(ctx context.Context, ep *srv6eg EndpointAddr: sp.EndpointAddr, BSID: sp.BSID, } - if err := r.BGP.Withdraw(ctx, owner, r.Encoder.ClusterAdvert(key, sp.SegmentList)); err != nil { + adv := r.Encoder.ClusterAdvert(key, sp.SegmentList) + if _, err := adv.BuildPath(); err != nil { + // Deterministic encode failure (e.g. sr-policy with an empty + // BSID/endpoint): nothing valid could have been announced, so there + // is nothing to withdraw. Log and drop the finalizer rather than + // wedging the object in Terminating forever on every reconcile. + log.Error(err, "cannot rebuild withdraw advert; dropping finalizer without withdraw", "name", ep.Name) + } else if err := r.BGP.Withdraw(ctx, owner, adv); err != nil { + // Transient transport error: keep the finalizer and retry. return ctrl.Result{}, err } }