From d369c7215d4345d9312a52fef3d52a6dc1a43df5 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 17 Aug 2026 15:26:04 +0800 Subject: [PATCH 1/3] review: keep sqlite Store type contiguous with its method set Move the standalone RefuseManifest below the type blocks so the asl funcpartition rule (type immediately followed by its methods) holds. --- meta/sqlite/store.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/meta/sqlite/store.go b/meta/sqlite/store.go index 3aab7ad5..c74dbb90 100644 --- a/meta/sqlite/store.go +++ b/meta/sqlite/store.go @@ -81,15 +81,6 @@ func OpenForRecovery(dbPath string, namespaces ...Namespace) (*Store, error) { return openStore(dbPath, namespaces) } -// RefuseManifest fails when a conversion manifest sits beside dbPath, meaning an offline conversion is unfinished. -func RefuseManifest(dbPath string) error { - manifest := filepath.Join(filepath.Dir(dbPath), ManifestName) - if utils.FileExists(manifest) { - return fmt.Errorf("%s exists: a conversion is in flight, run `cocoon meta convert` to finish it", manifest) - } - return nil -} - func openStore(dbPath string, namespaces []Namespace) (*Store, error) { // The driver creates a file on first touch; Open never creates — that is Init's job (§6) — and §4 refuses network filesystems before WAL work. if !utils.FileExists(dbPath) { @@ -381,6 +372,15 @@ func (h *txHandle) checkRead(ns string) error { return nil } +// RefuseManifest fails when a conversion manifest sits beside dbPath, meaning an offline conversion is unfinished. +func RefuseManifest(dbPath string) error { + manifest := filepath.Join(filepath.Dir(dbPath), ManifestName) + if utils.FileExists(manifest) { + return fmt.Errorf("%s exists: a conversion is in flight, run `cocoon meta convert` to finish it", manifest) + } + return nil +} + func tableName(ns, table string) string { return quoteIdent(ns + "__" + table) } From 8c5b38f89e1f16db9900839f3b30aa2bbc31779e Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 17 Aug 2026 15:26:04 +0800 Subject: [PATCH 2/3] net: scope host device families per installation (net_scope) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GC sweeps two host-wide name families by name — bridge TAPs bt- in the host netns and CNI netns cocoon- under /var/run/netns — and reclaims every entry whose VM its own DB does not know. Both prefixes were package constants, so a second installation on the same host (another root_dir, or a cocoon-derived runner such as cocoon-macos provisioning through cocoon's bridge/CNI backends) shares the families and has its live guests' TAPs and netns torn down by the peer's sweep. Add config net_scope: two alphanumerics keying an installation's families (-, -); empty keeps the legacy names. The fixed length keeps distinct scopes from being prefixes of each other, and bt / rm are rejected as aliases of the legacy and restore TAP families. The naming primitives live in package network next to TAPName; bridge and cni take the resolved prefix, and cocoon stays ignorant of any downstream. --- cmd/core/gc.go | 3 ++- cmd/core/network.go | 2 +- cmd/root.go | 1 + config/config.go | 6 +++++ config/config_test.go | 21 ++++++++++++++++ docs/cli.md | 1 + docs/gc.md | 4 ++-- docs/networking.md | 4 ++++ hypervisor/cloudhypervisor/clone.go | 4 +--- network/bridge/bridge_linux.go | 35 +++++++++++++-------------- network/bridge/bridge_other.go | 2 +- network/bridge/gc_linux.go | 14 +++++------ network/bridge/gc_linux_test.go | 32 ++++++++++++------------- network/bridge/gc_other.go | 2 +- network/cni/cni.go | 4 ++-- network/cni/cni_test.go | 24 +++++++++++++++++-- network/cni/config.go | 14 +++++------ network/cni/gc.go | 7 ++++-- network/cni/lifecycle.go | 8 +++---- network/cni/teardown.go | 8 +++---- network/network_test.go | 33 +++++++++++++++++++++++++ network/utils.go | 37 +++++++++++++++++++++++++++++ 22 files changed, 192 insertions(+), 74 deletions(-) diff --git a/cmd/core/gc.go b/cmd/core/gc.go index 8610e389..ae8503b5 100644 --- a/cmd/core/gc.go +++ b/cmd/core/gc.go @@ -7,6 +7,7 @@ import ( "github.com/cocoonstack/cocoon/gc" "github.com/cocoonstack/cocoon/hypervisor" "github.com/cocoonstack/cocoon/lock/vmlock" + "github.com/cocoonstack/cocoon/network" "github.com/cocoonstack/cocoon/network/bridge" "github.com/cocoonstack/cocoon/snapshot/localfile" ) @@ -39,7 +40,7 @@ func NewGCOrchestrator(ctx context.Context, conf *config.Config, snapOpts ...loc } gc.Register(o, hypervisor.CgroupGCModule(conf.CgroupParentDir())) netProvider.RegisterGC(o) - gc.Register(o, bridge.GCModule()) + gc.Register(o, bridge.GCModule(network.BridgeTAPPrefix(conf.NetScope))) gc.Register(o, vmlock.GCModule(conf.RootDir)) snapBackend.RegisterGC(o) return o, nil diff --git a/cmd/core/network.go b/cmd/core/network.go index 1a2ad0c7..b8a96159 100644 --- a/cmd/core/network.go +++ b/cmd/core/network.go @@ -96,7 +96,7 @@ func (n *NetProviders) Quiesce(ctx context.Context, vm *types.VM) error { // A partial CNI failure leaves the tombstone for retry or GC to resume. func (n *NetProviders) Cleanup(ctx context.Context, vmID string) error { - bridgenet.CleanupTAPs([]string{vmID}) + bridgenet.CleanupTAPs(network.BridgeTAPPrefix(n.conf.NetScope), []string{vmID}) p, err := n.cniOnly() if err != nil { // Lazy CNI; OK to skip for bridge-only setups. diff --git a/cmd/root.go b/cmd/root.go index 388a46c3..4679434d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -82,6 +82,7 @@ func newRootCmd() *cobra.Command { viper.SetDefault("meta_backend", "") viper.SetDefault("cgroup_parent", cgroup.DefaultParent) viper.SetDefault("cgroup_cpus", "") + viper.SetDefault("net_scope", "") viper.SetDefault("log.level", "info") viper.SetDefault("log.max_size", 500) viper.SetDefault("log.max_age", 28) diff --git a/config/config.go b/config/config.go index a1c881ec..c046a1b6 100644 --- a/config/config.go +++ b/config/config.go @@ -10,6 +10,7 @@ import ( coretypes "github.com/projecteru2/core/types" "github.com/cocoonstack/cocoon/cgroup" + "github.com/cocoonstack/cocoon/network" "github.com/cocoonstack/cocoon/utils" ) @@ -58,6 +59,8 @@ type Config struct { CNIBinDir string `json:"cni_bin_dir" mapstructure:"cni_bin_dir"` // DNS: comma/semicolon-separated DNS servers injected into VM net config. Env: COCOON_DNS. Default: "8.8.8.8,1.1.1.1". DNS string `json:"dns" mapstructure:"dns"` + // NetScope keys this installation's host network families (bridge TAPs -, CNI netns -) so co-hosted installations never GC each other's; two alphanumerics, empty keeps the legacy bt / cocoon- names. + NetScope string `json:"net_scope,omitempty" mapstructure:"net_scope"` // SocketWaitTimeoutSeconds: wait for the CH API socket after start. Default: 5; increase for slow storage. SocketWaitTimeoutSeconds int `json:"socket_wait_timeout_seconds" mapstructure:"socket_wait_timeout_seconds"` // TerminateGracePeriodSeconds: SIGTERM→SIGKILL window when force-killing CH. Default: 5. @@ -124,6 +127,9 @@ func (c *Config) Validate() error { if _, err := cgroup.ParseCPUList(c.CgroupCPUs); err != nil { return fmt.Errorf("cgroup_cpus: %w", err) } + if err := network.ValidateScope(c.NetScope); err != nil { + return fmt.Errorf("net_scope: %w", err) + } return nil } diff --git a/config/config_test.go b/config/config_test.go index 569d82ce..f202a965 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -99,6 +99,27 @@ func TestDNSServers(t *testing.T) { } } +func TestValidate_NetScope(t *testing.T) { + for _, tt := range []struct { + scope string + ok bool + }{ + {"", true}, {"mt", true}, {"m", false}, {"mtap", false}, {"m-", false}, {"bt", false}, {"rm", false}, + } { + c := &Config{ + RootDir: "/r", RunDir: "/r/run", LogDir: "/l", + StopTimeoutSeconds: 30, NetScope: tt.scope, + } + err := c.Validate() + if tt.ok && err != nil { + t.Fatalf("scope %q: unexpected error %v", tt.scope, err) + } + if !tt.ok && err == nil { + t.Fatalf("scope %q: want rejection", tt.scope) + } + } +} + func TestValidate_MetaBackend(t *testing.T) { for _, tt := range []struct { backend string diff --git a/docs/cli.md b/docs/cli.md index f82befb7..5942e879 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -81,6 +81,7 @@ Config-file / env-only keys (no CLI flag): | `pull_conns` | `COCOON_PULL_CONNS` | `8` | Concurrent HTTP Range connections per cloud-image download (`image pull`); raise for fat pipes, lower to be gentle on the registry | | `cgroup_parent` | `COCOON_CGROUP_PARENT` | `cocoon.slice` | cgroup v2 slice under `/sys/fs/cgroup` holding the per-VM CPU scopes; see [CPU Isolation](vm.md#cpu-isolation-cgroup-v2) | | `cgroup_cpus` | `COCOON_CGROUP_CPUS` | empty (all cores) | Host cpu list fencing the whole VM population (e.g. `0-14` reserves core 15 for the host); kernel cpu-list syntax | +| `net_scope` | `COCOON_NET_SCOPE` | empty (legacy names) | Two alphanumerics keying this installation's host network families — bridge TAPs `-`, CNI netns `-` — so co-hosted installations never GC each other's; see [Host device namespaces](networking.md#host-device-namespaces) | | `ch_binary` | `COCOON_CH_BINARY` | `cloud-hypervisor` | cloud-hypervisor executable, path or `$PATH` name | | `fc_binary` | `COCOON_FC_BINARY` | `firecracker` | firecracker executable, path or `$PATH` name | | `meta_backend` | `COCOON_META_BACKEND` | auto-resolved | Metadata engine, `json` or `sqlite`; see the note above `Global Flags` | diff --git a/docs/gc.md b/docs/gc.md index bbc5ae0a..4dd0a404 100644 --- a/docs/gc.md +++ b/docs/gc.md @@ -38,8 +38,8 @@ Reasons: - **snapshot**: `orphan` (dataDir without DB record), `stale-pending` (a dead save's pending record — its build lease is free), `lru-all` / `lru-age` / `lru-keep` / `lru-size` (multi-criterion uses `+` joiner) - **cloud-hypervisor / firecracker**: `orphan-runDir`, `orphan-logDir`, `stale-creating` (a dead create/clone's placeholder — its ops lock is free, no age wait) - **images (oci, cloudimg)**: `unreferenced` -- **cni**: `orphan` (netns without active VM) -- **bridge**: `orphan-tap` +- **cni**: `orphan` (netns in this installation's `net_scope` family without active VM) +- **bridge**: `orphan-tap` (TAP in this installation's `net_scope` family without active VM) - **vmlock**: `orphan-lease` (lease file for a VM no backend knows) ### Snapshot LRU Eviction diff --git a/docs/networking.md b/docs/networking.md index 5106953f..85bda350 100644 --- a/docs/networking.md +++ b/docs/networking.md @@ -27,6 +27,10 @@ Guest virtio-net ←→ TAP (multi-queue) ←TC redirect→ veth ←→ CN - **Bridge mode**: `--bridge ` creates TAP devices directly on an existing Linux bridge (e.g., `--bridge cni0`), bypassing CNI and TC redirect. VMs get IP via DHCP from the bridge. Mutually exclusive with `--network` - **DNS**: Use `--dns` to set custom DNS servers (comma separated) +### Host Device Namespaces + +Cocoon owns two host-wide name families that [GC](gc.md) sweeps by name: bridge-mode TAPs `bt-` in the host netns and per-VM CNI netns `cocoon-` under `/var/run/netns` (the `rm-` TAPs a clone restore hands to CH are transient — CH destroys them itself and no sweep touches them). GC reclaims every entry in its families whose VM it does not know, so two installations sharing a host — a second `root_dir`, or a cocoon-derived runner such as cocoon-macos that provisions through cocoon's bridge/CNI backends — must live in different families, or each sweep tears down the other's live guests. `net_scope` re-keys an installation: two alphanumerics (`mt` gives `mt-` and `mt-`); the fixed length keeps distinct scopes from being prefixes of each other, and `bt` / `rm` are rejected because they alias the legacy and restore families. Set it before the first VM is created — existing devices keep their old names. + ### CNI Configuration All `.conflist` files in `--cni-conf-dir` (default `/etc/cni/net.d`) are loaded at startup. Use `--network ` to select one by its `name` field; omitting defaults to the first file alphabetically. A typical bridge config: diff --git a/hypervisor/cloudhypervisor/clone.go b/hypervisor/cloudhypervisor/clone.go index 7603fafc..4ed19fce 100644 --- a/hypervisor/cloudhypervisor/clone.go +++ b/hypervisor/cloudhypervisor/clone.go @@ -19,8 +19,6 @@ import ( "github.com/cocoonstack/cocoon/utils" ) -const restoreTAPPrefix = "rm" - type cloneResumeOpts struct { vmID string vmCfg *types.VMConfig @@ -96,7 +94,7 @@ func (ch *CloudHypervisor) cloneAfterExtractParsed(ctx context.Context, vmID str // vm.restore attaches the taps named in the snapshot, so concurrent clones of one golden race to attach the source's tap (EBUSY); it cannot attach the clone's real taps either — vm.remove-device releases a tap only after the guest ACKs the eject, so hotSwapNets' add-net would EBUSY on its own tap. Give every restored NIC a clone-unique throwaway tap (auto-created by CH, auto-destroyed once the removed device drops it). netTAPs := make([]string, len(chCfg.Nets)) for i := range netTAPs { - netTAPs[i] = network.TAPName(restoreTAPPrefix, vmID, i) + netTAPs[i] = network.TAPName(network.RestoreTAPPrefix, vmID, i) } consoleSock := hypervisor.ConsoleSockPath(runDir) diff --git a/network/bridge/bridge_linux.go b/network/bridge/bridge_linux.go index 6b3f591f..bb9be5bc 100644 --- a/network/bridge/bridge_linux.go +++ b/network/bridge/bridge_linux.go @@ -25,7 +25,7 @@ var _ network.Network = (*Bridge)(nil) // Bridge is TAP-on-bridge; requires a pre-existing bridge with DHCP + routing. type Bridge struct { - conf *config.Config + tapPrefix string bridgeDev string bridgeIdx int } @@ -46,7 +46,7 @@ func New(conf *config.Config, bridgeDev string) (*Bridge, error) { return nil, fmt.Errorf("%s is not a bridge (type: %s)", bridgeDev, br.Type()) } return &Bridge{ - conf: conf, + tapPrefix: network.BridgeTAPPrefix(conf.NetScope), bridgeDev: bridgeDev, bridgeIdx: br.Attrs().Index, }, nil @@ -57,8 +57,9 @@ func (b *Bridge) Type() string { return typ } func (b *Bridge) Verify(_ context.Context, vmID string, expected []*types.NetworkConfig) error { // Legacy records persisted no NetworkConfigs, so empty means "assume tap0" — callers that legitimately resized to zero NICs must not call Verify. if len(expected) == 0 { - if _, err := netlink.LinkByName(tapName(vmID, 0)); err != nil { - return fmt.Errorf("tap %s: %w", tapName(vmID, 0), err) + name := network.TAPName(b.tapPrefix, vmID, 0) + if _, err := netlink.LinkByName(name); err != nil { + return fmt.Errorf("tap %s: %w", name, err) } return nil } @@ -94,12 +95,12 @@ func (b *Bridge) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, sp if retErr == nil || len(added) == 0 { return } - _ = tearDownTAPs(vmID, added, true) + _ = tearDownTAPs(b.tapPrefix, vmID, added, true) }() configs = make([]*types.NetworkConfig, 0, len(specs)) for _, spec := range specs { - name := tapName(vmID, spec.Index) + name := network.TAPName(b.tapPrefix, vmID, spec.Index) mac := generateMAC() if spec.Existing != nil { mac = spec.Existing.MAC @@ -137,7 +138,7 @@ func (b *Bridge) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, sp } func (b *Bridge) Remove(_ context.Context, vmID string, indices ...int) error { - return tearDownTAPs(vmID, indices, false) + return tearDownTAPs(b.tapPrefix, vmID, indices, false) } // Quiesce and Unquiesce are no-ops: bridge TAPs sit directly on the host bridge, with no TC redirect to storm when the VM stops. @@ -145,7 +146,7 @@ func (b *Bridge) Quiesce(_ context.Context, _ string) error { return nil } func (b *Bridge) Unquiesce(_ context.Context, _ string) error { return nil } func (b *Bridge) Delete(_ context.Context, vmIDs []string) ([]string, error) { - return CleanupTAPs(vmIDs), nil + return CleanupTAPs(b.tapPrefix, vmIDs), nil } // Inspect: bridge has no persistent records. @@ -158,23 +159,23 @@ func (b *Bridge) List(_ context.Context) ([]*types.Network, error) { return nil, nil } -// RegisterGC reclaims orphan bt* TAP devices. +// RegisterGC reclaims orphan bridge TAP devices. func (b *Bridge) RegisterGC(orch *gc.Orchestrator) { - gc.Register(orch, GCModule()) + gc.Register(orch, GCModule(b.tapPrefix)) } // CleanupTAPs removes bridge TAP devices per VM ID; safe without a Bridge instance. -func CleanupTAPs(vmIDs []string) []string { +func CleanupTAPs(tapPrefix string, vmIDs []string) []string { cleaned := make([]string, 0, len(vmIDs)) for _, vmID := range vmIDs { var indices []int for i := 0; ; i++ { - if _, err := netlink.LinkByName(tapName(vmID, i)); err != nil { + if _, err := netlink.LinkByName(network.TAPName(tapPrefix, vmID, i)); err != nil { break } indices = append(indices, i) } - _ = tearDownTAPs(vmID, indices, true) + _ = tearDownTAPs(tapPrefix, vmID, indices, true) cleaned = append(cleaned, vmID) } return cleaned @@ -200,9 +201,9 @@ func attachBridgeUp(tapIndex, bridgeIndex, mtu int) error { return nil } -func tearDownTAPs(vmID string, indices []int, bestEffort bool) error { +func tearDownTAPs(tapPrefix, vmID string, indices []int, bestEffort bool) error { for _, i := range indices { - name := tapName(vmID, i) + name := network.TAPName(tapPrefix, vmID, i) link, err := netlink.LinkByName(name) if err != nil { if bestEffort { @@ -220,10 +221,6 @@ func tearDownTAPs(vmID string, indices []int, bestEffort bool) error { return nil } -func tapName(vmID string, nic int) string { - return network.TAPName(tapPrefix, vmID, nic) -} - func generateMAC() string { buf := make([]byte, 6) //nolint:mnd _, _ = rand.Read(buf) diff --git a/network/bridge/bridge_other.go b/network/bridge/bridge_other.go index 3146eac9..ce72e119 100644 --- a/network/bridge/bridge_other.go +++ b/network/bridge/bridge_other.go @@ -50,4 +50,4 @@ func (b *Bridge) Inspect(_ context.Context, _ string) (*types.Network, error) { func (b *Bridge) List(_ context.Context) ([]*types.Network, error) { return nil, errUnsupported } -func CleanupTAPs(_ []string) []string { return nil } +func CleanupTAPs(_ string, _ []string) []string { return nil } diff --git a/network/bridge/gc_linux.go b/network/bridge/gc_linux.go index 84263ed9..0710856c 100644 --- a/network/bridge/gc_linux.go +++ b/network/bridge/gc_linux.go @@ -16,14 +16,12 @@ import ( "github.com/cocoonstack/cocoon/utils" ) -const tapPrefix = "bt" - type bridgeSnapshot struct { prefixes map[string]struct{} } -// GCModule returns a GC module reclaiming orphan bt* TAP devices; it needs no Bridge instance. -func GCModule() gc.Module[bridgeSnapshot] { +// GCModule returns a GC module reclaiming orphan TAP devices under tapPrefix; it needs no Bridge instance. +func GCModule(tapPrefix string) gc.Module[bridgeSnapshot] { return gc.Module[bridgeSnapshot]{ Name: typ, ReadDB: func(_ context.Context) (bridgeSnapshot, error) { @@ -34,7 +32,7 @@ func GCModule() gc.Module[bridgeSnapshot] { return snap, err } for _, l := range links { - if prefix, ok := parseTAPName(l.Attrs().Name); ok { + if prefix, ok := parseTAPName(tapPrefix, l.Attrs().Name); ok { snap.prefixes[prefix] = struct{}{} } } @@ -64,7 +62,7 @@ func GCModule() gc.Module[bridgeSnapshot] { } for _, l := range links { name := l.Attrs().Name - prefix, ok := parseTAPName(name) + prefix, ok := parseTAPName(tapPrefix, name) if !ok { continue } @@ -82,8 +80,8 @@ func GCModule() gc.Module[bridgeSnapshot] { } } -// parseTAPName extracts the vmID prefix from a bridge TAP name like "bt-". -func parseTAPName(name string) (string, bool) { +// parseTAPName extracts the vmID prefix from a bridge TAP name "-". +func parseTAPName(tapPrefix, name string) (string, bool) { rest, ok := strings.CutPrefix(name, tapPrefix) if !ok { return "", false diff --git a/network/bridge/gc_linux_test.go b/network/bridge/gc_linux_test.go index ed211f9c..101d2dd4 100644 --- a/network/bridge/gc_linux_test.go +++ b/network/bridge/gc_linux_test.go @@ -6,33 +6,33 @@ import "testing" func TestParseTAPName(t *testing.T) { tests := []struct { + tapPrefix string name string wantPrefix string wantOK bool }{ - {name: "bt12345678-0", wantPrefix: "12345678", wantOK: true}, - {name: "bt12345678-1", wantPrefix: "12345678", wantOK: true}, - {name: "btabc-3", wantPrefix: "abc", wantOK: true}, - {name: "btabc-def-5", wantPrefix: "abc-def", wantOK: true}, + {tapPrefix: "bt", name: "bt12345678-0", wantPrefix: "12345678", wantOK: true}, + {tapPrefix: "bt", name: "bt12345678-1", wantPrefix: "12345678", wantOK: true}, + {tapPrefix: "bt", name: "btabc-3", wantPrefix: "abc", wantOK: true}, + {tapPrefix: "bt", name: "btabc-def-5", wantPrefix: "abc-def", wantOK: true}, + {tapPrefix: "mt", name: "mt12345678-0", wantPrefix: "12345678", wantOK: true}, - {name: "wrong-prefix-0"}, - {name: "bt"}, - {name: "bt-0"}, // empty prefix - {name: "bt12345678"}, - {name: ""}, + {tapPrefix: "bt", name: "wrong-prefix-0"}, + {tapPrefix: "bt", name: "bt"}, + {tapPrefix: "bt", name: "bt-0"}, // empty prefix + {tapPrefix: "bt", name: "bt12345678"}, + {tapPrefix: "bt", name: ""}, + {tapPrefix: "mt", name: "bt12345678-0"}, // another installation's TAP } for _, tt := range tests { - label := tt.name - if label == "" { - label = "" - } + label := tt.tapPrefix + "/" + tt.name t.Run(label, func(t *testing.T) { - gotPrefix, gotOK := parseTAPName(tt.name) + gotPrefix, gotOK := parseTAPName(tt.tapPrefix, tt.name) if gotOK != tt.wantOK { - t.Errorf("parseTAPName(%q) ok = %v, want %v", tt.name, gotOK, tt.wantOK) + t.Errorf("parseTAPName(%q, %q) ok = %v, want %v", tt.tapPrefix, tt.name, gotOK, tt.wantOK) } if gotPrefix != tt.wantPrefix { - t.Errorf("parseTAPName(%q) prefix = %q, want %q", tt.name, gotPrefix, tt.wantPrefix) + t.Errorf("parseTAPName(%q, %q) prefix = %q, want %q", tt.tapPrefix, tt.name, gotPrefix, tt.wantPrefix) } }) } diff --git a/network/bridge/gc_other.go b/network/bridge/gc_other.go index f5544f97..b65213ff 100644 --- a/network/bridge/gc_other.go +++ b/network/bridge/gc_other.go @@ -12,7 +12,7 @@ import ( type bridgeSnapshot struct{} // GCModule returns a no-op GC module on non-Linux — bridge TAPs don't exist. -func GCModule() gc.Module[bridgeSnapshot] { +func GCModule(_ string) gc.Module[bridgeSnapshot] { return gc.Module[bridgeSnapshot]{ Name: "bridge", ReadDB: func(_ context.Context) (bridgeSnapshot, error) { diff --git a/network/cni/cni.go b/network/cni/cni.go index a79866e6..817a0fa5 100644 --- a/network/cni/cni.go +++ b/network/cni/cni.go @@ -82,7 +82,7 @@ func (c *CNI) Type() string { return typ } // Verify checks the netns and every expected TAP inside it. func (c *CNI) Verify(_ context.Context, vmID string, expected []*types.NetworkConfig) error { - nsPath := netnsPath(vmID) + nsPath := c.conf.netnsPath(vmID) if _, err := statNetnsFn(nsPath); err != nil { return fmt.Errorf("netns %s: %w", nsPath, err) } @@ -202,7 +202,7 @@ func (c *CNI) setLinkState(ctx context.Context, vmID string, up bool) error { for _, rec := range records { ifNames = append(ifNames, rec.IfName) } - nsPath := netnsPath(vmID) + nsPath := c.conf.netnsPath(vmID) // A host reboot wipes netns but not records; with no netns there is nothing left to quiesce, and failing would retry forever. if _, err := statNetnsFn(nsPath); errors.Is(err, fs.ErrNotExist) { return nil diff --git a/network/cni/cni_test.go b/network/cni/cni_test.go index c989837f..2f667778 100644 --- a/network/cni/cni_test.go +++ b/network/cni/cni_test.go @@ -13,6 +13,7 @@ import ( "github.com/containernetworking/cni/libcni" "github.com/containernetworking/cni/pkg/version" + "github.com/cocoonstack/cocoon/config" metajson "github.com/cocoonstack/cocoon/meta/json" "github.com/cocoonstack/cocoon/meta/tombstone" "github.com/cocoonstack/cocoon/network" @@ -48,6 +49,24 @@ var testNetTables = metajson.TableCodec{Specs: []metajson.TableSpec{ {Key: "tombstones", Table: tombstone.TableName, Optional: true}, }} +func TestNetnsNameHonorsScope(t *testing.T) { + for _, tt := range []struct { + scope string + want string + }{ + {"", "cocoon-vm1"}, + {"mt", "mt-vm1"}, + } { + c := NewConfig(&config.Config{NetScope: tt.scope}) + if got := c.netnsName("vm1"); got != tt.want { + t.Errorf("scope %q: netnsName = %q, want %q", tt.scope, got, tt.want) + } + if got, want := c.netnsPath("vm1"), filepath.Join(netnsBasePath, tt.want); got != want { + t.Errorf("scope %q: netnsPath = %q, want %q", tt.scope, got, want) + } + } +} + func TestLoadConfLists(t *testing.T) { t.Run("empty dir errors", func(t *testing.T) { dir := t.TempDir() @@ -349,8 +368,8 @@ func TestQuiesceUnquiesceTogglesEveryNIC(t *testing.T) { if err := c.Quiesce(ctx, "vm1"); err != nil { t.Fatalf("Quiesce: %v", err) } - if gotNS != netnsPath("vm1") { - t.Fatalf("nsPath = %q, want %q", gotNS, netnsPath("vm1")) + if gotNS != c.conf.netnsPath("vm1") { + t.Fatalf("nsPath = %q, want %q", gotNS, c.conf.netnsPath("vm1")) } slices.Sort(gotIfs) if !slices.Equal(gotIfs, []string{"eth0", "eth1"}) { @@ -400,6 +419,7 @@ func newTestCNIWithStore(t *testing.T) (*CNI, *recordingExec) { } t.Cleanup(func() { _ = store.Close() }) return &CNI{ + conf: NewConfig(&config.Config{RootDir: dir}), meta: store, confLists: map[string]*libcni.NetworkConfigList{"cni-bridge": cl}, defaultName: "cni-bridge", diff --git a/network/cni/config.go b/network/cni/config.go index a33a0a6d..206ca295 100644 --- a/network/cni/config.go +++ b/network/cni/config.go @@ -4,15 +4,13 @@ import ( "path/filepath" "github.com/cocoonstack/cocoon/config" + "github.com/cocoonstack/cocoon/network" "github.com/cocoonstack/cocoon/utils" ) const ( netnsBasePath = "/var/run/netns" - // netnsPrefix scopes GC to cocoon-owned netns (so docker/containerd entries survive). - netnsPrefix = "cocoon-" - - tapPrefix = "tap" + tapPrefix = "tap" ) type Config struct { @@ -32,10 +30,10 @@ func (c *Config) CacheDir() string { return filepath.Join(c.dir(), "cache") } func (c *Config) dir() string { return filepath.Join(c.RootDir, "cni") } func (c *Config) dbDir() string { return filepath.Join(c.dir(), "db") } -func netnsPath(vmID string) string { - return filepath.Join(netnsBasePath, netnsName(vmID)) +func (c *Config) netnsPath(vmID string) string { + return filepath.Join(netnsBasePath, c.netnsName(vmID)) } -func netnsName(vmID string) string { - return netnsPrefix + vmID +func (c *Config) netnsName(vmID string) string { + return network.NetnsPrefix(c.NetScope) + vmID } diff --git a/network/cni/gc.go b/network/cni/gc.go index 674478dd..42e10c3b 100644 --- a/network/cni/gc.go +++ b/network/cni/gc.go @@ -13,16 +13,19 @@ import ( "github.com/cocoonstack/cocoon/gc" "github.com/cocoonstack/cocoon/lock/vmlock" + "github.com/cocoonstack/cocoon/network" "github.com/cocoonstack/cocoon/utils" ) type cniSnapshot struct { dbVMIDs map[string]struct{} - netnsNames []string // VM IDs extracted from /var/run/netns/cocoon-* + netnsNames []string // VM IDs extracted from /var/run/netns/* } // GCModule returns the GC module for orphan netns and stale CNI record cleanup. func (c *CNI) GCModule() gc.Module[cniSnapshot] { + // The prefix scopes GC to this installation's netns, so docker/containerd and peer-installation entries survive. + netnsPrefix := network.NetnsPrefix(c.conf.NetScope) return gc.Module[cniSnapshot]{ Name: typ, Recover: c.gcRecover, @@ -76,7 +79,7 @@ func (c *CNI) GCModule() gc.Module[cniSnapshot] { errs = append(errs, fmt.Errorf("nic release incomplete for %s, netns kept: %w", vmID, tdErr)) continue } - logger.Infof(ctx, "collected id=%s netns=%s reason=orphan", vmID, netnsName(vmID)) + logger.Infof(ctx, "collected id=%s netns=%s reason=orphan", vmID, c.conf.netnsName(vmID)) } return errors.Join(errs...) }, diff --git a/network/cni/lifecycle.go b/network/cni/lifecycle.go index 57dc9fe2..0d7db0d1 100644 --- a/network/cni/lifecycle.go +++ b/network/cni/lifecycle.go @@ -26,8 +26,8 @@ func (c *CNI) Prepare(_ context.Context, vmID string, _ *types.VMConfig) (string if c.cniConf == nil { return "", nil } - nsName := netnsName(vmID) - nsPath := netnsPath(vmID) + nsName := c.conf.netnsName(vmID) + nsPath := c.conf.netnsPath(vmID) if _, err := ensureNetns(nsName, nsPath); err != nil { return "", fmt.Errorf("ensure netns %s: %w", nsName, err) } @@ -49,8 +49,8 @@ func (c *CNI) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, specs vmCfg.Network = confList.Name logger := log.WithFunc("cni.Add") - nsName := netnsName(vmID) - nsPath := netnsPath(vmID) + nsName := c.conf.netnsName(vmID) + nsPath := c.conf.netnsPath(vmID) if err = c.guardAdd(ctx, vmID); err != nil { return nil, err diff --git a/network/cni/teardown.go b/network/cni/teardown.go index b4dac031..a68b9434 100644 --- a/network/cni/teardown.go +++ b/network/cni/teardown.go @@ -54,7 +54,7 @@ func (c *CNI) teardownProtocol(ctx context.Context, vmID string, subset []string cl.Records = append(cl.Records, netCleanupRecord{ID: r.ID, Type: r.Type, IfName: r.IfName}) } if mode == tombstone.ModeAggregate { - cl.Netns = netnsPath(vmID) + cl.Netns = c.conf.netnsPath(vmID) } kind := tombstone.KindRecord if len(cl.Records) == 0 { @@ -91,13 +91,13 @@ func (c *CNI) finishTeardown(ctx context.Context, vmID, leaseID string, mode tom records = append(records, networkRecord{ID: r.ID, Type: r.Type, VMID: vmID, IfName: r.IfName}) } // A retry after the netns already went (crash between netns removal and the sweep) skips TAP deletion — the TAPs died with the ns; CNI DEL still runs, releasing IPAM by container ID without entering the ns. - if _, err := statNetnsFn(netnsPath(vmID)); errors.Is(err, fs.ErrNotExist) { + if _, err := statNetnsFn(c.conf.netnsPath(vmID)); errors.Is(err, fs.ErrNotExist) { deleteTAP = false } - downIDs, tdErr := c.tearDownNICs(ctx, vmID, netnsPath(vmID), records, deleteTAP) + downIDs, tdErr := c.tearDownNICs(ctx, vmID, c.conf.netnsPath(vmID), records, deleteTAP) // Slow cleanup stays outside the transaction (clause 1): the netns goes before the commit so a pure retryable closure never carries side effects. if tdErr == nil && mode == tombstone.ModeAggregate && cl.Netns != "" { - if err := deleteNetnsFn(ctx, netnsName(vmID)); err != nil && !errors.Is(err, fs.ErrNotExist) { + if err := deleteNetnsFn(ctx, c.conf.netnsName(vmID)); err != nil && !errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("remove netns %s (tombstone kept, retry resumes): %w", cl.Netns, err) } } diff --git a/network/network_test.go b/network/network_test.go index a1ecc41b..d26a4e32 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -6,6 +6,39 @@ import ( "github.com/cocoonstack/cocoon/types" ) +func TestScopeFamilies(t *testing.T) { + for _, tt := range []struct { + scope string + wantTAP string + wantNS string + }{ + {"", "bt", "cocoon-"}, + {"mt", "mt", "mt-"}, + } { + if got := BridgeTAPPrefix(tt.scope); got != tt.wantTAP { + t.Errorf("BridgeTAPPrefix(%q) = %q, want %q", tt.scope, got, tt.wantTAP) + } + if got := NetnsPrefix(tt.scope); got != tt.wantNS { + t.Errorf("NetnsPrefix(%q) = %q, want %q", tt.scope, got, tt.wantNS) + } + } +} + +func TestValidateScope(t *testing.T) { + for _, tt := range []struct { + scope string + ok bool + }{ + {"", true}, {"mt", true}, {"A9", true}, + {"m", false}, {"mtap", false}, {"m-", false}, {"m/", false}, + {"bt", false}, {"rm", false}, + } { + if err := ValidateScope(tt.scope); (err == nil) != tt.ok { + t.Errorf("ValidateScope(%q) = %v, want ok=%v", tt.scope, err, tt.ok) + } + } +} + func TestAddRecover(t *testing.T) { existing := []*types.NetworkConfig{ {TAP: "tap0", NumQueues: 2}, diff --git a/network/utils.go b/network/utils.go index 939a2fcd..6ba1b463 100644 --- a/network/utils.go +++ b/network/utils.go @@ -3,6 +3,7 @@ package network import ( "cmp" "fmt" + "regexp" "strconv" "strings" ) @@ -12,8 +13,17 @@ const ( // NetQueueSize: default virtio-net ring depth per queue; 512 balances throughput vs request latency. NetQueueSize = 512 + + // RestoreTAPPrefix names CH's throwaway restore TAPs; a scope equal to it would collide with a clone's own bridge TAPs. + RestoreTAPPrefix = "rm" + + legacyBridgeTAPPrefix = "bt" + legacyNetnsPrefix = "cocoon-" ) +// validScope pins net_scope at two chars: equal-length tags are never proper prefixes of each other, so scoped families stay disjoint by construction. +var validScope = regexp.MustCompile(`^[A-Za-z0-9]{2}$`) + // NetNumQueues returns the virtio-net queue count for cpu; CH uses TX+RX pairs, so the result is always even (>= 2). func NetNumQueues(cpu int) int { if cpu <= 1 { @@ -32,6 +42,33 @@ func ResolveQueues(specQueues, cpu int) int { return cmp.Or(specQueues, NetNumQueues(cpu)) } +// BridgeTAPPrefix returns the host-side bridge TAP name prefix for scope; "" keeps the legacy bt family. +func BridgeTAPPrefix(scope string) string { + return cmp.Or(scope, legacyBridgeTAPPrefix) +} + +// NetnsPrefix returns the per-VM CNI netns name prefix for scope; "" keeps the legacy cocoon- family. +func NetnsPrefix(scope string) string { + if scope == "" { + return legacyNetnsPrefix + } + return scope + "-" +} + +// ValidateScope accepts "" or two alphanumerics that do not alias the legacy or restore TAP families. +func ValidateScope(scope string) error { + if scope == "" { + return nil + } + if !validScope.MatchString(scope) { + return fmt.Errorf("%q must be exactly two alphanumeric chars", scope) + } + if scope == legacyBridgeTAPPrefix || scope == RestoreTAPPrefix { + return fmt.Errorf("%q is reserved", scope) + } + return nil +} + // VMIDPrefix returns the first 8 characters of a VM ID, matching the truncation used by both bridge and CNI TAP device naming. func VMIDPrefix(vmID string) string { if len(vmID) > vmIDPrefixLen { From 08192e25d824d6a403001ba0c243db4aadde4a47 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 17 Aug 2026 16:36:13 +0800 Subject: [PATCH 3/3] test: format net scope validation cases --- network/network_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/network/network_test.go b/network/network_test.go index d26a4e32..b6036794 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -29,9 +29,15 @@ func TestValidateScope(t *testing.T) { scope string ok bool }{ - {"", true}, {"mt", true}, {"A9", true}, - {"m", false}, {"mtap", false}, {"m-", false}, {"m/", false}, - {"bt", false}, {"rm", false}, + {"", true}, + {"mt", true}, + {"A9", true}, + {"m", false}, + {"mtap", false}, + {"m-", false}, + {"m/", false}, + {"bt", false}, + {"rm", false}, } { if err := ValidateScope(tt.scope); (err == nil) != tt.ok { t.Errorf("ValidateScope(%q) = %v, want ok=%v", tt.scope, err, tt.ok)