Skip to content

Commit b437497

Browse files
acmoreclaude
andauthored
feat(sync,exec): okdev sync wait; split command-exit from delivery-failure exit semantics (#159)
* feat(sync,exec): add okdev sync wait; split command-exit from delivery-failure semantics okdev sync wait [session] [--timeout 10m] blocks until every sync mapping has zero pending bytes in both directions, giving scripts the edit-run guarantee (vim f.py && okdev sync wait && okdev exec -- python f.py). It forces a rescan on both sides first so files written moments earlier are indexed immediately instead of after the FS-watcher delay, and fails fast with guidance when background sync is not running. Fanout exec failures are now layered: when every failing pod merely reported a non-zero command exit, the summary is COMMAND EXITED NON-ZERO and okdev exits with the remote status (single pod, preserving the documented verbatim-exit contract) or 1 (multi-pod). Any delivery failure (unreachable, timeout, container gone) keeps the FAILED framing and maps to new dedicated exit code 69 (sysexits EX_UNAVAILABLE) via the ErrExecInfraFailure sentinel, so scripts can tell "my command failed" from "okdev could not deliver the command" without parsing output. Covered by unit tests plus kind e2e scenarios (sync wait convergence guarantee, single-pod exit-status preservation, multi-pod non-zero framing). Docs and skill references updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(e2e): write sync-wait probe file inside the sync root Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 5104077 commit b437497

14 files changed

Lines changed: 382 additions & 28 deletions

docs/command-reference.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@ Commands that resolve a session distinguish failure classes so scripts and
1717
agents can react without launching a diagnostic chain on every blip:
1818

1919
- `0`: success.
20-
- non-zero from a remote command (e.g. `okdev exec -- false``1`, `exit 7``7`): the remote process's own status, always preserved.
20+
- non-zero from a remote command (e.g. `okdev exec -- false``1`, `exit 7``7`): the remote process's own status, preserved verbatim on single-pod runs. A multi-pod fanout where the command exited non-zero on some pods exits `1` (there is no single status to preserve).
2121
- `74`: the cluster was reachable but the session has no pods — it is genuinely gone.
2222
- `78`: a transient cluster-contact failure (API timeout, connection refused/reset, server overload). okdev already retries once with backoff before surfacing this; on `78` the caller should retry rather than treat the session as dead.
23-
- `1`: any other error (configuration, permissions/RBAC, a failed job result, etc.).
23+
- `69`: a fanout `okdev exec` could not run the command on at least one pod (unreachable, timeout, container gone) — a delivery failure, as opposed to the command itself exiting non-zero, which stays exit `1`.
24+
- `1`: any other error (configuration, permissions/RBAC, a failed job result, a fanout command that exited non-zero on some pods, etc.).
2425

2526
## Commands
2627

@@ -50,6 +51,7 @@ agents can react without launching a diagnostic chain on every blip:
5051
- `okdev ports`
5152
- `okdev port-forward [session] <local:remote>... [--address <addr>[,<addr>...]] [--pod <name> | --role <role>] [--ready-only]`
5253
- `okdev sync [--mode up|down|bi] [--foreground] [--reset] [--dry-run]`
54+
- `okdev sync wait [session] [--timeout 10m]`
5355
- `okdev prune [--ttl-hours 72] [--all-namespaces] [--all-users] [--include-pvc] [--dry-run]`
5456
- `okdev migrate [--template <name>] [--set key=value] [--dry-run] [--yes]`
5557
- `okdev upgrade`
@@ -98,6 +100,7 @@ agents can react without launching a diagnostic chain on every blip:
98100
- `--parallel` runs declared groups in parallel. `--fanout` caps total concurrent pod executions across all groups.
99101
- `--script <file>` uploads a local script file and runs it across the targeted pods. In v1 this is local-file-only; `--script -` is not supported.
100102
- With `--script`, everything after `--` is passed to the script as positional arguments (`$1`, `$2`, … / `"$@"`): `okdev exec --script ./bench.sh -- --steps 100 --warmup 10` runs `bench.sh` on each pod with `$1=--steps $2=100 $3=--warmup $4=10`. Arguments containing spaces are preserved.
103+
- Failure framing for `-- command...` fanout: when every failing pod merely reported a non-zero exit code from the command itself, the summary is headed `COMMAND EXITED NON-ZERO:` and `okdev` exits with the remote status on a single-pod run (`exit 3``3`) or `1` on a multi-pod run — either way `okdev exec -- cmd && next` still short-circuits `&&` chains. When any pod could not run the command at all (unreachable, timeout, container gone), the summary is headed `FAILED:` and `okdev` exits `69`, letting scripts tell "my command failed" apart from "delivery failed" without parsing output.
101104
- The `-- command...` path is non-interactive fanout execution, not a terminal session. TTY-dependent programs such as `watch`, `top`, `htop`, or full-screen TUIs are not expected to work there; use `okdev ssh` or `okdev exec` without `-- command...` to open an interactive shell first.
102105
- `-- command...` preserves raw argv to the remote process. If you choose `bash -lc '...'`, that remote shell layer is still yours.
103106
- `--script` avoids the quoting/stitching problem for non-trivial shell pipelines by uploading the file and running it remotely instead of embedding the body in a one-liner.
@@ -326,6 +329,13 @@ agents can react without launching a diagnostic chain on every blip:
326329
- `--reset --force --local` / `--reset --force --mesh`: force reset a specific component.
327330
- `okdev init` writes the starter config and, for built-in templates, a starter local `.stignore` file for the initialized sync root.
328331

332+
### `okdev sync wait [session] [--timeout 10m]`
333+
334+
- Blocks until every configured sync mapping has zero pending bytes in **both** directions, then returns — the edit-run loop guarantee: `vim train.py && okdev sync wait && okdev exec -- python train.py`.
335+
- Triggers an immediate rescan on both sides before waiting, so files written moments earlier are picked up now instead of after the filesystem-watcher delay.
336+
- Purely a wait: it does not start or repair sync. If background sync is not running it fails fast with guidance (`okdev sync` starts it, `okdev sync --reset` repairs it).
337+
- Prints pending-byte progress while waiting; exits non-zero if convergence is not reached within `--timeout`.
338+
329339
### `okdev upgrade`
330340

331341
- Checks the latest GitHub release and upgrades the `okdev` binary in place.

internal/cli/cluster_errors.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ const (
2121
// ExitSessionNotFound signals that the cluster was reachable but the
2222
// requested session has no pods — a real "it's gone" condition.
2323
ExitSessionNotFound = 74
24+
// ExitExecInfraFailure signals that a fanout exec could not run the
25+
// command on at least one pod (unreachable, timeout, container gone) —
26+
// as opposed to the command itself exiting non-zero, which stays exit
27+
// 1. Follows sysexits EX_UNAVAILABLE.
28+
ExitExecInfraFailure = 69
2429
)
2530

2631
// ErrSessionNotFound is returned when session resolution reaches the cluster
@@ -33,6 +38,13 @@ var ErrSessionNotFound = errors.New("session not found")
3338
// session that no longer exists.
3439
var ErrTransientCluster = errors.New("transient cluster contact failure")
3540

41+
// ErrExecInfraFailure marks a fanout exec where at least one pod could not
42+
// run the command at all (transport, timeout, container gone). Wrapped into
43+
// the returned error so the top-level classifier maps it to
44+
// ExitExecInfraFailure, letting scripts tell real delivery failures apart
45+
// from a command that simply exited non-zero (plain exit 1).
46+
var ErrExecInfraFailure = errors.New("exec delivery failure")
47+
3648
// ClassifiedExitCode maps okdev's sentinel cluster errors to their dedicated
3749
// exit codes. It returns ok=false for anything it does not recognize so the
3850
// caller can fall back to its default (e.g. a remote command's own exit code,
@@ -44,6 +56,8 @@ func ClassifiedExitCode(err error) (int, bool) {
4456
return ExitSessionNotFound, true
4557
case errors.Is(err, ErrTransientCluster):
4658
return ExitTransientCluster, true
59+
case errors.Is(err, ErrExecInfraFailure):
60+
return ExitExecInfraFailure, true
4761
default:
4862
return 0, false
4963
}

internal/cli/pdsh.go

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,20 +1003,46 @@ func runMultiExec(ctx context.Context, client connect.ExecClient, namespace stri
10031003
}
10041004
}
10051005
if len(failures) > 0 {
1006-
parts := make([]string, 0, len(failures))
1007-
for _, f := range failures {
1008-
part := formatPodExecFailureSummary(f.pod, container, f.err)
1009-
if isContainerUnavailableExecError(f.err) {
1010-
if hint := containerUnavailableHint(ctx, client, namespace, f.pod, container); hint != "" {
1011-
part += " " + hint
1012-
}
1006+
return reportFanoutFailures(ctx, client, namespace, container, failures, len(pods), stderr)
1007+
}
1008+
return nil
1009+
}
1010+
1011+
// reportFanoutFailures prints the per-pod failure summary and builds the
1012+
// command error. When every failure is the remote command merely exiting
1013+
// non-zero (grep with no match, a failing test), the run is reported as a
1014+
// command result — no "pods failed" framing; a single-pod run preserves the
1015+
// remote exit status verbatim, a multi-pod run exits 1. Any delivery
1016+
// failure (transport, timeout, container gone) keeps the FAILED framing and
1017+
// wraps ErrExecInfraFailure so okdev exits with the dedicated infra code
1018+
// (ExitExecInfraFailure), letting scripts and monitors tell the two apart.
1019+
func reportFanoutFailures(ctx context.Context, client connect.ExecClient, namespace, container string, failures []podExecResult, total int, stderr io.Writer) error {
1020+
remoteExitOnly := true
1021+
parts := make([]string, 0, len(failures))
1022+
for _, f := range failures {
1023+
if kind, _ := classifyPodExecFailure(f.err); kind != "remote-exit" {
1024+
remoteExitOnly = false
1025+
}
1026+
part := formatPodExecFailureSummary(f.pod, container, f.err)
1027+
if isContainerUnavailableExecError(f.err) {
1028+
if hint := containerUnavailableHint(ctx, client, namespace, f.pod, container); hint != "" {
1029+
part += " " + hint
10131030
}
1014-
parts = append(parts, part)
10151031
}
1016-
fmt.Fprintf(stderr, "\nFAILED:\n%s\n", strings.Join(parts, "\n"))
1017-
return fmt.Errorf("%d of %d pods failed", len(failures), len(pods))
1032+
parts = append(parts, part)
1033+
}
1034+
if remoteExitOnly {
1035+
fmt.Fprintf(stderr, "\nCOMMAND EXITED NON-ZERO:\n%s\n", strings.Join(parts, "\n"))
1036+
if total == 1 {
1037+
// Single-pod runs keep the documented "remote exit status is
1038+
// always preserved" contract: wrap the pod's own ExitError so
1039+
// the top-level exit-code mapper propagates it verbatim.
1040+
return fmt.Errorf("command exited non-zero: %w", failures[0].err)
1041+
}
1042+
return fmt.Errorf("command exited non-zero on %d of %d pod(s)", len(failures), total)
10181043
}
1019-
return nil
1044+
fmt.Fprintf(stderr, "\nFAILED:\n%s\n", strings.Join(parts, "\n"))
1045+
return fmt.Errorf("%d of %d pods failed: %w", len(failures), total, ErrExecInfraFailure)
10201046
}
10211047

10221048
func formatPodExecFailureSummary(podName, container string, err error) string {
@@ -1588,18 +1614,7 @@ func runMultiExecScript(ctx context.Context, client scriptCopyClient, namespace
15881614
}
15891615
}
15901616
if len(failures) > 0 {
1591-
parts := make([]string, 0, len(failures))
1592-
for _, f := range failures {
1593-
part := formatPodExecFailureSummary(f.pod, container, f.err)
1594-
if isContainerUnavailableExecError(f.err) {
1595-
if hint := containerUnavailableHint(ctx, client, namespace, f.pod, container); hint != "" {
1596-
part += " " + hint
1597-
}
1598-
}
1599-
parts = append(parts, part)
1600-
}
1601-
fmt.Fprintf(stderr, "\nFAILED:\n%s\n", strings.Join(parts, "\n"))
1602-
return fmt.Errorf("%d of %d pods failed", len(failures), len(pods))
1617+
return reportFanoutFailures(ctx, client, namespace, container, failures, len(pods), stderr)
16031618
}
16041619
return nil
16051620
}

