Skip to content

Commit 62ba22b

Browse files
committed
feat(cli): polish lifecycle UX and make ports/sync idempotent
1 parent 35009ec commit 62ba22b

8 files changed

Lines changed: 328 additions & 56 deletions

File tree

docs/command-reference.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,9 @@
3030
- Maintains managed host alias in `~/.ssh/config` as `okdev-<session>`.
3131
- `--no-tmux`: bypass tmux for this SSH session when tmux mode is enabled.
3232
- `okdev ports`
33+
- Advanced/recovery command. Rebuilds managed SSH `LocalForward` state from `spec.ports` after disconnects or local port changes.
34+
- No-op when managed forwards are already healthy and config is unchanged.
3335
- `okdev sync [--mode up|down|bi] [--background] [--dry-run]`
36+
- Advanced command. Use for foreground sync debugging, or explicit one-way sync (`up`/`down`).
37+
- For default `--mode bi`, no-op when background sync is already active for the session.
3438
- `okdev prune [--ttl-hours 72] [--all-namespaces] [--all-users] [--include-pvc] [--dry-run]`

docs/quickstart.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,17 @@ Notes:
110110
./bin/okdev list --all-users
111111
```
112112

113+
## Advanced Recovery Commands
114+
115+
```bash
116+
# Reconcile managed SSH LocalForward rules from spec.ports
117+
./bin/okdev ports
118+
119+
# Run sync in foreground for troubleshooting or one-way flows
120+
./bin/okdev sync --mode up
121+
./bin/okdev sync --mode down
122+
```
123+
113124
## Teardown and Cleanup
114125

115126
```bash

internal/cli/down.go

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"path/filepath"
77
"strings"
88

