amadan
carlos move: 429s retried on mutations, the copy gate traces forwarded text, regions bound per move, strict L19 polling (Task 1a fix round 1)
Rulings 1 to 8 of the Task 1a review:
1. A 429 on request, cut, abort or retry sends the same call again after
the 10 s back-off, said once; the verb never exits on a 429.
2. The copy gate traces a forwarding sink's argument where it is called:
sayError and sayRecorded are local sinks; text whose provenance is the
console's answer or an error that crossed the API is exempt by name
(movesExternalText, each with its reason), through package helpers'
returns; a locally made error is a sentence that needs its row. The
general skip is gone: only an allow-listed function's value made from
none of its parameters (the flag help) needs no row. Fixture:
sayError(out, errors.New("unreviewed copy")) fails the gate.
3. The outcome's regions are the move's own, bound from its own answers
(the mutation's answer and its detail right after); another move's
record never names them.
4. A missed rollback says M-C8 only when the outcome carries a reason.
5. The wait reads detail=1 only when the state or the phase changes (the
flat view now carries phase); a release deadline that passes in between
is seen from the detail already read.
6. The flag help is said through moveFlagHelp, registered in
movesSinkAllow.
7. docs/site/cli.md: plain status reads once; exit 0 includes accepted
--no-wait commands.
8. Copy ids renumbered to MV-N350 to MV-N364 (Plan 5 owns N325 to N349).
B8 census: moveMain's digest (the session error's name).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
10 files changed,
+700
−158
approved_moves_copy_scan_test.go+133 −2approved_moves_copy_test.go+79 −12docs/site/cli.md+5 −4docs/superpowers/plans/2026-10-06-moves-phase1-copy.md+16 −16internal/console/apiinstancemove.go+21 −1internal/console/apiinstancemove_test.go+32 −1internal/provision/plan9_b8_census_test.go+1 −1move.go+165 −113move_test.go+190 −8testdata/moves-copy/sinks/forwarded-local-error/emit.go+58 −0
diff --git a/approved_moves_copy_scan_test.go b/approved_moves_copy_scan_test.go| index 37edacf3a..7f0199c7b 100644 |
| --- a/approved_moves_copy_scan_test.go |
| +++ b/approved_moves_copy_scan_test.go |
| @@ -1717,6 +1717,7 @@ type movesEmission struct { |
| pattern string |
| fragment bool // a piece of a sentence, matched by containment |
| underived bool // the gate could not derive a sentence: it matches no row |
| + fromParam bool // the value is made from a string or error parameter of fn |
| } |
| func (e movesEmission) String() string { |
| @@ -1825,7 +1826,14 @@ func scanMovesFileIn(t *testing.T, u *movesUniverse, root, file string, noCalls |
| emit := func(pos token.Pos, kind string, e ast.Expr, fragment bool) { |
| line := fset.Position(pos).Line |
| end := fset.Position(e.End()).Line |
| + // Text from outside the file (movesExternalText), or a local sink's |
| + // own forwarded parameter, is no sentence of this file here: the |
| + // parameter is checked where the sink is called. |
| + if ctx.external(e, 0) { |
| + return |
| + } |
| alts, probs := ctx.eval(e, 0) |
| + fromParam := false |
| if ctx.fd != nil { |
| params := map[string]bool{} |
| if ctx.fd.Type.Params != nil { |
| @@ -1841,6 +1849,7 @@ func scanMovesFileIn(t *testing.T, u *movesUniverse, root, file string, noCalls |
| mentions(e, ctx.locals, params, hit, 0) |
| if len(hit) > 0 { |
| u.paramSinks[ctx.fd.Name.Name] = true |
| + fromParam = true |
| } |
| } |
| underived := len(probs) > 0 |
| @@ -1853,7 +1862,7 @@ func scanMovesFileIn(t *testing.T, u *movesUniverse, root, file string, noCalls |
| case strings.Contains(a, fwdMark) && !informative(a): |
| // forwarded text with no sentence of its own around it: checked where it is made |
| case informative(a): |
| - out = append(out, movesEmission{file, line, end, fn, kind, strings.ReplaceAll(a, fwdMark, wild), fragment, false}) |
| + out = append(out, movesEmission{file, line, end, fn, kind, strings.ReplaceAll(a, fwdMark, wild), fragment, false, fromParam}) |
| default: |
| underived = true |
| if !fragment { |
| @@ -1862,7 +1871,7 @@ func scanMovesFileIn(t *testing.T, u *movesUniverse, root, file string, noCalls |
| } |
| } |
| if underived && !fragment { |
| - out = append(out, movesEmission{file, line, end, fn, kind, "", false, true}) |
| + out = append(out, movesEmission{file, line, end, fn, kind, "", false, true, fromParam}) |
| } |
| } |
| isCheck := func(x ast.Expr) bool { |
| @@ -1998,6 +2007,128 @@ func scanMovesFileIn(t *testing.T, u *movesUniverse, root, file string, noCalls |
| return out, vios |
| } |
| +// external reports whether e is text from outside the file, by a name in |
| +// movesExternalText (a call's result, or a field of a decoded answer, read |
| +// through .Error() and parentheses), or a call to a package helper whose |
| +// result is external at every return, or a local whose every assignment is |
| +// (or the zero value, or nil), or the local sink's own forwarded parameter, |
| +// which is checked where the sink is called. Anything else is read as usual. |
| +func (c *evalCtx) external(e ast.Expr, depth int) bool { |
| + if depth > maxEvalDepth { |
| + return false |
| + } |
| + ext := movesExternalText[c.file] |
| + named := func(n string) bool { _, ok := ext[n]; return ok } |
| + switch e := e.(type) { |
| + case *ast.ParenExpr: |
| + return c.external(e.X, depth+1) |
| + case *ast.CallExpr: |
| + if f, ok := e.Fun.(*ast.SelectorExpr); ok && f.Sel.Name == "Error" && len(e.Args) == 0 { |
| + return c.external(f.X, depth+1) |
| + } |
| + return c.externalCall(e, 0, depth+1) |
| + case *ast.SelectorExpr: |
| + return named(e.Sel.Name) |
| + case *ast.Ident: |
| + if e.Name == "nil" || c.fwd[e.Name] { |
| + return true |
| + } |
| + as := c.locals[e.Name] |
| + if len(as) == 0 { |
| + return false |
| + } |
| + for _, a := range as { |
| + switch { |
| + case a.zero: |
| + case a.rhs != nil: |
| + if !c.external(a.rhs, depth+1) { |
| + return false |
| + } |
| + case a.call != nil: |
| + if !c.externalCall(a.call, a.pos, depth+1) { |
| + return false |
| + } |
| + default: |
| + return false |
| + } |
| + } |
| + return true |
| + } |
| + return false |
| +} |
| + |
| +// externalCall reports whether result pos of call is external: the callee is |
| +// named in movesExternalText, or it is a package helper each of whose returns |
| +// gives an external value at pos (read in the helper's own context). |
| +func (c *evalCtx) externalCall(call *ast.CallExpr, pos, depth int) bool { |
| + if depth > maxEvalDepth { |
| + return false |
| + } |
| + name := "" |
| + switch f := call.Fun.(type) { |
| + case *ast.SelectorExpr: |
| + name = f.Sel.Name |
| + case *ast.Ident: |
| + name = f.Name |
| + } |
| + if _, ok := movesExternalText[c.file][name]; ok { |
| + return true |
| + } |
| + decls, p := c.resolveCallees(call) |
| + if p != nil || len(decls) == 0 { |
| + return false |
| + } |
| + for _, d := range decls { |
| + if d.Body == nil { |
| + return false |
| + } |
| + key := fmt.Sprintf("external %p %d", d, pos) |
| + if c.visiting[key] { |
| + return false |
| + } |
| + c.visiting[key] = true |
| + hc := c.u.newCtx(strings.TrimPrefix(movesFset.Position(d.Pos()).Filename, c.u.root+"/"), d, c.noCalls) |
| + hc.visiting = c.visiting |
| + ok := hc.returnsExternal(d, pos, depth+1) |
| + delete(c.visiting, key) |
| + if !ok { |
| + return false |
| + } |
| + } |
| + return true |
| +} |
| + |
| +// returnsExternal reports whether every return of d gives an external value at |
| +// pos (c is d's own context). |
| +func (c *evalCtx) returnsExternal(d *ast.FuncDecl, pos, depth int) bool { |
| + nres := resultCount(d) |
| + named := namedResults(d) |
| + ok := true |
| + ast.Inspect(d.Body, func(n ast.Node) bool { |
| + if !ok { |
| + return false |
| + } |
| + switch n := n.(type) { |
| + case *ast.FuncLit: |
| + return false |
| + case *ast.ReturnStmt: |
| + switch { |
| + case len(n.Results) == nres && pos < len(n.Results): |
| + ok = c.external(n.Results[pos], depth) |
| + case len(n.Results) == 1 && nres > 1: |
| + cl, isCall := n.Results[0].(*ast.CallExpr) |
| + ok = isCall && c.externalCall(cl, pos, depth) |
| + case len(n.Results) == 0 && pos < len(named): |
| + ok = c.external(&ast.Ident{Name: named[pos], NamePos: n.Pos()}, depth) |
| + default: |
| + ok = false |
| + } |
| + } |
| + return true |
| + }) |
| + return ok |
| +} |
| + |
| // checkFuncArg checks the function a call hands to a sentence-function |
| // parameter: a package function that is a sentence helper. |
| func (c *evalCtx) checkFuncArg(e ast.Expr) string { |
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index b614dc39a..3f7f4a1f7 100644 |
| --- a/approved_moves_copy_test.go |
| +++ b/approved_moves_copy_test.go |
| @@ -76,8 +76,8 @@ var movesCopyIDs = []string{ |
| "MV-N313", "MV-N314", |
| "MV-N315", "MV-N316", "MV-N317", "MV-N318", |
| "MV-N319", |
| - "MV-N325", "MV-N326", "MV-N327", "MV-N328", "MV-N329", "MV-N330", "MV-N331", |
| - "MV-N332", "MV-N333", "MV-N334", "MV-N335", "MV-N336", "MV-N337", "MV-N338", "MV-N339", |
| + "MV-N350", "MV-N351", "MV-N352", "MV-N353", "MV-N354", "MV-N355", "MV-N356", |
| + "MV-N357", "MV-N358", "MV-N359", "MV-N360", "MV-N361", "MV-N362", "MV-N363", "MV-N364", |
| } |
| // copyRow is one row of the record. The File cell is the file the sentence is |
| @@ -738,7 +738,9 @@ var movesLocalSinks = map[string]map[string]int{ |
| // Every operator sentence of `carlos move` goes through moveSay, L13's |
| // say (named so because the scan resolves helpers by name, and |
| // internal/release has a say of its own). |
| - "move.go": {"moveSay": 1}, |
| + // sayError and sayRecorded forward text this file does not write; their |
| + // arguments are traced where they are called (movesExternalText). |
| + "move.go": {"moveSay": 1, "sayError": 1, "sayRecorded": 1}, |
| } |
| var movesCopyTables = map[string][]string{ |
| @@ -819,9 +821,27 @@ var movesFuncSinks = map[string]map[string]int{ |
| // code instead. TestMovesSinkAllowListIsHonest holds the list to this. |
| var movesSinkAllow = map[string]map[string]string{ |
| "move.go": { |
| - "sayStuck": "prints, after M-C11, the record's stuck reason, a sentence the provision transitions record (MV-N300 to MV-N312 and the rows before them), on its own line (preflight L28)", |
| - "sayRefusal": "passes on the console's refusal, a sentence recorded with the door that speaks it (M-C13, M-C17, MV-N1 and the rest of the console rows)", |
| - "sayError": "reports a transport or decoding error from net/http or encoding/json, Go's text and not a sentence of this file", |
| + "moveFlagHelp": "the flag help: the flag package's error for a bad flag and its rendering of the usage strings parseMoveArgs declares; they go to copy review with the CLI page (preflight L13, L16), and no row can carry the flag package's layout", |
| + }, |
| +} |
| + |
| +// movesExternalText names, per file, what a forwarding sink may pass on |
| +// without a row: text whose provenance is the console's answer, or an error |
| +// that crossed the API, never a sentence the file makes itself. A name is a |
| +// called function or method (its result, error or text) or a field read from |
| +// a decoded answer. Each entry says why. A local sink's argument is traced to |
| +// these names where the sink is called; a locally made error |
| +// (errors.New, fmt.Errorf) is a sentence and needs its row. |
| +var movesExternalText = map[string]map[string]string{ |
| + "move.go": { |
| + "apiErrorFromBody": "the console's answer: its own JSON error sentence, recorded with the door that speaks it (M-C13, M-C17, MV-N1 and the rest), or apimode.go's reading of a body with none", |
| + "getJSON": "an error that crossed the API on a read: net/http's transport error, or apimode.go's console-moved explanation", |
| + "postJSON": "an error that crossed the API on a mutation, as getJSON's", |
| + "deleteJSON": "an error that crossed the API on an abort, as getJSON's", |
| + "Unmarshal": "encoding/json's error decoding the console's answer", |
| + "sleep": "mover.sleep's only error is ctx.Err(): the context's own cancellation, Go's text", |
| + "session": "resolveMode's refusal (moveDeps.session): apimode.go's own sentences, which every API verb says", |
| + "StuckReason": "the stuck reason the console's detail answer carries from the record: a sentence a provision transition recorded (MV-N300 to MV-N312 and the rows before them)", |
| }, |
| } |
| @@ -901,16 +921,18 @@ func movesLegacy(e movesEmission) bool { |
| } |
| // findUnrecorded returns the emissions no row covers. A value the gate cannot |
| -// derive, inside a function movesSinkAllow lists, is not this file's sentence: |
| -// the entry's reason says whose it is, so no row could carry it. A sentence |
| -// the gate can derive in such a function still needs its row. |
| +// derive, inside a function movesSinkAllow lists, made from none of its string |
| +// or error parameters, is that entry's (flag help): its reason says whose it |
| +// is, so no row could carry it. A value made from a parameter is never |
| +// skipped (a forwarding helper's text is traced where it is called, not |
| +// exempted), and a sentence the gate can derive still needs its row. |
| func findUnrecorded(ems []movesEmission, rows []copyRow) []movesEmission { |
| var out []movesEmission |
| for _, e := range ems { |
| if movesLegacy(e) { |
| continue |
| } |
| - if _, allowed := movesSinkAllow[e.file][e.fn]; allowed && e.underived { |
| + if _, allowed := movesSinkAllow[e.file][e.fn]; allowed && e.underived && !e.fromParam { |
| continue |
| } |
| found := false |
| @@ -1536,10 +1558,11 @@ func TestMovesSinkAllowListIsHonest(t *testing.T) { |
| ems := []movesEmission{ |
| {file: "a.go", fn: "f", kind: "say", underived: true}, |
| {file: "a.go", fn: "f", kind: "say", pattern: "an unrecorded sentence of f"}, |
| + {file: "a.go", fn: "f", kind: "say", underived: true, fromParam: true}, |
| {file: "a.go", fn: "g", kind: "say", underived: true}, |
| } |
| - if un := findUnrecorded(ems, nil); len(un) != 2 || un[0].fn != "f" || un[0].underived || un[1].fn != "g" { |
| - t.Errorf("unrecorded = %v, want f's derived sentence and g's value", un) |
| + if un := findUnrecorded(ems, nil); len(un) != 3 || un[0].fn != "f" || un[0].underived || !un[1].fromParam || un[2].fn != "g" { |
| + t.Errorf("unrecorded = %v, want f's derived sentence, f's value made from a parameter, and g's value", un) |
| } |
| } |
| @@ -1672,3 +1695,47 @@ func checkFuncValueAllow(allow map[string]map[string]string, refs []funcRef) []s |
| sort.Strings(out) |
| return out |
| } |
| + |
| +// Fix round 1 of Plan 6 Task 1a, ruling 2: a forwarding sink's argument is |
| +// traced where the sink is called. Text whose provenance is the console's |
| +// answer or an error that crossed the API is exempt by name |
| +// (movesExternalText); a locally made error is a sentence that needs its row |
| +// (sayError(out, errors.New("unreviewed copy")) fails the gate); an error the |
| +// gate cannot trace is a violation. |
| +func TestMovesForwardedErrorsAreTraced(t *testing.T) { |
| + file := filepath.Join("testdata", "moves-copy", "sinks", "forwarded-local-error", "emit.go") |
| + savedSinks, savedExt := movesLocalSinks[file], movesExternalText[file] |
| + movesLocalSinks[file] = map[string]int{"moveSay": 1, "sayError": 1} |
| + movesExternalText[file] = map[string]string{"apiErrorFromBody": "the console's answer", "getJSON": "an error that crossed the API"} |
| + defer func() { |
| + movesLocalSinks[file], movesExternalText[file] = savedSinks, savedExt |
| + if savedSinks == nil { |
| + delete(movesLocalSinks, file) |
| + } |
| + if savedExt == nil { |
| + delete(movesExternalText, file) |
| + } |
| + }() |
| + u := newUniverse(t, ".", []string{file}) |
| + ems, vios := scanMovesFileIn(t, u, ".", file, false) |
| + var unrecorded []string |
| + for _, e := range findUnrecorded(ems, nil) { |
| + unrecorded = append(unrecorded, e.fn+": "+strings.ReplaceAll(e.pattern, wild, "%")) |
| + } |
| + sort.Strings(unrecorded) |
| + // The two locally made errors are sentences with no row. The untraced |
| + // error, and a helper's own error, are values the gate cannot derive (and |
| + // violations, below). A helper that passes on getJSON's error (viaFetch) |
| + // is external. |
| + want := []string{"formatted: % boxes are unreviewed", "local: unreviewed copy", "untraced: ", "viaHelper: "} |
| + if strings.Join(unrecorded, "|") != strings.Join(want, "|") { |
| + t.Errorf("unrecorded = %q, want %q", unrecorded, want) |
| + } |
| + var where []string |
| + for _, v := range vios { |
| + where = append(where, v.fn) |
| + } |
| + if strings.Join(where, ",") != "untraced,viaHelper" { |
| + t.Errorf("violations in %v, want untraced and viaHelper: %v", where, vios) |
| + } |
| +} |
diff --git a/docs/site/cli.md b/docs/site/cli.md| index f3928b8f5..a1bd6a429 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -2065,15 +2065,16 @@ carlos move abort --host <host> |
| carlos move resolve --host <host> --retry |
| ``` |
| -Each command waits until the move ends unless you pass `--no-wait`. |
| -`--manual-cut` stops once the move is ready, and `carlos move cut` starts the |
| -cut. `carlos move status --wait` picks a wait back up after a disconnect. |
| +`carlos move`, `cut`, `abort` and `resolve` wait until the move ends unless |
| +you pass `--no-wait`. `carlos move status` reads the move once; with `--wait` |
| +it waits too, which picks a wait back up after a disconnect. `--manual-cut` |
| +stops once the move is ready, and `carlos move cut` starts the cut. |
| `--app` and `--account` name the instance's app and account, as for |
| `carlos instances`. |
| | exit code | meaning | |
| |---|---| |
| -| `0` | the move finished | |
| +| `0` | the move finished, the console accepted a command given `--no-wait`, or `status` found the move still under way | |
| | `3` | the move rolled back or was aborted | |
| | `4` | the move needs an operator: it is stuck, waiting for a box, or ready for a manual cut, or its outcome was not recorded | |
| | `5` | refused, by the console or before anything was sent | |
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md| index c7ce8f74a..6ff1be194 100644 |
| --- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| +++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md |
| @@ -284,26 +284,26 @@ Columns: id, file, text, status. |
| ## Plan 6 |
| -(completed by Plan 6 Task 7. Task 2 takes MV-N319 to MV-N324, and made the abort door speak M-C13 on a console without the host claim, so M-C13 is now approved verbatim. Task 2b made the delete door speak M-C19 through moveDeleteRefusal in internal/console/apiinstancemove.go, so its File cell moves there (preflight L6) and it is approved verbatim. Task 1a takes MV-N325 to MV-N339 for `carlos move` (move.go, and its usage line in main.go), and speaks M-C1 to M-C12 and M-C22 verbatim from move.go, so those rows are approved) |
| +(completed by Plan 6 Task 7. Task 2 takes MV-N319 to MV-N324, and made the abort door speak M-C13 on a console without the host claim, so M-C13 is now approved verbatim. Task 2b made the delete door speak M-C19 through moveDeleteRefusal in internal/console/apiinstancemove.go, so its File cell moves there (preflight L6) and it is approved verbatim. Task 1a takes MV-N350 to MV-N364 (Plan 5 owns MV-N325 to MV-N349) for `carlos move` (move.go, and its usage line in main.go), and speaks M-C1 to M-C12 and M-C22 verbatim from move.go, so those rows are approved) |
| | id | file | text | status | |
| |---|---|---|---| |
| | MV-N319 | internal/console/apiinstancemove.go | could not read the move's reports. try again | pending | |
| -| MV-N325 | move.go | Moves are not enabled on this console. | pending | |
| -| MV-N326 | move.go | carlos move needs --app: name the app the instance belongs to. | pending | |
| -| MV-N327 | move.go | Run carlos move as yourself, not as root. It uses your own credentials. | pending | |
| -| MV-N328 | move.go | carlos move works through a console. Unset CARLOS_DEPLOYMENT_BUCKET and CARLOS_DEPLOYMENT_DIR and run carlos auth login. | pending | |
| -| MV-N329 | move.go | This move's outcome is not recorded. | pending | |
| -| MV-N330 | move.go | The console is limiting requests. Checking every 10 seconds. | pending | |
| -| MV-N331 | move.go | carlos move needs --host <host>. | pending | |
| -| MV-N332 | move.go | carlos move needs --to <region>. | pending | |
| -| MV-N333 | move.go | carlos move resolve needs --retry. | pending | |
| -| MV-N334 | move.go | No move is in flight for <host>. | pending | |
| -| MV-N335 | move.go | Move <id> of <host> is <state>. | pending | |
| -| MV-N336 | move.go | The move was aborted. <host> stays in <from>. | pending | |
| -| MV-N337 | move.go | <host> is relocating after its box was terminated. This move will not finish. | pending | |
| -| MV-N338 | main.go | move an instance to another region: status | cut | abort | resolve --retry (operators; off unless the console enables it) | pending | |
| -| MV-N339 | move.go (M-C10's <box> when the claim names none) | the target box | pending | |
| +| MV-N350 | move.go | Moves are not enabled on this console. | pending | |
| +| MV-N351 | move.go | carlos move needs --app: name the app the instance belongs to. | pending | |
| +| MV-N352 | move.go | Run carlos move as yourself, not as root. It uses your own credentials. | pending | |
| +| MV-N353 | move.go | carlos move works through a console. Unset CARLOS_DEPLOYMENT_BUCKET and CARLOS_DEPLOYMENT_DIR and run carlos auth login. | pending | |
| +| MV-N354 | move.go | This move's outcome is not recorded. | pending | |
| +| MV-N355 | move.go | The console is limiting requests. Checking every 10 seconds. | pending | |
| +| MV-N356 | move.go | carlos move needs --host <host>. | pending | |
| +| MV-N357 | move.go | carlos move needs --to <region>. | pending | |
| +| MV-N358 | move.go | carlos move resolve needs --retry. | pending | |
| +| MV-N359 | move.go | No move is in flight for <host>. | pending | |
| +| MV-N360 | move.go | Move <id> of <host> is <state>. | pending | |
| +| MV-N361 | move.go | The move was aborted. <host> stays in <from>. | pending | |
| +| MV-N362 | move.go | <host> is relocating after its box was terminated. This move will not finish. | pending | |
| +| MV-N363 | main.go | move an instance to another region: status | cut | abort | resolve --retry (operators; off unless the console enables it) | pending | |
| +| MV-N364 | move.go (M-C10's <box> when the claim names none) | the target box | pending | |
| ## Review of 2026-10-09 |
diff --git a/internal/console/apiinstancemove.go b/internal/console/apiinstancemove.go| index 7c0837864..106f6f704 100644 |
| --- a/internal/console/apiinstancemove.go |
| +++ b/internal/console/apiinstancemove.go |
| @@ -164,6 +164,26 @@ type moveView struct { |
| ID string `json:"id,omitempty"` |
| Last *provision.MoveOutcome `json:"last,omitempty"` |
| Protocol string `json:"protocol,omitempty"` |
| + // Phase is the step within the state (movePhase), so a poller reads |
| + // detail=1 only when the state or the phase changes (preflight L19). |
| + Phase string `json:"phase,omitempty"` |
| +} |
| + |
| +// movePhase is a move's phase as the flat view shows it: the cut's phase |
| +// while cutting, the landing's while landing, and the rollback's target and |
| +// source phases while rolling back. Other states have none. |
| +func movePhase(m *provision.MoveState) string { |
| + switch m.State { |
| + case provision.MoveCutting: |
| + return m.CutPhase |
| + case provision.MoveLanding: |
| + return m.LandingPhase |
| + case provision.MoveRollingBack: |
| + if m.Rollback != nil { |
| + return m.Rollback.TargetPhase + "/" + m.Rollback.SourcePhase |
| + } |
| + } |
| + return "" |
| } |
| // moveProtocolPhase1 is the Phase 1 marker every move answer carries with |
| @@ -184,7 +204,7 @@ func (s *Server) moveViewOf(rec *provision.Record) moveView { |
| v.Protocol = moveProtocolPhase1 |
| if rec != nil { |
| if rec.Move != nil { |
| - v.ID = rec.Move.ID |
| + v.ID, v.Phase = rec.Move.ID, movePhase(rec.Move) |
| } |
| v.Last = rec.LastMove |
| } |
diff --git a/internal/console/apiinstancemove_test.go b/internal/console/apiinstancemove_test.go| index a6626ca6e..bbf282aa5 100644 |
| --- a/internal/console/apiinstancemove_test.go |
| +++ b/internal/console/apiinstancemove_test.go |
| @@ -1624,7 +1624,7 @@ func TestResolveRetryFromStuckOnly(t *testing.T) { |
| if err := json.Unmarshal([]byte(out), &view); err != nil { |
| t.Fatal(err) |
| } |
| - if len(view) != 5 || view["state"] != provision.MoveRollingBack || view["target"] != "uk" || view["updated_at"] == nil || view["id"] != "mv_abcdefghijklmnopqrstuvwxyz" || view["protocol"] != "phase1" { |
| + if len(view) != 6 || view["state"] != provision.MoveRollingBack || view["target"] != "uk" || view["updated_at"] == nil || view["id"] != "mv_abcdefghijklmnopqrstuvwxyz" || view["protocol"] != "phase1" || view["phase"] != provision.RbTargetCleaned+"/"+c.at { |
| t.Fatalf("retry after %s answered %s, want the flat view in rolling_back", c.name, out) |
| } |
| got := read().Move |
| @@ -2118,3 +2118,34 @@ func TestMoveAbortAfterTheCutStartedSaysSo(t *testing.T) { |
| t.Fatalf("abort from cutting/requested = %s", out) |
| } |
| } |
| + |
| +// Task 1a fix round 1 (L19): with the switch on the flat view carries the |
| +// move's phase, so a poller reads detail=1 only when the state or the phase |
| +// changes. |
| +func TestMoveViewCarriesThePhase(t *testing.T) { |
| + fx, token, moveURL := moveDoorWithClaim(t, "code-inst-move-phase") |
| + readMoveAnswer(t, http.MethodPost, moveURL, token, `{"region":"uk","cut":"manual"}`, http.StatusAccepted) |
| + const rb = "rb_abcdefghijklmnopqrstuvwxyz" |
| + for _, c := range []struct { |
| + mut func(*provision.MoveState) |
| + want string |
| + }{ |
| + {func(m *provision.MoveState) { m.State = provision.MovePreparing }, ""}, |
| + {func(m *provision.MoveState) { m.State, m.CutPhase = provision.MoveCutting, provision.CutAdmitted }, provision.CutAdmitted}, |
| + {func(m *provision.MoveState) { m.State, m.LandingPhase = provision.MoveLanding, provision.LandProbing }, provision.LandProbing}, |
| + {func(m *provision.MoveState) { |
| + m.State, m.Rollback = provision.MoveRollingBack, &provision.MoveRollback{ID: rb, TargetPhase: provision.RbTargetReleasing, SourcePhase: provision.RbSourceVerified} |
| + }, provision.RbTargetReleasing + "/" + provision.RbSourceVerified}, |
| + } { |
| + setMove(t, fx, func(rec *provision.Record) { c.mut(rec.Move) }) |
| + code, _, out := rawReq(t, http.MethodGet, moveURL, token, "") |
| + var v map[string]any |
| + if code != http.StatusOK || json.Unmarshal([]byte(out), &v) != nil { |
| + t.Fatalf("status = %d %s", code, out) |
| + } |
| + got, has := v["phase"] |
| + if (c.want == "" && has) || (c.want != "" && got != c.want) { |
| + t.Fatalf("phase = %v (present %v), want %q: %s", got, has, c.want, out) |
| + } |
| + } |
| +} |
diff --git a/internal/provision/plan9_b8_census_test.go b/internal/provision/plan9_b8_census_test.go| index 2d77a0491..28e1059c2 100644 |
| --- a/internal/provision/plan9_b8_census_test.go |
| +++ b/internal/provision/plan9_b8_census_test.go |
| @@ -564,7 +564,7 @@ var plan9B8CensusInventory = []censusEntry{ |
| {"main.go", "main", "key-site", "lit:instances,lit:sidecars", "3eb1b1a9b8a7fd9cc2820b5a4348046b4f6bac3496d5ac6c4e2c32722233bd15"}, |
| // Plan 6 Task 1a: `carlos move` builds the console's move door path |
| // (/api/cli/apps/.../instances/<host>/move); it writes no record. |
| - {"move.go", "moveMain", "key-site", "lit:instances", "16fb7f2e98a020b9bc69ce0de4af807aee690f6baec51b4e11ff12aa7e705358"}, |
| + {"move.go", "moveMain", "key-site", "lit:instances", "b41a91fb74208d9f9a08ad797bcb483162eb19f79d176bd4acd52de76805dcd8"}, |
| {"schedule.go", "renderScheduleList", "key-site", "lit:instances", "af6d666083fbbc753eecc98809d3d307344cc15110362a7e93afab573749ea6e"}, |
| {"internal/activator/layout.go", "LayoutInputs.sockets", "key-site", "lit:instances", "1d11ac5ac5be6a42e676e064771141fe8f4367c78cbb7e5c5cca072f40c27c42"}, |
| {"internal/activator/paths.go", "ExecBackend.pathsFor", "key-site", "lit:instances", "cbc01865e27934cb38baab9329a0867b899149531b4706a533a6956afc7c71b3"}, |
diff --git a/move.go b/move.go| index 88100e4bb..b9898baef 100644 |
| --- a/move.go |
| +++ b/move.go |
| @@ -23,20 +23,24 @@ import ( |
| // console whose move answer lacks the Phase 1 marker (preflight L4), so it |
| // can never start a legacy move. |
| // |
| -// Waiting is the default. The wait polls the flat view every two seconds, |
| -// reads detail=1 when the state changes (and now and then while it does |
| -// not), backs off to ten seconds on a 429 and keeps going (L19), and reads |
| -// the outcome only from `last`, bound to the id the move was requested |
| -// under (L2, Review Focus 1). Exit codes (L24): 0 done; 3 rolled back or |
| -// aborted; 4 stuck, rolling back past its release deadline, relocating, |
| -// ready for a manual cut, or an outcome not recorded; 5 refused (by the |
| -// console, or before anything was sent); 1 a transport error or a 5xx; 2 a |
| -// usage error. |
| +// Waiting is the default. The wait polls the flat view every two seconds and |
| +// reads detail=1 only when the state or the phase changes (L19). A 429, on a |
| +// read or on a mutation, backs off to ten seconds, is said once, and never |
| +// ends the command: the doors' rate limit answers before any handler, so the |
| +// same call is sent again. The outcome is read only from `last`, bound to the |
| +// id the console gave the move (L2, Review Focus 1), and its regions are that |
| +// move's own, bound from its own answers. Exit codes (L24): 0 done, or a |
| +// request accepted with --no-wait; 3 rolled back or aborted; 4 stuck, |
| +// rolling back past its release deadline, relocating, ready for a manual |
| +// cut, or an outcome not recorded; 5 refused (by the console, or before |
| +// anything was sent); 1 a transport error or a 5xx; 2 a usage error. |
| // |
| -// Every operator sentence goes through say, the copy gate's sink for this |
| -// file (movesLocalSinks). The functions that pass on a sentence recorded |
| -// elsewhere (the console's refusals, the record's stuck reason, transport |
| -// errors) are listed in movesSinkAllow with the reason. |
| +// Every operator sentence goes through moveSay, the copy gate's sink for this |
| +// file (movesLocalSinks). Text this file does not write goes through the two |
| +// forwarding sinks, sayError and sayRecorded, whose arguments the gate traces |
| +// where they are called, to the console's answer or an error that crossed the |
| +// API (movesExternalText). The flag help is said by moveFlagHelp |
| +// (movesSinkAllow). |
| const ( |
| moveExitDone = 0 |
| @@ -48,7 +52,7 @@ const ( |
| moveProtocolMark = "phase1" |
| ) |
| -// The sentences this file adds (copy record MV-N325 to MV-N339, pending). |
| +// The sentences this file adds (copy record MV-N350 to MV-N364, pending). |
| const ( |
| moveNotEnabled = "Moves are not enabled on this console." |
| moveNoApp = "carlos move needs --app: name the app the instance belongs to." |
| @@ -65,6 +69,21 @@ func moveSay(out io.Writer, line string) { |
| fmt.Fprintln(out, line) |
| } |
| +// sayError passes on an error this file did not write: the console's answer |
| +// (apiErrorFromBody) or an error from a call across the API. The copy gate |
| +// traces each call's argument to one of those (movesExternalText); a locally |
| +// made error is a sentence and needs its row. |
| +func sayError(out io.Writer, err error) { |
| + moveSay(out, err.Error()) |
| +} |
| + |
| +// sayRecorded passes on a sentence the console's answer carries from the |
| +// record: the stuck reason a provision transition recorded (after M-C11, on |
| +// its own line, preflight L28). |
| +func sayRecorded(out io.Writer, text string) { |
| + moveSay(out, text) |
| +} |
| + |
| func runMove(args []string) { |
| deps := moveDeps{session: resolveMode, euid: os.Geteuid, poll: 2 * time.Second} |
| os.Exit(moveMain(context.Background(), args, os.Stdout, deps)) |
| @@ -99,7 +118,9 @@ func parseMoveArgs(args []string, out io.Writer) (moveArgs, error) { |
| } |
| } |
| fs := flag.NewFlagSet("move "+a.verb, flag.ContinueOnError) |
| - fs.SetOutput(out) |
| + var parsed strings.Builder |
| + fs.SetOutput(&parsed) |
| + fs.Usage = func() {} // the help is said by moveFlagHelp |
| fs.StringVar(&a.host, "host", "", "the instance's host name") |
| fs.StringVar(&a.app, "app", "", "the app the instance belongs to") |
| fs.StringVar(&a.app, "a", "", "shorthand for --app") |
| @@ -120,6 +141,7 @@ func parseMoveArgs(args []string, out io.Writer) (moveArgs, error) { |
| fs.BoolVar(&a.noWait, "no-wait", false, "return once the console has accepted it") |
| } |
| if err := fs.Parse(args); err != nil { |
| + moveFlagHelp(out, fs, &parsed) |
| return moveArgs{}, err |
| } |
| if fs.NArg() > 0 { |
| @@ -143,6 +165,16 @@ func parseMoveArgs(args []string, out io.Writer) (moveArgs, error) { |
| return a, nil |
| } |
| +// moveFlagHelp says the flag help: what the flag package wrote while parsing |
| +// (its error for a bad flag, if any) and its rendering of the verb's flags |
| +// and their usage strings. It is the copy gate's one help emission |
| +// (movesSinkAllow); the usage strings go to copy review with the CLI page. |
| +func moveFlagHelp(out io.Writer, fs *flag.FlagSet, parsed *strings.Builder) { |
| + fs.SetOutput(parsed) |
| + fs.PrintDefaults() |
| + moveSay(out, strings.TrimRight(parsed.String(), "\n")) |
| +} |
| + |
| // moveMain runs one `carlos move` and returns its exit code. |
| func moveMain(ctx context.Context, args []string, out io.Writer, d moveDeps) int { |
| a, err := parseMoveArgs(args, out) |
| @@ -159,9 +191,9 @@ func moveMain(ctx context.Context, args []string, out io.Writer, d moveDeps) int |
| moveSay(out, moveAsRoot) |
| return moveExitRefused |
| } |
| - sess, err := d.session(a.console) |
| - if err != nil { |
| - sayError(out, err) |
| + sess, sessErr := d.session(a.console) |
| + if sessErr != nil { |
| + sayError(out, sessErr) |
| return moveExitError |
| } |
| if sess == nil { |
| @@ -206,6 +238,7 @@ func moveMain(ctx context.Context, args []string, out io.Writer, d moveDeps) int |
| // here as the CLI convention is). |
| type moveFlat struct { |
| State string `json:"state"` |
| + Phase string `json:"phase"` |
| Target string `json:"target"` |
| UpdatedAt int64 `json:"updated_at"` |
| ID string `json:"id"` |
| @@ -213,11 +246,15 @@ type moveFlat struct { |
| Protocol string `json:"protocol"` |
| } |
| +// moveOutcome is the record's last close (D11). Reason is read when the |
| +// console's answer carries one; today's record does not, and the CLI never |
| +// makes one up. |
| type moveOutcome struct { |
| ID string `json:"id"` |
| Outcome string `json:"outcome"` |
| At time.Time `json:"at"` |
| OfflineS float64 `json:"offline_s"` |
| + Reason string `json:"reason"` |
| } |
| type moveAnswer struct { |
| @@ -271,8 +308,8 @@ type mover struct { |
| host string |
| path string |
| poll time.Duration |
| - // limited: a 429 was answered, so the wait polls every 10 seconds now |
| - // and has said so (once). |
| + // limited: a 429 was answered, so the command waits 10 seconds between |
| + // calls now and has said so (once). |
| limited bool |
| } |
| @@ -287,6 +324,15 @@ func (m *mover) withDetail() string { |
| // it gives up with exit 1: a console restart is not a failed move. |
| const moveTransient = 5 |
| +// limit notes a 429: from now on the command waits 10 seconds between calls, |
| +// and says so the first time. |
| +func (m *mover) limit() { |
| + if !m.limited { |
| + m.limited = true |
| + moveSay(m.out, moveLimited) |
| + } |
| +} |
| + |
| // get reads one answer, riding out 429s (backing off, said once) and a few |
| // transient failures. code is -1 on success, else the exit code to return. |
| func (m *mover) get(ctx context.Context, path string, into any) (code int) { |
| @@ -301,12 +347,9 @@ func (m *mover) get(ctx context.Context, path string, into any) (code int) { |
| } |
| return -1 |
| case err == nil && status == http.StatusTooManyRequests: |
| - if !m.limited { |
| - m.limited = true |
| - moveSay(m.out, moveLimited) |
| - } |
| + m.limit() |
| case err == nil && status >= 400 && status < 500: |
| - sayRefusal(m.out, status, body) |
| + sayError(m.out, apiErrorFromBody(status, body)) |
| return moveExitRefused |
| default: |
| failures++ |
| @@ -357,24 +400,39 @@ func (m *mover) readDetail(ctx context.Context) (moveAnswer, int) { |
| return a, code |
| } |
| +// send sends one mutation, sending the same call again after the back-off |
| +// while the doors' rate limit answers 429 (ruling 1 of Task 1a's fix round: |
| +// the limit answers before any handler, so nothing was done). |
| +func (m *mover) send(ctx context.Context, method, tail string, body any) (status int, raw []byte, err error) { |
| + for { |
| + if method == http.MethodDelete { |
| + status, raw, err = m.sess.deleteJSON(ctx, m.path) |
| + } else { |
| + status, raw, err = m.sess.postJSON(ctx, m.path+tail, body) |
| + } |
| + if err != nil || status != http.StatusTooManyRequests { |
| + return status, raw, err |
| + } |
| + m.limit() |
| + if err = m.sleep(ctx); err != nil { |
| + return 0, nil, err |
| + } |
| + } |
| +} |
| + |
| // mutate sends one move mutation. A refusal is the console's own sentence |
| -// and exit 5. On acceptance the answer names the move (Task 2); a request |
| -// prints M-C1, and then the wait (unless --no-wait) follows that id. |
| +// and exit 5. On acceptance the answer names the move (Task 2), and the move's |
| +// detail, read once now, binds its regions: the sentences about this move |
| +// name them, whatever a later move does to the record. A request says M-C1; |
| +// then the wait (unless --no-wait) follows the move's id. |
| func (m *mover) mutate(ctx context.Context, method, tail string, body any, wait, request bool) int { |
| - var status int |
| - var raw []byte |
| - var err error |
| - if method == http.MethodDelete { |
| - status, raw, err = m.sess.deleteJSON(ctx, m.path) |
| - } else { |
| - status, raw, err = m.sess.postJSON(ctx, m.path+tail, body) |
| - } |
| + status, raw, err := m.send(ctx, method, tail, body) |
| if err != nil { |
| sayError(m.out, err) |
| return moveExitError |
| } |
| if status >= 400 && status < 500 { |
| - sayRefusal(m.out, status, raw) |
| + sayError(m.out, apiErrorFromBody(status, raw)) |
| return moveExitRefused |
| } |
| if status < 200 || status > 299 { |
| @@ -391,16 +449,14 @@ func (m *mover) mutate(ctx context.Context, method, tail string, body any, wait, |
| return moveExitRefused |
| } |
| w := newMoveWatch(m, view.ID) |
| - w.to = view.Target |
| - if request { |
| - a, code := m.readDetail(ctx) |
| - if code >= 0 { |
| - return code |
| - } |
| - w.learn(a) |
| - if w.first("start") { |
| - moveSay(w.m.out, fmt.Sprintf("Moving %s from %s to %s.", m.host, w.from, w.to)) |
| - } |
| + w.toRegion = view.Target |
| + a, code := m.readDetail(ctx) |
| + if code >= 0 { |
| + return code |
| + } |
| + w.learn(a) |
| + if request && w.first("start") { |
| + moveSay(m.out, fmt.Sprintf("Moving %s from %s to %s.", m.host, w.fromRegion, w.toRegion)) |
| } |
| if !wait { |
| return moveExitDone |
| @@ -443,23 +499,18 @@ func (m *mover) status(ctx context.Context, view moveFlat, wait bool) int { |
| // moveWatch is the plan's moveWait: it follows one move, by id, and prints |
| // each sentence once. |
| type moveWatch struct { |
| - m *mover |
| - id string |
| - from, to string |
| - said map[string]bool |
| - lastKey string |
| - sinceRead int |
| + m *mover |
| + id string |
| + fromRegion, toRegion string // this move's, from its own answers only |
| + said map[string]bool |
| + lastKey string |
| + last moveAnswer // the detail read last |
| } |
| func newMoveWatch(m *mover, id string) *moveWatch { |
| return &moveWatch{m: m, id: id, said: map[string]bool{}} |
| } |
| -// moveDetailEvery is how many flat polls may pass with no change before the |
| -// wait reads detail=1 anyway (a target report appears without a record |
| -// change, M-C3 and M-C4): every 30 s at the 2 s poll. |
| -const moveDetailEvery = 15 |
| - |
| // first reports whether key's sentence is due: true the first time only. |
| func (w *moveWatch) first(key string) bool { |
| if w.said[key] { |
| @@ -469,23 +520,25 @@ func (w *moveWatch) first(key string) bool { |
| return true |
| } |
| -// learn keeps the regions the sentences name. |
| +// learn binds the move's regions from an answer about this move, and only |
| +// such an answer: another move's record never names this one's regions. |
| func (w *moveWatch) learn(a moveAnswer) { |
| - d := a.Detail |
| - if d.From != "" { |
| - w.from = d.From |
| + if a.ID != w.id { |
| + return |
| } |
| - if a.Target != "" && a.ID == w.id { |
| - w.to = a.Target |
| + if a.Detail.From != "" { |
| + w.fromRegion = a.Detail.From |
| } |
| - if w.from == "" && a.State != "none" && d.Region != "" { |
| - w.from = d.Region |
| + if a.Target != "" { |
| + w.toRegion = a.Target |
| } |
| } |
| // run polls until this move ends or needs an operator, and returns the exit |
| -// code. It never infers the outcome from a state it saw: the outcome is |
| -// last, bound to this move's id. |
| +// code. The detail is read only when the state or the phase changes (L19); |
| +// between reads, the detail already read is looked at again, so a release |
| +// deadline that passes while nothing changes is still seen. The outcome is |
| +// never inferred from a state seen: it is last, bound to this move's id. |
| func (w *moveWatch) run(ctx context.Context) int { |
| for first := true; ; first = false { |
| if !first { |
| @@ -498,15 +551,12 @@ func (w *moveWatch) run(ctx context.Context) int { |
| if code >= 0 { |
| return code |
| } |
| - key := fmt.Sprintf("%s|%s|%d", view.State, view.ID, view.UpdatedAt) |
| - w.sinceRead++ |
| - if key == w.lastKey && w.sinceRead < moveDetailEvery && view.ID == w.id { |
| - continue |
| - } |
| - w.lastKey, w.sinceRead = key, 0 |
| - a, code := w.m.readDetail(ctx) |
| - if code >= 0 { |
| - return code |
| + a := w.last |
| + if key := view.State + "|" + view.Phase + "|" + view.ID; key != w.lastKey { |
| + if a, code = w.m.readDetail(ctx); code >= 0 { |
| + return code |
| + } |
| + w.lastKey, w.last = key, a |
| } |
| if code := w.observe(a); code >= 0 { |
| return code |
| @@ -518,7 +568,7 @@ func (w *moveWatch) run(ctx context.Context) int { |
| // exit code when the wait ends here (else -1). |
| func (w *moveWatch) observe(a moveAnswer) int { |
| w.learn(a) |
| - host, from, to := w.m.host, w.from, w.to |
| + host, from, to := w.m.host, w.fromRegion, w.toRegion |
| if a.ID != w.id { |
| // This move is not in flight: its outcome is last, or it is not |
| // recorded (Review Focus 1). |
| @@ -587,7 +637,10 @@ func (w *moveWatch) observe(a moveAnswer) int { |
| return moveExitWaiting |
| } |
| case "stuck": |
| - sayStuck(w.m.out, host, from, to, j.StuckReason) |
| + moveSay(w.m.out, fmt.Sprintf("%s could not start in %s or in %s. It is offline. Run carlos move resolve --host %s --retry once the cause is fixed.", host, to, from, host)) |
| + if j.StuckReason != "" { |
| + sayRecorded(w.m.out, j.StuckReason) |
| + } |
| return moveExitWaiting |
| case "relocating": |
| if region := a.Detail.WaitingForBox; region != "" { |
| @@ -601,7 +654,10 @@ func (w *moveWatch) observe(a moveAnswer) int { |
| } |
| // outcome is the end of a move that is no longer in flight: last, if it is |
| -// this move's, else not recorded. |
| +// this move's, else not recorded. Its regions are the move's own. Only with |
| +// no move in flight at all may the record's region stand in for one the wait |
| +// never learnt: then no later move can have changed it (a later move that |
| +// closed would be last, and one in flight would be shown). |
| func (w *moveWatch) outcome(a moveAnswer) int { |
| host := w.m.host |
| l := a.Last |
| @@ -609,33 +665,51 @@ func (w *moveWatch) outcome(a moveAnswer) int { |
| moveSay(w.m.out, moveNotRecorded) |
| return moveExitWaiting |
| } |
| - region := a.Detail.Region |
| + from, to := w.fromRegion, w.toRegion |
| + if a.State == "none" { |
| + if from == "" { |
| + from = a.Detail.Region |
| + } |
| + if to == "" { |
| + to = a.Detail.Region |
| + } |
| + } |
| switch l.Outcome { |
| case "done": |
| - if region == "" { |
| - region = w.to |
| + if to == "" { |
| + return w.outcomeLine(l, moveExitDone) |
| } |
| - moveSay(w.m.out, fmt.Sprintf("Moved. %s is served from %s. It was offline for %d seconds.", host, region, int(math.Round(l.OfflineS)))) |
| + moveSay(w.m.out, fmt.Sprintf("Moved. %s is served from %s. It was offline for %d seconds.", host, to, int(math.Round(l.OfflineS)))) |
| return moveExitDone |
| case "rolled_back": |
| - // M-C8 is said only from a rollback seen with its reason: the |
| - // outcome carries none, and the line is in the present tense. |
| - if region == "" { |
| - region = w.from |
| + if from == "" { |
| + return w.outcomeLine(l, moveExitRolled) |
| + } |
| + // M-C8 for a rollback the wait missed only when the outcome carries |
| + // its reason (ruling 4): a reason is never made up. |
| + if l.Reason != "" && to != "" && w.first("rollback") { |
| + moveSay(w.m.out, fmt.Sprintf("%s could not start %s: %s. Moving it back to %s.", to, host, l.Reason, from)) |
| } |
| - moveSay(w.m.out, fmt.Sprintf("%s is back in %s and serving. Nothing was lost.", host, region)) |
| + moveSay(w.m.out, fmt.Sprintf("%s is back in %s and serving. Nothing was lost.", host, from)) |
| return moveExitRolled |
| case "aborted": |
| - if region == "" { |
| - region = w.from |
| + if from == "" { |
| + return w.outcomeLine(l, moveExitRolled) |
| } |
| - moveSay(w.m.out, fmt.Sprintf("The move was aborted. %s stays in %s.", host, region)) |
| + moveSay(w.m.out, fmt.Sprintf("The move was aborted. %s stays in %s.", host, from)) |
| return moveExitRolled |
| } |
| moveSay(w.m.out, moveNotRecorded) |
| return moveExitWaiting |
| } |
| +// outcomeLine says the outcome without a region, when the wait never learnt |
| +// the move's own and another move holds the record now. |
| +func (w *moveWatch) outcomeLine(l *moveOutcome, code int) int { |
| + moveSay(w.m.out, fmt.Sprintf("Move %s of %s is %s.", l.ID, w.m.host, l.Outcome)) |
| + return code |
| +} |
| + |
| // moveEstimate renders a cut budget for M-C5. |
| func moveEstimate(seconds int) string { |
| if seconds < 90 { |
| @@ -643,25 +717,3 @@ func moveEstimate(seconds int) string { |
| } |
| return fmt.Sprintf("%d minutes", (seconds+59)/60) |
| } |
| - |
| -// sayStuck is M-C11 and then the record's stuck reason on its own line |
| -// (preflight L28). The reason is a sentence the provision transitions |
| -// record (MV-N300 to MV-N312 and the rows before them), passed on as it is. |
| -func sayStuck(out io.Writer, host, from, to, reason string) { |
| - moveSay(out, fmt.Sprintf("%s could not start in %s or in %s. It is offline. Run carlos move resolve --host %s --retry once the cause is fixed.", host, to, from, host)) |
| - if reason != "" { |
| - moveSay(out, reason) |
| - } |
| -} |
| - |
| -// sayRefusal passes on the console's refusal, a sentence recorded with the |
| -// door that speaks it (M-C13, M-C17, MV-N1 and the rest). |
| -func sayRefusal(out io.Writer, status int, body []byte) { |
| - moveSay(out, apiErrorFromBody(status, body).Error()) |
| -} |
| - |
| -// sayError reports a transport or decoding error: Go's text, not a sentence |
| -// of this file. |
| -func sayError(out io.Writer, err error) { |
| - moveSay(out, err.Error()) |
| -} |
diff --git a/move_test.go b/move_test.go| index ccb267660..f6d85006b 100644 |
| --- a/move_test.go |
| +++ b/move_test.go |
| @@ -4,6 +4,8 @@ import ( |
| "bytes" |
| "context" |
| "encoding/json" |
| + "errors" |
| + "flag" |
| "io" |
| "net/http" |
| "net/http/httptest" |
| @@ -48,6 +50,9 @@ type fakeMoveConsole struct { |
| posts []string // method path body |
| mutated bool // a mutation since the last flat GET |
| answer func(method, path, body string) (int, any) |
| + // startDetail, when set, is the detail answer right after a mutation |
| + // (the move's own start), whatever the next scripted frame is. |
| + startDetail frame |
| } |
| func (c *fakeMoveConsole) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| @@ -67,7 +72,9 @@ func (c *fakeMoveConsole) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| if r.URL.Query().Get("detail") == "1" { |
| c.details++ |
| cur := c.last |
| - if cur == nil || c.mutated { |
| + if c.mutated && c.startDetail != nil { |
| + cur = c.startDetail |
| + } else if cur == nil || c.mutated { |
| // Right after a mutation the console shows its result: the |
| // next scripted frame. |
| n := c.i |
| @@ -153,7 +160,30 @@ func inFlight(state string, move frame, extra frame) frame { |
| for k, v := range extra { |
| d[k] = v |
| } |
| - return frame{"state": state, "target": "uk", "updated_at": time.Now().Unix(), "id": tMoveID, "protocol": "phase1", "detail": d} |
| + f := frame{"state": state, "target": "uk", "updated_at": time.Now().Unix(), "id": tMoveID, "protocol": "phase1", "detail": d} |
| + if p := flatPhase(state, m); p != "" { |
| + f["phase"] = p |
| + } |
| + return f |
| +} |
| + |
| +// flatPhase is the flat view's phase, as the console derives it from the |
| +// journal (moveViewOf). |
| +func flatPhase(state string, m frame) string { |
| + str := func(k string) string { v, _ := m[k].(string); return v } |
| + switch state { |
| + case "cutting": |
| + return str("cut_phase") |
| + case "landing": |
| + return str("landing_phase") |
| + case "rolling_back": |
| + if rb, ok := m["rollback"].(frame); ok { |
| + tp, _ := rb["target_phase"].(string) |
| + sp, _ := rb["source_phase"].(string) |
| + return tp + "/" + sp |
| + } |
| + } |
| + return "" |
| } |
| func outcome(id, what string, offline float64) frame { |
| @@ -198,12 +228,9 @@ func TestMoveWaitExitCodes(t *testing.T) { |
| rb := frame{"id": "rb_x", "reason": "the landing ran past its deadline", "target_phase": "cleaning"} |
| report := frame{"checks": frame{"t1_protocol": frame{"ok": true, "reason": ""}}} |
| t.Run("rolled back", func(t *testing.T) { |
| - frames := []frame{none(nil), inFlight("preparing", nil, nil)} |
| - // A report appears with no record change: only the wait's periodic |
| - // detail read sees it, so it stays up for that many polls. |
| - for i := 0; i < moveDetailEvery+1; i++ { |
| - frames = append(frames, inFlight("preparing", nil, frame{"target_report": report})) |
| - } |
| + // The detail is read on each change of state or phase (L19): the |
| + // target's report is there when the move is first seen preparing. |
| + frames := []frame{none(nil), inFlight("preparing", nil, frame{"target_report": report})} |
| c := &fakeMoveConsole{frames: append(frames, |
| inFlight("ready", nil, nil), |
| inFlight("cutting", frame{"cut_phase": "admitted", "cut_budget_s": 45}, nil), |
| @@ -562,3 +589,158 @@ func TestMoveDocsNameEveryVerb(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// Ruling 1 (fix round 1): a 429 from request, cut, abort or retry comes |
| +// before the handler, so the same call is sent again after the back-off, |
| +// said once; the verb never exits on it. |
| +func TestMoveMutationRetriesOn429(t *testing.T) { |
| + for _, v := range []struct { |
| + args []string |
| + want string |
| + }{ |
| + {[]string{"--host", tHost, "--to", "uk", "--no-wait"}, "POST " + tMovePth + ` {"cut":"auto","region":"uk"}`}, |
| + {[]string{"cut", "--host", tHost, "--no-wait"}, "POST " + tMovePth + "/cut {}"}, |
| + {[]string{"abort", "--host", tHost, "--no-wait"}, "DELETE " + tMovePth + " "}, |
| + {[]string{"resolve", "--host", tHost, "--retry", "--no-wait"}, "POST " + tMovePth + `/resolve {"retry":true}`}, |
| + } { |
| + limited := 2 |
| + c := &fakeMoveConsole{frames: []frame{none(nil), inFlight("preparing", nil, nil)}} |
| + c.answer = func(string, string, string) (int, any) { |
| + if limited > 0 { |
| + limited-- |
| + return http.StatusTooManyRequests, map[string]string{"error": "too many instance requests. wait a minute and try again"} |
| + } |
| + return http.StatusAccepted, frame{"state": "preparing", "target": "uk", "id": tMoveID, "protocol": "phase1"} |
| + } |
| + if v.args[0] == "cut" || v.args[0] == "abort" || v.args[0] == "resolve" { |
| + c.frames = []frame{inFlight("ready", nil, nil)} |
| + } |
| + code, out := runMoveAgainst(t, c, v.args...) |
| + if code != 0 { |
| + t.Fatalf("%v: exit %d, want 0:\n%s", v.args, code, out) |
| + } |
| + m := c.mutations() |
| + if len(m) != 3 || m[0] != v.want || m[1] != v.want || m[2] != v.want { |
| + t.Fatalf("%v: mutations %q, want the same call three times", v.args, m) |
| + } |
| + if n := strings.Count(out, "The console is limiting requests. Checking every 10 seconds."); n != 1 { |
| + t.Fatalf("%v: the back-off line appears %d times, want once:\n%s", v.args, n, out) |
| + } |
| + wantNoLine(t, out, "too many instance requests") |
| + } |
| +} |
| + |
| +// Ruling 3: the outcome's regions are this move's own, bound from its own |
| +// answers, never from a later move's record. Here a second move flips to fr |
| +// while the first one's outcome is read. |
| +func TestMoveOutcomeRegionsAreThisMoves(t *testing.T) { |
| + second := func(last frame) frame { |
| + f := inFlight("landing", frame{"id": "mv_another", "from": "uk", "to": "fr", "landing_phase": "probing"}, frame{"from": "uk", "region": "fr"}) |
| + f["id"], f["target"], f["last"] = "mv_another", "fr", last |
| + return f |
| + } |
| + for _, c := range []struct { |
| + name string |
| + last frame |
| + code int |
| + want string |
| + }{ |
| + {"done", outcome(tMoveID, "done", 41.2), 0, mc7}, |
| + {"rolled back", outcome(tMoveID, "rolled_back", 0), 3, mc9}, |
| + {"aborted", outcome(tMoveID, "aborted", 0), 3, "The move was aborted. acme.go.tito.io stays in ie."}, |
| + } { |
| + fc := &fakeMoveConsole{frames: []frame{none(nil), inFlight("preparing", nil, nil), second(c.last)}} |
| + code, out := runMoveAgainst(t, fc, "--host", tHost, "--to", "uk") |
| + if code != c.code { |
| + t.Fatalf("%s: exit %d, want %d:\n%s", c.name, code, c.code, out) |
| + } |
| + wantLines(t, out, c.want) |
| + wantNoLine(t, out, "from fr", "in fr") |
| + } |
| + // Bound from the request's own start answer, before any poll: the |
| + // first poll already shows the second move, flipped to fr. |
| + fc := &fakeMoveConsole{frames: []frame{none(nil), second(outcome(tMoveID, "done", 41.2))}, startDetail: inFlight("preparing", nil, nil)} |
| + code, out := runMoveAgainst(t, fc, "--host", tHost, "--to", "uk") |
| + if code != 0 { |
| + t.Fatalf("bound at the start: exit %d:\n%s", code, out) |
| + } |
| + wantLines(t, out, mc1, mc7) |
| + wantNoLine(t, out, "from fr", "in fr") |
| +} |
| + |
| +// Ruling 4: a rollback the wait missed says M-C8 only when the outcome |
| +// carries the reason, and never invents one. |
| +func TestMoveMissedRollbackSaysItsReasonOnlyFromTheOutcome(t *testing.T) { |
| + withReason := outcome(tMoveID, "rolled_back", 0) |
| + withReason["reason"] = "the landing ran past its deadline" |
| + c := &fakeMoveConsole{frames: []frame{none(nil), inFlight("preparing", nil, nil), none(withReason)}} |
| + code, out := runMoveAgainst(t, c, "--host", tHost, "--to", "uk") |
| + if code != 3 { |
| + t.Fatalf("exit %d, want 3:\n%s", code, out) |
| + } |
| + wantLines(t, out, mc8, mc9) |
| + if strings.Index(out, mc8) > strings.Index(out, mc9) { |
| + t.Fatalf("M-C8 after M-C9:\n%s", out) |
| + } |
| +} |
| + |
| +// L19 strictly: the detail is read only when the state or the phase changes, |
| +// whatever else (updated_at) does. A release deadline that passes while |
| +// nothing changes is still seen, from the detail already read. |
| +func TestMoveWaitReadsDetailOnlyOnStateOrPhase(t *testing.T) { |
| + frames := []frame{none(nil)} |
| + for i := 0; i < 6; i++ { |
| + f := inFlight("preparing", nil, nil) |
| + f["updated_at"] = int64(1000 + i) // an unrelated record update each poll |
| + frames = append(frames, f) |
| + } |
| + for _, p := range []string{"admitted", "admitted", "stopping", "stopping"} { |
| + frames = append(frames, inFlight("cutting", frame{"cut_phase": p, "cut_budget_s": 45}, nil)) |
| + } |
| + done := none(outcome(tMoveID, "done", 41.2)) |
| + frames = append(frames, done) |
| + c := &fakeMoveConsole{frames: frames} |
| + code, out := runMoveAgainst(t, c, "--host", tHost, "--to", "uk") |
| + if code != 0 { |
| + t.Fatalf("exit %d:\n%s", code, out) |
| + } |
| + // One detail read at the request's start, then one per change: preparing, |
| + // cutting/admitted, cutting/stopping, none. |
| + if c.details != 5 { |
| + t.Fatalf("%d detail reads, want 5 (the start and four changes) over %d flat polls", c.details, c.flatGets) |
| + } |
| + |
| + soon := time.Now().Add(150 * time.Millisecond).UTC().Format(time.RFC3339Nano) |
| + rb := frame{"id": "rb_x", "reason": "the landing ran past its deadline", "target_phase": "releasing", "release_deadline": soon} |
| + c = &fakeMoveConsole{frames: []frame{none(nil), inFlight("rolling_back", frame{"rollback": rb}, nil)}} |
| + code, out = runMoveAgainst(t, c, "--host", tHost, "--to", "uk") |
| + if code != 4 { |
| + t.Fatalf("deadline passing: exit %d, want 4:\n%s", code, out) |
| + } |
| + wantLines(t, out, "Waiting for uk-box in uk to confirm it has stopped acme.go.tito.io. acme.go.tito.io is offline.") |
| + if c.details > 2 { |
| + t.Fatalf("%d detail reads while nothing changed, want at most 2", c.details) |
| + } |
| +} |
| + |
| +// Ruling 6: the flag help is said through moveFlagHelp, the copy gate's |
| +// registered help emission: on -h, and after a bad flag, with the flag |
| +// package's own error. |
| +func TestMoveFlagHelp(t *testing.T) { |
| + var out bytes.Buffer |
| + if _, err := parseMoveArgs([]string{"-h"}, &out); !errors.Is(err, flag.ErrHelp) { |
| + t.Fatalf("-h: %v", err) |
| + } |
| + for _, want := range []string{"-host", "the instance's host name", "-to", "-manual-cut"} { |
| + if !strings.Contains(out.String(), want) { |
| + t.Errorf("help lacks %q:\n%s", want, out.String()) |
| + } |
| + } |
| + out.Reset() |
| + if _, err := parseMoveArgs([]string{"resolve", "--host", tHost, "--declare-lost", "uk-box"}, &out); err == nil { |
| + t.Fatal("--declare-lost parsed") |
| + } |
| + if !strings.Contains(out.String(), "flag provided but not defined: -declare-lost") || !strings.Contains(out.String(), "-retry") { |
| + t.Errorf("a bad flag's help:\n%s", out.String()) |
| + } |
| +} |
diff --git a/testdata/moves-copy/sinks/forwarded-local-error/emit.go b/testdata/moves-copy/sinks/forwarded-local-error/emit.go| new file mode 100644 |
| index 000000000..2af6950ee |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/forwarded-local-error/emit.go |
| @@ -0,0 +1,58 @@ |
| +package fixture |
| + |
| +// Not compiled. sayError is a local sink that forwards an error, as move.go's |
| +// does. What reaches it is traced at each call: the console's own text |
| +// (apiErrorFromBody) and an error from a call across the API (getJSON) are |
| +// external, by name; a locally made error is a sentence and needs its row; |
| +// an error the gate cannot trace is a violation. |
| + |
| +func moveSay(out int, line string) {} |
| + |
| +func sayError(out int, err error) { |
| + moveSay(out, err.Error()) |
| +} |
| + |
| +func local(out int) { |
| + sayError(out, errors.New("unreviewed copy")) |
| +} |
| + |
| +func formatted(out int, n int) { |
| + sayError(out, fmt.Errorf("%d boxes are unreviewed", n)) |
| +} |
| + |
| +func external(out int, status int, body []byte) { |
| + sayError(out, apiErrorFromBody(status, body)) |
| +} |
| + |
| +func crossed(out int) { |
| + _, _, err := getJSON("/x") |
| + if err != nil { |
| + sayError(out, err) |
| + } |
| +} |
| + |
| +func untraced(out int, err error) { |
| + sayError(out, err) |
| +} |
| + |
| +// A package helper's error result is traced through its returns: one that |
| +// makes its own error is not external and fails the gate; one that passes on |
| +// an external error is external. |
| +func helper() error { |
| + return errors.New("helper copy") |
| +} |
| + |
| +func viaHelper(out int) { |
| + sayError(out, helper()) |
| +} |
| + |
| +func fetch() (int, error) { |
| + _, _, err := getJSON("/y") |
| + return 0, err |
| +} |
| + |
| +func viaFetch(out int) { |
| + if _, err := fetch(); err != nil { |
| + sayError(out, err) |
| + } |
| +} |