internal/cli/pdsh_fanout_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func TestRunExecPodGroupsSurfacesGroupErrors(t *testing.T) {
307307
Stdout: &stdout,
308308
Stderr: &stderr,
309309
})
310-
if err == nil || err.Error() != "1 of 1 pods failed" {
310+
if err == nil || err.Error() != "1 of 1 pods failed: exec delivery failure" {
311311
t.Fatalf("expected verbatim single-group error, got %v", err)
312312
}
313313
}

internal/cli/pdsh_test.go

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"github.com/acmore/okdev/internal/kube"
1919
k8sexec "k8s.io/client-go/util/exec"
20+
utilexec "k8s.io/utils/exec"
2021
)
2122

2223
func TestFilterPodsByRole(t *testing.T) {
@@ -711,15 +712,50 @@ func TestRunMultiExecPartialFailure(t *testing.T) {
711712
if err == nil {
712713
t.Fatal("expected error for partial failure")
713714
}
715+
// A pure remote non-zero exit is a command result, not an okdev
716+
// failure: softer framing, plain error (exit 1), no infra sentinel.
717+
if !strings.Contains(err.Error(), "command exited non-zero on 1 of 2 pod(s)") {
718+
t.Fatalf("unexpected error message: %v", err)
719+
}
720+
if errors.Is(err, ErrExecInfraFailure) {
721+
t.Fatalf("remote-exit-only run must not carry the infra sentinel: %v", err)
722+
}
714723
got := stderr.String()
715-
if !strings.Contains(got, "\nFAILED:\n") {
716-
t.Fatalf("expected blank line before multiline failure header, got %q", got)
724+
if !strings.Contains(got, "\nCOMMAND EXITED NON-ZERO:\n") || strings.Contains(got, "FAILED:") {
725+
t.Fatalf("expected command-result framing without FAILED header, got %q", got)
717726
}
718727
if !strings.Contains(got, "pod=okdev-sess-worker-1 container=dev kind=remote-exit exit=1") {
719728
t.Fatalf("expected classified failure summary for worker-1, got %q", got)
720729
}
721730
}
722731

732+
func TestRunMultiExecSinglePodPreservesRemoteExitStatus(t *testing.T) {
733+
client := &fakePdshExecClient{
734+
outputs: map[string]string{},
735+
errs: map[string]error{
736+
"okdev-sess-master-0": k8sexec.CodeExitError{Err: errors.New("command terminated with exit code 7"), Code: 7},
737+
},
738+
}
739+
pods := []kube.PodSummary{{Name: "okdev-sess-master-0", Phase: "Running"}}
740+
var stdout, stderr bytes.Buffer
741+
err := runMultiExec(context.Background(), client, "default", pods, "dev", []string{"sh", "-lc", "exit 7"}, "", 0, false, make(chan struct{}, pdshDefaultFanout), &stdout, &stderr)
742+
if err == nil {
743+
t.Fatal("expected error for non-zero remote exit")
744+
}
745+
// The documented contract: a single-pod remote exit status is preserved
746+
// verbatim, so the wrapped ExitError must survive for main's exit mapper.
747+
var exitErr utilexec.ExitError
748+
if !errors.As(err, &exitErr) || exitErr.ExitStatus() != 7 {
749+
t.Fatalf("expected wrapped ExitError with status 7, got %v", err)
750+
}
751+
if errors.Is(err, ErrExecInfraFailure) {
752+
t.Fatalf("remote-exit-only run must not carry the infra sentinel: %v", err)
753+
}
754+
if got := stderr.String(); !strings.Contains(got, "\nCOMMAND EXITED NON-ZERO:\n") || strings.Contains(got, "FAILED:") {
755+
t.Fatalf("expected command-result framing without FAILED header, got %q", got)
756+
}
757+
}
758+
723759
func TestRunMultiExecClassifiesTimeoutAndTransportFailures(t *testing.T) {
724760
client := &fakePdshExecClient{
725761
outputs: map[string]string{},
@@ -1027,3 +1063,30 @@ func TestResolveExecNoPrefix(t *testing.T) {
10271063
})
10281064
}
10291065
}
1066+
1067+
func TestRunMultiExecInfraFailureCarriesSentinel(t *testing.T) {
1068+
// Mixed remote-exit + timeout: any delivery failure keeps the FAILED
1069+
// framing and maps to the dedicated infra exit code so monitors can
1070+
// tell it apart from a command that merely exited non-zero.
1071+
client := &fakePdshExecClient{
1072+
errs: map[string]error{
1073+
"okdev-sess-worker-0": k8sexec.CodeExitError{Err: errors.New("command terminated with exit code 1"), Code: 1},
1074+
"okdev-sess-worker-1": context.DeadlineExceeded,
1075+
},
1076+
}
1077+
pods := []kube.PodSummary{
1078+
{Name: "okdev-sess-worker-0", Phase: "Running"},
1079+
{Name: "okdev-sess-worker-1", Phase: "Running"},
1080+
}
1081+
var stdout, stderr bytes.Buffer
1082+
err := runMultiExec(context.Background(), client, "default", pods, "dev", []string{"true"}, "", 0, false, make(chan struct{}, pdshDefaultFanout), &stdout, &stderr)
1083+
if err == nil || !errors.Is(err, ErrExecInfraFailure) {
1084+
t.Fatalf("expected infra sentinel, got %v", err)
1085+
}
1086+
if code, ok := ClassifiedExitCode(err); !ok || code != ExitExecInfraFailure {
1087+
t.Fatalf("expected exit code %d, got %d ok=%v", ExitExecInfraFailure, code, ok)
1088+
}
1089+
if !strings.Contains(stderr.String(), "FAILED:") {
1090+
t.Fatalf("expected FAILED framing for infra failures, got %q", stderr.String())
1091+
}
1092+
}

internal/cli/sync.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ func newSyncCmd(opts *Options) *cobra.Command {
185185
cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Preview sync actions without transferring files")
186186

187187
cmd.AddCommand(newSyncResetRemoteCmd(opts))
188+
cmd.AddCommand(newSyncWaitCmd(opts))
188189
return cmd
189190
}
190191

0 commit comments

Comments
 (0)