review round and refactor

README.md

@@ -7,18 +7,16 @@ iptables DNAT, no external DHCP server dependency.
 
 **Documentation: [cocoonstack.github.io/cocoon-net](https://cocoonstack.github.io/cocoon-net/)** (source in [`docs/`](docs/)).
 
-```
-cocoon-net init          cocoon-net daemon
-      |                        |
-      v                        v
-Cloud provisioning       Node setup (NICs up, bridge, sysctl, iptables, CNI conflist)
-(alias IPs / ENIs)             |
-      |                        v
-      v                  DHCP server on cni0
-pool.json  <----------        |
-                               v
-                         On lease: add /32 route
-                         On release: del /32 route
+## Architecture
+
+```mermaid
+flowchart LR
+    provision["init / adopt"] --> state["pool.json"]
+    provision --> setup["Node networking"]
+    state --> daemon["daemon"]
+    daemon --> setup
+    daemon --> dhcp["DHCP on cni0"]
+    dhcp --> routes["Lease-based /32 routes"]
 ```
 
 **Two-phase operation**: `cocoon-net init` does one-time cloud provisioning
@@ -30,7 +28,7 @@ long-running service that sets up node networking, serves DHCP, and manages
 routes.
 Full details in [Architecture](docs/architecture.md).
 
-## Supported platforms
+### Supported platforms
 
 | Platform | Mechanism | Max IPs/node |
 |---|---|---|
@@ -47,18 +45,15 @@ sudo cocoon-net daemon
 
 Full steps in [Installation](docs/installation.md).
 
-## Documentation
+## Related projects
 
-- [Architecture](docs/architecture.md) -- the two-phase model, package
-  layout, CNI integration, credentials
-- [Embedded DHCP server](docs/dhcp.md) -- lease lifecycle, the lease file,
-  dynamic host routes, VM traffic isolation
-- [Configuration](docs/configuration.md) -- every flag and environment
-  variable, plus the `pool.json` state schema
-- [Installation](docs/installation.md) -- prebuilt binary, building from
-  source, the systemd unit, command usage
-- [GKE VPC-native networking](docs/gke.md)
-- [Volcengine VPC-native networking](docs/volcengine.md)
+| Project | Role |
+|---|---|
+| [cocoon](https://github.com/cocoonstack/cocoon) | MicroVM engine (Cloud Hypervisor + Firecracker) |
+| [cocoon-common](https://github.com/cocoonstack/cocoon-common) | Shared metadata, Kubernetes, and logging helpers |
+| [cocoon-operator](https://github.com/cocoonstack/cocoon-operator) | CocoonSet and Hibernation CRDs |
+| [cocoon-webhook](https://github.com/cocoonstack/cocoon-webhook) | Admission webhook for sticky scheduling |
+| [vk-cocoon](https://github.com/cocoonstack/vk-cocoon) | Virtual kubelet provider |
 
 ## Development
 
@@ -67,19 +62,8 @@ make build      # build binary
 make test       # run tests with coverage
 make lint       # golangci-lint (linux + darwin)
 make fmt        # gofumpt + goimports
-make help       # show all targets
 ```
 
-## Related projects
-
-| Project | Role |
-|---|---|
-| [cocoon](https://github.com/cocoonstack/cocoon) | MicroVM engine (Cloud Hypervisor + Firecracker) |
-| [cocoon-common](https://github.com/cocoonstack/cocoon-common) | Shared metadata, Kubernetes, and logging helpers |
-| [cocoon-operator](https://github.com/cocoonstack/cocoon-operator) | CocoonSet and Hibernation CRDs |
-| [cocoon-webhook](https://github.com/cocoonstack/cocoon-webhook) | Admission webhook for sticky scheduling |
-| [vk-cocoon](https://github.com/cocoonstack/vk-cocoon) | Virtual kubelet provider |
-
 ## License
 
 [MIT](LICENSE)

cmd/adopt.go

@@ -45,7 +45,14 @@ func runAdopt(cmd *cobra.Command, _ []string) error {
 	if err := resolveSubnet(); err != nil {
 		return err
 	}
-	dnsServers := splitTrim(flagDNS, ",")
+	dnsServers, err := parseDNSFlag()
+	if err != nil {
+		return err
+	}
+	poolSize, err := cmd.Flags().GetInt("pool-size")
+	if err != nil {
+		return err
+	}
 
 	gateway, err := platform.ResolveGateway(flagGateway, flagSubnet)
 	if err != nil {
@@ -57,7 +64,7 @@ func runAdopt(cmd *cobra.Command, _ []string) error {
 	if err != nil {
 		return fmt.Errorf("adopt platform: %w", err)
 	}
-	ips, poolENIs, err := adoptPool(ctx, plat, flagSubnet, gateway, flagPoolSize)
+	ips, poolENIs, err := adoptPool(ctx, plat, flagSubnet, gateway, poolSize)
 	if err != nil {
 		return fmt.Errorf("compute ip list: %w", err)
 	}

cmd/daemon.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cocoonstack/cocoon-net/metrics"
 	"github.com/cocoonstack/cocoon-net/node"
 	"github.com/cocoonstack/cocoon-net/platform"
+	"github.com/cocoonstack/cocoon-net/platform/gke"
 )
 
 const (
@@ -79,6 +80,11 @@ func runDaemon(cmd *cobra.Command, _ []string) error {
 	logger.Infof(ctx, "pool loaded: %d IPs, subnet %s, gateway %s", len(state.IPs), state.Subnet, state.Gateway)
 
 	state.PrimaryNIC = cmp.Or(state.PrimaryNIC, platform.DefaultNIC(state.Platform))
+	if state.Platform == platform.PlatformGKE {
+		if err = gke.New().Adopt(ctx, &platform.Config{NodeName: state.NodeName, SubnetCIDR: state.Subnet, Gateway: state.Gateway, PrimaryNIC: state.PrimaryNIC}); err != nil {
+			return fmt.Errorf("re-apply the guest-agent route fix: %w", err)
+		}
+	}
 	if setupErr := node.Setup(ctx, nodeConfigFromState(state, flagSkipIPTables)); setupErr != nil {
 		return fmt.Errorf("node setup: %w", setupErr)
 	}

cmd/init.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"cmp"
 	"fmt"
 	"strings"
 
@@ -34,12 +35,19 @@ func runInit(cmd *cobra.Command, _ []string) error {
 	if err := resolveSubnet(); err != nil {
 		return err
 	}
-	dnsServers := splitTrim(flagDNS, ",")
+	dnsServers, err := parseDNSFlag()
+	if err != nil {
+		return err
+	}
+	poolSize, err := cmd.Flags().GetInt("pool-size")
+	if err != nil {
+		return err
+	}
 
 	cfg := &platform.Config{
 		NodeName:   flagNodeName,
 		SubnetCIDR: flagSubnet,
-		PoolSize:   flagPoolSize,
+		PoolSize:   poolSize,
 		Gateway:    flagGateway,
 		PrimaryNIC: flagPrimaryNIC,
 	}
@@ -67,9 +75,10 @@ func runInit(cmd *cobra.Command, _ []string) error {
 		DropCIDRs:          flagDropCIDRs,
 		StateDir:           flagStateDir,
 	}
-	if _, loadErr := pool.Load(ctx, flagStateDir); loadErr != nil {
+	existing, loadErr := pool.Load(ctx, flagStateDir)
+	if loadErr != nil {
 		// a first provisioning that fails midway still leaves teardown something to act on
-		if err := state.Save(ctx); err != nil {
+		if err = state.Save(ctx); err != nil {
 			return fmt.Errorf("save seed pool state: %w", err)
 		}
 	}
@@ -82,6 +91,13 @@ func runInit(cmd *cobra.Command, _ []string) error {
 
 	result, err := plat.ProvisionNetwork(ctx, cfg)
 	if err != nil {
+		if result != nil && len(result.ENIIDs) > 0 {
+			recorded := cmp.Or(existing, state)
+			recorded.ENIIDs = mergeENIIDs(recorded.ENIIDs, result.ENIIDs)
+			if saveErr := recorded.Save(ctx); saveErr != nil {
+				logger.Error(ctx, saveErr, "save the attached ENIs after the failed provisioning")
+			}
+		}
 		return fmt.Errorf("provision network: %w", err)
 	}
 	logger.Infof(ctx, "provisioned %d IPs on subnet %s", len(result.IPs), result.SubnetCIDR)

cmd/pidfile.go

@@ -34,7 +34,7 @@ func acquirePIDFile() error {
 		if !errors.Is(err, fs.ErrExist) {
 			return fmt.Errorf("create pid file: %w", err)
 		}
-		if err := checkExistingPID(); err != nil {
+		if err := checkExistingPID(pidFile); err != nil {
 			return err
 		}
 		_ = os.Remove(pidFile)
@@ -43,8 +43,8 @@ func acquirePIDFile() error {
 }
 
 // a missing, corrupt, or stale (process dead) PID file is safe to overwrite
-func checkExistingPID() error {
-	data, err := os.ReadFile(pidFile)
+func checkExistingPID(path string) error {
+	data, err := os.ReadFile(filepath.Clean(path))
 	if err != nil {
 		return nil
 	}

cmd/status.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/projecteru2/core/log"
 	"github.com/spf13/cobra"
@@ -44,7 +45,7 @@ func runStatus(cmd *cobra.Command, _ []string) error {
 	fmt.Printf("Subnet:     %s\n", state.Subnet)
 	fmt.Printf("Gateway:    %s\n", state.Gateway)
 	fmt.Printf("IPs:        %d\n", len(state.IPs))
-	fmt.Printf("Updated:    %s\n", state.UpdatedAt.Format("2006-01-02T15:04:05Z"))
+	fmt.Printf("Updated:    %s\n", state.UpdatedAt.Format(time.RFC3339))
 	if status != nil {
 		if len(status.ENIs) > 0 {
 			fmt.Printf("ENIs:       %d\n", len(status.ENIs))

cmd/teardown.go

@@ -23,6 +23,7 @@ func newTeardownCmd() *cobra.Command {
 	cmd.Flags().StringVar(&flagStateDir, "state-dir", defaultStateDir, "state directory")
 	cmd.Flags().StringVar(&flagLeaseFile, "lease-file", defaultLeaseFile, "lease persistence file")
 	cmd.Flags().BoolVar(&flagDryRun, "dry-run", false, "show what would be done without making changes")
+	cmd.Flags().BoolVar(&flagForce, "force", false, "tear down even while a cocoon-net daemon is running")
 
 	return cmd
 }
@@ -41,6 +42,11 @@ func runTeardown(cmd *cobra.Command, _ []string) error {
 			state.Platform, state.NodeName, state.Subnet)
 		return nil
 	}
+	if !flagForce {
+		if err = checkExistingPID(pidFile); err != nil {
+			return fmt.Errorf("%w; stop it first or pass --force", err)
+		}
+	}
 
 	plat, err := newPlatform(ctx, state.Platform)
 	if err != nil {

cmd/teardown_test.go

@@ -1,6 +1,11 @@
 package cmd
 
-import "testing"
+import (
+	"os"
+	"path/filepath"
+	"strconv"
+	"testing"
+)
 
 func TestTeardownLeaseFileFlag(t *testing.T) {
 	cmd := newTeardownCmd()
@@ -12,3 +17,28 @@ func TestTeardownLeaseFileFlag(t *testing.T) {
 		t.Fatalf("lease-file default = %q, want %q", flag.DefValue, defaultLeaseFile)
 	}
 }
+
+func TestTeardownForceFlag(t *testing.T) {
+	if newTeardownCmd().Flags().Lookup("force") == nil {
+		t.Fatal("teardown has no force flag")
+	}
+}
+
+func TestCheckExistingPIDRefusesALiveDaemon(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "cocoon-net.pid")
+	if err := os.WriteFile(path, []byte(strconv.Itoa(os.Getpid())), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := checkExistingPID(path); err == nil {
+		t.Fatal("a pidfile naming a live process must be refused")
+	}
+	if err := os.WriteFile(path, []byte("999999"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := checkExistingPID(path); err != nil {
+		t.Fatalf("a stale pidfile must not block: %v", err)
+	}
+	if err := checkExistingPID(filepath.Join(t.TempDir(), "absent")); err != nil {
+		t.Fatalf("a missing pidfile must not block: %v", err)
+	}
+}

cmd/utils.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"slices"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -22,12 +23,12 @@ var (
 	flagPlatform     string
 	flagNodeName     string
 	flagSubnet       string
-	flagPoolSize     int
 	flagGateway      string
 	flagPrimaryNIC   string
 	flagDNS          string
 	flagStateDir     string
 	flagDryRun       bool
+	flagForce        bool
 	flagDropInternal bool
 	flagDropCIDRs    []string
 
@@ -39,10 +40,10 @@ func registerCommonFlags(cmd *cobra.Command, defaultPoolSize int) {
 	cmd.Flags().StringVar(&flagPlatform, "platform", "", "cloud platform (gke|volcengine); auto-detected from instance metadata if omitted")
 	cmd.Flags().StringVar(&flagNodeName, "node-name", "", "virtual node name (required)")
 	cmd.Flags().StringVar(&flagSubnet, "subnet", "", "VM subnet CIDR, e.g. 172.20.100.0/24 (required)")
-	cmd.Flags().IntVar(&flagPoolSize, "pool-size", defaultPoolSize, "number of IPs in the pool")
+	cmd.Flags().Int("pool-size", defaultPoolSize, "number of IPs in the pool")
 	cmd.Flags().StringVar(&flagGateway, "gateway", "", "gateway IP on cni0 (default: first IP in subnet)")
 	cmd.Flags().StringVar(&flagPrimaryNIC, "primary-nic", "", "host primary NIC (default: eth0 on volcengine, ens4 otherwise)")
-	cmd.Flags().StringVar(&flagDNS, "dns", "8.8.8.8,1.1.1.1", "comma-separated DNS servers for DHCP clients")
+	cmd.Flags().StringVar(&flagDNS, "dns", "8.8.8.8,1.1.1.1", "comma-separated IPv4 DNS servers for DHCP clients")
 	cmd.Flags().StringVar(&flagStateDir, "state-dir", defaultStateDir, "state directory")
 	cmd.Flags().BoolVar(&flagDryRun, "dry-run", false, "show what would be done without making changes")
 	cmd.Flags().BoolVar(&flagDropInternal, "drop-internal-access", false, "block VM-to-VM traffic within the cocoon subnet")
@@ -94,6 +95,25 @@ func resolveSubnet() error {
 	return nil
 }
 
+func parseDNSFlag() ([]string, error) {
+	servers := splitTrim(flagDNS, ",")
+	for _, s := range servers {
+		if net.ParseIP(s).To4() == nil {
+			return nil, fmt.Errorf("--dns entry %q is not an IPv4 address", s)
+		}
+	}
+	return servers, nil
+}
+
+func mergeENIIDs(recorded, attached []string) []string {
+	for _, id := range attached {
+		if !slices.Contains(recorded, id) {
+			recorded = append(recorded, id)
+		}
+	}
+	return recorded
+}
+
 func splitTrim(s, sep string) []string {
 	var parts []string
 	for p := range strings.SplitSeq(s, sep) {