From a6d2ccf6ef31559e0628c001db32cdf5c90c9dd5 Mon Sep 17 00:00:00 2001 From: bussyjd Date: Sat, 18 Jul 2026 18:14:34 +0400 Subject: [PATCH] fix(stack): don't treat own cluster's ports as conflicts under --force Reported by a teammate during v0.14.0-rc0 field testing. https://claude.ai/code/session_01PnhCQLz7CHuDBUhWd5xF8v --- internal/stack/backend.go | 7 +++-- internal/stack/backend_k3d.go | 54 ++++++++++++++++++++++++++++++---- internal/stack/backend_k3s.go | 2 +- internal/stack/backend_test.go | 4 +-- internal/stack/stack.go | 2 +- internal/stack/stack_test.go | 37 +++++++++++++++++++++-- 6 files changed, 92 insertions(+), 14 deletions(-) diff --git a/internal/stack/backend.go b/internal/stack/backend.go index c3719504..cb272d6b 100644 --- a/internal/stack/backend.go +++ b/internal/stack/backend.go @@ -24,8 +24,11 @@ type Backend interface { // Name returns the backend identifier (e.g., "k3d", "k3s") Name() string - // Init generates backend-specific cluster configuration files - Init(cfg *config.Config, u *ui.UI, stackID string) error + // Init generates backend-specific cluster configuration files. + // force is true when the caller is about to tear down and recreate the + // existing cluster (obol stack init --force); backends may use it to + // treat ports currently held by that cluster as non-conflicting. + Init(cfg *config.Config, u *ui.UI, stackID string, force bool) error // Up creates or starts the cluster and returns kubeconfig contents Up(cfg *config.Config, u *ui.UI, stackID string) (kubeconfigData []byte, err error) diff --git a/internal/stack/backend_k3d.go b/internal/stack/backend_k3d.go index 9a8baa70..bf46e03d 100644 --- a/internal/stack/backend_k3d.go +++ b/internal/stack/backend_k3d.go @@ -52,7 +52,7 @@ func (b *K3dBackend) Prerequisites(cfg *config.Config) error { return nil } -func (b *K3dBackend) Init(cfg *config.Config, u *ui.UI, stackID string) error { +func (b *K3dBackend) Init(cfg *config.Config, u *ui.UI, stackID string, force bool) error { absDataDir, err := filepath.Abs(cfg.DataDir) if err != nil { return fmt.Errorf("failed to get absolute path for data directory: %w", err) @@ -63,6 +63,8 @@ func (b *K3dBackend) Init(cfg *config.Config, u *ui.UI, stackID string) error { return fmt.Errorf("failed to get absolute path for config directory: %w", err) } + stackName := "obol-stack-" + stackID + // Template k3d config with actual values k3dConfig := embed.K3dConfig k3dConfig = strings.ReplaceAll(k3dConfig, "{{STACK_ID}}", stackID) @@ -71,7 +73,8 @@ func (b *K3dBackend) Init(cfg *config.Config, u *ui.UI, stackID string) error { // Rewrite occupied host-port mappings so k3d cluster create won't fail // when another local stack is already bound to the default ingress ports. - k3dConfig = stripConflictingPorts(k3dConfig, u) + // Under --force, ports held by this cluster's own serverlb are not conflicts. + k3dConfig = stripConflictingPorts(k3dConfig, u, force, stackName) k3dConfigPath := filepath.Join(cfg.ConfigDir, k3dConfigFile) if err := os.WriteFile(k3dConfigPath, []byte(k3dConfig), 0o600); err != nil { @@ -273,8 +276,14 @@ func portBlock(host, container int) string { // stripConflictingPorts removes occupied default k3d ingress mappings. If all // default host ports for a container port are occupied, it adds an ephemeral // host-port mapping so multiple dev stacks can coexist on the same machine. -func stripConflictingPorts(k3dConfig string, u *ui.UI) string { - return rewriteConflictingPorts(k3dConfig, u, hostPortAvailable, pickAvailableHostPort) +// When force is true, ports published by the existing clusterName load-balancer +// are treated as owned (not foreign conflicts). +func stripConflictingPorts(k3dConfig string, u *ui.UI, force bool, clusterName string) string { + owned := map[int]bool{} + if force { + owned = k3dOwnedHostPorts(clusterName) + } + return rewriteConflictingPorts(k3dConfig, u, hostPortAvailable, pickAvailableHostPort, owned) } func rewriteConflictingPorts( @@ -282,6 +291,7 @@ func rewriteConflictingPorts( u *ui.UI, available func(int) bool, pickPort func() (int, error), + owned map[int]bool, ) string { hasMapping := map[int]bool{} @@ -294,7 +304,7 @@ func rewriteConflictingPorts( if !strings.Contains(k3dConfig, block) { continue } - if available(c.hostPort) { + if available(c.hostPort) || owned[c.hostPort] { hasMapping[c.containerPort] = true continue } @@ -368,6 +378,36 @@ func hostPortAvailable(port int) bool { return checkPortsAvailable([]int{port}) == nil } +// k3dOwnedHostPorts returns the host ports currently published by the +// existing obol-managed k3d cluster's load-balancer container — the +// cluster that "--force" is about to tear down and recreate. Used so +// rewriteConflictingPorts does not treat a port as a foreign conflict when +// it is actually held by the very cluster being recreated. Returns an +// empty map (no exceptions) if the container can't be found/inspected — +// callers fall back to the normal availability check in that case. +func k3dOwnedHostPorts(clusterName string) map[int]bool { + out, err := exec.Command("docker", "port", "k3d-"+clusterName+"-serverlb").Output() + if err != nil { + return map[int]bool{} + } + + owned := map[int]bool{} + for _, line := range strings.Split(string(out), "\n") { + line = strings.TrimSpace(line) + + idx := strings.LastIndex(line, ":") + if idx == -1 { + continue + } + + if port, err := strconv.Atoi(strings.TrimSpace(line[idx+1:])); err == nil { + owned[port] = true + } + } + + return owned +} + func pickAvailableHostPort() (int, error) { ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -403,7 +443,9 @@ func ensureK3dPortsAvailable(configPath string, u *ui.UI) { } original := string(data) - updated := stripConflictingPorts(original, u) + // force=false: this runs from Up() right before a fresh k3d cluster create, + // which only happens when IsRunning() is false — any prior instance is gone. + updated := stripConflictingPorts(original, u, false, "") if updated != original { if err := os.WriteFile(configPath, []byte(updated), 0o600); err != nil { diff --git a/internal/stack/backend_k3s.go b/internal/stack/backend_k3s.go index c8e56be5..b536cea7 100644 --- a/internal/stack/backend_k3s.go +++ b/internal/stack/backend_k3s.go @@ -53,7 +53,7 @@ func (b *K3sBackend) Prerequisites(cfg *config.Config) error { return nil } -func (b *K3sBackend) Init(cfg *config.Config, u *ui.UI, stackID string) error { +func (b *K3sBackend) Init(cfg *config.Config, u *ui.UI, stackID string, force bool) error { absDataDir, err := filepath.Abs(cfg.DataDir) if err != nil { return fmt.Errorf("failed to get absolute path for data directory: %w", err) diff --git a/internal/stack/backend_test.go b/internal/stack/backend_test.go index 5ff99ab7..a9b96454 100644 --- a/internal/stack/backend_test.go +++ b/internal/stack/backend_test.go @@ -219,7 +219,7 @@ func TestK3dBackendInit(t *testing.T) { b := &K3dBackend{} u := ui.New(false) - if err := b.Init(cfg, u, "test-stack"); err != nil { + if err := b.Init(cfg, u, "test-stack", false); err != nil { t.Fatalf("K3dBackend.Init() error: %v", err) } @@ -267,7 +267,7 @@ func TestK3sBackendInit(t *testing.T) { b := &K3sBackend{} u := ui.New(false) - if err := b.Init(cfg, u, "my-cluster"); err != nil { + if err := b.Init(cfg, u, "my-cluster", false); err != nil { t.Fatalf("K3sBackend.Init() error: %v", err) } diff --git a/internal/stack/stack.go b/internal/stack/stack.go index 9ef3fcc6..6c1b5464 100644 --- a/internal/stack/stack.go +++ b/internal/stack/stack.go @@ -117,7 +117,7 @@ func Init(cfg *config.Config, u *ui.UI, force bool, backendName string, skipConf } // Generate backend-specific config - if err := backend.Init(cfg, u, stackID); err != nil { + if err := backend.Init(cfg, u, stackID, force); err != nil { return err } diff --git a/internal/stack/stack_test.go b/internal/stack/stack_test.go index 2d4866e8..cb78df55 100644 --- a/internal/stack/stack_test.go +++ b/internal/stack/stack_test.go @@ -175,7 +175,7 @@ func TestRewriteConflictingPorts_PreservesAvailableFallbacks(t *testing.T) { }, func() (int, error) { t.Fatal("should not pick an ephemeral port when fallbacks are available") return 0, nil - }) + }, nil) for _, unexpected := range []string{"- port: 80:80", "- port: 443:443"} { if strings.Contains(got, unexpected) { @@ -207,7 +207,7 @@ func TestRewriteConflictingPorts_PicksEphemeralWhenAllDefaultsBusy(t *testing.T) port := picks[0] picks = picks[1:] return port, nil - }) + }, nil) for _, unexpected := range []string{"- port: 80:80", "- port: 8080:80", "- port: 443:443", "- port: 8443:443"} { if strings.Contains(got, unexpected) { @@ -224,6 +224,39 @@ func TestRewriteConflictingPorts_PicksEphemeralWhenAllDefaultsBusy(t *testing.T) } } +func TestRewriteConflictingPorts_ForceSkipsOwnedPorts(t *testing.T) { + fullConfig := "ports:\n" + + portBlock(80, 80) + + portBlock(8080, 80) + + portBlock(443, 443) + + portBlock(8443, 443) + + "options:\n" + + // Every default host port reads as occupied. 80 and 443 are "owned" — + // held by the existing obol cluster that --force is about to recreate + // — and must be kept. 8080/8443 are genuinely foreign occupants (not + // owned) and must still be stripped. + got := rewriteConflictingPorts(fullConfig, ui.New(false), + func(int) bool { return false }, + func() (int, error) { + t.Fatal("should not need an ephemeral port: both container ports resolve via the owned mapping") + return 0, nil + }, + map[int]bool{80: true, 443: true}, + ) + + for _, expected := range []string{"- port: 80:80", "- port: 443:443"} { + if !strings.Contains(got, expected) { + t.Fatalf("expected owned mapping %s to be preserved under force:\n%s", expected, got) + } + } + for _, unexpected := range []string{"- port: 8080:80", "- port: 8443:443"} { + if strings.Contains(got, unexpected) { + t.Fatalf("expected genuinely foreign-occupied mapping %s to still be stripped:\n%s", unexpected, got) + } + } +} + func TestEnsureK3dPortsAvailable_NoDefaultMappings(t *testing.T) { // Verify the file read/write path stays a no-op for configs that do not // contain the default ingress mappings.