diff --git a/AGENTS.md b/AGENTS.md index 5dae62bb..471aaa6b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,7 +70,7 @@ Summary: - **`config/system/`** — Creates the `galactic-system` namespace both components deploy into. Apply with `kubectl apply -k config/system/`. - **`config/cni/`** — Production manifests for the CNI installer DaemonSet, ConfigMap, RBAC, and ServiceAccount. Apply with `kubectl apply -k config/cni/`. -- **`config/router/`** — Shared RBAC/ServiceAccount plus DaemonSet roles, all running `GALACTIC_ROUTER_ROUTER_MODE=tenant`: +- **`config/router/`** — Shared RBAC/ServiceAccount plus DaemonSet roles: - **`config/router/tenant/`** — the per-node role (`galactic-router`); runs on every node except Kubernetes control-plane nodes and nodes labeled for the route-reflector or gateway roles. - **`config/router/tenant-control/`** — the BGP route-reflector role (`galactic-router-control`, `GALACTIC_ROUTER_REFLECTOR=true`); opt-in only, requires nodes labeled `galactic.datumapis.com/node: control` (stays at zero replicas otherwise). `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS` is auto-detected from the host's `lo` interface by default; see the comments in `daemonset-patch.yaml` for when to override it. - **`config/router/base/`** — the DaemonSet spec shared by both roles above; not applied directly. diff --git a/README.md b/README.md index b9913830..4c93628c 100644 --- a/README.md +++ b/README.md @@ -60,9 +60,9 @@ kubectl apply -k config/fabric/ - **Talos: gRPC health port.** `galactic-router` runs `hostNetwork: true` and defaults to gRPC health checks on port `5000`, which collides with Talos's built-in dashboard (`/sbin/dashboard` permanently binds `127.0.0.1:5000` on every Talos node). `config/router/base/daemonset.yaml` already ships with `GALACTIC_ROUTER_GRPC_HEALTH_PORT=5179` (and matching probe/containerPort) to avoid this; if you run `galactic-router` outside these manifests on Talos, set `GALACTIC_ROUTER_GRPC_HEALTH_PORT` to something other than `5000` yourself. -- **`galactic-router` tenant mode: BGP local address.** The node needs a global-unicast IPv6 address assigned to `lo` (typically by `config/fabric/`'s underlay eBGP daemon, which must start and converge before `galactic-router`), or you must set `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS` explicitly — this is required even when `GALACTIC_ROUTER_BGP_LISTEN_PORT=-1` (no inbound listener), since `galactic-router` still needs a source address for outbound BGP connections. Without one of these, startup fails with `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS not set and no address could be detected on lo: no global-unicast IPv6 address found on lo`. See [`docs/router/configuration.md`](./docs/router/configuration.md) for details. +- **`galactic-router`: BGP local address.** The node needs a global-unicast IPv6 address assigned to `lo` (typically by `config/fabric/`'s underlay eBGP daemon, which must start and converge before `galactic-router`), or you must set `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS` explicitly — this is required even when `GALACTIC_ROUTER_BGP_LISTEN_PORT=-1` (no inbound listener), since `galactic-router` still needs a source address for outbound BGP connections. Without one of these, startup fails with `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS not set and no address could be detected on lo: no global-unicast IPv6 address found on lo`. See [`docs/router/configuration.md`](./docs/router/configuration.md) for details. -See [`docs/router/configuration.md`](./docs/router/configuration.md) for the full `galactic-router` CLI flag / environment variable reference — note that env var names generally follow `GALACTIC_ROUTER_` but aren't always the naive uppercased guess (e.g. `--mode` is `GALACTIC_ROUTER_ROUTER_MODE`, not `GALACTIC_ROUTER_MODE`); the reference table has the exact name for every flag. +See [`docs/router/configuration.md`](./docs/router/configuration.md) for the full `galactic-router` CLI flag / environment variable reference — env var names follow `GALACTIC_ROUTER_` (hyphens become underscores, uppercased); the reference table has the exact name for every flag. ## Development diff --git a/cmd/galactic-router/main.go b/cmd/galactic-router/main.go index 8dd0e8b3..59570689 100644 --- a/cmd/galactic-router/main.go +++ b/cmd/galactic-router/main.go @@ -3,8 +3,7 @@ // SPDX-License-Identifier: AGPL-3.0-or-later // Command galactic-router is the BGP control-plane reconciler for the Galactic -// data plane. It watches BGP CRDs and drives a BGP runtime backend -// (GoBGP for tenant role, FRR stub for fabric role). +// data plane. It watches BGP CRDs and drives an embedded GoBGP server. package main import ( diff --git a/cmd/galactic-router/root.go b/cmd/galactic-router/root.go index bb1fe16d..97a62e35 100644 --- a/cmd/galactic-router/root.go +++ b/cmd/galactic-router/root.go @@ -32,7 +32,6 @@ import ( "go.datum.net/galactic/internal/plumbing/loaddr" "go.datum.net/galactic/internal/reconcile" galacticruntime "go.datum.net/galactic/internal/runtime" - "go.datum.net/galactic/internal/runtime/frr" "go.datum.net/galactic/internal/runtime/gobgp" networkwebhook "go.datum.net/galactic/internal/webhook" bgpv1alpha1 "go.datum.net/network/api/v1alpha1" @@ -66,7 +65,6 @@ func resolveBGPLocalAddress(explicit string, detect func() (string, error)) (str // the provided config and initializes the BGP runtime. func runCmd(cfg *config.RouterConfig) error { nodeName := cfg.NodeName - mode := cfg.Mode bgpListenPort := cfg.BGPListenPort metricsPort := cfg.MetricsPort grpcHealthPort := cfg.GRPCHealthPort @@ -76,15 +74,7 @@ func runCmd(cfg *config.RouterConfig) error { return err } - var factory galacticruntime.RuntimeFactory - switch mode { - case config.ModeTenant: - factory = gobgp.NewRuntimeFactory(int32(bgpListenPort), bgpLocalAddr) - case config.ModeFabric: - factory = frr.NewRuntimeFactory() - case config.ModeTransit: - return errors.New("mode=transit is not yet supported") - } + factory := gobgp.NewRuntimeFactory(int32(bgpListenPort), bgpLocalAddr) ctrl.SetLogger(zap.New(zap.UseDevMode(true))) @@ -174,7 +164,7 @@ func runCmd(cfg *config.RouterConfig) error { runtimeMgr := galacticruntime.NewRuntimeManager(factory) // Create reconciler. - rec := reconcile.New(mgr.GetClient(), nodeName, mode, bgpLocalAddr) + rec := reconcile.New(mgr.GetClient(), nodeName, bgpLocalAddr) // Register BGPRouter controller. if err := (&controller.BGPRouterReconciler{ @@ -184,7 +174,6 @@ func runCmd(cfg *config.RouterConfig) error { RuntimeManager: runtimeMgr, Hasher: hash.DesiredRouter, NodeName: nodeName, - RouterMode: mode, }).SetupWithManager(mgr); err != nil { return fmt.Errorf("setup BGPRouter controller: %w", err) } @@ -333,10 +322,7 @@ func newRootCommand() *cobra.Command { } cmd.Flags().StringP("node-name", "n", "", "Kubernetes node name (required)") - cmd.Flags().StringP("mode", "m", "", - "Operating mode: '"+config.ModeTransit+"', '"+config.ModeFabric+"', or '"+config.ModeTenant+"' (required)") - cmd.Flags().Bool("reflector", false, - "Enable route reflector mode (requires --mode="+config.ModeFabric+" or --mode="+config.ModeTenant+")") + cmd.Flags().Bool("reflector", false, "Enable route reflector mode") cmd.Flags().IntP("bgp-listen-port", "p", config.DefaultRouterBGPListenPort, "BGP listen port") cmd.Flags().StringP("bgp-local-address", "", diff --git a/cmd/galactic-router/root_test.go b/cmd/galactic-router/root_test.go index d3cd684d..d1a89185 100644 --- a/cmd/galactic-router/root_test.go +++ b/cmd/galactic-router/root_test.go @@ -21,7 +21,6 @@ func testCmd(t *testing.T) *cobra.Command { t.Helper() cmd := &cobra.Command{Use: testCmdUse} cmd.Flags().StringP("node-name", "n", "", "Kubernetes node name (required)") - cmd.Flags().StringP("mode", "m", "", "Operating mode") cmd.Flags().Bool("reflector", false, "Enable route reflector mode") cmd.Flags().IntP("bgp-listen-port", "p", config.DefaultRouterBGPListenPort, "BGP listen port") cmd.Flags().StringP("bgp-local-address", "", "", "BGP local address") @@ -56,13 +55,12 @@ func TestRequiredFlags(t *testing.T) { cmd := testCmd(t) cfg.BindFlags(cmd.Flags()) if err := cfg.Validate(); err == nil { - t.Error("Validate() with empty node-name and mode returned nil error") + t.Error("Validate() with empty node-name returned nil error") } } func TestEnvVarDefaults(t *testing.T) { t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeTenant) cfg := config.NewRouterConfig() cmd := testCmd(t) @@ -72,21 +70,8 @@ func TestEnvVarDefaults(t *testing.T) { } } -func TestInvalidMode(t *testing.T) { - t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, "invalid") - - cfg := config.NewRouterConfig() - cmd := testCmd(t) - cfg.BindFlags(cmd.Flags()) - if err := cfg.Validate(); err == nil { - t.Error("Validate() with invalid mode returned nil error") - } -} - func TestBGPListenPortMinusOne(t *testing.T) { t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeTenant) t.Setenv(config.EnvRouterBGPListenPort, "-1") cfg := config.NewRouterConfig() @@ -99,7 +84,6 @@ func TestBGPListenPortMinusOne(t *testing.T) { func TestBGPListenPortOverflow(t *testing.T) { t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeTenant) t.Setenv(config.EnvRouterBGPListenPort, "70000") cfg := config.NewRouterConfig() @@ -119,56 +103,8 @@ func TestNodeNameRequired(t *testing.T) { } } -func TestModeRequired(t *testing.T) { - t.Setenv(config.EnvRouterNodeName, "test-node") - - cfg := config.NewRouterConfig() - cmd := testCmd(t) - cfg.BindFlags(cmd.Flags()) - if err := cfg.Validate(); err == nil { - t.Error("Validate() with empty mode returned nil error") - } -} - -func TestValidModes(t *testing.T) { - for _, mode := range []string{config.ModeTransit, config.ModeFabric, config.ModeTenant} { - t.Run(mode, func(t *testing.T) { - t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, mode) - - cfg := config.NewRouterConfig() - cmd := testCmd(t) - cfg.BindFlags(cmd.Flags()) - - if cfg.Mode != mode { - t.Errorf("Mode = %q, want %q", cfg.Mode, mode) - } - if err := cfg.Validate(); err != nil { - t.Errorf("Validate() with mode %q: %v", mode, err) - } - }) - } -} - -func TestReflectorInvalidMode(t *testing.T) { - t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeTransit) - - cfg := config.NewRouterConfig() - cmd := testCmd(t) - if err := cmd.Flags().Set("reflector", "true"); err != nil { - t.Fatalf("set --reflector flag: %v", err) - } - cfg.BindFlags(cmd.Flags()) - - if err := cfg.Validate(); err == nil { - t.Error("Validate() with --reflector and --mode=transit returned nil error") - } -} - func TestMetricsPortOverride(t *testing.T) { t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeTenant) t.Setenv(config.EnvRouterMetricsPort, "9090") cfg := config.NewRouterConfig() @@ -181,7 +117,6 @@ func TestMetricsPortOverride(t *testing.T) { func TestGRPCHealthPortOverride(t *testing.T) { t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeTenant) t.Setenv(config.EnvRouterGRPCHealthPort, "9091") cfg := config.NewRouterConfig() @@ -192,25 +127,8 @@ func TestGRPCHealthPortOverride(t *testing.T) { } } -func TestModeFlagOverridesEnv(t *testing.T) { - t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeFabric) - - cfg := config.NewRouterConfig() - cmd := testCmd(t) - if err := cmd.Flags().Set("mode", config.ModeTenant); err != nil { - t.Fatalf("set --mode flag: %v", err) - } - cfg.BindFlags(cmd.Flags()) - - if cfg.Mode != config.ModeTenant { - t.Errorf("Mode = %q, want %q (flag should override env var)", cfg.Mode, config.ModeTenant) - } -} - func TestGRPCHealthPortFlagOverridesEnv(t *testing.T) { t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeTenant) t.Setenv(config.EnvRouterGRPCHealthPort, "9091") cfg := config.NewRouterConfig() @@ -227,7 +145,6 @@ func TestGRPCHealthPortFlagOverridesEnv(t *testing.T) { func TestWebhookFlagsOverrideEnv(t *testing.T) { t.Setenv(config.EnvRouterNodeName, "test-node") - t.Setenv(config.EnvRouterMode, config.ModeTenant) t.Setenv(config.EnvRouterWebhookEnabled, "false") t.Setenv(config.EnvRouterWebhookPort, "9443") diff --git a/config/gateway/base/daemonset.yaml b/config/gateway/base/daemonset.yaml index b17b1f21..407ec007 100644 --- a/config/gateway/base/daemonset.yaml +++ b/config/gateway/base/daemonset.yaml @@ -74,8 +74,6 @@ spec: valueFrom: fieldRef: fieldPath: spec.nodeName - - name: GALACTIC_ROUTER_ROUTER_MODE - value: tenant - name: GALACTIC_ROUTER_GC_NAMESPACE value: galactic-system - name: GALACTIC_ROUTER_BGP_LISTEN_PORT diff --git a/config/router/base/daemonset.yaml b/config/router/base/daemonset.yaml index 9e51ecb9..a1c64063 100644 --- a/config/router/base/daemonset.yaml +++ b/config/router/base/daemonset.yaml @@ -47,8 +47,6 @@ spec: valueFrom: fieldRef: fieldPath: spec.nodeName - - name: GALACTIC_ROUTER_ROUTER_MODE - value: tenant - name: GALACTIC_ROUTER_GC_NAMESPACE value: galactic-system # 5179 is also the binary's own default. Set explicitly here diff --git a/docs/agent-startup.md b/docs/agent-startup.md index 34022543..ef60d981 100644 --- a/docs/agent-startup.md +++ b/docs/agent-startup.md @@ -6,8 +6,8 @@ sequenceDiagram participant GoBGP participant Kubernetes - Router->>Router: validate config (--node-name, --mode: transit/fabric/tenant, ...) - Router->>Router: select RuntimeFactory: tenant→GoBGP, fabric→FRR stub, transit→error (unsupported) + Router->>Router: validate config (--node-name, ...) + Router->>Router: create RuntimeFactory (GoBGP — the only backend) Router->>Kubernetes: build controller-runtime manager (metrics on :9179, no HTTP health) Router->>Router: start gRPC health server (:5179, SERVING immediately) Router->>Kubernetes: RBAC pre-flight — SelfSubjectAccessReview per watched resource (logs if watch denied) @@ -16,10 +16,10 @@ sequenceDiagram Router->>Kubernetes: start GC ticker goroutine (waits for cache sync, then runs on --gc-interval, default 5m) Router->>Kubernetes: start controller-runtime manager (watch BGPRouter/BGPPeer/BGPAdvertisement/BGPVRFInstance/BGPPolicy/Secret/Node) Note over Router: on each BGPRouter reconcile (not just the first) - Router->>GoBGP: lazy-start embedded server (only for --mode=tenant; listenPort defaults to 179, or -1 for outbound-only deployments) + Router->>GoBGP: lazy-start embedded server (listenPort defaults to 179, or -1 for outbound-only deployments) Router->>GoBGP: StartBgp (ASN, RouterID from BGPRouter spec) — skipped if already started with the same values; Reconfigure (fresh BgpServer) if ASN/RouterID changed Router->>GoBGP: apply peers, VRFs (route targets + kernel VRF wiring), start RIB monitor, apply EVPN advertisements, apply policies Note over Router: on shutdown (SIGTERM/SIGINT): gRPC health server GracefulStop, manager exits when its signal context is cancelled. GoBGP has no explicit shutdown hook — it stops only because the process exits. ``` -`--mode=fabric` uses an FRR runtime stub instead of GoBGP; `--mode=transit` is accepted by validation but returns an error at startup (not yet implemented). See [docs/router/configuration.md](router/configuration.md) for the full set of flags/environment variables. +See [docs/router/configuration.md](router/configuration.md) for the full set of flags/environment variables. diff --git a/docs/agents/ARCHITECTURE-ROUTER.md b/docs/agents/ARCHITECTURE-ROUTER.md index aacd15b9..73b21bcb 100644 --- a/docs/agents/ARCHITECTURE-ROUTER.md +++ b/docs/agents/ARCHITECTURE-ROUTER.md @@ -75,8 +75,7 @@ galactic/ │ ├── reconcile/ # CRD → DesiredRouter translation (node/role checks, │ │ # secret resolution, IPv6 next-hop from Node) │ ├── runtime/ # RouterRuntime interface + RuntimeManager -│ │ ├── gobgp/ # GoBGP RouterRuntime (tenant mode) -│ │ └── frr/ # FRR RouterRuntime stub (fabric mode) +│ │ └── gobgp/ # GoBGP RouterRuntime (the only backend) │ ├── model/ # DesiredRouter and family; re-exports BGP API enums │ ├── hash/ # SHA-256 change detection over DesiredRouter │ ├── metadata/ # Build-time version info (Version, GitCommit, etc.) @@ -115,8 +114,7 @@ See [docs/agent-startup.md](../agent-startup.md) for the router startup sequence | ------------------------ | ------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `internal/controller` | `galactic-router` | controller-runtime reconcilers (BGPRouter, BGPPeer, BGPAdvertisement, BGPVRFInstance, BGPPolicy, Secret, Node, GC — not NetworkGateway/NetworkRule, see the note above); field index registration; CRD status helpers | | `internal/reconcile` | `galactic-router` | CRD → DesiredRouter translation | -| `internal/runtime/gobgp` | `galactic-router` | Embedded GoBGP server (`--mode=tenant`) | -| `internal/runtime/frr` | `galactic-router` | FRR stub (`--mode=fabric`) — returns "not implemented" for every method | +| `internal/runtime/gobgp` | `galactic-router` | Embedded GoBGP server (the only backend) | | `internal/model` | `galactic-router` | Internal BGP model types | | `internal/hash` | `galactic-router` | Change detection | | `internal/metadata` | every binary in the repo | Build-time version info stamped via `-ldflags` | @@ -134,14 +132,13 @@ See [docs/agent-startup.md](../agent-startup.md) for the router startup sequence `main.go` is a 3-line wrapper around `newRootCommand().Execute()`; all startup logic lives in `root.go`'s `runCmd`: -1. Validate config (`--node-name` and `--mode` required; `--mode` must be `transit`, - `fabric`, or `tenant`). Env vars: `GALACTIC_ROUTER_NODE_NAME`, - `GALACTIC_ROUTER_ROUTER_MODE`, plus optional `GALACTIC_ROUTER_BGP_LISTEN_PORT`, +1. Validate config (`--node-name` required). Env vars: `GALACTIC_ROUTER_NODE_NAME`, + plus optional `GALACTIC_ROUTER_BGP_LISTEN_PORT`, `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS`, `GALACTIC_ROUTER_METRICS_PORT`, `GALACTIC_ROUTER_GRPC_HEALTH_PORT`, `GALACTIC_ROUTER_GC_NAMESPACE`, `GALACTIC_ROUTER_GC_INTERVAL`, `GALACTIC_ROUTER_REFLECTOR`. -2. Select `RuntimeFactory`: `tenant` → GoBGP, `fabric` → FRR stub, `transit` → returns - an error ("not yet supported"). +2. Create the `RuntimeFactory`: an unconditional `gobgp.NewRuntimeFactory(...)` call — + GoBGP is the only backend, so there is no mode-based selection to make. 3. Build controller-runtime manager (metrics on configurable port, default `:9179`; no HTTP health endpoint). 4. Start gRPC health server on a configurable port (default `:5179`). @@ -173,8 +170,7 @@ sibling container instead. | Variable | Required | Default | Description | | ----------------------------------- | -------- | ----------------- | ----------------------------------------------------------------------- | | `GALACTIC_ROUTER_NODE_NAME` | Yes | — | Kubernetes node name; filters which BGPRouter CRDs this instance owns | -| `GALACTIC_ROUTER_ROUTER_MODE` | Yes | — | `transit` (unsupported stub), `fabric` (FRR stub), or `tenant` (GoBGP) | -| `GALACTIC_ROUTER_REFLECTOR` | No | `false` | Enable route reflector mode; only valid for `fabric`/`tenant` | +| `GALACTIC_ROUTER_REFLECTOR` | No | `false` | Enable route reflector mode | | `GALACTIC_ROUTER_BGP_LISTEN_PORT` | No | `179` | BGP TCP listen port; `-1` disables inbound connections (outbound-only) | | `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS` | No | — | Source address for outgoing BGP TCP connections (numbered underlay use) | | `GALACTIC_ROUTER_METRICS_PORT` | No | `9179` | controller-runtime Prometheus metrics port | @@ -203,7 +199,6 @@ co-located `galactic-gateway` container's own health port on the same | `internal/reconcile` | galactic-router | Translates BGPRouter + related CRDs into `model.DesiredRouter`; enforces node/role filtering, timer validation, AFI validation | No | | `internal/runtime` | galactic-router | `RouterRuntime` interface; `RuntimeManager` (keyed map of live runtimes, double-checked lock create) | Yes (runtime map) | | `internal/runtime/gobgp` | galactic-router | Embeds GoBGP v4; lazy-starts on first Apply; handles peer/VRF/EVPN-path/policy add/update/delete; tracks established timestamps | Yes (per-router) | -| `internal/runtime/frr` | galactic-router | FRR stub — returns "not implemented" for every method | No | | `internal/model` | galactic-router | `DesiredRouter`, `DesiredPeer`, `DesiredAdvertisement`, `DesiredPolicy`, `DesiredVRFInstance`, `RuntimeStatus`; re-exports BGP API enums | No | | `internal/hash` | galactic-router | SHA-256 fingerprint of `DesiredRouter` for no-op suppression | No | | `internal/metadata` | every binary | Build-time vars (`Version`, `GitCommit`, `GitTreeState`, `BuildDate`) stamped via `-ldflags` | No | @@ -218,7 +213,7 @@ co-located `galactic-gateway` container's own health port on the same | Dependency | Version | Purpose | | -------------------------------- | --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `github.com/osrg/gobgp/v4` | v4.7.0 | Embedded BGP server (tenant mode) | +| `github.com/osrg/gobgp/v4` | v4.7.0 | Embedded BGP server | | `go.datum.net/network` | bumped frequently | BGP CRD API types (BGPRouter, BGPPeer, BGPAdvertisement, BGPPolicy, BGPVRFInstance) | | `sigs.k8s.io/controller-runtime` | v0.24.1 | Full manager + reconciler framework: manager, field indexes, eight registered controllers (see Entry Points) | | `github.com/spf13/cobra` | v1.10.2 | CLI command/flag handling | @@ -231,12 +226,12 @@ co-located `galactic-gateway` container's own health port on the same ## Key Design Decisions -- **GoBGP embedded, lazy-started.** GoBGP runs in-process (`--mode=tenant` only) and starts only when the first `BGPRouter` is reconciled for that router; `Apply` re-runs on every subsequent reconcile too (subject to hash-based no-op suppression), re-applying peers/VRFs/EVPN/policies each time. `listenPort` defaults to `179`; `-1` (outbound-only) is an operator choice for specific deployments, not the codebase default. ASN or RouterID changes trigger a full `Reconfigure` (fresh `BgpServer` — `StopBgp` is not called because it permanently terminates the v4 Serve loop). +- **GoBGP embedded, lazy-started.** GoBGP runs in-process and starts only when the first `BGPRouter` is reconciled for that router; `Apply` re-runs on every subsequent reconcile too (subject to hash-based no-op suppression), re-applying peers/VRFs/EVPN/policies each time. `listenPort` defaults to `179`; `-1` (outbound-only) is an operator choice for specific deployments, not the codebase default. ASN or RouterID changes trigger a full `Reconfigure` (fresh `BgpServer` — `StopBgp` is not called because it permanently terminates the v4 Serve loop). - **Overlay BGP port.** galactic-router peers connect outbound on port `1790` by default (configurable per-peer via `BGPPeer.spec.remotePort`). Port `179` is occupied by the underlay FRR `bgpd` on every node, so the overlay uses a non-conflicting port. The `BGPPeer` CRD defaults `remotePort` to `179` (the IANA BGP port); galactic-router overrides this to `1790` when the field is unset, so existing CRDs without an explicit value continue to work. Set `remotePort: 179` explicitly when peering with external BGP speakers that listen on the standard port. - **VRF/route-target model via BGPVRFInstance.** `galactic-bgp` (the CNI chain, see [ARCHITECTURE-CNI.md](ARCHITECTURE-CNI.md)) creates a `BGPVRFInstance` (RouteDistinguisher + import/export Route Targets, all set to the derived RT) before the `BGPAdvertisement`; `galactic-router`'s GoBGP runtime applies VRFs (`applyVRFs`) before originating EVPN paths (`applyEVPN`). `galactic-gateway`'s own `BGPAdvertisement`s (VIP/self-address routes, see [ARCHITECTURE-GATEWAY.md](ARCHITECTURE-GATEWAY.md)) leave VRFID/Function unset entirely — they carry no SRv6 decap behavior of their own. - **CRD-driven config, no sidecar gRPC.** `galactic-router` watches BGP CRDs directly via controller-runtime. `galactic-bgp` writes `BGPVRFInstance`/`BGPAdvertisement` CRDs; the router reconciler picks them up. No in-node gRPC calls between any of the CNI-chain binaries and `galactic-router`. - **Hash-based no-op suppression.** SHA-256 over the sorted `DesiredRouter` prevents redundant GoBGP Apply calls on every CRD event. -- **RuntimeFactory pattern.** `--mode=tenant` (`GALACTIC_ROUTER_ROUTER_MODE=tenant`) selects GoBGP; `--mode=fabric` selects the FRR stub; `--mode=transit` is accepted by validation but returns an error at startup (not yet implemented). The mode is selected at startup; no controller changes are needed to add a new mode. +- **RuntimeFactory pattern.** `galactic-router` always builds a `gobgp.NewRuntimeFactory(...)` at startup — GoBGP is the only backend. A `RuntimeFactory` is a plain function value (`func(types.NamespacedName) (RouterRuntime, error)`), so a future backend still only needs a new factory function and a call-site change here, not a controller change. - **DEL is intentionally minimal everywhere in the CNI chain; GC reclaims shared state asynchronously.** See [ARCHITECTURE-CNI.md](ARCHITECTURE-CNI.md#key-design-decisions) for the CNI-side half of this decision. `galactic-router`'s GC controller (ticker-driven, default every 5m) reclaims orphaned CRDs, stale kernel VRFs, and stale eBPF entries once no live container still references them. - **gRPC health, configurable port.** Liveness and readiness probes use the gRPC health protocol (`google.golang.org/grpc/health`) on a configurable port (default `5179`). No HTTP health endpoint. `galactic-gateway` follows the identical convention on its own port — see [ARCHITECTURE-GATEWAY.md](ARCHITECTURE-GATEWAY.md). - **`galactic-router` carries no gateway-role code.** The edge XDP NAT+LB gateway used to be a `galactic-router` mode; it was split into its own `galactic-gateway` binary specifically so a crash on either side no longer takes the other down with it — see [ARCHITECTURE-GATEWAY.md](ARCHITECTURE-GATEWAY.md) for the full rationale and design. @@ -247,7 +242,7 @@ co-located `galactic-gateway` container's own health port on the same | Layer | Command | Framework | Scope | | ------- | ---------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| Unit | `task test:unit` | `go test -race` | `internal/plumbing/srv6`, `internal/gc`, `internal/reconcile`, `internal/controller` (BGP-family reconcilers), `internal/plumbing/intf`, `internal/metadata`, `internal/runtime/gobgp` (partial), `internal/runtime/frr` | +| Unit | `task test:unit` | `go test -race` | `internal/plumbing/srv6`, `internal/gc`, `internal/reconcile`, `internal/controller` (BGP-family reconcilers), `internal/plumbing/intf`, `internal/metadata`, `internal/runtime/gobgp` (partial) | | E2E | `task test:e2e` | Kind + `go test` | Full BGPRouter lifecycle coverage for `galactic-router` comes from this Kind cluster's separate reconciler tests, run alongside the CNI e2e suite described in [ARCHITECTURE-CNI.md](ARCHITECTURE-CNI.md#testing). | | CI full | `task ci` | all of the above | lint → build → test:unit → test:e2e | @@ -281,7 +276,6 @@ own publish/image details. - **GoBGP RIB is ephemeral.** All BGP state is in-process memory. On restart, sessions and paths must be re-established from CRD state; controller-runtime's reconcile loop handles this automatically. - **EVPN Type 5 is implemented, not deferred.** `internal/runtime/gobgp/paths.go`'s `buildEVPNPaths` builds real `EVPNIPPrefixRoute` NLRIs, deriving the Route Distinguisher from `routerID + ":0"` (not from the CRD). The `BGPVRFInstance` CRD carries its own explicit `RouteDistinguisher` and import/export Route Targets (see Key Design Decisions above), applied via `internal/runtime/gobgp/runtime.go`'s `applyVRFs`. There is no `ErrMissingRouteDistinguisher` or similar rejection path in the current code. -- **`--mode=transit` is unimplemented.** Accepted by CLI/env validation, but `runCmd` returns an error at startup ("mode=transit is not yet supported"). - **No binary's `cmdDel` tears down shared kernel/CRD state.** See [ARCHITECTURE-CNI.md#known-constraints](ARCHITECTURE-CNI.md#known-constraints) for the CNI-side half; this reconciler's GC controller (`internal/gc`) is the asynchronous cleanup path for all of it. --- @@ -306,7 +300,6 @@ own publish/image details. **Stable vs. frequently changed:** - Stable: `internal/plumbing/` (pure kernel primitives), `internal/model/types.go`, `internal/runtime/runtime.go` (interface) - Active: `internal/controller/` (status conditions, watch graph), `internal/runtime/gobgp/` (EVPN path construction), `internal/reconcile/` (validation rules), `internal/gc/` (GC rules) -- Stub / incomplete: `internal/runtime/frr/` (returns "not implemented" everywhere), `--mode=transit` (rejected at startup) **Non-obvious patterns:** - `BGPPeer` and `BGPPolicy` reconcilers do not call Apply themselves — they enqueue their owning `BGPRouter`, which is the only reconciler that calls `RuntimeManager.Apply`. This means touching any associated resource triggers a full router reconcile. diff --git a/docs/agents/CONVENTIONS.md b/docs/agents/CONVENTIONS.md index 376de886..c76e64cf 100644 --- a/docs/agents/CONVENTIONS.md +++ b/docs/agents/CONVENTIONS.md @@ -11,11 +11,11 @@ This document defines the coding standards, naming rules, error handling pattern - Module: `go.datum.net/galactic` - `cmd/galactic-cni/main.go` — CNI installer entry point; a cobra command (no viper) with `init`/`run` subcommands wrapping `internal/installer.Bootstrap`/`Run` for the DaemonSet. Never itself a CNI plugin. - `cmd/galactic-veth/main.go` — CNI plugin entry point; a cobra command (no viper) with the plugin invocation on the root command (calls `cni.RunPlugin()`), no other subcommands -- `cmd/galactic-router/main.go` / `root.go` — router entry point; `main.go` is a thin wrapper, startup logic (reads `GALACTIC_ROUTER_NODE_NAME`/`GALACTIC_ROUTER_ROUTER_MODE`, starts the controller-runtime manager) lives in `root.go` +- `cmd/galactic-router/main.go` / `root.go` — router entry point; `main.go` is a thin wrapper, startup logic (reads `GALACTIC_ROUTER_NODE_NAME`, starts the controller-runtime manager) lives in `root.go` - `internal/plumbing/` — low-level kernel and network primitives shared between router and CNI (`intf`, `srv6`, `sysctl`, `vrf`) - `internal/controller/` — controller-runtime reconcilers (BGPRouter, BGPPeer, BGPAdvertisement, BGPVRFInstance, BGPPolicy, Secret, Node, GC); also contains field index registration (`indexer.go`) and CRD status helpers (`status.go`) - `internal/reconcile/` — CRD → DesiredRouter translation -- `internal/runtime/` — RouterRuntime interface; `gobgp/` (tenant mode) and `frr/` (fabric mode stub) +- `internal/runtime/` — RouterRuntime interface; `gobgp/` (the only backend) - `internal/gc/` — orphaned `BGPAdvertisement`/`BGPVRFInstance` CRD and stale kernel VRF cleanup, invoked by the GC controller's ticker - `internal/cni/` — CNI plugin (`cmdAdd`/`cmdDel`/`cmdCheck`, split across `ops_add.go`/`ops_del.go`/`ops_check.go`/`bgp.go`); `ipam/`, `route/`, `tap/`, `veth/` subpackages - `internal/installer/` — support for `galactic-cni`'s `init`/`run` subcommands (binary staging, conflist/kubeconfig templating, credential refresh, gRPC health server); not a subpackage of `internal/cni` diff --git a/docs/router/configuration.md b/docs/router/configuration.md index 58827812..bbce154b 100644 --- a/docs/router/configuration.md +++ b/docs/router/configuration.md @@ -4,21 +4,19 @@ or a combination of both. CLI flags take precedence over environment variables. All configuration is managed centrally by the `internal/config` package. The -`RouterConfig` struct and its constants (`EnvRouterNodeName`, `ModeTenant`, -etc.) are the single source of truth. +`RouterConfig` struct and its constants (`EnvRouterNodeName`, +`EnvRouterReflector`, etc.) are the single source of truth. ## Quick Reference Environment variable names are not a naive uppercased guess from the CLI flag name — the mapping is defined in `internal/config` (see `RouterConfig` -and the `EnvRouter*` constants). The most common trip-up: `--mode` is -`GALACTIC_ROUTER_ROUTER_MODE`, not `GALACTIC_ROUTER_MODE`. Always use the -exact name from the table below. +and the `EnvRouter*` constants). Always use the exact name from the table +below. | Option | Environment Variable | CLI Flag | Default | |---|---|---|---| | Node name | `GALACTIC_ROUTER_NODE_NAME` | `--node-name` | _(required)_ | -| Router mode | `GALACTIC_ROUTER_ROUTER_MODE` | `--mode` | _(required)_ | | Route reflector | `GALACTIC_ROUTER_REFLECTOR` | `--reflector` | `false` | | BGP listen port | `GALACTIC_ROUTER_BGP_LISTEN_PORT` | `--bgp-listen-port` | `179` | | BGP local address | `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS` | `--bgp-local-address` | _(auto-detected from `lo`)_ | @@ -33,7 +31,6 @@ The following options are **required**. If unset, `galactic-router` exits with an error: - `--node-name` (`GALACTIC_ROUTER_NODE_NAME`) — Kubernetes node name where the router runs. -- `--mode` (`GALACTIC_ROUTER_ROUTER_MODE`) — Router mode: `transit`, `fabric`, or `tenant`. ## Option Details @@ -45,17 +42,9 @@ used to scope BGP configuration to the correct node. **Type:** string **Required:** yes -### `--mode` / `GALACTIC_ROUTER_ROUTER_MODE` - -The operating mode of this instance. Determines which BGP backend is used: - -- `tenant` — uses GoBGP for EVPN path distribution (production mode). -- `fabric` — uses the FRR stub backend (not yet implemented). -- `transit` — reserved for future transit mode (not yet implemented). - ### `--reflector` / `GALACTIC_ROUTER_REFLECTOR` -Enable route reflector mode. Only valid when `--mode=fabric` or `--mode=tenant`. +Enable route reflector mode. **Type:** boolean **Default:** `false` @@ -83,8 +72,8 @@ value is set and no such address is found on `lo`. This detection always runs, even when `--bgp-listen-port`/ `GALACTIC_ROUTER_BGP_LISTEN_PORT` is `-1` (no inbound listener) — `galactic-router` still needs a source address for outbound BGP connections. -In practice this means every node running `galactic-router` in `tenant` mode -needs a global-unicast IPv6 address on `lo` before startup, or an explicit +In practice this means every node running `galactic-router` needs a +global-unicast IPv6 address on `lo` before startup, or an explicit `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS`. **Type:** string @@ -150,8 +139,6 @@ env: valueFrom: fieldRef: fieldPath: spec.nodeName - - name: GALACTIC_ROUTER_ROUTER_MODE - value: tenant - name: GALACTIC_ROUTER_GC_NAMESPACE value: galactic-system ``` @@ -165,7 +152,6 @@ command: - /galactic-router args: - --node-name=$(GALACTIC_ROUTER_NODE_NAME) - - --mode=tenant - --metrics-port=9090 env: - name: GALACTIC_ROUTER_NODE_NAME @@ -184,14 +170,11 @@ env: valueFrom: fieldRef: fieldPath: spec.nodeName - - name: GALACTIC_ROUTER_ROUTER_MODE - value: tenant - name: GALACTIC_ROUTER_METRICS_PORT value: "9090" command: - /galactic-router args: - --node-name=$(GALACTIC_ROUTER_NODE_NAME) - - --mode=tenant - --metrics-port=9100 # overrides GALACTIC_ROUTER_METRICS_PORT env var ``` diff --git a/internal/config/router.go b/internal/config/router.go index 18d8ecb7..9c5d278c 100644 --- a/internal/config/router.go +++ b/internal/config/router.go @@ -36,7 +36,6 @@ const ( const ( EnvRouterNodeName = "GALACTIC_ROUTER_NODE_NAME" - EnvRouterMode = "GALACTIC_ROUTER_ROUTER_MODE" EnvRouterReflector = "GALACTIC_ROUTER_REFLECTOR" EnvRouterBGPListenPort = "GALACTIC_ROUTER_BGP_LISTEN_PORT" EnvRouterBGPLocalAddr = "GALACTIC_ROUTER_BGP_LOCAL_ADDRESS" @@ -58,14 +57,6 @@ const ( EnvRouterWebhookCertDir = "GALACTIC_ROUTER_WEBHOOK_CERT_DIR" ) -// --- Router mode constants ------------------------------------------------- - -const ( - ModeTransit = "transit" - ModeFabric = "fabric" - ModeTenant = "tenant" -) - // --- RouterConfig ---------------------------------------------------------- // RouterConfig resolves router configuration with three-tier precedence: CLI @@ -77,7 +68,6 @@ type RouterConfig struct { // Resolved fields. NodeName string - Mode string Reflector bool BGPListenPort int BGPLocalAddr string @@ -103,7 +93,6 @@ func NewRouterConfig() *RouterConfig { v.AutomaticEnv() v.SetDefault("node_name", "") - v.SetDefault("router_mode", "") v.SetDefault("reflector", false) v.SetDefault("bgp_listen_port", DefaultRouterBGPListenPort) v.SetDefault("bgp_local_address", "") @@ -131,7 +120,6 @@ func (c *RouterConfig) BindFlags(flags *pflag.FlagSet) { key string }{ {"node-name", "node_name"}, - {"mode", "router_mode"}, {"reflector", "reflector"}, {"bgp-listen-port", "bgp_listen_port"}, {"bgp-local-address", "bgp_local_address"}, @@ -157,7 +145,6 @@ func (c *RouterConfig) BindFlags(flags *pflag.FlagSet) { // readFields populates the exported fields from the current Viper state. func (c *RouterConfig) readFields() { c.NodeName = c.v.GetString("node_name") - c.Mode = c.v.GetString("router_mode") c.Reflector = c.v.GetBool("reflector") c.BGPListenPort = c.v.GetInt("bgp_listen_port") c.BGPLocalAddr = c.v.GetString("bgp_local_address") @@ -170,24 +157,12 @@ func (c *RouterConfig) readFields() { c.WebhookCertDir = c.v.GetString("webhook_cert_dir") } -// Validate checks that the required configuration fields are set and that -// mode/reflector constraints are satisfied. +// Validate checks that the required configuration fields are set and within +// range. func (c *RouterConfig) Validate() error { if c.NodeName == "" { return fmt.Errorf("node name is required (use --node-name flag or %s env var)", EnvRouterNodeName) } - if c.Mode == "" { - return fmt.Errorf("router mode is required (use --mode flag or %s env var)", EnvRouterMode) - } - switch c.Mode { - case ModeTransit, ModeFabric, ModeTenant: - default: - return fmt.Errorf("invalid router mode %q: must be %s, %s, or %s", - c.Mode, ModeTransit, ModeFabric, ModeTenant) - } - if c.Reflector && c.Mode != ModeFabric && c.Mode != ModeTenant { - return fmt.Errorf("route reflector mode requires --mode=%s or --mode=%s", ModeFabric, ModeTenant) - } if c.BGPListenPort != -1 && (c.BGPListenPort < 1 || c.BGPListenPort > 65535) { return errors.New("bgp listen port must be between 1 and 65535, or -1 for outbound-only mode") } diff --git a/internal/config/router_test.go b/internal/config/router_test.go index 8d7fce7d..a8f9b63e 100644 --- a/internal/config/router_test.go +++ b/internal/config/router_test.go @@ -54,7 +54,6 @@ func TestRouterConfigDefaults(t *testing.T) { func TestRouterConfigEnvOverride(t *testing.T) { t.Setenv(EnvRouterNodeName, "env-node") - t.Setenv(EnvRouterMode, ModeTenant) t.Setenv(EnvRouterReflector, testBoolTrue) t.Setenv(EnvRouterBGPListenPort, "1790") t.Setenv(EnvRouterBGPLocalAddr, "2001:db8::1") @@ -71,9 +70,6 @@ func TestRouterConfigEnvOverride(t *testing.T) { if cfg.NodeName != "env-node" { t.Errorf("NodeName = %q, want %q", cfg.NodeName, "env-node") } - if cfg.Mode != ModeTenant { - t.Errorf("Mode = %q, want %q", cfg.Mode, ModeTenant) - } if !cfg.Reflector { t.Error("Reflector = false, want true") } @@ -114,50 +110,20 @@ func TestRouterConfigValidate(t *testing.T) { }{ { name: "missing node name", - envVars: map[string]string{EnvRouterMode: ModeTenant}, + envVars: map[string]string{}, wantErr: "node name is required", }, { - name: "missing mode", - envVars: map[string]string{EnvRouterNodeName: testRouterNodeName}, - wantErr: "router mode is required", - }, - { - name: "invalid mode", - envVars: map[string]string{EnvRouterNodeName: testRouterNodeName, EnvRouterMode: "invalid"}, - wantErr: "invalid router mode", - }, - { - name: "reflector without valid mode", - envVars: map[string]string{ - EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeTransit, - EnvRouterReflector: testBoolTrue, - }, - wantErr: "route reflector mode requires", - }, - { - name: "valid tenant mode", + name: "valid node name", envVars: map[string]string{ EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeTenant, - }, - wantErr: "", - }, - { - name: "valid fabric mode with reflector", - envVars: map[string]string{ - EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeFabric, - EnvRouterReflector: testBoolTrue, }, wantErr: "", }, { - name: "valid tenant mode with reflector", + name: "valid node name with reflector", envVars: map[string]string{ EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeTenant, EnvRouterReflector: testBoolTrue, }, wantErr: "", @@ -166,7 +132,6 @@ func TestRouterConfigValidate(t *testing.T) { name: "invalid bgp listen port", envVars: map[string]string{ EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeTenant, EnvRouterBGPListenPort: "0", }, wantErr: "bgp listen port must be between", @@ -175,7 +140,6 @@ func TestRouterConfigValidate(t *testing.T) { name: "outbound-only bgp listen port", envVars: map[string]string{ EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeTenant, EnvRouterBGPListenPort: "-1", }, wantErr: "", @@ -184,7 +148,6 @@ func TestRouterConfigValidate(t *testing.T) { name: "invalid metrics port", envVars: map[string]string{ EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeTenant, EnvRouterMetricsPort: "0", }, wantErr: "metrics port must be between", @@ -193,7 +156,6 @@ func TestRouterConfigValidate(t *testing.T) { name: "invalid grpc health port", envVars: map[string]string{ EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeTenant, EnvRouterGRPCHealthPort: "0", }, wantErr: "grpc health port must be between", @@ -202,7 +164,6 @@ func TestRouterConfigValidate(t *testing.T) { name: "invalid webhook port", envVars: map[string]string{ EnvRouterNodeName: testRouterNodeName, - EnvRouterMode: ModeTenant, EnvRouterWebhookPort: "0", }, wantErr: "webhook port must be between", diff --git a/internal/controller/bgprouter_controller.go b/internal/controller/bgprouter_controller.go index 211802de..d517d1e5 100644 --- a/internal/controller/bgprouter_controller.go +++ b/internal/controller/bgprouter_controller.go @@ -45,7 +45,6 @@ type BGPRouterReconciler struct { RuntimeManager galacticruntime.RuntimeManager Hasher func(model.DesiredRouter) (string, error) NodeName string - RouterMode string } // Reconcile reconciles a single BGPRouter. diff --git a/internal/reconcile/reconcile.go b/internal/reconcile/reconcile.go index 4de9bab2..36893b60 100644 --- a/internal/reconcile/reconcile.go +++ b/internal/reconcile/reconcile.go @@ -36,19 +36,17 @@ const legacySRv6SIDAnnotation = "galactic.datum.net/srv6-sid" type Reconciler struct { client client.Client nodeName string - routerMode string localAddress string } -// New returns a Reconciler for the given node, router mode, and local BGP address. +// New returns a Reconciler for the given node and local BGP address. // localAddress, when non-empty, is used as the EVPN next-hop instead of the // node's first IPv6 InternalIP — required when the node InternalIP is not // reachable via the SRv6 transit mesh (e.g. Kind/ContainerLab Docker bridge). -func New(c client.Client, nodeName, routerMode, localAddress string) *Reconciler { +func New(c client.Client, nodeName, localAddress string) *Reconciler { return &Reconciler{ client: c, nodeName: nodeName, - routerMode: routerMode, localAddress: localAddress, } } @@ -64,9 +62,11 @@ func (r *Reconciler) BuildDesiredRouter( return nil, nil } - // Role check. - wantRole := bgpv1alpha1.RouterRole(r.routerMode) - if !slices.Contains(router.Spec.Roles, wantRole) { + // Role check. galactic-router only ever runs the tenant role; a router + // carrying a different role (e.g. fabric, transit — both exist in the + // external RouterRole enum but have no implementation here) is safely + // skipped rather than reconciled against the GoBGP runtime. + if !slices.Contains(router.Spec.Roles, bgpv1alpha1.RouterRoleTenant) { return nil, nil } if len(router.Spec.Roles) > 1 { diff --git a/internal/reconcile/reconcile_test.go b/internal/reconcile/reconcile_test.go index dbbe0db1..78199dbd 100644 --- a/internal/reconcile/reconcile_test.go +++ b/internal/reconcile/reconcile_test.go @@ -5,9 +5,13 @@ package reconcile import ( + "context" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "go.datum.net/galactic/internal/model" bgpv1alpha1 "go.datum.net/network/api/v1alpha1" @@ -59,6 +63,99 @@ func TestBuildVRFInstance(t *testing.T) { } } +// newTestScheme returns a runtime.Scheme with the BGP API types registered, +// for constructing a fake.Client in tests. +func newTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + if err := bgpv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("add bgpv1alpha1 to scheme: %v", err) + } + return scheme +} + +// routerRefIndexer indexes BGPAdvertisement/BGPVRFInstance by +// .spec.routerRef.name, mirroring internal/controller/indexer.go's +// BGPAdvByRouterName/BGPVRFInstanceByRouterName registration on the real +// manager cache — BuildDesiredRouter's client.MatchingFields lookups require +// it, and the fake client has no cache to register it on implicitly. +const routerRefIndexer = ".spec.routerRef.name" + +func TestBuildDesiredRouter_NodeAndRoleFilter(t *testing.T) { + const thisNode = "node-a" + + tests := []struct { + name string + targetNode string + roles []bgpv1alpha1.RouterRole + wantSkip bool + }{ + { + name: "skips a router targeting a different node", + targetNode: "node-b", + roles: []bgpv1alpha1.RouterRole{bgpv1alpha1.RouterRoleTenant}, + wantSkip: true, + }, + { + name: "skips a router with a non-tenant role", + targetNode: thisNode, + roles: []bgpv1alpha1.RouterRole{"fabric"}, + wantSkip: true, + }, + { + name: "proceeds for a tenant-role router on this node", + targetNode: thisNode, + roles: []bgpv1alpha1.RouterRole{bgpv1alpha1.RouterRoleTenant}, + wantSkip: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + router := &bgpv1alpha1.BGPRouter{ + ObjectMeta: metav1.ObjectMeta{Name: "r1", Namespace: "default"}, + Spec: bgpv1alpha1.BGPRouterSpec{ + TargetRef: bgpv1alpha1.TargetRef{Name: tc.targetNode}, + Roles: tc.roles, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(newTestScheme(t)). + WithIndex(&bgpv1alpha1.BGPAdvertisement{}, routerRefIndexer, func(obj client.Object) []string { + adv := obj.(*bgpv1alpha1.BGPAdvertisement) + return []string{adv.Spec.RouterRef.Name} + }). + WithIndex(&bgpv1alpha1.BGPVRFInstance{}, routerRefIndexer, func(obj client.Object) []string { + vrf := obj.(*bgpv1alpha1.BGPVRFInstance) + if vrf.Spec.RouterRef == nil { + return nil + } + return []string{vrf.Spec.RouterRef.Name} + }). + Build() + + // localAddress set so the "proceeds" case doesn't also need a + // fake Node object just to resolve an EVPN next-hop. + r := New(fakeClient, thisNode, "2001:db8::1") + + got, err := r.BuildDesiredRouter(context.Background(), router) + if err != nil { + t.Fatalf("BuildDesiredRouter() error = %v, want nil", err) + } + if tc.wantSkip { + if got != nil { + t.Errorf("BuildDesiredRouter() = %+v, want nil (skip)", got) + } + return + } + if got == nil { + t.Error("BuildDesiredRouter() = nil, want a non-nil DesiredRouter") + } + }) + } +} + func TestResolveSRv6SID(t *testing.T) { tests := []struct { name string diff --git a/internal/runtime/frr/frr.go b/internal/runtime/frr/frr.go deleted file mode 100644 index 58d6eb0e..00000000 --- a/internal/runtime/frr/frr.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2025 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -// Package frr provides a stub FRR RouterRuntime implementation. -// NOTE: The fabric role is not yet implemented. Running galactic-router -// with ROUTER_ROLE=fabric will fail on the first reconcile. -package frr - -import ( - "context" - "errors" - - "k8s.io/apimachinery/pkg/types" - - "go.datum.net/galactic/internal/model" - "go.datum.net/galactic/internal/runtime" -) - -var errNotImplemented = errors.New("frr runtime not implemented") - -type frrRuntime struct{} - -func (f *frrRuntime) Apply(_ context.Context, _ model.DesiredRouter) error { - return errNotImplemented -} - -func (f *frrRuntime) Status(_ context.Context) (model.RuntimeStatus, error) { - return model.RuntimeStatus{}, errNotImplemented -} - -func (f *frrRuntime) Stop(_ context.Context) error { - return nil -} - -// NewRuntimeFactory returns a RuntimeFactory that creates stub FRR runtimes. -func NewRuntimeFactory() runtime.RuntimeFactory { - return func(_ types.NamespacedName) (runtime.RouterRuntime, error) { - return &frrRuntime{}, nil - } -}