9+
"github.com/acmore/okdev/internal/config"
910
"github.com/acmore/okdev/internal/session"
1011
"github.com/spf13/cobra"
1112
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -19,21 +20,31 @@ func newDownCmd(opts *Options) *cobra.Command {
1920
Use: "down",
2021
Short: "Delete a dev session pod",
2122
RunE: func(cmd *cobra.Command, args []string) error {
23+
ui := newUpUI(cmd.OutOrStdout(), cmd.ErrOrStderr())
24+
ui.section("Validate")
2225
cfg, ns, err := loadConfigAndNamespace(opts)
2326
if err != nil {
2427
return err
2528
}
29+
cfgPath, pathErr := config.ResolvePath(opts.ConfigPath)
30+
if pathErr == nil {
31+
ui.stepDone("config", cfgPath)
32+
}
2633
sn, err := resolveSessionNameForUpDown(opts, cfg, ns)
2734
if err != nil {
2835
return err
2936
}
37+
ui.stepDone("session", sn)
38+
ui.stepDone("namespace", ns)
3039
k := newKubeClient(opts)
3140
if err := ensureSessionOwnership(opts, k, ns, sn, false); err != nil {
3241
return err
3342
}
43+
ui.stepDone("ownership", "ok")
3444
ctx, cancel := defaultContext()
3545
defer cancel()
3646
if dryRun {
47+
ui.section("Dry Run")
3748
fmt.Fprintf(cmd.OutOrStdout(), "DRY RUN: session=%s namespace=%s\n", sn, ns)
3849
fmt.Fprintf(cmd.OutOrStdout(), "- would delete pod/%s\n", podName(sn))
3950
if deletePVC && cfg.Spec.Workspace.PVC.ClaimName == "" {
@@ -44,23 +55,48 @@ func newDownCmd(opts *Options) *cobra.Command {
4455
return nil
4556
}
4657

58+
ui.section("Delete")
59+
ui.stepRun("pod", podName(sn))
4760
if err := k.Delete(ctx, ns, "pod", podName(sn), true); err != nil && !apierrors.IsNotFound(err) {
4861
return fmt.Errorf("delete session pod: %w", err)
4962
}
63+
ui.stepDone("pod", "deleted")
5064
if deletePVC && cfg.Spec.Workspace.PVC.ClaimName == "" {
65+
ui.stepRun("pvc", pvcName(cfg, sn))
5166
if err := k.Delete(ctx, ns, "pvc", pvcName(cfg, sn), true); err != nil && !apierrors.IsNotFound(err) {
5267
return fmt.Errorf("delete workspace pvc: %w", err)
5368
}
69+
ui.stepDone("pvc", "deleted")
70+
} else if cfg.Spec.Workspace.PVC.ClaimName == "" {
71+
ui.stepDone("pvc", "retained")
72+
} else {
73+
ui.stepDone("pvc", "external claim")
5474
}
5575
alias := sshHostAlias(sn)
76+
ui.section("Cleanup")
5677
_ = stopManagedSSHForward(alias)
57-
_ = removeSSHConfigEntry(alias)
78+
ui.stepDone("ssh forward", "stopped")
79+
if err := removeSSHConfigEntry(alias); err != nil {
80+
ui.warnf("failed to remove SSH config entry for %s: %v", alias, err)
81+
} else {
82+
ui.stepDone("ssh config", "cleaned")
83+
}
5884
if active, err := session.LoadActiveSession(); err == nil && active == sn {
5985
_ = session.ClearActiveSession()
86+
ui.stepDone("active session", "cleared")
6087
}
61-
fmt.Fprintf(cmd.OutOrStdout(), "Session stopped: %s\n", sn)
88+
ui.printWarnings()
89+
ui.section("Ready")
90+
fmt.Fprintf(cmd.OutOrStdout(), "session: %s\n", sn)
91+
fmt.Fprintf(cmd.OutOrStdout(), "namespace: %s\n", ns)
92+
fmt.Fprintln(cmd.OutOrStdout(), "status: stopped")
6293
if !deletePVC && cfg.Spec.Workspace.PVC.ClaimName == "" {
63-
fmt.Fprintf(cmd.OutOrStdout(), "Workspace PVC retained: %s (use --delete-pvc to remove)\n", pvcName(cfg, sn))
94+
fmt.Fprintf(cmd.OutOrStdout(), "workspace: retained (%s)\n", pvcName(cfg, sn))
95+
fmt.Fprintln(cmd.OutOrStdout(), "note: use --delete-pvc to remove workspace storage")
96+
} else if deletePVC && cfg.Spec.Workspace.PVC.ClaimName == "" {
97+
fmt.Fprintln(cmd.OutOrStdout(), "workspace: deleted")
98+
} else {
99+
fmt.Fprintf(cmd.OutOrStdout(), "workspace: external claim (%s)\n", cfg.Spec.Workspace.PVC.ClaimName)
64100
}
65101
return nil
66102
},

internal/cli/ports.go

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,27 @@ package cli
22

33
import (
44
"fmt"
5-
"log/slog"
6-
"os"
7-
"strconv"
5+
"strings"
86
"time"
97

10-
portsrunner "github.com/acmore/okdev/internal/ports"
8+
"github.com/acmore/okdev/internal/config"
119
"github.com/spf13/cobra"
1210
)
1311

1412
func newPortsCmd(opts *Options) *cobra.Command {
1513
return &cobra.Command{
1614
Use: "ports",
17-
Short: "Start port-forwarding for configured ports",
15+
Short: "Reconcile managed SSH port forwards for configured ports",
1816
RunE: func(cmd *cobra.Command, args []string) error {
1917
cfg, ns, err := loadConfigAndNamespace(opts)
2018
if err != nil {
2119
return err
2220
}
23-
sn, err := resolveSessionName(opts, cfg)
21+
cfgPath, err := config.ResolvePath(opts.ConfigPath)
22+
if err != nil {
23+
return err
24+
}
25+
sn, err := resolveSessionNameForUpDown(opts, cfg, ns)
2426
if err != nil {
2527
return err
2628
}
@@ -33,23 +35,52 @@ func newPortsCmd(opts *Options) *cobra.Command {
3335
if len(cfg.Spec.Ports) == 0 {
3436
return fmt.Errorf("no ports configured in config")
3537
}
36-
37-
forwards := make([]string, 0, len(cfg.Spec.Ports))
38+
validForwards := make([]string, 0, len(cfg.Spec.Ports))
3839
for _, p := range cfg.Spec.Ports {
3940
if p.Local <= 0 || p.Remote <= 0 {
4041
continue
4142
}
42-
forwards = append(forwards, strconv.Itoa(p.Local)+":"+strconv.Itoa(p.Remote))
43+
validForwards = append(validForwards, fmt.Sprintf("%d:%d", p.Local, p.Remote))
4344
}
44-
if len(forwards) == 0 {
45+
if len(validForwards) == 0 {
4546
return fmt.Errorf("no valid ports configured")
4647
}
4748

48-
ctx, cancel := interactiveContext()
49-
defer cancel()
50-
fmt.Fprintf(cmd.OutOrStdout(), "Forwarding %v from session %s\n", forwards, sn)
51-
slog.Debug("ports start", "namespace", ns, "session", sn, "forwards", forwards)
52-
return portsrunner.ForwardWithRetry(ctx, k, ns, podName(sn), forwards, os.Stdout, cmd.ErrOrStderr(), 30*time.Second)
49+
keyPath, err := defaultSSHKeyPath(cfg)
50+
if err != nil {
51+
return fmt.Errorf("resolve SSH key path: %w", err)
52+
}
53+
if err := ensureSSHKeyOnPod(opts, cfg, ns, podName(sn), keyPath); err != nil {
54+
return fmt.Errorf("setup SSH key in pod: %w", err)
55+
}
56+
if err := waitForSSHDReady(opts, cfg, ns, podName(sn), 20*time.Second); err != nil {
57+
return fmt.Errorf("wait for sshd ready: %w", err)
58+
}
59+
alias := sshHostAlias(sn)
60+
changed, err := ensureSSHConfigEntry(alias, sn, ns, cfg.Spec.SSH.User, cfg.Spec.SSH.RemotePort, keyPath, cfgPath, cfg.Spec.Ports)
61+
if err != nil {
62+
return fmt.Errorf("update ~/.ssh/config for managed forwards: %w", err)
63+
}
64+
running, runErr := isManagedSSHForwardRunning(alias)
65+
if runErr != nil {
66+
return fmt.Errorf("check managed SSH forward state: %w", runErr)
67+
}
68+
if running && !changed {
69+
fmt.Fprintf(cmd.OutOrStdout(), "Managed forwards already active for %s: %s\n", alias, strings.Join(validForwards, ", "))
70+
return nil
71+
}
72+
if running {
73+
_ = stopManagedSSHForward(alias)
74+
}
75+
if err := startManagedSSHForward(alias); err != nil {
76+
return fmt.Errorf("start managed SSH forwards: %w", err)
77+
}
78+
if changed {
79+
fmt.Fprintf(cmd.OutOrStdout(), "Managed forwards updated for %s: %s\n", alias, strings.Join(validForwards, ", "))
80+
} else {
81+
fmt.Fprintf(cmd.OutOrStdout(), "Managed forwards started for %s: %s\n", alias, strings.Join(validForwards, ", "))
82+
}
83+
return nil
5384
},
5485
}
5586
}

internal/cli/ssh.go

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func newSSHCmd(opts *Options) *cobra.Command {
9090

9191
sshHost := sshHostAlias(sn)
9292
cfgPath, _ := config.ResolvePath(opts.ConfigPath)
93-
if cfgErr := ensureSSHConfigEntry(sshHost, sn, ns, user, remotePort, keyPath, cfgPath, cfg.Spec.Ports); cfgErr != nil {
93+
if _, cfgErr := ensureSSHConfigEntry(sshHost, sn, ns, user, remotePort, keyPath, cfgPath, cfg.Spec.Ports); cfgErr != nil {
9494
fmt.Fprintf(cmd.ErrOrStderr(), "warning: failed to update ~/.ssh/config: %v\n", cfgErr)
9595
}
9696

@@ -313,14 +313,14 @@ func sshHostAlias(sessionName string) string {
313313
return "okdev-" + sessionName
314314
}
315315

316-
func ensureSSHConfigEntry(hostAlias, sessionName, namespace, user string, remotePort int, keyPath, okdevConfigPath string, forwards []config.PortMapping) error {
316+
func ensureSSHConfigEntry(hostAlias, sessionName, namespace, user string, remotePort int, keyPath, okdevConfigPath string, forwards []config.PortMapping) (bool, error) {
317317
home, err := os.UserHomeDir()
318318
if err != nil {
319-
return err
319+
return false, err
320320
}
321321
sshDir := filepath.Join(home, ".ssh")
322322
if err := os.MkdirAll(sshDir, 0o700); err != nil {
323-
return err
323+
return false, err
324324
}
325325
sshConfigPath := filepath.Join(sshDir, "config")
326326
begin := "# BEGIN OKDEV " + hostAlias
@@ -355,10 +355,41 @@ func ensureSSHConfigEntry(hostAlias, sessionName, namespace, user string, remote
355355
end,
356356
)
357357
block := strings.Join(blockLines, "\n") + "\n"
358-
return updateSSHConfigWithLock(sshConfigPath, func(existing string) string {
359-
updated := stripManagedSSHBlock(existing, begin, end) + "\n" + block
360-
return strings.TrimSpace(updated) + "\n"
361-
})
358+
lockPath := sshConfigPath + ".okdev.lock"
359+
lockFile, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600)
360+
if err != nil {
361+
return false, err
362+
}
363+
defer lockFile.Close()
364+
if err := syscall.Flock(int(lockFile.Fd()), syscall.LOCK_EX); err != nil {
365+
return false, err
366+
}
367+
defer func() {
368+
_ = syscall.Flock(int(lockFile.Fd()), syscall.LOCK_UN)
369+
}()
370+
371+
existingBytes, _ := os.ReadFile(sshConfigPath)
372+
existing := string(existingBytes)
373+
updated := strings.TrimSpace(stripManagedSSHBlock(existing, begin, end)+"\n"+block) + "\n"
374+
if updated == existing {
375+
return false, nil
376+
}
377+
if err := os.WriteFile(sshConfigPath, []byte(updated), 0o600); err != nil {
378+
return false, err
379+
}
380+
return true, nil
381+
}
382+
383+
func isManagedSSHForwardRunning(hostAlias string) (bool, error) {
384+
socketPath, err := sshControlSocketPath(hostAlias)
385+
if err != nil {
386+
return false, err
387+
}
388+
check := exec.Command("ssh", "-S", socketPath, "-O", "check", hostAlias)
389+
if err := check.Run(); err == nil {
390+
return true, nil
391+
}
392+
return false, nil
362393
}
363394

364395
func stripManagedSSHBlock(content, begin, end string) string {

internal/cli/ssh_config_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func TestEnsureSSHConfigEntryIncludesNamespaceInProxyCommand(t *testing.T) {
1919
_ = os.Setenv("HOME", origHome)
2020
}()
2121

22-
err := ensureSSHConfigEntry(
22+
_, err := ensureSSHConfigEntry(
2323
"okdev-test",
2424
"test-session",
2525
"dev-ns",

internal/cli/sync.go

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"syscall"
1111
"time"
1212

13+
"github.com/acmore/okdev/internal/config"
1314
syncengine "github.com/acmore/okdev/internal/sync"
1415
"github.com/spf13/cobra"
1516
)
@@ -27,7 +28,11 @@ func newSyncCmd(opts *Options) *cobra.Command {
2728
if err != nil {
2829
return err
2930
}
30-
sn, err := resolveSessionName(opts, cfg)
31+
cfgPath, err := config.ResolvePath(opts.ConfigPath)
32+
if err != nil {
33+
return err
34+
}
35+
sn, err := resolveSessionNameForUpDown(opts, cfg, ns)
3136
if err != nil {
3237
return err
3338
}
@@ -57,8 +62,17 @@ func newSyncCmd(opts *Options) *cobra.Command {
5762
stopMaintenance := startSessionMaintenance(opts, cfg, ns, sn, cmd.OutOrStdout(), false, true)
5863
defer stopMaintenance()
5964

65+
if !background && mode == "bi" && os.Getenv("OKDEV_SYNCTHING_BACKGROUND_CHILD") != "1" {
66+
if pidPath, err := syncthingPIDPath(sn); err == nil {
67+
if pid, ok := readSyncthingPID(pidPath); ok && processAlive(pid) && processLooksLikeSyncthingSync(pid) {
68+
fmt.Fprintf(cmd.OutOrStdout(), "Syncthing sync already active in background for session %s\n", sn)
69+
return nil
70+
}
71+
}
72+
}
73+
6074
if background && os.Getenv("OKDEV_SYNCTHING_BACKGROUND_CHILD") != "1" {
61-
logPath, started, err := startDetachedSyncthingSync(opts, mode, sn)
75+
logPath, started, err := startDetachedSyncthingSync(opts, mode, sn, ns, cfgPath)
6276
if err != nil {
6377
return err
6478
}
@@ -80,7 +94,7 @@ func newSyncCmd(opts *Options) *cobra.Command {
8094
return cmd
8195
}
8296

83-
func startDetachedSyncthingSync(opts *Options, mode, sessionName string) (string, bool, error) {
97+
func startDetachedSyncthingSync(opts *Options, mode, sessionName, namespace, cfgPath string) (string, bool, error) {
8498
exe, err := os.Executable()
8599
if err != nil {
86100
return "", false, err
@@ -110,14 +124,14 @@ func startDetachedSyncthingSync(opts *Options, mode, sessionName string) (string
110124
}
111125

112126
args := make([]string, 0, 16)
113-
if opts.ConfigPath != "" {
114-
args = append(args, "--config", opts.ConfigPath)
127+
if cfgPath != "" {
128+
args = append(args, "--config", cfgPath)
115129
}
116-
if opts.Session != "" {
117-
args = append(args, "--session", opts.Session)
130+
if sessionName != "" {
131+
args = append(args, "--session", sessionName)
118132
}
119-
if opts.Namespace != "" {
120-
args = append(args, "--namespace", opts.Namespace)
133+
if namespace != "" {
134+
args = append(args, "--namespace", namespace)
121135
}
122136
if opts.Context != "" {
123137
args = append(args, "--context", opts.Context)

0 commit comments

Comments
 (0)