From 431dea86034cb9fe743c3a5f7e08014634155881 Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Tue, 16 Jun 2026 22:21:55 -0400 Subject: [PATCH] refactor(cni): rename cmd/galactic to cmd/galactic-cni and simplify to pure CNI plugin Rename the source directory from cmd/galactic to cmd/galactic-cni to match the binary name. Strip the CLI layer entirely: galactic-cni is always invoked by the container runtime with CNI_COMMAND set, so the cobra root command, cni/version subcommands, and CNI_COMMAND env-var dispatch were all dead code. - Replace NewCommand() in internal/cni with RunPlugin(); main() calls it directly - Delete internal/cmd/cni (pass-through cobra wrapper) - Remove NewCommand() and cobra import from internal/cmd/version - Update CNI type from "galactic" to "galactic-cni" in all NADs and test fixtures - Remove TestGalacticBinaryVersion e2e test (tested the deleted version subcommand) - Update ARCHITECTURE.md, CONVENTIONS.md, AGENTS.md, and Taskfile to match Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 2 +- ARCHITECTURE.md | 8 ++-- CONVENTIONS.md | 5 +- Taskfile.yaml | 3 +- cmd/galactic-cni/main.go | 11 +++++ cmd/galactic/main.go | 47 ------------------- containers/galactic/Dockerfile | 2 +- .../kindest-node-galactic/Dockerfile | 2 +- .../containerlab/examples/nginx-vpc-test.yaml | 2 +- .../resources/overlay/dfw/nad.yaml | 2 +- .../resources/overlay/iad/nad.yaml | 2 +- .../resources/overlay/sjc/nad.yaml | 2 +- internal/cmd/cni/cni.go | 30 ------------ internal/cmd/version/version.go | 20 -------- internal/cni/cni.go | 24 ++++------ internal/cni/cni_test.go | 2 +- tests/e2e/e2e_test.go | 35 -------------- 17 files changed, 35 insertions(+), 164 deletions(-) create mode 100644 cmd/galactic-cni/main.go delete mode 100644 cmd/galactic/main.go delete mode 100644 internal/cmd/cni/cni.go diff --git a/AGENTS.md b/AGENTS.md index db49c0c..494bad0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ Galactic is the SRv6 data plane for multi-cloud VPC networking. It consists of a **Non-obvious decisions:** - VPC identifiers are 48-bit hex; VPCAttachment identifiers are 16-bit hex. These are embedded into IPv6 SRv6 endpoint addresses for deterministic route lookups. Both are supplied by an external operator via the CNI config. - Identifiers are also Base62-encoded for interface naming (VRF: `vrfX-Y`, veth host side: `galX-Y`) to keep kernel interface name length within limits. -- The binary auto-detects CNI mode via the `CNI_COMMAND` env var; otherwise runs as a Cobra CLI with `agent`, `cni`, and `version` subcommands. +- `galactic-cni` is a pure CNI plugin; `main()` calls `cni.RunPlugin()` directly with no CLI layer. `galactic-agent` uses flag parsing for its configuration flags. - The Kubernetes operator, VPC/VPCAttachment CRDs, and webhook code have been removed from this repository. They live in a separate companion operator project. ## Tech Stack diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 16e667e..898c84e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -40,7 +40,7 @@ enabling automatic cross-node path import without explicit RT configuration. ``` galactic/ ├── cmd/ -│ ├── galactic/ # CNI binary +│ ├── galactic-cni/ # CNI binary │ └── galactic-agent/ # Agent binary ├── internal/ │ ├── agent/ # Agent run loop; wires GoBGP, health, metrics, bootstrap @@ -83,8 +83,8 @@ See [docs/agent-startup.md](docs/agent-startup.md) for the agent startup sequenc | `internal/bootstrap` | `galactic-agent` | BGPProvider CR lifecycle | | `internal/gobgp` | `galactic-agent` | Embedded GoBGP server | | `internal/metrics` | `galactic-agent` | Prometheus metrics | -| `internal/cni` | `galactic` | CNI cmdAdd / cmdDel | -| `internal/cni/bgp` | `galactic` | L3VPN path injection into GoBGP | +| `internal/cni` | `galactic-cni` | CNI cmdAdd / cmdDel | +| `internal/cni/bgp` | `galactic-cni` | L3VPN path injection into GoBGP | | `pkg/common/vrf` | both | Linux VRF management | | `pkg/common/util` | both | SRv6 encoding, interface naming | @@ -95,7 +95,7 @@ See [docs/agent-startup.md](docs/agent-startup.md) for the agent startup sequenc - **Identifiers in the SID.** VPC (48-bit) and VPCAttachment (16-bit) identifiers are packed into the low 64 bits of the SRv6 SID, making forwarding state fully self-describing without a lookup table. - **Base62 interface names.** Kernel interface names are Base62-encoded to stay within the 15-character limit (`vrfX-Y`, `galX-Y`). The hex form is used for BGP and SRv6; base62 for kernel interfaces. - **GoBGP embedded, not sidecar.** GoBGP runs in-process so the agent owns its lifecycle and can gate readiness on BGP availability. Peer and policy config is applied by the cosmos operator via `BGPProvider` / `BGPInstance` / `BGPPeer` CRDs. -- **CNI binary auto-detects mode.** The `galactic` binary runs as both the CNI plugin (when `CNI_COMMAND` is set) and a CLI tool. This avoids shipping two separate binaries on the node. +- **CNI binary auto-detects mode.** The `galactic-cni` binary runs as both the CNI plugin (when `CNI_COMMAND` is set) and a CLI tool. This avoids shipping two separate binaries on the node. --- diff --git a/CONVENTIONS.md b/CONVENTIONS.md index 71df786..4d54b10 100644 --- a/CONVENTIONS.md +++ b/CONVENTIONS.md @@ -9,12 +9,13 @@ This document defines the coding standards, naming rules, error handling pattern ### Module and package layout - Module: `go.datum.net/galactic` -- `cmd/galactic/main.go` — binary entry point; all Cobra subcommands registered here +- `cmd/galactic-cni/main.go` — CNI plugin entry point; calls `cni.RunPlugin()` directly +- `cmd/galactic-agent/main.go` — agent entry point; parses flags and calls `agent.Run()` - `pkg/common/` — utilities shared between agent and CNI - `pkg/proto/local/` — gRPC / protobuf generated files plus hand-written convenience wrapper for CNI-to-agent communication - `internal/agent/` — agent entry point and gRPC server; `srv6/` subdirectory owns kernel SRv6 route and VRF management - `internal/cni/` — CNI plugin (cmdAdd / cmdDel implementation) -- `internal/cmd/` — one sub-package per Cobra subcommand (`cni`, `version`) +- `internal/cmd/version/` — ldflags variables (Version, GitCommit, etc.) set at build time - `internal/gobgp/` — embedded GoBGP server lifecycle - `internal/bootstrap/` — agent startup sequencing (BGPProvider resource management) - `internal/metrics/` — Prometheus metrics registration diff --git a/Taskfile.yaml b/Taskfile.yaml index baa589b..20c1ba2 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -80,9 +80,8 @@ tasks: desc: Build binary deps: [fmt, vet] cmds: - - go build -o bin/galactic-cni cmd/galactic/main.go + - go build -o bin/galactic-cni cmd/galactic-cni/main.go - file bin/galactic-cni - - ./bin/galactic-cni version docker-build: desc: Build container image diff --git a/cmd/galactic-cni/main.go b/cmd/galactic-cni/main.go new file mode 100644 index 0000000..346a68f --- /dev/null +++ b/cmd/galactic-cni/main.go @@ -0,0 +1,11 @@ +// Copyright 2025 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import "go.datum.net/galactic/internal/cni" + +func main() { + cni.RunPlugin() +} diff --git a/cmd/galactic/main.go b/cmd/galactic/main.go deleted file mode 100644 index 956cef8..0000000 --- a/cmd/galactic/main.go +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2025 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package main - -import ( - "fmt" - "os" - - "github.com/spf13/cobra" - - "go.datum.net/galactic/internal/cmd/cni" - "go.datum.net/galactic/internal/cmd/version" -) - -func main() { - // Auto-detect CNI mode via CNI_COMMAND environment variable - // When run as a CNI plugin, the CNI runtime sets this variable - if os.Getenv("CNI_COMMAND") != "" { - if err := cni.NewCommand().Execute(); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) - } - return - } - - // Normal CLI mode with subcommands - rootCmd := &cobra.Command{ - Use: "galactic", - Short: "Galactic multi-cloud networking solution", - Long: `Galactic provides VPC connectivity across multiple clouds using SRv6 packet routing. - -This unified binary supports multiple modes of operation: -- cni: CNI plugin for container network attachment -`, - } - - // Add subcommands - rootCmd.AddCommand(cni.NewCommand()) - rootCmd.AddCommand(version.NewCommand()) - - if err := rootCmd.Execute(); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) - } -} diff --git a/containers/galactic/Dockerfile b/containers/galactic/Dockerfile index cb70805..2a75a5b 100644 --- a/containers/galactic/Dockerfile +++ b/containers/galactic/Dockerfile @@ -31,7 +31,7 @@ RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build \ -X go.datum.net/galactic/internal/cmd/version.GitCommit=${GIT_COMMIT} \ -X go.datum.net/galactic/internal/cmd/version.GitTreeState=${GIT_TREE_STATE} \ -X go.datum.net/galactic/internal/cmd/version.BuildDate=${BUILD_DATE}" \ - -o galactic-cni cmd/galactic/main.go + -o galactic-cni cmd/galactic-cni/main.go # Use distroless as minimal base image to package the binary # Refer to https://github.com/GoogleContainerTools/distroless for more details diff --git a/deploy/containerlab/containers/kindest-node-galactic/Dockerfile b/deploy/containerlab/containers/kindest-node-galactic/Dockerfile index 199b4d6..50deb63 100644 --- a/deploy/containerlab/containers/kindest-node-galactic/Dockerfile +++ b/deploy/containerlab/containers/kindest-node-galactic/Dockerfile @@ -4,7 +4,7 @@ FROM golang:1.26 AS builder WORKDIR /src COPY . . RUN go mod download -RUN CGO_ENABLED=0 GOOS=linux go build -o bin/galactic-cni cmd/galactic/main.go +RUN CGO_ENABLED=0 GOOS=linux go build -o bin/galactic-cni cmd/galactic-cni/main.go FROM kindest/node:${KINDEST_VER} diff --git a/deploy/containerlab/examples/nginx-vpc-test.yaml b/deploy/containerlab/examples/nginx-vpc-test.yaml index 79b3157..17784bb 100644 --- a/deploy/containerlab/examples/nginx-vpc-test.yaml +++ b/deploy/containerlab/examples/nginx-vpc-test.yaml @@ -43,7 +43,7 @@ spec: { "cniVersion": "0.3.1", "name": "galactic-test", - "type": "galactic", + "type": "galactic-cni", "vpc": "2", "vpcattachment": "1", "srv6_locator": "2001:db8:ff01::/48", diff --git a/deploy/containerlab/resources/overlay/dfw/nad.yaml b/deploy/containerlab/resources/overlay/dfw/nad.yaml index 25aa8c8..083c087 100644 --- a/deploy/containerlab/resources/overlay/dfw/nad.yaml +++ b/deploy/containerlab/resources/overlay/dfw/nad.yaml @@ -8,7 +8,7 @@ spec: { "cniVersion": "0.3.1", "name": "galactic", - "type": "galactic", + "type": "galactic-cni", "vpc": "1", "vpcattachment": "1", "srv6_locator": "2001:db8:ff01::/48", diff --git a/deploy/containerlab/resources/overlay/iad/nad.yaml b/deploy/containerlab/resources/overlay/iad/nad.yaml index d8c854a..b722e3e 100644 --- a/deploy/containerlab/resources/overlay/iad/nad.yaml +++ b/deploy/containerlab/resources/overlay/iad/nad.yaml @@ -8,7 +8,7 @@ spec: { "cniVersion": "0.3.1", "name": "galactic", - "type": "galactic", + "type": "galactic-cni", "vpc": "1", "vpcattachment": "1", "srv6_locator": "2001:db8:ff03::/48", diff --git a/deploy/containerlab/resources/overlay/sjc/nad.yaml b/deploy/containerlab/resources/overlay/sjc/nad.yaml index 7067299..fc0c47e 100644 --- a/deploy/containerlab/resources/overlay/sjc/nad.yaml +++ b/deploy/containerlab/resources/overlay/sjc/nad.yaml @@ -8,7 +8,7 @@ spec: { "cniVersion": "0.3.1", "name": "galactic", - "type": "galactic", + "type": "galactic-cni", "vpc": "1", "vpcattachment": "1", "srv6_locator": "2001:db8:ff02::/48", diff --git a/internal/cmd/cni/cni.go b/internal/cmd/cni/cni.go deleted file mode 100644 index 653ddf4..0000000 --- a/internal/cmd/cni/cni.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2025 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package cni - -import ( - "github.com/spf13/cobra" - - cniimpl "go.datum.net/galactic/internal/cni" -) - -func NewCommand() *cobra.Command { - cmd := &cobra.Command{ - Use: "cni", - Short: "Run as a CNI plugin", - Long: `The CNI plugin is invoked by the container runtime to set up network interfaces -for pods. It configures VRF, veth pairs, SRv6 ingress routes, and BGP paths. - -This command is typically invoked automatically by the container runtime when the -CNI_COMMAND environment variable is set. The main binary auto-detects CNI mode -and invokes this subcommand.`, - Run: func(cmd *cobra.Command, args []string) { - // Delegate to the CNI implementation - cniimpl.NewCommand().Run(cmd, args) - }, - } - - return cmd -} diff --git a/internal/cmd/version/version.go b/internal/cmd/version/version.go index 4f011fb..2793ed9 100644 --- a/internal/cmd/version/version.go +++ b/internal/cmd/version/version.go @@ -7,8 +7,6 @@ package version import ( "fmt" "runtime" - - "github.com/spf13/cobra" ) var ( @@ -20,21 +18,3 @@ var ( GoVersion = runtime.Version() Platform = fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH) ) - -func NewCommand() *cobra.Command { - cmd := &cobra.Command{ - Use: "version", - Short: "Print version information", - Long: `Print the version, git commit, build date, and platform information for this binary.`, - Run: func(cmd *cobra.Command, args []string) { - fmt.Printf("Galactic Version: %s\n", Version) - fmt.Printf("Git Commit: %s\n", GitCommit) - fmt.Printf("Git Tree State: %s\n", GitTreeState) - fmt.Printf("Build Date: %s\n", BuildDate) - fmt.Printf("Go Version: %s\n", GoVersion) - fmt.Printf("Platform: %s\n", Platform) - }, - } - - return cmd -} diff --git a/internal/cni/cni.go b/internal/cni/cni.go index 0f52631..0513e74 100644 --- a/internal/cni/cni.go +++ b/internal/cni/cni.go @@ -15,8 +15,6 @@ import ( "strings" "time" - "github.com/spf13/cobra" - "github.com/containernetworking/cni/pkg/invoke" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" @@ -65,21 +63,15 @@ type PluginConf struct { SRv6Locator string `json:"srv6_locator"` } -func NewCommand() *cobra.Command { - return &cobra.Command{ - Use: "galactic", - Short: "Galactic CNI plugin", - Run: func(cmd *cobra.Command, args []string) { - skel.PluginMainFuncs( - skel.CNIFuncs{ - Add: cmdAdd, - Del: cmdDel, - }, - version.All, - fmt.Sprintf("CNI galactic plugin %s", galversion.Version), - ) +func RunPlugin() { + skel.PluginMainFuncs( + skel.CNIFuncs{ + Add: cmdAdd, + Del: cmdDel, }, - } + version.All, + fmt.Sprintf("CNI galactic plugin %s", galversion.Version), + ) } func parseConf(data []byte) (*PluginConf, error) { diff --git a/internal/cni/cni_test.go b/internal/cni/cni_test.go index d518040..1a84592 100644 --- a/internal/cni/cni_test.go +++ b/internal/cni/cni_test.go @@ -69,7 +69,7 @@ func TestParseConf(t *testing.T) { }{ { name: "valid config", - input: `{"cniVersion":"1.0.0","name":"test","type":"galactic","vpc":"abc","vpcattachment":"def","srv6_locator":"2001:db8::/48"}`, + input: `{"cniVersion":"1.0.0","name":"test","type":"galactic-cni","vpc":"abc","vpcattachment":"def","srv6_locator":"2001:db8::/48"}`, wantVPC: "abc", }, { diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 4904246..d5e3465 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -50,41 +50,6 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -// TestGalacticBinaryVersion runs the galactic binary with the "version" -// subcommand inside a pod and verifies the output contains the expected fields. -func TestGalacticBinaryVersion(t *testing.T) { - name := "e2e-version-check" - t.Cleanup(func() { deletePod(t, name) }) - deletePod(t, name) // remove any leftover from a prior run - - _, err := kubectl( - "run", name, - "--image="+image(), - "--image-pull-policy=Never", - "--restart=Never", - "--command", "--", - "/galactic-cni", "version", - ) - if err != nil { - t.Fatalf("kubectl run failed: %v", err) - } - - if err := waitForPodPhase(t, name, "Succeeded", podReadyTimeout); err != nil { - t.Fatalf("pod did not succeed: %v", err) - } - - logs, err := kubectl("logs", name) - if err != nil { - t.Fatalf("kubectl logs failed: %v", err) - } - - for _, field := range []string{"Galactic Version:", "Git Commit:", "Go Version:", "Platform:"} { - if !strings.Contains(logs, field) { - t.Errorf("expected %q in version output, got:\n%s", field, logs) - } - } -} - // TestCNIPluginVersionReport invokes the binary with CNI_COMMAND=VERSION, which // causes it to report the CNI spec versions it supports. The response must be // valid JSON containing a "cniVersion" key.